Bug 1817401

Jano,

How serious do you think this is?

Is the debug log enabled by default for all guests, or
is it something the user has to enable?

If the user has to enable, then I'd say it's less serious.

The log is created for all guests with a virtiofs filesystem.

The filename length is problematic for the combination of:
* using uuids as user-defined device aliases
* using very long guest names

By default the device alias for the first device is just "fs0", so it
does not limit the domain name as much.

Also, the current naming scheme is not foolproof - it can clash with
another domain's log file if someone puts the log filename as the
domain name:
https://www.redhat.com/archives/libvir-list/2020-March/msg00818.html

Each domain should just have its own directory in the common log directory, just like we had to migrate to this in the case with other files.  Ideally most of the files should move to that structure.

After evaluating this issue, there are no plans to address it further or fix it in an upcoming release.  Therefore, it is being closed.  If plans change such that this issue will be fixed in an upcoming release, then the bug can be reopened.

Jan,

This looks like an important issue we'd like to get fix, so I'm reopening it.

After evaluating this issue, there are no plans to address it further or fix it in an upcoming release.  Therefore, it is being closed.  If plans change such that this issue will be fixed in an upcoming release, then the bug can be reopened.

Hi, Jarda

I reopened this bug. please help to confirm whether this bug should be fixed. Thanks

Patch proposed upstream by MartinK:
https://listman.redhat.com/archives/libvir-list/2022-April/229916.html

How serious is this issue? I mean, we can try to shorten the log path but then the next problem is collision in log filename. And even if we'd figure a workaround for this, the path limit is going to be hit for something else (e.g. domain monitor socket/log file/...). My suggestion is: don't use long names for domains. Use <description/> or <title/> for that. At very best we can document that there is a limit wrt virtiofsd.

------- Comment From danielhb.com 2022-04-07 10:24 EDT-------
(In reply to comment #13)
> How serious is this issue? I mean, we can try to shorten the log path but
> then the next problem is collision in log filename. And even if we'd figure
> a workaround for this, the path limit is going to be hit for something else
> (e.g. domain monitor socket/log file/...). My suggestion is: don't use long
> names for domains. Use <description/> or <title/> for that. At very best we
> can document that there is a limit wrt virtiofsd.

Not only document, but also limit the VM instance name size. For what it's worth VMWare caps the instance name in 128 chars:

https://docs.vmware.com/en/VMware-Cloud-Director/9.7/com.vmware.vcloud.admin.doc/GUID-023D4FBC-3016-4757-B52B-4A97984ABD7F.html

I don't see a reason not to have a VM name size limit in libvirt as well. There might be a number where we can mitigate/avoid these file name size issues. 40 chars seems like a good size for a VM name since we have other fields such as "Description" where a long explanation can be provided. Of course that such limit would be imposed just for new VMs - VMs already in the wild can't be impacted by this rule.

Without any form of name size sanitization this bug will keep appearing over and over again, impacting different parts of the VM lifecycle, because we thought that a documentation hint alone would do the trick.

(In reply to IBM Bug Proxy from comment #18)
> ------- Comment From danielhb.com 2022-04-07 10:24 EDT-------
> (In reply to comment #13)
> > How serious is this issue? I mean, we can try to shorten the log path but
> > then the next problem is collision in log filename. And even if we'd figure
> > a workaround for this, the path limit is going to be hit for something else
> > (e.g. domain monitor socket/log file/...). My suggestion is: don't use long
> > names for domains. Use <description/> or <title/> for that. At very best we
> > can document that there is a limit wrt virtiofsd.
> 
> Not only document, but also limit the VM instance name size. For what it's
> worth VMWare caps the instance name in 128 chars:
> 
> https://docs.vmware.com/en/VMware-Cloud-Director/9.7/com.vmware.vcloud.admin.
> doc/GUID-023D4FBC-3016-4757-B52B-4A97984ABD7F.html
> 
> I don't see a reason not to have a VM name size limit in libvirt as well.
> There might be a number where we can mitigate/avoid these file name size
> issues. 40 chars seems like a good size for a VM name since we have other
> fields such as "Description" where a long explanation can be provided. Of
> course that such limit would be imposed just for new VMs - VMs already in
> the wild can't be impacted by this rule.
> 
> Without any form of name size sanitization this bug will keep appearing over
> and over again, impacting different parts of the VM lifecycle, because we
> thought that a documentation hint alone would do the trick.

And if we enforce the length of the domain name we will hit the same issue with other user supplied strings.  We'll always hit those.  And enforcing the limit is not as easy as it seems because of backward compatibility.  I am not against limiting the name length to something reasonable (well, who uses names longer than 30 characters anyway, right?), but doing it properly seems like too much of a trouble when the user actually gets a reasonable error (see the description) from which it is very clear why that is.  The same will happen if you use a very long alias because the socket name does not fit in sockaddr_t.

I personally think we should expect the users to be able to figure this out for themselves without us hand-holding them and checking for every single possible failure to provide a slightly nicer error message.

(In reply to Jaroslav Suchanek from comment #16)
> Patch proposed upstream by MartinK:
> https://listman.redhat.com/archives/libvir-list/2022-April/229916.html

Patch dismissed upstream by JanT:
https://listman.redhat.com/archives/libvir-list/2022-April/229917.html

Closing this as WONTFIX per bug severity/priority.