Re: [PATCH 1/4] qemu: Generate shorter channel target paths

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