This helps when starting domains with long names which could possibly end up
creating too long of a name for the filesystem.

The path is not being saved in the domain config, so there is no need for
backwards compatibility.  User aliases will always start with "ua-" and the only
way paths could collide is to:

1) create a domain named e.g. "asdf-ua"
2) start the domain with virtiofsd debug logs
3) destroy the domain
4) restart libvirt daemon for domain IDs to start from 1 again
5) create a domain named "asdf"
6) add user alias for the virtiofs that is the same as was generated for the
   first domain, with the required "ua-" prefix
7) start the "asdf" domain

at which point the logs for the two domains would end up in the same logfile.
Since this is still better than what we had before I think it is not worth
fixing this peculiar scenario in this patch.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1817401
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/qemu/qemu_virtiofs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index 7e3324b017ce..67df8b3890b6 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -81,8 +81,9 @@ qemuVirtioFSCreateLogFilename(virQEMUDriverConfig *cfg,
                               const char *alias)
 {
     g_autofree char *name = NULL;
+    g_autofree char *shortname = virDomainDefGetShortName(def);
 
-    name = g_strdup_printf("%s-%s", def->name, alias);
+    name = g_strdup_printf("%s-%s", shortname, alias);
 
     return virFileBuildPath(cfg->logDir, name, "-virtiofsd.log");
 }
-- 
2.35.1

On a Thursday in 2022, Martin Kletzander wrote:
>This helps when starting domains with long names which could possibly end up
>creating too long of a name for the filesystem.
>
>The path is not being saved in the domain config, so there is no need for
>backwards compatibility.  User aliases will always start with "ua-" and the only
>way paths could collide is to:
>
>1) create a domain named e.g. "asdf-ua"
>2) start the domain with virtiofsd debug logs
>3) destroy the domain
>4) restart libvirt daemon for domain IDs to start from 1 again
>5) create a domain named "asdf"
>6) add user alias for the virtiofs that is the same as was generated for the
>   first domain, with the required "ua-" prefix
>7) start the "asdf" domain
>
>at which point the logs for the two domains would end up in the same logfile.

>Since this is still better than what we had before I think it is not worth
>fixing this peculiar scenario in this patch.
>

I don't think this is better. It does fix it for the strange use case
of using very long domain names, and I agree that people combining
domain names with "-ua" suffixes and no user aliases along with
exact same domain names without the -ua suffix, but using user aliases
are not worth worrying about.

But for the 99 % of reasonable users, this patch includes the domain ID
in the log path, so the path can change across restarts of the same domain.
I don't like including this transient identifier in the log path.
We also don't shorten the domain log, since we expect it to stay after
the domain stops running.

Jano

>Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1817401
>Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>---
> src/qemu/qemu_virtiofs.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)

On Thu, Apr 07, 2022 at 02:14:30PM +0200, Ján Tomko wrote:
>On a Thursday in 2022, Martin Kletzander wrote:
>>This helps when starting domains with long names which could possibly end up
>>creating too long of a name for the filesystem.
>>
>>The path is not being saved in the domain config, so there is no need for
>>backwards compatibility.  User aliases will always start with "ua-" and the only
>>way paths could collide is to:
>>
>>1) create a domain named e.g. "asdf-ua"
>>2) start the domain with virtiofsd debug logs
>>3) destroy the domain
>>4) restart libvirt daemon for domain IDs to start from 1 again
>>5) create a domain named "asdf"
>>6) add user alias for the virtiofs that is the same as was generated for the
>>   first domain, with the required "ua-" prefix
>>7) start the "asdf" domain
>>
>>at which point the logs for the two domains would end up in the same logfile.
>
>>Since this is still better than what we had before I think it is not worth
>>fixing this peculiar scenario in this patch.
>>
>
>I don't think this is better. It does fix it for the strange use case
>of using very long domain names, and I agree that people combining
>domain names with "-ua" suffixes and no user aliases along with
>exact same domain names without the -ua suffix, but using user aliases
>are not worth worrying about.
>
>But for the 99 % of reasonable users, this patch includes the domain ID
>in the log path, so the path can change across restarts of the same domain.
>I don't like including this transient identifier in the log path.
>We also don't shorten the domain log, since we expect it to stay after
>the domain stops running.
>

I think it is even better.  If you are looking at the virtiofsd debug
log then you probably want to look at the current run, you can see when
the file was created and open the newest one.  And if you need to look
at all the log files, then you can just:

cat $(ls -cr *-$domain-$alias-virtiofsd.log) | less

or similar.  But maybe my assumption about virtiofsd debug logs being
usable only in corner case scenarios is wrong.

I wouldn't hate domain logfiles including the id either.

>Jano
>
>>Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1817401
>>Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>>---
>> src/qemu/qemu_virtiofs.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)

On Thu, Apr 07, 2022 at 03:19:27PM +0200, Martin Kletzander wrote:
> On Thu, Apr 07, 2022 at 02:14:30PM +0200, Ján Tomko wrote:
> > On a Thursday in 2022, Martin Kletzander wrote:
> > > This helps when starting domains with long names which could possibly end up
> > > creating too long of a name for the filesystem.
> > > 
> > > The path is not being saved in the domain config, so there is no need for
> > > backwards compatibility.  User aliases will always start with "ua-" and the only
> > > way paths could collide is to:
> > > 
> > > 1) create a domain named e.g. "asdf-ua"
> > > 2) start the domain with virtiofsd debug logs
> > > 3) destroy the domain
> > > 4) restart libvirt daemon for domain IDs to start from 1 again
> > > 5) create a domain named "asdf"
> > > 6) add user alias for the virtiofs that is the same as was generated for the
> > >   first domain, with the required "ua-" prefix
> > > 7) start the "asdf" domain
> > > 
> > > at which point the logs for the two domains would end up in the same logfile.
> > 
> > > Since this is still better than what we had before I think it is not worth
> > > fixing this peculiar scenario in this patch.
> > > 
> > 
> > I don't think this is better. It does fix it for the strange use case
> > of using very long domain names, and I agree that people combining
> > domain names with "-ua" suffixes and no user aliases along with
> > exact same domain names without the -ua suffix, but using user aliases
> > are not worth worrying about.
> > 
> > But for the 99 % of reasonable users, this patch includes the domain ID
> > in the log path, so the path can change across restarts of the same domain.
> > I don't like including this transient identifier in the log path.
> > We also don't shorten the domain log, since we expect it to stay after
> > the domain stops running.
> > 
> 
> I think it is even better.  If you are looking at the virtiofsd debug
> log then you probably want to look at the current run, you can see when
> the file was created and open the newest one.  And if you need to look
> at all the log files, then you can just:
> 
> cat $(ls -cr *-$domain-$alias-virtiofsd.log) | less
> 
> or similar.  But maybe my assumption about virtiofsd debug logs being
> usable only in corner case scenarios is wrong.
> 
> I wouldn't hate domain logfiles including the id either.

That would lead to a big growth in the number of log files, as we
would be creating a new one every time, instead of appending to
the existing one. 


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