public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: "Ján Tomko" <jtomko@redhat.com>
To: Laine Stump <laine@redhat.com>
Cc: Libvirt <libvir-list@redhat.com>,
	sbrivio@redhat.com, passt-dev@passt.top
Subject: Re: [libvirt PATCH 8/9] qemu: hook up passt config to qemu domains
Date: Mon, 9 Jan 2023 15:51:59 +0100	[thread overview]
Message-ID: <Y7wqD24bp8tYY3m4@fedora> (raw)
In-Reply-To: <ba7e3882-39cb-9121-3f05-843282f81065@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1682 bytes --]

On a Monday in 2023, Laine Stump wrote:
>On 1/9/23 2:32 AM, Ján Tomko wrote:
>>On a Sunday in 2023, Laine Stump wrote:
>>>+static char *
>>>+qemuPasstCreatePidFilename(virDomainObj *vm,
>>>+                           virDomainNetDef *net)
>>>+{
>>>+    qemuDomainObjPrivate *priv = vm->privateData;
>>>+    virQEMUDriver *driver = priv->driver;
>>>+    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
>>>+    g_autofree char *name = NULL;
>>>+
>>>+    name = g_strdup_printf("%s-%s-passt", vm->def->name, 
>>>net->info.alias);
>>
>>Please use virDomainDefGetShortName for filename purposes.
>
>Why? If I use GetShortName, then there's the possibility that two 
>domains would want to use the same name for the pidfile.
>

Because otherwise the PID filename might exceed maximum path length for
domains with very long names.

The short name should be unique since it contains the domain ID. The
ShortName function is also used for slirp and virtiofsd pid filenames,
so we have the cult argument too :)

(If you know of an issue with this usage, please share it - it could
  be the cause of us not cleaning up virtiofsd properly sometimes [0])

Jano

>Would it be better to use the domain's UUID (as I did for the socket 
>path?) The advantage of using the name is that it's easier for a human 
>to find, but while the uuid is usually longer, its length is at least 
>predictable/consistent, and I suppose a human will probably never need 
>to find the pidfile anyway...
>
>>
>>>+
>>>+    return virPidFileBuildPath(cfg->passtStateDir, name);
>>>+}
>>>+
>>>+

[0] https://bugzilla.redhat.com/show_bug.cgi?id=2151808

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  reply	other threads:[~2023-01-09 14:51 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-09  4:11 [libvirt PATCH 0/9] Support libvirt-managed QEMU domain <interface> backed by a passt process Laine Stump
2023-01-09  4:11 ` [libvirt PATCH 1/9] conf: rename virDomainNetBackend* to virDomainNetDriver* Laine Stump
2023-01-09  5:40   ` Ján Tomko
2023-01-09  4:11 ` [libvirt PATCH 2/9] conf: move anonymous backend struct from virDomainNetDef into its own struct Laine Stump
2023-01-09  5:41   ` Ján Tomko
2023-01-09  4:11 ` [libvirt PATCH 3/9] conf: put interface <backend> parsing/formatting separate functions Laine Stump
2023-01-09  5:47   ` Ján Tomko
2023-01-09  7:04     ` Laine Stump
2023-01-09  4:11 ` [libvirt PATCH 4/9] conf: add passt XML additions to schema Laine Stump
2023-01-09  5:48   ` Ján Tomko
2023-01-11 18:33   ` Daniel P. Berrangé
2023-01-12 14:45     ` Laine Stump
2023-01-12 17:28       ` Stefano Brivio
2023-01-12 18:12       ` Jiri Denemark
2023-01-09  4:11 ` [libvirt PATCH 5/9] conf: parse/format passt-related XML additions Laine Stump
2023-01-09  6:18   ` Ján Tomko
2023-01-09  4:11 ` [libvirt PATCH 6/9] qemu: new capability QEMU_CAPS_NETDEV_STREAM Laine Stump
2023-01-09  6:20   ` Ján Tomko
2023-01-09  4:11 ` [libvirt PATCH 7/9] qemu: add passtStateDir to qemu driver config Laine Stump
2023-01-09  6:23   ` Ján Tomko
2023-01-09 14:02     ` Laine Stump
2023-01-09  4:11 ` [libvirt PATCH 8/9] qemu: hook up passt config to qemu domains Laine Stump
2023-01-09  6:31   ` Ján Tomko
2023-01-09 14:14     ` Laine Stump
2023-01-09 14:51       ` Ján Tomko [this message]
2023-01-09 16:05         ` Laine Stump
2023-01-09  4:11 ` [libvirt PATCH 9/9] specfile: require passt for the build if fedora >= 36 or rhel >= 9 Laine Stump
2023-01-09  6:32   ` Ján Tomko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y7wqD24bp8tYY3m4@fedora \
    --to=jtomko@redhat.com \
    --cc=laine@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).