From: Daniel P. Berrange <berrange@redhat.com>
Date: Tue, 15 Jun 2010 15:40:47 +0000 (+0100)
Subject: Convert all disk backing store loops to shared helper API
X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=a8853344994a7c6aaca882a5e949ab5536821ab5;p=libvirt.git

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()
---

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 143266de5b..6a3942813a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3044,107 +3044,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);
 }
 
 
@@ -3208,7 +3183,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;
         }
 
@@ -8039,7 +8014,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;
         }
 
@@ -8084,7 +8059,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));
         }
@@ -8284,7 +8259,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;
         }
 
@@ -8307,7 +8282,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));
         }
@@ -8434,7 +8409,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));
     }
@@ -8497,7 +8472,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 acfe48eff8..770010d1f4 100644
--- a/src/qemu/qemu_security_dac.c
+++ b/src/qemu/qemu_security_dac.c
@@ -97,46 +97,29 @@ 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 5c0f002dc6..d191118366 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -438,55 +438,44 @@ 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 2c045e67cd..9ed0cd315f 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;
 
@@ -800,6 +799,28 @@ file_iterate_pci_cb(pciDevice *dev ATTRIBUTE_UNUSED,
     return vah_add_file(buf, file, "rw");
 }
 
+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)
 {
@@ -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)