On Wed, Jul 12, 2023 at 04:49:53PM +0200, Michal Privoznik wrote:
> A <channel/> device is basically an UNIX socket into guest.
> Whatever is sent from the host, appears in the guest and vice
> versa. But because of that, the length of the path to the socket
> is important (underscored by fact that we derive the path from
> domain short name). But there are still cases where we might not
> fit into UNIX_PATH_MAX limit (usually 108 characters), because
> the path is derived also from other variables, e.g.
> XDG_CONFIG_HOME for session domains.
>
> There is one component though, that's needless: "/target/". Drop
> it. This is safe to do, because running domains have their path
> saved in status XML and even though paths are dropped on
> migration, they are not part of guest ABI and thus we are free to
> change them.
This mentions dropping '/target/' which makes sense, but...
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 94587638c3..b4844351ba 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1780,7 +1780,7 @@ qemuDomainSetPrivatePaths(virQEMUDriver *driver,
> priv->libDir = g_strdup_printf("%s/domain-%s", cfg->libDir, domname);
>
> if (!priv->channelTargetDir)
> - priv->channelTargetDir = g_strdup_printf("%s/domain-%s",
> + priv->channelTargetDir = g_strdup_printf("%s/%s",
> cfg->channelTargetDir, domname);
...this is also dropping 'domain-' which also makes sense, but
is not mentioned in the commit message.
> @@ -5352,9 +5352,12 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
> * libvirt 1.2.19 - 1.3.2:
> * {cfg->channelTargetDir}/domain-{dom-name}/{target-name}
> *
> - * libvirt 1.3.3 and newer:
> + * libvirt 1.3.3 - 9.4.0:
> * {cfg->channelTargetDir}/domain-{dom-id}-{short-dom-name}/{target-name}
> *
> + * libvirt 9.6.0 and newer:
> + * {cfg->channelTargetDir}/{dom-id}-{short-dom-name}/{target-name}
> + *
This doesn't make it clear that 'cfg->channelTargetDir' is actually
different in those two examples. Should we change th old ones to
libvirt 1.2.19 - 1.3.2:
{cfg->channelTargetDir}/target/domain-{dom-name}/{target-name}
libvirt 1.3.3 - 9.4.0:
{cfg->channelTargetDir}/target/domain-{dom-id}-{short-dom-name}/{target-name}
> * The unix socket path was stored in config XML until libvirt 1.3.0.
> * If someone specifies the same path as we generate, they shouldn't do it.
> *
> @@ -5379,7 +5382,7 @@ qemuDomainChrDefDropDefaultPath(virDomainChrDef *chr,
> cfg = virQEMUDriverGetConfig(driver);
>
> virBufferEscapeRegex(&buf, "^%s", cfg->channelTargetDir);
> - virBufferAddLit(&buf, "/([^/]+\\.)|(domain-[^/]+/)");
> + virBufferAddLit(&buf, "/(target/)?([^/]+\\.)|(domain-[^/]+/)|([0-9]+-[^/]+/)");
> virBufferEscapeRegex(&buf, "%s$", chr->target.name);
> diff --git a/tests/qemuhotplugtestdevices/qemuhotplug-qemu-agent-detach.xml b/tests/qemuhotplugtestdevices/qemuhotplug-qemu-agent-detach.xml
> index 0c3c70a78e..2e5cbfe09c 100644
> --- a/tests/qemuhotplugtestdevices/qemuhotplug-qemu-agent-detach.xml
> +++ b/tests/qemuhotplugtestdevices/qemuhotplug-qemu-agent-detach.xml
> @@ -1,5 +1,5 @@
> <channel type='unix'>
> - <source mode='bind' path='/var/lib/libvirt/qemu/channel/target/domain-7-hotplug/org.qemu.guest_agent.0'/>
> + <source mode='bind' path='/var/lib/libvirt/qemu/channel/target/7-hotplug/org.qemu.guest_agent.0'/>
You dropped 'domain-' but didn't drop '/target'. Same in a few other cases below
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
