[libvirt] [PATCH 3/3] qemuDomainGetPreservedMounts: Fix suffixes for corner cases

Michal Privoznik posted 3 patches 7 years, 11 months ago
[libvirt] [PATCH 3/3] qemuDomainGetPreservedMounts: Fix suffixes for corner cases
Posted by Michal Privoznik 7 years, 11 months ago
https://bugzilla.redhat.com/show_bug.cgi?id=1431112

Imagine a FS mounted on /dev/blah/blah2. Our process of creating
suffix for temporary location where all the mounted filesystems
are moved is very simplistic. We want:

/var/run/libvirt/qemu/$domName.$suffix\

were $suffix is just the mount point path stripped of the "/dev/"
preffix. For instance:

/var/run/libvirt/qemu/fedora.mqueue  for /dev/mqueue
/var/run/libvirt/qemu/fedora.pts     for /dev/pts

and so on. Now if we plug /dev/blah/blah2 into the example we see
some misbehaviour:

/var/run/libvirt/qemu/fedora.blah/blah2

Well, misbehaviour if /dev/blah/blah2 is a file, because in that
case we call virFileTouch() instead of virFileMakePath().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_domain.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index accf05a6f..547c9fbfb 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7570,7 +7570,9 @@ qemuDomainGetPreservedMounts(virQEMUDriverConfigPtr cfg,
         goto error;
 
     for (i = 0; i < nmounts; i++) {
+        char *tmp;
         const char *suffix = mounts[i] + strlen(DEVPREFIX);
+        size_t off;
 
         if (STREQ(mounts[i], "/dev"))
             suffix = "dev";
@@ -7578,6 +7580,20 @@ qemuDomainGetPreservedMounts(virQEMUDriverConfigPtr cfg,
         if (virAsprintf(&paths[i], "%s/%s.%s",
                         cfg->stateDir, vm->def->name, suffix) < 0)
             goto error;
+
+        /* Now consider that mounts[i] is "/dev/blah/blah2".
+         * @suffix then points to "blah/blah2". However, caller
+         * expects all the @paths to be the same depth. The
+         * caller doesn't always do `mkdir -p` but sometimes bare
+         * `touch`. Therefore fix all the suffixes. */
+        off = strlen(paths[i]) - strlen(suffix);
+
+        tmp = paths[i] + off;
+        while (*tmp) {
+            if (*tmp == '/')
+                *tmp = '.';
+            tmp++;
+        }
     }
 
     if (devPath)
-- 
2.13.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemuDomainGetPreservedMounts: Fix suffixes for corner cases
Posted by John Ferlan 7 years, 11 months ago

On 06/12/2017 11:57 AM, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1431112
> 
> Imagine a FS mounted on /dev/blah/blah2. Our process of creating
> suffix for temporary location where all the mounted filesystems
> are moved is very simplistic. We want:
> 
> /var/run/libvirt/qemu/$domName.$suffix\
> 
> were $suffix is just the mount point path stripped of the "/dev/"
> preffix. For instance:

s/preffix/prefix

> 
> /var/run/libvirt/qemu/fedora.mqueue  for /dev/mqueue
> /var/run/libvirt/qemu/fedora.pts     for /dev/pts
> 
> and so on. Now if we plug /dev/blah/blah2 into the example we see
> some misbehaviour:
> 
> /var/run/libvirt/qemu/fedora.blah/blah2
> 
> Well, misbehaviour if /dev/blah/blah2 is a file, because in that
> case we call virFileTouch() instead of virFileMakePath().
> 

You didn't finish my bedtime story!

Am I to assume that instead of :

/var/run/libvirt/qemu/fedora.blah/blah2

we would get

/var/run/libvirt/qemu/fedora.blah.blah2

taking things one step further...

would /dev/blah/blah2/blah3

be

/var/run/libvirt/qemu/fedora.blah.blah2.blah3

That's what I see coded at least... Or should the path be:

/var/run/libvirt/qemu/fedora.blah/blah2.blah3


It would seem you'd want to get to the end, reverse search on '/' then
if that spot is greater than @off, then convert it to a '.', but what do
I know. I keep to the simple life and don't use namespaces.


John

> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_domain.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index accf05a6f..547c9fbfb 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -7570,7 +7570,9 @@ qemuDomainGetPreservedMounts(virQEMUDriverConfigPtr cfg,
>          goto error;
>  
>      for (i = 0; i < nmounts; i++) {
> +        char *tmp;
>          const char *suffix = mounts[i] + strlen(DEVPREFIX);
> +        size_t off;
>  
>          if (STREQ(mounts[i], "/dev"))
>              suffix = "dev";
> @@ -7578,6 +7580,20 @@ qemuDomainGetPreservedMounts(virQEMUDriverConfigPtr cfg,
>          if (virAsprintf(&paths[i], "%s/%s.%s",
>                          cfg->stateDir, vm->def->name, suffix) < 0)
>              goto error;
> +
> +        /* Now consider that mounts[i] is "/dev/blah/blah2".
> +         * @suffix then points to "blah/blah2". However, caller
> +         * expects all the @paths to be the same depth. The
> +         * caller doesn't always do `mkdir -p` but sometimes bare
> +         * `touch`. Therefore fix all the suffixes. */
> +        off = strlen(paths[i]) - strlen(suffix);
> +
> +        tmp = paths[i] + off;
> +        while (*tmp) {
> +            if (*tmp == '/')
> +                *tmp = '.';
> +            tmp++;
> +        }
>      }
>  
>      if (devPath)
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list