[libvirt] [PATCHv3 00/19] qemu block-commit support

Eric Blake eblake at redhat.com
Wed Oct 17 22:30:11 UTC 2012


This v3 posting resolves all the comments I had from Doug, Laine,
and myself, and has passed my testing with SELinux enabled.

v2 was here:
https://www.redhat.com/archives/libvir-list/2012-October/msg00633.html
See below for interdiff, and individual patches for more notes
about changes

Also available at:
 http://repo.or.cz/w/libvirt/ericb.git/shortlog/refs/heads/blockjob
 git fetch git://repo.or.cz/libvirt/ericb.git blockjob

Eric Blake (19):
  storage: list more file types
  storage: treat 'aio' like 'raw' at parse time
  storage: match RNG to supported driver types
  storage: use enum for default driver type
  storage: use enum for disk driver type
  storage: use enum for snapshot driver type
  storage: don't probe non-files
  storage: get entire metadata chain in one call
  storage: don't require caller to pre-allocate metadata struct
  storage: remember relative names in backing chain
  storage: make it easier to find file within chain
  storage: cache backing chain while qemu domain is live
  storage: use cache to walk backing chain
  blockjob: remove unused parameters after previous patch
  blockjob: manage qemu block-commit monitor command
  blockjob: wire up online qemu block-commit
  blockjob: implement shallow commit flag in qemu
  blockjob: refactor qemu disk chain permission grants
  blockjob: properly label disks for qemu block-commit

 docs/schemas/domaincommon.rng                      |  27 +-
 docs/schemas/domainsnapshot.rng                    |   2 +-
 src/conf/capabilities.h                            |   4 +-
 src/conf/domain_conf.c                             | 165 +++------
 src/conf/domain_conf.h                             |   8 +-
 src/conf/snapshot_conf.c                           |  23 +-
 src/conf/snapshot_conf.h                           |   2 +-
 src/conf/storage_conf.c                            |  15 +-
 src/libvirt.c                                      |   2 -
 src/libvirt_private.syms                           |   1 +
 src/libxl/libxl_conf.c                             |  42 ++-
 src/libxl/libxl_driver.c                           |   6 +-
 src/qemu/qemu_capabilities.c                       |   3 +
 src/qemu/qemu_capabilities.h                       |   1 +
 src/qemu/qemu_cgroup.c                             |  14 +-
 src/qemu/qemu_cgroup.h                             |   6 +-
 src/qemu/qemu_command.c                            |  18 +-
 src/qemu/qemu_domain.c                             |  33 +-
 src/qemu/qemu_domain.h                             |   3 +
 src/qemu/qemu_driver.c                             | 371 ++++++++++++++-------
 src/qemu/qemu_hotplug.c                            |  13 +-
 src/qemu/qemu_monitor.c                            |  30 ++
 src/qemu/qemu_monitor.h                            |   7 +
 src/qemu/qemu_monitor_json.c                       |  34 ++
 src/qemu/qemu_monitor_json.h                       |   7 +
 src/qemu/qemu_process.c                            |  11 +
 src/security/security_dac.c                        |   7 -
 src/security/security_selinux.c                    |  11 -
 src/security/virt-aa-helper.c                      |  20 +-
 src/storage/storage_backend_fs.c                   |  15 +-
 src/util/storage_file.c                            | 282 ++++++++++++----
 src/util/storage_file.h                            |  43 ++-
 src/vbox/vbox_tmpl.c                               |   6 +-
 src/xenxs/xen_sxpr.c                               |  26 +-
 src/xenxs/xen_xm.c                                 |  28 +-
 tests/sexpr2xmldata/sexpr2xml-curmem.xml           |   2 +-
 .../sexpr2xml-disk-block-shareable.xml             |   2 +-
 .../sexpr2xml-disk-drv-blktap-raw.xml              |   2 +-
 .../sexpr2xml-disk-drv-blktap2-raw.xml             |   2 +-
 39 files changed, 859 insertions(+), 435 deletions(-)


diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 81cb3aa..afa4cfe 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -971,7 +971,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
     VIR_FREE(def->src);
     VIR_FREE(def->dst);
     VIR_FREE(def->driverName);
-    virStorageFileFreeMetadata(def->chain);
+    virStorageFileFreeMetadata(def->backingChain);
     VIR_FREE(def->mirror);
     VIR_FREE(def->auth.username);
     VIR_FREE(def->wwn);
@@ -3723,8 +3723,12 @@ virDomainDiskDefParseXML(virCapsPtr caps,
                        xmlStrEqual(cur->name, BAD_CAST "driver")) {
                 driverName = virXMLPropString(cur, "name");
                 driverType = virXMLPropString(cur, "type");
-                if (STREQ_NULLABLE(driverType, "aio"))
-                    memcpy(driverType, "raw", strlen("raw"));
+                if (STREQ_NULLABLE(driverType, "aio")) {
+                    /* In-place conversion to "raw", for Xen back-compat */
+                    driverType[0] = 'r';
+                    driverType[1] = 'a';
+                    driverType[2] = 'w';
+                }
                 cachetag = virXMLPropString(cur, "cache");
                 error_policy = virXMLPropString(cur, "error_policy");
                 rerror_policy = virXMLPropString(cur, "rerror_policy");
@@ -4185,7 +4189,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
                            driverType);
             goto error;
         }
-    } else {
+    } else if (def->type == VIR_DOMAIN_DISK_TYPE_FILE ||
+               def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) {
         def->format = caps->defaultDiskDriverType;
     }

@@ -14763,10 +14768,10 @@ done:


 /* Call iter(disk, name, depth, opaque) for each element of disk and
-   its backing chain in the pre-populated disk->chain.
-   ignoreOpenFailure determines whether to warn about a chain that
-   mentions a backing file without also having metadata on that
-   file.  */
+ * its backing chain in the pre-populated disk->backingChain.
+ * ignoreOpenFailure determines whether to warn about a chain that
+ * mentions a backing file without also having metadata on that
+ * file.  */
 int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
                                 bool ignoreOpenFailure,
                                 virDomainDiskDefPathIterator iter,
@@ -14782,7 +14787,7 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
     if (iter(disk, disk->src, 0, opaque) < 0)
         goto cleanup;

-    tmp = disk->chain;
+    tmp = disk->backingChain;
     while (tmp && tmp->backingStoreIsFile) {
         if (!ignoreOpenFailure && !tmp->backingMeta) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9c3abec..10ef841 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -569,7 +569,7 @@ struct _virDomainDiskDef {
     } auth;
     char *driverName;
     int format; /* enum virStorageFileFormat */
-    virStorageFileMetadataPtr chain;
+    virStorageFileMetadataPtr backingChain;

     char *mirror;
     int mirrorFormat; /* enum virStorageFileFormat */
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 428befd..db371a0 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -87,8 +87,7 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk,
 }


-int qemuSetupDiskCgroup(struct qemud_driver *driver ATTRIBUTE_UNUSED,
-                        virDomainObjPtr vm,
+int qemuSetupDiskCgroup(virDomainObjPtr vm,
                         virCgroupPtr cgroup,
                         virDomainDiskDefPtr disk)
 {
@@ -127,8 +126,7 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
 }


-int qemuTeardownDiskCgroup(struct qemud_driver *driver ATTRIBUTE_UNUSED,
-                           virDomainObjPtr vm,
+int qemuTeardownDiskCgroup(virDomainObjPtr vm,
                            virCgroupPtr cgroup,
                            virDomainDiskDefPtr disk)
 {
@@ -230,7 +228,7 @@ int qemuSetupCgroup(struct qemud_driver *driver,
         for (i = 0; i < vm->def->ndisks ; i++) {
             if (qemuDomainDetermineDiskChain(driver, vm->def->disks[i],
                                              false) < 0 ||
-                qemuSetupDiskCgroup(driver, vm, cgroup, vm->def->disks[i]) < 0)
+                qemuSetupDiskCgroup(vm, cgroup, vm->def->disks[i]) < 0)
                 goto cleanup;
         }

diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index 362080a..c552162 100644
--- a/src/qemu/qemu_cgroup.h
+++ b/src/qemu/qemu_cgroup.h
@@ -36,12 +36,10 @@ typedef struct _qemuCgroupData qemuCgroupData;

 bool qemuCgroupControllerActive(struct qemud_driver *driver,
                                 int controller);
-int qemuSetupDiskCgroup(struct qemud_driver *driver,
-                        virDomainObjPtr vm,
+int qemuSetupDiskCgroup(virDomainObjPtr vm,
                         virCgroupPtr cgroup,
                         virDomainDiskDefPtr disk);
-int qemuTeardownDiskCgroup(struct qemud_driver *driver,
-                           virDomainObjPtr vm,
+int qemuTeardownDiskCgroup(virDomainObjPtr vm,
                            virCgroupPtr cgroup,
                            virDomainDiskDefPtr disk);
 int qemuSetupHostUsbDeviceCgroup(usbDevice *dev,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f071769..4196caf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2129,7 +2129,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
           disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) {
         if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) {
             /* QEMU only supports magic FAT format for now */
-            if (disk->format && disk->format != VIR_STORAGE_FILE_FAT) {
+            if (disk->format > 0 && disk->format != VIR_STORAGE_FILE_FAT) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("unsupported disk driver type for '%s'"),
                                virStorageFileFormatTypeToString(disk->format));
@@ -5210,7 +5210,7 @@ qemuBuildCommandLine(virConnectPtr conn,

             if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) {
                 /* QEMU only supports magic FAT format for now */
-                if (disk->format && disk->format != VIR_STORAGE_FILE_FAT) {
+                if (disk->format > 0 && disk->format != VIR_STORAGE_FILE_FAT) {
                     virReportError(VIR_ERR_INTERNAL_ERROR,
                                    _("unsupported disk driver type for '%s'"),
                                    virStorageFileFormatTypeToString(disk->format));
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9675454..45f3a5e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2016,30 +2016,23 @@ qemuDomainDetermineDiskChain(struct qemud_driver *driver,
                              virDomainDiskDefPtr disk,
                              bool force)
 {
-    int format;
+    bool probe = driver->allowDiskFormatProbing;

     if (!disk->src)
         return 0;

-    if (disk->chain) {
+    if (disk->backingChain) {
         if (force) {
-            virStorageFileFreeMetadata(disk->chain);
-            disk->chain = NULL;
+            virStorageFileFreeMetadata(disk->backingChain);
+            disk->backingChain = NULL;
         } else {
             return 0;
         }
     }
-    if (disk->format > 0)
-        format = disk->format;
-    else if (driver->allowDiskFormatProbing)
-        format = VIR_STORAGE_FILE_AUTO;
-    else
-        format = VIR_STORAGE_FILE_RAW;
-
-    disk->chain = virStorageFileGetMetadata(disk->src, format,
-                                            driver->user, driver->group,
-                                            driver->allowDiskFormatProbing);
-    if (!disk->chain && !force)
+    disk->backingChain = virStorageFileGetMetadata(disk->src, disk->format,
+                                                   driver->user, driver->group,
+                                                   probe);
+    if (!disk->backingChain)
         return -1;
     return 0;
 }
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7a47cf7..3829a89 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5826,7 +5826,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
                            vm->def->name);
             goto end;
         }
-        if (qemuSetupDiskCgroup(driver, vm, cgroup, disk) < 0)
+        if (qemuSetupDiskCgroup(vm, cgroup, disk) < 0)
             goto end;
     }
     switch (disk->device)  {
@@ -5862,7 +5862,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
     }

     if (ret != 0 && cgroup) {
-        if (qemuTeardownDiskCgroup(driver, vm, cgroup, disk) < 0)
+        if (qemuTeardownDiskCgroup(vm, cgroup, disk) < 0)
             VIR_WARN("Failed to teardown cgroup for disk path %s",
                      NULLSTR(disk->src));
     }
@@ -6058,7 +6058,7 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm,
                            vm->def->name);
             goto end;
         }
-        if (qemuSetupDiskCgroup(driver, vm, cgroup, disk) < 0)
+        if (qemuSetupDiskCgroup(vm, cgroup, disk) < 0)
             goto end;
     }

@@ -6077,7 +6077,7 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm,
     }

     if (ret != 0 && cgroup) {
-        if (qemuTeardownDiskCgroup(driver, vm, cgroup, disk) < 0)
+        if (qemuTeardownDiskCgroup(vm, cgroup, disk) < 0)
              VIR_WARN("Failed to teardown cgroup for disk path %s",
                       NULLSTR(disk->src));
     }
@@ -10462,7 +10462,8 @@ typedef enum {
 /* Several operations end up adding or removing a single element of a
  * disk backing file chain; this helper function ensures that the lock
  * manager, cgroup device controller, and security manager labelling
- * are all aware of each new file before it is added to a chain.  */
+ * are all aware of each new file before it is added to a chain, and
+ * can revoke access to a file no longer needed in a chain.  */
 static int
 qemuDomainPrepareDiskChainElement(struct qemud_driver *driver,
                                   virDomainObjPtr vm,
@@ -10476,26 +10477,26 @@ qemuDomainPrepareDiskChainElement(struct qemud_driver *driver,
      * temporarily modify the disk in place.  */
     char *origsrc = disk->src;
     int origformat = disk->format;
-    virStorageFileMetadataPtr origchain = disk->chain;
+    virStorageFileMetadataPtr origchain = disk->backingChain;
     bool origreadonly = disk->readonly;
     int ret = -1;

     disk->src = (char *) file; /* casting away const is safe here */
     disk->format = VIR_STORAGE_FILE_RAW;
-    disk->chain = NULL;
+    disk->backingChain = NULL;
     disk->readonly = mode == VIR_DISK_CHAIN_READ_ONLY;

     if (mode == VIR_DISK_CHAIN_NO_ACCESS) {
         if (virSecurityManagerRestoreImageLabel(driver->securityManager,
                                                 vm->def, disk) < 0)
             VIR_WARN("Unable to restore security label on %s", disk->src);
-        if (cgroup && qemuTeardownDiskCgroup(driver, vm, cgroup, disk) < 0)
+        if (cgroup && qemuTeardownDiskCgroup(vm, cgroup, disk) < 0)
             VIR_WARN("Failed to teardown cgroup for disk path %s", disk->src);
         if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
             VIR_WARN("Unable to release lock on %s", disk->src);
     } else if (virDomainLockDiskAttach(driver->lockManager, driver->uri,
                                        vm, disk) < 0 ||
-               (cgroup && qemuSetupDiskCgroup(driver, vm, cgroup, disk) < 0) ||
+               (cgroup && qemuSetupDiskCgroup(vm, cgroup, disk) < 0) ||
                virSecurityManagerSetImageLabel(driver->securityManager,
                                                vm->def, disk) < 0) {
         goto cleanup;
@@ -10506,7 +10507,7 @@ qemuDomainPrepareDiskChainElement(struct qemud_driver *driver,
 cleanup:
     disk->src = origsrc;
     disk->format = origformat;
-    disk->chain = origchain;
+    disk->backingChain = origchain;
     disk->readonly = origreadonly;
     return ret;
 }
@@ -10802,7 +10803,6 @@ cleanup:
     return ret;
 }

-
 /* The domain is expected to hold monitor lock.  */
 static int
 qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver,
@@ -10848,14 +10848,14 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver,
         VIR_FORCE_CLOSE(fd);
     }

-    /* XXX Here, we know we are about to alter disk->chain if
+    /* XXX Here, we know we are about to alter disk->backingChain if
      * successful, so we nuke the existing chain so that future
      * commands will recompute it.  Better would be storing the chain
      * ourselves rather than reprobing, but this requires modifying
      * domain_conf and our XML to fully track the chain across
      * libvirtd restarts.  */
-    virStorageFileFreeMetadata(disk->chain);
-    disk->chain = NULL;
+    virStorageFileFreeMetadata(disk->backingChain);
+    disk->backingChain = NULL;

     if (qemuDomainPrepareDiskChainElement(driver, vm, cgroup, disk, source,
                                           VIR_DISK_CHAIN_READ_WRITE) < 0) {
@@ -12737,8 +12737,9 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,

     if (!top) {
         top_canon = disk->src;
-        top_meta = disk->chain;
-    } else if (!(top_canon = virStorageFileChainLookup(disk->chain, disk->src,
+        top_meta = disk->backingChain;
+    } else if (!(top_canon = virStorageFileChainLookup(disk->backingChain,
+                                                       disk->src,
                                                        top, &top_meta,
                                                        &top_parent))) {
         virReportError(VIR_ERR_INVALID_ARG,
@@ -12762,6 +12763,9 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
                        base ? base : "(default)", top_canon, path);
         goto endjob;
     }
+    /* Note that this code exploits the fact that
+     * virStorageFileChainLookup guarantees a simple pointer
+     * comparison will work, rather than needing full-blown STREQ.  */
     if ((flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) &&
         base_canon != top_meta->backingStore) {
         virReportError(VIR_ERR_INVALID_ARG,
@@ -14165,7 +14169,7 @@ static virDriver qemuDriver = {
     .domainBlockJobSetSpeed = qemuDomainBlockJobSetSpeed, /* 0.9.4 */
     .domainBlockPull = qemuDomainBlockPull, /* 0.9.4 */
     .domainBlockRebase = qemuDomainBlockRebase, /* 0.9.10 */
-    .domainBlockCommit = qemuDomainBlockCommit, /* 0.10.3 */
+    .domainBlockCommit = qemuDomainBlockCommit, /* 1.0.0 */
     .isAlive = qemuIsAlive, /* 0.9.8 */
     .nodeSuspendForDuration = nodeSuspendForDuration, /* 0.9.8 */
     .domainSetBlockIoTune = qemuDomainSetBlockIoTune, /* 0.9.8 */
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ca441f2..7381921 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2006,7 +2006,7 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver,
         VIR_WARN("Unable to restore security label on %s", dev->data.disk->src);

     if (cgroup != NULL) {
-        if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0)
+        if (qemuTeardownDiskCgroup(vm, cgroup, dev->data.disk) < 0)
             VIR_WARN("Failed to teardown cgroup for disk path %s",
                      NULLSTR(dev->data.disk->src));
     }
@@ -2089,7 +2089,7 @@ int qemuDomainDetachDiskDevice(struct qemud_driver *driver,
         VIR_WARN("Unable to restore security label on %s", dev->data.disk->src);

     if (cgroup != NULL) {
-        if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0)
+        if (qemuTeardownDiskCgroup(vm, 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_monitor_json.c b/src/qemu/qemu_monitor_json.c
index d0ecd1e..d1ce9cc 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3284,7 +3284,7 @@ cleanup:
 int
 qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device,
                            const char *top, const char *base,
-                           unsigned long speed)
+                           unsigned long long speed)
 {
     int ret = -1;
     virJSONValuePtr cmd;
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 71bc6aa..61127a7 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -239,7 +239,7 @@ int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon,
                                const char *device,
                                const char *top,
                                const char *base,
-                               unsigned long bandwidth)
+                               unsigned long long bandwidth)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);

 int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1eb93a6..3a087e2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4121,18 +4121,6 @@ void qemuProcessStop(struct qemud_driver *driver,
         networkReleaseActualDevice(net);
     }

-    /* XXX For now, disk chains should only be cached while qemu is
-     * running.  Since we don't track the chain in XML, a user is free
-     * to update the chain while the domain is offline, and then when
-     * they next boot the domain we should re-read the chain from the
-     * files at that point in time.  Only when we track the chain in
-     * XML can we forbid the user from altering the chain of an
-     * offline domain.  */
-    for (i = 0; i < def->ndisks; i++) {
-        virStorageFileFreeMetadata(def->disks[i]->chain);
-        def->disks[i]->chain = NULL;
-    }
-
 retry:
     if ((ret = qemuRemoveCgroup(driver, vm, 0)) < 0) {
         if (ret == -EBUSY && (retries++ < 5)) {
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 729c0d1..263fc92 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -919,21 +919,13 @@ get_files(vahControl * ctl)
     }

     for (i = 0; i < ctl->def->ndisks; i++) {
-        int ret;
-        int format;
         virDomainDiskDefPtr disk = ctl->def->disks[i];

-        if (disk->format > 0)
-            format = disk->format;
-        else if (ctl->allowDiskFormatProbing)
-            format = VIR_STORAGE_FILE_AUTO;
-        else
-            format = VIR_STORAGE_FILE_RAW;
-
         /* XXX - if we knew the qemu user:group here we could send it in
          *        so that the open could be re-tried as that user:group.
          */
-        disk->chain = virStorageFileGetMetadata(disk->src, format, -1, -1,
+        disk->chain = virStorageFileGetMetadata(disk->src, disk->format,
+                                                -1, -1,
                                                 ctl->allowDiskFormatProbing,
                                                 NULL);

@@ -942,8 +934,7 @@ get_files(vahControl * ctl)
          * be passing ignoreOpenFailure = false and handle open errors more
          * careful than just ignoring them.
          */
-        ret = virDomainDiskDefForeachPath(disk, true, add_file_path, &buf);
-        if (ret != 0)
+        if (virDomainDiskDefForeachPath(disk, true, add_file_path, &buf) < 0)
             goto clean;
     }

diff --git a/src/util/storage_file.c b/src/util/storage_file.c
index 218891e..882df6e 100644
--- a/src/util/storage_file.c
+++ b/src/util/storage_file.c
@@ -271,7 +271,8 @@ qcow2GetBackingStoreFormat(int *format,
                 break;
             *format = virStorageFileFormatTypeFromString(
                 ((const char *)buf)+offset);
-            break;
+            if (*format <= VIR_STORAGE_FILE_NONE)
+                return -1;
         }

         offset += len;
@@ -353,12 +354,10 @@ qcowXGetBackingStore(char **res,
      * between the end of the header (QCOW2_HDR_TOTAL_SIZE)
      * and the start of the backingStoreName (offset)
      */
-    if (isQCow2 && format) {
+    if (isQCow2 && format &&
         qcow2GetBackingStoreFormat(format, buf, buf_size, QCOW2_HDR_TOTAL_SIZE,
-                                   offset);
-        if (*format <= VIR_STORAGE_FILE_NONE)
-            return BACKING_STORE_INVALID;
-    }
+                                   offset) < 0)
+        return BACKING_STORE_INVALID;

     return BACKING_STORE_OK;
 }
@@ -517,7 +516,7 @@ qedGetBackingStore(char **res,
     (*res)[size] = '\0';

     if (flags & QED_F_BACKING_FORMAT_NO_PROBE)
-        *format = virStorageFileFormatTypeFromString("raw");
+        *format = VIR_STORAGE_FILE_RAW;
     else
         *format = VIR_STORAGE_FILE_AUTO_SAFE;

@@ -954,7 +953,7 @@ virStorageFileGetMetadataRecurse(const char *path, int format,
     ret = virStorageFileGetMetadataFromFD(path, fd, format);

     if (VIR_CLOSE(fd) < 0)
-        virReportSystemError(errno, _("could not close file %s"), path);
+        VIR_WARN("could not close file %s", path);

     if (ret && ret->backingStoreIsFile) {
         if (ret->backingStoreFormat == VIR_STORAGE_FILE_AUTO && !allow_probe)
@@ -1004,6 +1003,9 @@ virStorageFileGetMetadata(const char *path, int format,

     if (!cycle)
         return NULL;
+
+    if (format <= VIR_STORAGE_FILE_NONE)
+        format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW;
     ret = virStorageFileGetMetadataRecurse(path, format, uid, gid,
                                            allow_probe, cycle);
     virHashFree(cycle);
@@ -1261,13 +1263,14 @@ const char *virStorageFileGetSCSIKey(const char *path)
 }
 #endif

-/* Given a CHAIN that starts at the named file START, return the
- * canonical name for the backing file NAME within that chain, or pass
- * NULL to find the base of the chain.  If *META is not NULL, set it
+/* Given a CHAIN that starts at the named file START, return a string
+ * pointing to either START or within CHAIN that gives the preferred
+ * name for the backing file NAME within that chain.  Pass NULL for
+ * NAME to find the base of the chain.  If META is not NULL, set *META
  * to the point in the chain that describes NAME (or to NULL if the
- * backing element is not a file).  If *PARENT is not NULL, set it to
- * the canonical name of the parent (or to NULL if NAME matches
- * START).  The results point within CHAIN, and must not be
+ * backing element is not a file).  If PARENT is not NULL, set *PARENT
+ * to the preferred name of the parent (or to NULL if NAME matches
+ * START).  Since the results point within CHAIN, they must not be
  * independently freed.  */
 const char *
 virStorageFileChainLookup(virStorageFileMetadataPtr chain, const char *start,
@@ -1301,12 +1304,12 @@ virStorageFileChainLookup(virStorageFileMetadataPtr chain, const char *start,
                    STREQ(name, owner->backingStore)) {
             break;
         } else if (owner->backingStoreIsFile) {
-            char *abs = absolutePathFromBaseFile(*parent, name);
-            if (abs && STREQ(abs, owner->backingStore)) {
-                VIR_FREE(abs);
+            char *absName = absolutePathFromBaseFile(*parent, name);
+            if (absName && STREQ(absName, owner->backingStore)) {
+                VIR_FREE(absName);
                 break;
             }
-            VIR_FREE(abs);
+            VIR_FREE(absName);
         }
         *parent = owner->backingStore;
         owner = owner->backingMeta;


-- 
1.7.11.7




More information about the libvir-list mailing list