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

[Michal Prívozník]

On 7/21/23 14:27, Daniel P. Berrangé wrote:
> 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}
> 
> 

Good point, let me fix that.

> 
>>   * 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

Yeah, I need to squash in the next patch that handles this. Let me fix
that in v2.

Michal