[libvirt] [PATCH 2/2] qemu: Also treat directories properly when using namespaces

Martin Kletzander posted 2 patches 7 years, 9 months ago
There is a newer version of this series
[libvirt] [PATCH 2/2] qemu: Also treat directories properly when using namespaces
Posted by Martin Kletzander 7 years, 9 months ago
Partially-resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1443434

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/qemu/qemu_domain.c | 38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 40608554c473..b5fc6697ed29 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7837,6 +7837,7 @@ qemuDomainCreateDeviceRecursive(const char *device,
     isLink = S_ISLNK(sb.st_mode);
     isDev = S_ISCHR(sb.st_mode) || S_ISBLK(sb.st_mode);
     isReg = S_ISREG(sb.st_mode) || S_ISFIFO(sb.st_mode) || S_ISSOCK(sb.st_mode);
+    isDir = S_ISDIR(sb.st_mode);
 
     /* Here, @device might be whatever path in the system. We
      * should create the path in the namespace iff it's "/dev"
@@ -7954,6 +7955,10 @@ qemuDomainCreateDeviceRecursive(const char *device,
             goto cleanup;
         /* Just create the file here so that code below sets
          * proper owner and mode. Bind mount only after that. */
+    } else if (isDir) {
+        if (create &&
+            virFileMakePathWithMode(devicePath, sb.st_mode) < 0)
+            goto cleanup;
     } else {
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
                        _("unsupported device type %s 0%o"),
@@ -8015,7 +8020,7 @@ qemuDomainCreateDeviceRecursive(const char *device,
 #endif
 
     /* Finish mount process started earlier. */
-    if (isReg &&
+    if ((isReg || isDir) &&
         virFileBindMountDevice(device, devicePath) < 0)
         goto cleanup;
 
@@ -8644,6 +8649,7 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED,
     bool isLink = S_ISLNK(data->sb.st_mode);
     bool isDev = S_ISCHR(data->sb.st_mode) || S_ISBLK(data->sb.st_mode);
     bool isReg = S_ISREG(data->sb.st_mode) || S_ISFIFO(data->sb.st_mode) || S_ISSOCK(data->sb.st_mode);
+    bool isDir = S_ISDIR(data->sb.st_mode);
 
     qemuSecurityPostFork(data->driver->securityManager);
 
@@ -8699,6 +8705,23 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED,
         delDevice = true;
         /* Just create the file here so that code below sets
          * proper owner and mode. Move the mount only after that. */
+    } else if (isDir) {
+        /* We are not cleaning up disks on virDomainDetachDevice
+         * because disk might be still in use by different disk
+         * as its backing chain. This might however clash here.
+         * Therefore do the cleanup here. */
+        if (umount(data->file) < 0 &&
+            errno != ENOENT && errno != EINVAL) {
+            virReportSystemError(errno,
+                                 _("Unable to umount %s"),
+                                 data->file);
+            goto cleanup;
+        }
+        if (virFileMakePathWithMode(data->file, data->sb.st_mode) < 0)
+            goto cleanup;
+        delDevice = true;
+        /* Just create the folder here so that code below sets
+         * proper owner and mode. Move the mount only after that. */
     } else {
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
                        _("unsupported device type %s 0%o"),
@@ -8746,14 +8769,18 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED,
 # endif
 
     /* Finish mount process started earlier. */
-    if (isReg &&
+    if ((isReg || isDir) &&
         virFileMoveMount(data->target, data->file) < 0)
         goto cleanup;
 
     ret = 0;
  cleanup:
-    if (ret < 0 && delDevice)
-        unlink(data->file);
+    if (ret < 0 && delDevice) {
+        if (isDir)
+            virFileDeleteTree(data->file);
+        else
+            unlink(data->file);
+    }
 # ifdef WITH_SELINUX
     freecon(data->tcon);
 # endif
@@ -8798,8 +8825,9 @@ qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr driver,
 
     isLink = S_ISLNK(data.sb.st_mode);
     isReg = S_ISREG(data.sb.st_mode) || S_ISFIFO(data.sb.st_mode) || S_ISSOCK(data.sb.st_mode);
+    isDir = S_ISDIR(data.sb.st_mode);
 
-    if (isReg && STRPREFIX(file, DEVPREFIX)) {
+    if ((isReg || isDir) && STRPREFIX(file, DEVPREFIX)) {
         cfg = virQEMUDriverGetConfig(driver);
         if (!(target = qemuDomainGetPreservedMountPath(cfg, vm, file)))
             goto cleanup;
-- 
2.14.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: Also treat directories properly when using namespaces
Posted by Martin Kletzander 7 years, 8 months ago
On Fri, Aug 18, 2017 at 05:36:04PM +0200, Martin Kletzander wrote:
>Partially-resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1443434
>
>Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>---
> src/qemu/qemu_domain.c | 38 +++++++++++++++++++++++++++++++++-----
> 1 file changed, 33 insertions(+), 5 deletions(-)
>
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index 40608554c473..b5fc6697ed29 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -7837,6 +7837,7 @@ qemuDomainCreateDeviceRecursive(const char *device,
>     isLink = S_ISLNK(sb.st_mode);
>     isDev = S_ISCHR(sb.st_mode) || S_ISBLK(sb.st_mode);
>     isReg = S_ISREG(sb.st_mode) || S_ISFIFO(sb.st_mode) || S_ISSOCK(sb.st_mode);
>+    isDir = S_ISDIR(sb.st_mode);
>

I forgot to squash this in before sending.  It is already part of the
patch in my local tree, though.

diff --git i/src/qemu/qemu_domain.c w/src/qemu/qemu_domain.c
index 3fc1183cbe7a..fd91dc430252 100644
--- i/src/qemu/qemu_domain.c
+++ w/src/qemu/qemu_domain.c
@@ -7850,6 +7850,7 @@ qemuDomainCreateDeviceRecursive(const char *device,
     bool isLink = false;
     bool isDev = false;
     bool isReg = false;
+    bool isDir = false;
     bool create = false;
 #ifdef WITH_SELINUX
     char *tcon = NULL;
@@ -8840,6 +8841,7 @@ qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr driver,
     char *target = NULL;
     bool isLink;
     bool isReg;
+    bool isDir;

     if (!ttl) {
         virReportSystemError(ELOOP,
--

Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: Also treat directories properly when using namespaces
Posted by John Ferlan 7 years, 8 months ago

On 08/18/2017 11:36 AM, Martin Kletzander wrote:
> Partially-resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1443434
> 

Similar to 1/2 - it's really not clear what this patch has to do with
the referenced bz...  Perhaps a few words on the relationship would make
things clearer.


> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  src/qemu/qemu_domain.c | 38 +++++++++++++++++++++++++++++++++-----
>  1 file changed, 33 insertions(+), 5 deletions(-)
> 

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

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