qemu: Deduplicate code in qemuSecurityChownCallback()

The DAC security driver has an option to register a callback that
is called instead of chown(). So far QEMU is the only user of
this feature and it's used to set labels on non-local disks (like
gluster), where exists notion of owners but regular chown() can't
be used.

However, this callback (if set) is called always, even for local
disks. And thus the QEMU's implementation duplicated parts of the
DAC driver to deal with chown().

If the DAC driver would call the callback only for non-local
disks then the QEMU's callback can be shorter.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1ee0e7ebc0abe012e78257288cf1b42dca888344..235f575901cf6bc8e011cf89c06bfeb61bc5ab1c 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -228,31 +228,13 @@ qemuSecurityChownCallback(const virStorageSource *src,
                           uid_t uid,
                           gid_t gid)
 {
-    struct stat sb;
     int save_errno = 0;
     int ret = -1;
     int rv;
     g_autoptr(virStorageSource) cpy = NULL;
 
-    if (virStorageSourceIsLocalStorage(src)) {
-        /* use direct chown for local files so that the file doesn't
-         * need to be initialized */
-        if (!src->path)
-            return 0;
-
-        if (stat(src->path, &sb) >= 0) {
-            if (sb.st_uid == uid &&
-                sb.st_gid == gid) {
-                /* It's alright, there's nothing to change anyway. */
-                return 0;
-            }
-        }
-
-        if (chown(src->path, uid, gid) < 0)
-            return -1;
-
-        return 0;
-    }
+    if (virStorageSourceIsLocalStorage(src))
+        return -3;
 
     if ((rv = virStorageSourceSupportsSecurityDriver(src)) <= 0)
         return rv;

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 7ba367755a8bb3f49a5c11eba33f5ed2409d05b9..4909107fcc1d13dab0d397b42eecde75aa3f79ba 100644 (file)
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -672,7 +672,7 @@ virSecurityDACSetOwnershipInternal(const virSecurityDACData *priv,
                                    uid_t uid,
                                    gid_t gid)
 {
-    int rc;
+    int rc = 0;
 
     /* Be aware that this function might run in a separate process.
      * Therefore, any driver state changes would be thrown away. */
@@ -683,7 +683,9 @@ virSecurityDACSetOwnershipInternal(const virSecurityDACData *priv,
         /* on -2 returned an error was already reported */
         if (rc == -2)
             return -1;
-    } else {
+    }
+
+    if (rc == 0 || rc == -3) {
         struct stat sb;
 
         if (!path)

diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index b5c81e9d989f286268c24e5bd3337d2833752dd8..57047ccb137d7e0f098bf887b0171e420687d78a 100644 (file)
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -53,9 +53,16 @@ int virSecurityManagerStackAddNested(virSecurityManager *stack,
  * @uid: target uid
  * @gid: target gid
  *
- * A function callback to chown image files described by the disk source struct
- * @src. The callback shall return 0 on success, -1 on error and errno set (no
- * libvirt error reported) OR -2 and a libvirt error reported. */
+ * A function callback to chown image files described by the disk
+ * source struct @src. The callback can decide to skip given @src
+ * and thus let DAC driver chown the file instead (signalled by
+ * returning -3).
+ *
+ * Returns: 0 on success,
+ *         -1 on error and errno set (no libvirt error reported),
+ *         -2 and a libvirt error reported.
+ *         -3 if callback did not handle chown
+ */
 typedef int
 (*virSecurityManagerDACChownCallback)(const virStorageSource *src,
                                       uid_t uid,