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

[PATCH] qemu: Move virQEMUFileOpenAs to qemu_domain.c



Commit 43620689794507308fbd3def6992a68ee2f8fa97 moved the function to
util/virqemu.c which is compiled also on win32 and geteuid()/getegid()
doesn't exist there.

Move it to qemu_domain.c which is compiled only when the qemu driver is
enabled. Originally I didn't want to put it here as qemu_domain.c is a
code dump for helper functions but this is the least invasive fix.

Signed-off-by: Peter Krempa <pkrempa redhat com>
---
 src/libvirt_private.syms |   1 -
 src/qemu/qemu_domain.c   | 124 +++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_domain.h   |   7 +++
 src/util/virqemu.c       | 130 ---------------------------------------
 src/util/virqemu.h       |   7 ---
 5 files changed, 131 insertions(+), 138 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d737e4d9e4..01c2e710cd 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2939,7 +2939,6 @@ virQEMUBuildDriveCommandlineFromJSON;
 virQEMUBuildNetdevCommandlineFromJSON;
 virQEMUBuildObjectCommandlineFromJSON;
 virQEMUBuildQemuImgKeySecretOpts;
-virQEMUFileOpenAs;


 # util/virrandom.h
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 715815f224..b321722f0e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -10828,6 +10828,130 @@ qemuDomainDiskBlockJobIsSupported(virDomainObjPtr vm,
 }


+int
+virQEMUFileOpenAs(uid_t fallback_uid,
+                  gid_t fallback_gid,
+                  bool dynamicOwnership,
+                  const char *path,
+                  int oflags,
+                  bool *needUnlink)
+{
+    struct stat sb;
+    bool is_reg = true;
+    bool need_unlink = false;
+    unsigned int vfoflags = 0;
+    int fd = -1;
+    int path_shared = virFileIsSharedFS(path);
+    uid_t uid = geteuid();
+    gid_t gid = getegid();
+
+    /* 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 (oflags & O_CREAT) {
+        need_unlink = true;
+
+        /* Don't force chown on network-shared FS
+         * as it is likely to fail. */
+        if (path_shared <= 0 || dynamicOwnership)
+            vfoflags |= VIR_FILE_OPEN_FORCE_OWNER;
+
+        if (stat(path, &sb) == 0) {
+            /* It already exists, we don't want to delete it on error */
+            need_unlink = false;
+
+            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 its ownership, just open it as-is */
+            if (is_reg && !dynamicOwnership) {
+                uid = sb.st_uid;
+                gid = sb.st_gid;
+            }
+        }
+    }
+
+    /* First try creating the file as root */
+    if (!is_reg) {
+        if ((fd = open(path, oflags & ~O_CREAT)) < 0) {
+            fd = -errno;
+            goto error;
+        }
+    } else {
+        if ((fd = virFileOpenAs(path, oflags, S_IRUSR | S_IWUSR, uid, gid,
+                                vfoflags | VIR_FILE_OPEN_NOFORK)) < 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 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) || fallback_uid == geteuid())
+                goto error;
+
+            /* On Linux we can also verify the FS-type of the directory. */
+            switch (path_shared) {
+                case 1:
+                    /* it was on a network share, so we'll continue
+                     * as outlined above
+                     */
+                    break;
+
+                case -1:
+                    virReportSystemError(-fd, oflags & O_CREAT
+                                         ? _("Failed to create file "
+                                             "'%s': couldn't determine fs type")
+                                         : _("Failed to open file "
+                                             "'%s': couldn't determine fs type"),
+                                         path);
+                    goto cleanup;
+
+                case 0:
+                default:
+                    /* local file - log the error returned by virFileOpenAs */
+                    goto error;
+            }
+
+            /* If we created the file above, then we need to remove it;
+             * otherwise, the next attempt to create will fail. If the
+             * file had already existed before we got here, then we also
+             * don't want to delete it and allow the following to succeed
+             * or fail based on existing protections
+             */
+            if (need_unlink)
+                unlink(path);
+
+            /* Retry creating the file as qemu user */
+
+            /* Since we're passing different modes... */
+            vfoflags |= VIR_FILE_OPEN_FORCE_MODE;
+
+            if ((fd = virFileOpenAs(path, oflags,
+                                    S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP,
+                                    fallback_uid, fallback_gid,
+                                    vfoflags | VIR_FILE_OPEN_FORK)) < 0) {
+                virReportSystemError(-fd, oflags & O_CREAT
+                                     ? _("Error from child process creating '%s'")
+                                     : _("Error from child process opening '%s'"),
+                                     path);
+                goto cleanup;
+            }
+        }
+    }
+ cleanup:
+    if (needUnlink)
+        *needUnlink = need_unlink;
+    return fd;
+
+ error:
+    virReportSystemError(-fd, oflags & O_CREAT
+                         ? _("Failed to create file '%s'")
+                         : _("Failed to open file '%s'"),
+                         path);
+    goto cleanup;
+}
+
+
 /**
  * qemuDomainOpenFile:
  * @driver: driver object
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 70fdb48317..e6d4acb8d4 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1024,6 +1024,13 @@ void qemuDomainRemoveInactiveJob(virQEMUDriverPtr driver,
 void qemuDomainRemoveInactiveJobLocked(virQEMUDriverPtr driver,
                                        virDomainObjPtr vm);

+int virQEMUFileOpenAs(uid_t fallback_uid,
+                      gid_t fallback_gid,
+                      bool dynamicOwnership,
+                      const char *path,
+                      int oflags,
+                      bool *needUnlink);
+
 int
 qemuDomainOpenFile(virQEMUDriverPtr driver,
                    virDomainObjPtr vm,
diff --git a/src/util/virqemu.c b/src/util/virqemu.c
index e1c8673390..20cb09d878 100644
--- a/src/util/virqemu.c
+++ b/src/util/virqemu.c
@@ -22,18 +22,12 @@

 #include <config.h>

-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <unistd.h>
-
 #include "virbuffer.h"
 #include "virerror.h"
 #include "virlog.h"
 #include "virqemu.h"
 #include "virstring.h"
 #include "viralloc.h"
-#include "virfile.h"

 #define VIR_FROM_THIS VIR_FROM_NONE

@@ -447,127 +441,3 @@ virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf,
         virBufferAddLit(buf, ",");
     }
 }
-
-
-int
-virQEMUFileOpenAs(uid_t fallback_uid,
-                  gid_t fallback_gid,
-                  bool dynamicOwnership,
-                  const char *path,
-                  int oflags,
-                  bool *needUnlink)
-{
-    struct stat sb;
-    bool is_reg = true;
-    bool need_unlink = false;
-    unsigned int vfoflags = 0;
-    int fd = -1;
-    int path_shared = virFileIsSharedFS(path);
-    uid_t uid = geteuid();
-    gid_t gid = getegid();
-
-    /* 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 (oflags & O_CREAT) {
-        need_unlink = true;
-
-        /* Don't force chown on network-shared FS
-         * as it is likely to fail. */
-        if (path_shared <= 0 || dynamicOwnership)
-            vfoflags |= VIR_FILE_OPEN_FORCE_OWNER;
-
-        if (stat(path, &sb) == 0) {
-            /* It already exists, we don't want to delete it on error */
-            need_unlink = false;
-
-            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 its ownership, just open it as-is */
-            if (is_reg && !dynamicOwnership) {
-                uid = sb.st_uid;
-                gid = sb.st_gid;
-            }
-        }
-    }
-
-    /* First try creating the file as root */
-    if (!is_reg) {
-        if ((fd = open(path, oflags & ~O_CREAT)) < 0) {
-            fd = -errno;
-            goto error;
-        }
-    } else {
-        if ((fd = virFileOpenAs(path, oflags, S_IRUSR | S_IWUSR, uid, gid,
-                                vfoflags | VIR_FILE_OPEN_NOFORK)) < 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 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) || fallback_uid == geteuid())
-                goto error;
-
-            /* On Linux we can also verify the FS-type of the directory. */
-            switch (path_shared) {
-                case 1:
-                    /* it was on a network share, so we'll continue
-                     * as outlined above
-                     */
-                    break;
-
-                case -1:
-                    virReportSystemError(-fd, oflags & O_CREAT
-                                         ? _("Failed to create file "
-                                             "'%s': couldn't determine fs type")
-                                         : _("Failed to open file "
-                                             "'%s': couldn't determine fs type"),
-                                         path);
-                    goto cleanup;
-
-                case 0:
-                default:
-                    /* local file - log the error returned by virFileOpenAs */
-                    goto error;
-            }
-
-            /* If we created the file above, then we need to remove it;
-             * otherwise, the next attempt to create will fail. If the
-             * file had already existed before we got here, then we also
-             * don't want to delete it and allow the following to succeed
-             * or fail based on existing protections
-             */
-            if (need_unlink)
-                unlink(path);
-
-            /* Retry creating the file as qemu user */
-
-            /* Since we're passing different modes... */
-            vfoflags |= VIR_FILE_OPEN_FORCE_MODE;
-
-            if ((fd = virFileOpenAs(path, oflags,
-                                    S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP,
-                                    fallback_uid, fallback_gid,
-                                    vfoflags | VIR_FILE_OPEN_FORK)) < 0) {
-                virReportSystemError(-fd, oflags & O_CREAT
-                                     ? _("Error from child process creating '%s'")
-                                     : _("Error from child process opening '%s'"),
-                                     path);
-                goto cleanup;
-            }
-        }
-    }
- cleanup:
-    if (needUnlink)
-        *needUnlink = need_unlink;
-    return fd;
-
- error:
-    virReportSystemError(-fd, oflags & O_CREAT
-                         ? _("Failed to create file '%s'")
-                         : _("Failed to open file '%s'"),
-                         path);
-    goto cleanup;
-}
diff --git a/src/util/virqemu.h b/src/util/virqemu.h
index e4e071f7c5..b1296cb657 100644
--- a/src/util/virqemu.h
+++ b/src/util/virqemu.h
@@ -63,10 +63,3 @@ void virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf,
                                       virStorageEncryptionInfoDefPtr enc,
                                       const char *alias)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
-
-int virQEMUFileOpenAs(uid_t fallback_uid,
-                      gid_t fallback_gid,
-                      bool dynamicOwnership,
-                      const char *path,
-                      int oflags,
-                      bool *needUnlink);
-- 
2.26.2


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