[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCHv3 42/43] snapshot: refactor qemu file opening



In a SELinux or root-squashing NFS environment, libvirt has to go
through some hoops to create a new file that qemu can then open()
by name.  Snapshots are a case where we want to guarantee an empty
file that qemu can open, so refactor some existing code to make
it easier to reuse in the next patch.

* src/qemu/qemu_driver.c (qemuOpenFile): New function, pulled from...
(qemuDomainSaveInternal): ...here.
---
 src/qemu/qemu_driver.c |  228 ++++++++++++++++++++++++++----------------------
 1 files changed, 123 insertions(+), 105 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 673d751..48313ad 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2450,6 +2450,125 @@ qemuCompressProgramName(int compress)
             qemudSaveCompressionTypeToString(compress));
 }

+/* Internal function to properly create or open existing files, with
+ * ownership affected by qemu driver setup.  */
+static int
+qemuOpenFile(struct qemud_driver *driver, const char *path, bool *isReg,
+             bool *bypassSecurityDriver, bool bypassCache)
+{
+    struct stat sb;
+    bool is_reg;
+    bool bypass_security = false;
+    int fd = -1;
+    uid_t uid = getuid();
+    gid_t gid = getgid();
+    int directFlag = 0;
+
+    /* path might be a pre-existing block dev, in which case
+     * we need to skip the create step, and also avoid unlink
+     * in the failure case */
+    if (stat(path, &sb) < 0) {
+        /* Avoid throwing an error here, since it is possible
+         * that with NFS we can't actually stat() the file.
+         * The subsequent codepaths will still raise an error
+         * if a truly fatal problem is hit */
+        is_reg = true;
+    } else {
+        is_reg = !!S_ISREG(sb.st_mode);
+        /* If the path is regular file which exists
+         * already and dynamic_ownership is off, we don't
+         * want to change it's ownership, just open it as-is */
+        if (is_reg && !driver->dynamicOwnership) {
+            uid = sb.st_uid;
+            gid = sb.st_gid;
+        }
+    }
+
+    /* First try creating the file as root */
+    if (bypassCache) {
+        directFlag = virFileDirectFdFlag();
+        if (directFlag < 0) {
+            qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
+                            _("bypass cache unsupported by this system"));
+            goto cleanup;
+        }
+    }
+    if (!is_reg) {
+        fd = open(path, O_WRONLY | O_TRUNC | directFlag);
+        if (fd < 0) {
+            virReportSystemError(errno, _("unable to open %s"), path);
+            goto cleanup;
+        }
+    } else {
+        int oflags = O_CREAT | O_TRUNC | O_WRONLY | directFlag;
+        if ((fd = virFileOpenAs(path, oflags, S_IRUSR | S_IWUSR,
+                                uid, gid, 0)) < 0) {
+            /* If we failed as root, and the error was permission-denied
+               (EACCES or EPERM), assume it's on a network-connected share
+               where root access is restricted (eg, root-squashed NFS). If the
+               qemu user (driver->user) is non-root, just set a flag to
+               bypass security driver shenanigans, and retry the operation
+               after doing setuid to qemu user */
+            if ((fd != -EACCES && fd != -EPERM) ||
+                driver->user == getuid()) {
+                virReportSystemError(-fd,
+                                     _("Failed to create file '%s'"),
+                                     path);
+                goto cleanup;
+            }
+
+            /* On Linux we can also verify the FS-type of the directory. */
+            switch (virStorageFileIsSharedFS(path)) {
+                case 1:
+                   /* it was on a network share, so we'll continue
+                    * as outlined above
+                    */
+                   break;
+
+                case -1:
+                   virReportSystemError(errno,
+                                        _("Failed to create file "
+                                          "'%s': couldn't determine fs type"),
+                                        path);
+                   goto cleanup;
+
+                case 0:
+                default:
+                   /* local file - log the error returned by virFileOpenAs */
+                   virReportSystemError(-fd,
+                                        _("Failed to create file '%s'"),
+                                        path);
+                   goto cleanup;
+            }
+
+            /* Retry creating the file as driver->user */
+
+            if ((fd = virFileOpenAs(path, oflags,
+                                    S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP,
+                                    driver->user, driver->group,
+                                    VIR_FILE_OPEN_AS_UID)) < 0) {
+                virReportSystemError(-fd,
+                                   _("Error from child process creating '%s'"),
+                                     path);
+                goto cleanup;
+            }
+
+            /* Since we had to setuid to create the file, and the fstype
+               is NFS, we assume it's a root-squashing NFS share, and that
+               the security driver stuff would have failed anyway */
+
+            bypass_security = true;
+        }
+    }
+cleanup:
+    if (isReg)
+        *isReg = is_reg;
+    if (bypassSecurityDriver)
+        *bypassSecurityDriver = bypass_security;
+
+    return fd;
+}
+
 /* This internal function expects the driver lock to already be held on
  * entry and the vm must be active + locked. Vm will be unlocked and
  * potentially free'd after this returns (eg transient VMs are freed
@@ -2468,15 +2587,11 @@ qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom,
     int rc;
     virDomainEventPtr event = NULL;
     qemuDomainObjPrivatePtr priv;
-    struct stat sb;
     bool is_reg = false;
     size_t len;
     unsigned long long offset;
     unsigned long long pad;
     int fd = -1;
-    uid_t uid = getuid();
-    gid_t gid = getgid();
-    int directFlag = 0;
     virFileDirectFdPtr directFd = NULL;

     memset(&header, 0, sizeof(header));
@@ -2557,107 +2672,10 @@ qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom,
     header.xml_len = len;

     /* Obtain the file handle.  */
-    /* path might be a pre-existing block dev, in which case
-     * we need to skip the create step, and also avoid unlink
-     * in the failure case */
-    if (stat(path, &sb) < 0) {
-        /* Avoid throwing an error here, since it is possible
-         * that with NFS we can't actually stat() the file.
-         * The subsequent codepaths will still raise an error
-         * if a truly fatal problem is hit */
-        is_reg = true;
-    } else {
-        is_reg = !!S_ISREG(sb.st_mode);
-        /* If the path is regular file which exists
-         * already and dynamic_ownership is off, we don't
-         * want to change it's ownership, just open it as-is */
-        if (is_reg && !driver->dynamicOwnership) {
-            uid=sb.st_uid;
-            gid=sb.st_gid;
-        }
-    }
-
-    /* First try creating the file as root */
-    if (bypass_cache) {
-        directFlag = virFileDirectFdFlag();
-        if (directFlag < 0) {
-            qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
-                            _("bypass cache unsupported by this system"));
-            goto endjob;
-        }
-    }
-    if (!is_reg) {
-        fd = open(path, O_WRONLY | O_TRUNC | directFlag);
-        if (fd < 0) {
-            virReportSystemError(errno, _("unable to open %s"), path);
-            goto endjob;
-        }
-    } else {
-        int oflags = O_CREAT | O_TRUNC | O_WRONLY | directFlag;
-        if ((fd = virFileOpenAs(path, oflags, S_IRUSR | S_IWUSR,
-                                uid, gid, 0)) < 0) {
-            /* If we failed as root, and the error was permission-denied
-               (EACCES or EPERM), assume it's on a network-connected share
-               where root access is restricted (eg, root-squashed NFS). If the
-               qemu user (driver->user) is non-root, just set a flag to
-               bypass security driver shenanigans, and retry the operation
-               after doing setuid to qemu user */
-            rc = fd;
-            if (((rc != -EACCES) && (rc != -EPERM)) ||
-                driver->user == getuid()) {
-                virReportSystemError(-rc,
-                                    _("Failed to create domain save file '%s'"),
-                                     path);
-                goto endjob;
-            }
-
-            /* On Linux we can also verify the FS-type of the directory. */
-            switch (virStorageFileIsSharedFS(path)) {
-                case 1:
-                   /* it was on a network share, so we'll continue
-                    * as outlined above
-                    */
-                   break;
-
-                case -1:
-                   virReportSystemError(errno,
-                                        _("Failed to create domain save file "
-                                          "'%s': couldn't determine fs type"),
-                                        path);
-                   goto endjob;
-                   break;
-
-                case 0:
-                default:
-                   /* local file - log the error returned by virFileOpenAs */
-                   virReportSystemError(-rc,
-                                    _("Failed to create domain save file '%s'"),
-                                        path);
-                   goto endjob;
-                   break;
-
-            }
-
-            /* Retry creating the file as driver->user */
-
-            if ((fd = virFileOpenAs(path, oflags,
-                                    S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP,
-                                    driver->user, driver->group,
-                                    VIR_FILE_OPEN_AS_UID)) < 0) {
-                virReportSystemError(-fd,
-                                   _("Error from child process creating '%s'"),
-                                     path);
-                goto endjob;
-            }
-
-            /* Since we had to setuid to create the file, and the fstype
-               is NFS, we assume it's a root-squashing NFS share, and that
-               the security driver stuff would have failed anyway */
-
-            bypassSecurityDriver = true;
-        }
-    }
-
+    fd = qemuOpenFile(driver, path, &is_reg, &bypassSecurityDriver,
+                      bypass_cache);
+    if (fd < 0)
+        goto endjob;
     if (bypass_cache && (directFd = virFileDirectFdNew(&fd, path)) == NULL)
         goto endjob;

-- 
1.7.4.4


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]