From 6d00b685c31dfa3c049a5a1ce8ec66884a1887a4 Mon Sep 17 00:00:00 2001
Message-Id: <6d00b685c31dfa3c049a5a1ce8ec66884a1887a4.1283420934.git.jdenemar@redhat.com>
From: Daniel P. Berrange <berrange@redhat.com>
Date: Tue, 15 Jun 2010 16:40:47 +0100
Subject: [PATCH] Convert all disk backing store loops to shared helper API

Update the QEMU cgroups code, QEMU DAC security driver, SELinux
and AppArmour security drivers over to use the shared helper API
virDomainDiskDefForeachPath().

* src/qemu/qemu_driver.c, src/qemu/qemu_security_dac.c,
  src/security/security_selinux.c, src/security/virt-aa-helper.c:
  Convert over to use virDomainDiskDefForeachPath()
(cherry picked from commit a8853344994a7c6aaca882a5e949ab5536821ab5)
---
 src/qemu/qemu_driver.c          |  161 ++++++++++++++++----------------------
 src/qemu/qemu_security_dac.c    |   47 ++++--------
 src/security/security_selinux.c |   67 +++++++----------
 src/security/virt-aa-helper.c   |   71 ++++++++----------
 4 files changed, 142 insertions(+), 204 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6ca897b..244f551 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3040,107 +3040,82 @@ static const char *const defaultDeviceACL[] = {
 #define DEVICE_PTY_MAJOR 136
 #define DEVICE_SND_MAJOR 116
 
-static int qemuSetupDiskCgroup(virCgroupPtr cgroup,
-                               virDomainObjPtr vm,
-                               virDomainDiskDefPtr disk)
-{
-    char *path = disk->src;
-    int ret = -1;
 
-    while (path != NULL) {
-        virStorageFileMetadata meta;
-        int rc;
+static int qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
+                                  const char *path,
+                                  size_t depth ATTRIBUTE_UNUSED,
+                                  void *opaque)
+{
+    virCgroupPtr cgroup = opaque;
+    int rc;
 
-        VIR_DEBUG("Process path '%s' for disk", path);
-        rc = virCgroupAllowDevicePath(cgroup, path);
-        if (rc != 0) {
-            /* Get this for non-block devices */
-            if (rc == -EINVAL) {
-                VIR_DEBUG("Ignoring EINVAL for %s", path);
-            } else if (rc == -EACCES) { /* Get this for root squash NFS */
-                VIR_DEBUG("Ignoring EACCES for %s", path);
-            } else {
-                virReportSystemError(-rc,
-                                     _("Unable to allow device %s for %s"),
-                                     path, vm->def->name);
-                if (path != disk->src)
-                    VIR_FREE(path);
-                goto cleanup;
-            }
+    VIR_DEBUG("Process path %s for disk", path);
+    /* XXX RO vs RW */
+    rc = virCgroupAllowDevicePath(cgroup, path);
+    if (rc != 0) {
+        /* Get this for non-block devices */
+        if (rc == -EINVAL) {
+            VIR_DEBUG("Ignoring EINVAL for %s", path);
+        } else if (rc == -EACCES) { /* Get this for root squash NFS */
+            VIR_DEBUG("Ignoring EACCES for %s", path);
+        } else {
+            virReportSystemError(-rc,
+                                 _("Unable to allow access for disk path %s"),
+                                 path);
+            return -1;
         }
-
-        rc = virStorageFileGetMetadata(path,
-                                       VIR_STORAGE_FILE_AUTO,
-                                       &meta);
-        if (rc < 0)
-            VIR_WARN("Unable to lookup parent image for %s", path);
-
-        if (path != disk->src)
-            VIR_FREE(path);
-        path = NULL;
-
-        if (rc < 0)
-            break; /* Treating as non fatal */
-
-        path = meta.backingStore;
     }
+    return 0;
+}
 
-    ret = 0;
 
-cleanup:
-    return ret;
+static int qemuSetupDiskCgroup(virCgroupPtr cgroup,
+                               virDomainDiskDefPtr disk)
+{
+    return virDomainDiskDefForeachPath(disk,
+                                       true,
+                                       true,
+                                       qemuSetupDiskPathAllow,
+                                       cgroup);
 }
 
 
-static int qemuTeardownDiskCgroup(virCgroupPtr cgroup,
-                                  virDomainObjPtr vm,
-                                  virDomainDiskDefPtr disk)
+static int qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
+                                    const char *path,
+                                    size_t depth ATTRIBUTE_UNUSED,
+                                    void *opaque)
 {
-    char *path = disk->src;
-    int ret = -1;
-
-    while (path != NULL) {
-        virStorageFileMetadata meta;
-        int rc;
+    virCgroupPtr cgroup = opaque;
+    int rc;
 
-        VIR_DEBUG("Process path '%s' for disk", path);
-        rc = virCgroupDenyDevicePath(cgroup, path);
-        if (rc != 0) {
-            /* Get this for non-block devices */
-            if (rc == -EINVAL) {
-                VIR_DEBUG("Ignoring EINVAL for %s", path);
-            } else if (rc == -EACCES) { /* Get this for root squash NFS */
-                VIR_DEBUG("Ignoring EACCES for %s", path);
-            } else {
-                virReportSystemError(-rc,
-                                     _("Unable to deny device %s for %s"),
-                                     path, vm->def->name);
-                if (path != disk->src)
-                    VIR_FREE(path);
-                goto cleanup;
-            }
+    VIR_DEBUG("Process path %s for disk", path);
+    /* XXX RO vs RW */
+    rc = virCgroupDenyDevicePath(cgroup, path);
+    if (rc != 0) {
+        /* Get this for non-block devices */
+        if (rc == -EINVAL) {
+            VIR_DEBUG("Ignoring EINVAL for %s", path);
+        } else if (rc == -EACCES) { /* Get this for root squash NFS */
+            VIR_DEBUG("Ignoring EACCES for %s", path);
+        } else {
+            virReportSystemError(-rc,
+                                 _("Unable to allow access for disk path %s"),
+                                 path);
+            return -1;
         }
-
-        rc = virStorageFileGetMetadata(path,
-                                       VIR_STORAGE_FILE_AUTO,
-                                       &meta);
-        if (rc < 0)
-            VIR_WARN("Unable to lookup parent image for %s", path);
-
-        if (path != disk->src)
-            VIR_FREE(path);
-        path = NULL;
-
-        if (rc < 0)
-            break; /* Treating as non fatal */
-
-        path = meta.backingStore;
     }
+    return 0;
+}
 
-    ret = 0;
 
-cleanup:
-    return ret;
+static int qemuTeardownDiskCgroup(virCgroupPtr cgroup,
+                                  virDomainDiskDefPtr disk)
+{
+    return virDomainDiskDefForeachPath(disk,
+                                       true,
+                                       true,
+                                       qemuTeardownDiskPathDeny,
+                                       cgroup);
 }
 
 
@@ -3204,7 +3179,7 @@ static int qemuSetupCgroup(struct qemud_driver *driver,
         }
 
         for (i = 0; i < vm->def->ndisks ; i++) {
-            if (qemuSetupDiskCgroup(cgroup, vm, vm->def->disks[i]) < 0)
+            if (qemuSetupDiskCgroup(cgroup, vm->def->disks[i]) < 0)
                 goto cleanup;
         }
 
@@ -8042,7 +8017,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
                                 vm->def->name);
                 goto endjob;
             }
-            if (qemuSetupDiskCgroup(cgroup, vm, dev->data.disk) < 0)
+            if (qemuSetupDiskCgroup(cgroup, dev->data.disk) < 0)
                 goto endjob;
         }
 
@@ -8087,7 +8062,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
             /* Fallthrough */
         }
         if (ret != 0 && cgroup) {
-            if (qemuTeardownDiskCgroup(cgroup, vm, dev->data.disk) < 0)
+            if (qemuTeardownDiskCgroup(cgroup, dev->data.disk) < 0)
                 VIR_WARN("Failed to teardown cgroup for disk path %s",
                          NULLSTR(dev->data.disk->src));
         }
@@ -8287,7 +8262,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
                                 vm->def->name);
                 goto endjob;
             }
-            if (qemuSetupDiskCgroup(cgroup, vm, dev->data.disk) < 0)
+            if (qemuSetupDiskCgroup(cgroup, dev->data.disk) < 0)
                 goto endjob;
         }
 
@@ -8310,7 +8285,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
         }
 
         if (ret != 0 && cgroup) {
-            if (qemuTeardownDiskCgroup(cgroup, vm, dev->data.disk) < 0)
+            if (qemuTeardownDiskCgroup(cgroup, dev->data.disk) < 0)
                 VIR_WARN("Failed to teardown cgroup for disk path %s",
                          NULLSTR(dev->data.disk->src));
         }
@@ -8437,7 +8412,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver,
         VIR_WARN("Unable to restore security label on %s", dev->data.disk->src);
 
     if (cgroup != NULL) {
-        if (qemuTeardownDiskCgroup(cgroup, vm, dev->data.disk) < 0)
+        if (qemuTeardownDiskCgroup(cgroup, dev->data.disk) < 0)
             VIR_WARN("Failed to teardown cgroup for disk path %s",
                      NULLSTR(dev->data.disk->src));
     }
@@ -8500,7 +8475,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver,
         VIR_WARN("Unable to restore security label on %s", dev->data.disk->src);
 
     if (cgroup != NULL) {
-        if (qemuTeardownDiskCgroup(cgroup, vm, dev->data.disk) < 0)
+        if (qemuTeardownDiskCgroup(cgroup, dev->data.disk) < 0)
             VIR_WARN("Failed to teardown cgroup for disk path %s",
                      NULLSTR(dev->data.disk->src));
     }
diff --git a/src/qemu/qemu_security_dac.c b/src/qemu/qemu_security_dac.c
index acfe48e..770010d 100644
--- a/src/qemu/qemu_security_dac.c
+++ b/src/qemu/qemu_security_dac.c
@@ -98,45 +98,28 @@ err:
 
 
 static int
+qemuSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
+                                    const char *path,
+                                    size_t depth ATTRIBUTE_UNUSED,
+                                    void *opaque ATTRIBUTE_UNUSED)
+{
+    return qemuSecurityDACSetOwnership(path, driver->user, driver->group);
+}
+
+
+static int
 qemuSecurityDACSetSecurityImageLabel(virDomainObjPtr vm ATTRIBUTE_UNUSED,
                                      virDomainDiskDefPtr disk)
 
 {
-    const char *path;
-
     if (!driver->privileged || !driver->dynamicOwnership)
         return 0;
 
-    if (!disk->src)
-        return 0;
-
-    path = disk->src;
-    do {
-        virStorageFileMetadata meta;
-        int ret;
-
-        ret = virStorageFileGetMetadata(path,
-                                        VIR_STORAGE_FILE_AUTO,
-                                        &meta);
-
-        if (path != disk->src)
-            VIR_FREE(path);
-        path = NULL;
-
-        if (ret < 0)
-            return -1;
-
-        if (meta.backingStore != NULL &&
-            qemuSecurityDACSetOwnership(meta.backingStore,
-                                        driver->user, driver->group) < 0) {
-            VIR_FREE(meta.backingStore);
-            return -1;
-        }
-
-        path = meta.backingStore;
-    } while (path != NULL);
-
-    return qemuSecurityDACSetOwnership(disk->src, driver->user, driver->group);
+    return virDomainDiskDefForeachPath(disk,
+                                       true,
+                                       false,
+                                       qemuSecurityDACSetSecurityFileLabel,
+                                       NULL);
 }
 
 
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 5c0f002..d191118 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -439,54 +439,43 @@ SELinuxRestoreSecurityImageLabel(virDomainObjPtr vm,
 
 
 static int
+SELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk,
+                            const char *path,
+                            size_t depth,
+                            void *opaque)
+{
+    const virSecurityLabelDefPtr secdef = opaque;
+
+    if (depth == 0) {
+        if (disk->shared) {
+            return SELinuxSetFilecon(path, default_image_context);
+        } else if (disk->readonly) {
+            return SELinuxSetFilecon(path, default_content_context);
+        } else if (secdef->imagelabel) {
+            return SELinuxSetFilecon(path, secdef->imagelabel);
+        } else {
+            return 0;
+        }
+    } else {
+        return SELinuxSetFilecon(path, default_content_context);
+    }
+}
+
+static int
 SELinuxSetSecurityImageLabel(virDomainObjPtr vm,
                              virDomainDiskDefPtr disk)
 
 {
     const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
-    const char *path;
 
     if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC)
         return 0;
 
-    if (!disk->src)
-        return 0;
-
-    path = disk->src;
-    do {
-        virStorageFileMetadata meta;
-        int ret;
-
-        ret = virStorageFileGetMetadata(path,
-                                        VIR_STORAGE_FILE_AUTO,
-                                        &meta);
-
-        if (path != disk->src)
-            VIR_FREE(path);
-        path = NULL;
-
-        if (ret < 0)
-           break;
-
-        if (meta.backingStore != NULL &&
-            SELinuxSetFilecon(meta.backingStore,
-                              default_content_context) < 0) {
-            VIR_FREE(meta.backingStore);
-            return -1;
-        }
-
-        path = meta.backingStore;
-    } while (path != NULL);
-
-    if (disk->shared) {
-        return SELinuxSetFilecon(disk->src, default_image_context);
-    } else if (disk->readonly) {
-        return SELinuxSetFilecon(disk->src, default_content_context);
-    } else if (secdef->imagelabel) {
-        return SELinuxSetFilecon(disk->src, secdef->imagelabel);
-    }
-
-    return 0;
+    return virDomainDiskDefForeachPath(disk,
+                                       true,
+                                       false,
+                                       SELinuxSetSecurityFileLabel,
+                                       secdef);
 }
 
 
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 2c045e6..9ed0cd3 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -36,7 +36,6 @@
 #include "uuid.h"
 #include "hostusb.h"
 #include "pci.h"
-#include "storage_file.h"
 
 static char *progname;
 
@@ -801,6 +800,28 @@ file_iterate_pci_cb(pciDevice *dev ATTRIBUTE_UNUSED,
 }
 
 static int
+add_file_path(virDomainDiskDefPtr disk,
+              const char *path,
+              size_t depth,
+              void *opaque)
+{
+    virBufferPtr buf = opaque;
+    int ret;
+
+    if (depth == 0) {
+        if (disk->readonly)
+            ret = vah_add_file(buf, path, "r");
+        else
+            ret = vah_add_file(buf, path, "rw");
+    } else {
+        ret = vah_add_file(buf, path, "r");
+    }
+
+    return ret;
+}
+
+
+static int
 get_files(vahControl * ctl)
 {
     virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -821,45 +842,15 @@ get_files(vahControl * ctl)
         goto clean;
     }
 
-    for (i = 0; i < ctl->def->ndisks; i++)
-        if (ctl->def->disks[i] && ctl->def->disks[i]->src) {
-            int ret;
-            const char *path;
-
-            path = ctl->def->disks[i]->src;
-            do {
-                virStorageFileMetadata meta;
-
-                ret = virStorageFileGetMetadata(path,
-                                                VIR_STORAGE_FILE_AUTO,
-                                                &meta);
-
-                if (path != ctl->def->disks[i]->src)
-                    VIR_FREE(path);
-                path = NULL;
-
-                if (ret < 0) {
-                    vah_warning("could not open path, skipping");
-                    continue;
-                }
-
-                if (meta.backingStore != NULL &&
-                    (ret = vah_add_file(&buf, meta.backingStore, "rw")) != 0) {
-                    VIR_FREE(meta.backingStore);
-                    goto clean;
-                }
-
-                path = meta.backingStore;
-            } while (path != NULL);
-
-            if (ctl->def->disks[i]->readonly)
-                ret = vah_add_file(&buf, ctl->def->disks[i]->src, "r");
-            else
-                ret = vah_add_file(&buf, ctl->def->disks[i]->src, "rw");
-
-            if (ret != 0)
-                goto clean;
-        }
+    for (i = 0; i < ctl->def->ndisks; i++) {
+        int ret = virDomainDiskDefForeachPath(ctl->def->disks[i],
+                                              true,
+                                              false,
+                                              add_file_path,
+                                              &buf);
+        if (ret != 0)
+            goto clean;
+    }
 
     for (i = 0; i < ctl->def->nserials; i++)
         if (ctl->def->serials[i] && ctl->def->serials[i]->data.file.path)
-- 
1.7.2.2