util: storagefile: Add helpers to check presence of backing store

Add helpers that will simplify checking if a backing file is valid or
whether it has backing store. The helper virStorageSourceIsBacking
returns true if the given virStorageSource is a valid backing store
member. virStorageSourceHasBacking returns true if the virStorageSource
has a backing store child.

Adding these functions creates a central points for further refactors.

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 02c1facdb0ff27596c93f4405628d40f86e61a5b..e41f2e764d69d0c86ea4a388af0de413061dd633 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -26619,7 +26619,7 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
         }
     }
 
-    for (tmp = disk->src; tmp; tmp = tmp->backingStore) {
+    for (tmp = disk->src; virStorageSourceIsBacking(tmp); tmp = tmp->backingStore) {
         /* execute the callback only for local storage */
         if (virStorageSourceIsLocalStorage(tmp) &&
             tmp->path) {

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 7c373e7810225d4b50b663d21332886c25cb909a..f808cd291e4fd7fc1af604296875bf8c8720c014 100644 (file)
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1169,7 +1169,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
         if (virStorageSize(unit, capacity, &ret->target.capacity) < 0)
             goto error;
     } else if (!(flags & VIR_VOL_XML_PARSE_NO_CAPACITY) &&
-               !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) && ret->target.backingStore)) {
+               !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) &&
+                 virStorageSourceHasBacking(&ret->target))) {
         virReportError(VIR_ERR_XML_ERROR, "%s", _("missing capacity element"));
         goto error;
     }
@@ -1497,7 +1498,7 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
                                      &def->target, "target") < 0)
         goto cleanup;
 
-    if (def->target.backingStore &&
+    if (virStorageSourceHasBacking(&def->target) &&
         virStorageVolTargetDefFormat(options, &buf,
                                      def->target.backingStore,
                                      "backingStore") < 0)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6eea6f103f1e49a77eb1c3ad7b592be40d904b2b..44c968834797297b53cf2ebe792650f2dc567437 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2694,7 +2694,9 @@ virStorageSourceFindByNodeName;
 virStorageSourceFree;
 virStorageSourceGetActualType;
 virStorageSourceGetSecurityLabelDef;
+virStorageSourceHasBacking;
 virStorageSourceInitChainElement;
+virStorageSourceIsBacking;
 virStorageSourceIsBlockLocal;
 virStorageSourceIsEmpty;
 virStorageSourceIsLocalStorage;

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 8d232de3e3b5efcc0ce16d2b169da3f8ab6a5d0d..544b4893b40cee1631075b11e5b91cda35170635 100644 (file)
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -257,7 +257,7 @@ qemuBlockDiskClearDetectedNodes(virDomainDiskDefPtr disk)
 {
     virStorageSourcePtr next = disk->src;
 
-    while (next) {
+    while (virStorageSourceIsBacking(next)) {
         VIR_FREE(next->nodeformat);
         VIR_FREE(next->nodestorage);
 
@@ -287,7 +287,7 @@ qemuBlockDiskDetectNodes(virDomainDiskDefPtr disk,
         goto cleanup;
     }
 
-    while (src && entry) {
+    while (virStorageSourceIsBacking(src) && entry) {
         if (src->nodeformat || src->nodestorage) {
             if (STRNEQ_NULLABLE(src->nodeformat, entry->nodeformat) ||
                 STRNEQ_NULLABLE(src->nodestorage, entry->nodestorage))

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 6fc413098ae8949a34fe54d39420ad575c8775da..0f75e22f98541a98e73cd8aa400a44a17509ff8f 100644 (file)
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -142,7 +142,7 @@ qemuSetupDiskCgroup(virDomainObjPtr vm,
     virStorageSourcePtr next;
     bool forceReadonly = false;
 
-    for (next = disk->src; next; next = next->backingStore) {
+    for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) {
         if (qemuSetupImageCgroupInternal(vm, next, forceReadonly) < 0)
             return -1;
 
@@ -160,7 +160,7 @@ qemuTeardownDiskCgroup(virDomainObjPtr vm,
 {
     virStorageSourcePtr next;
 
-    for (next = disk->src; next; next = next->backingStore) {
+    for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) {
         if (qemuTeardownImageCgroup(vm, next) < 0)
             return -1;
     }

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ed27a91fa613b181733ceb718cc19d94831e6a7f..173fbca6ed09942f096ed6735134cb3f2634104d 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5929,7 +5929,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
     if (virStorageSourceIsEmpty(disk->src))
         goto cleanup;
 
-    if (disk->src->backingStore) {
+    if (virStorageSourceHasBacking(disk->src)) {
         if (force_probe)
             virStorageSourceBackingStoreClear(disk->src);
         else
@@ -8548,7 +8548,7 @@ qemuDomainSetupDisk(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED,
     char *dst = NULL;
     int ret = -1;
 
-    for (next = disk->src; next; next = next->backingStore) {
+    for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) {
         if (!next->path || !virStorageSourceIsLocalStorage(next)) {
             /* Not creating device. Just continue. */
             continue;
@@ -9449,7 +9449,7 @@ qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver,
                                      &ndevMountsPath) < 0)
         goto cleanup;
 
-    for (next = src; next; next = next->backingStore) {
+    for (next = src; virStorageSourceIsBacking(next); next = next->backingStore) {
         if (virStorageSourceIsEmpty(next) ||
             !virStorageSourceIsLocalStorage(next)) {
             /* Not creating device. Just continue. */

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 01992e0ae530e49e58a84d40ee72876428e7b75e..f889003ba6249d75c4b3893a9ff565284a1ca176 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14435,7 +14435,7 @@ qemuDomainSnapshotUpdateDiskSourcesRenumber(virStorageSourcePtr src)
     virStorageSourcePtr next;
     unsigned int idx = 1;
 
-    for (next = src->backingStore; next; next = next->backingStore)
+    for (next = src->backingStore; virStorageSourceIsBacking(next); next = next->backingStore)
         next->id = idx++;
 }
 
@@ -17035,7 +17035,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
     }
 
     /* clear the _SHALLOW flag if there is only one layer */
-    if (!disk->src->backingStore)
+    if (!virStorageSourceHasBacking(disk->src))
         flags &= ~VIR_DOMAIN_BLOCK_COPY_SHALLOW;
 
     /* unless the user provides a pre-created file, shallow copy into a raw
@@ -17426,7 +17426,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
         goto endjob;
     }
 
-    if (!topSource->backingStore) {
+    if (!virStorageSourceHasBacking(topSource)) {
         virReportError(VIR_ERR_INVALID_ARG,
                        _("top '%s' in chain for '%s' has no backing file"),
                        topSource->path, path);
@@ -19874,7 +19874,8 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
         virStorageSourcePtr src = disk->src;
         unsigned int backing_idx = 0;
 
-        while (src && (backing_idx == 0 || visitBacking)) {
+        while (virStorageSourceIsBacking(src) &&
+               (backing_idx == 0 || visitBacking)) {
             if (qemuDomainGetStatsOneBlock(driver, cfg, dom, record, maxparams,
                                            disk, src, visited, backing_idx,
                                            stats, nodestats) < 0)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 349dbe81dc10fb06c426cddabdf9b477e6c695eb..244b300a9f1a0b7f25dfefbd28c921d4398b206b 100644 (file)
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -730,7 +730,7 @@ virSecurityDACSetDiskLabel(virSecurityManagerPtr mgr,
 {
     virStorageSourcePtr next;
 
-    for (next = disk->src; next; next = next->backingStore) {
+    for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) {
         if (virSecurityDACSetImageLabel(mgr, def, next) < 0)
             return -1;
     }

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 2e3082b7a8d77a241f0aa88d08e4b0e46a611a17..cd3e41193100ddf67b159c09951f46b693b72746 100644 (file)
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1539,7 +1539,7 @@ virSecuritySELinuxRestoreImageLabelInt(virSecurityManagerPtr mgr,
      * be tracked in domain XML, at which point labelskip should be a
      * per-file attribute instead of a disk attribute. */
     if (disk_seclabel && disk_seclabel->labelskip &&
-        !src->backingStore)
+        !virStorageSourceHasBacking(src))
         return 0;
 
     /* Don't restore labels on readonly/shared disks, because other VMs may
@@ -1673,7 +1673,7 @@ virSecuritySELinuxSetDiskLabel(virSecurityManagerPtr mgr,
     bool first = true;
     virStorageSourcePtr next;
 
-    for (next = disk->src; next; next = next->backingStore) {
+    for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) {
         if (virSecuritySELinuxSetImageLabelInternal(mgr, def, next, first) < 0)
             return -1;
 

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 95906e68a38216a9d3dac26421a9d9e3c907f1d8..ef1bf0136f90c7bb7f676c2de71ecf5ed822d612 100644 (file)
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -942,7 +942,7 @@ get_files(vahControl * ctl)
         /* 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.
          */
-        if (!disk->src->backingStore) {
+        if (!virStorageSourceHasBacking(disk->src)) {
             bool probe = ctl->allowDiskFormatProbing;
             virStorageFileGetMetadata(disk->src, -1, -1, probe, false);
         }

diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
index 1e0f04e4e1c8473ddc094ca33fbbc412975dde30..a872a2f8811f5180cf1127960dd9eef12fdc740f 100644 (file)
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -977,7 +977,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
     }
     virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(vol->target.capacity,
                                                     1024));
-    if (vol->target.backingStore)
+    if (virStorageSourceHasBacking(&vol->target))
         virCommandAddArgList(cmd, "-s", vol->target.backingStore->path, NULL);
     else
         virCommandAddArg(cmd, def->source.name);

diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c
index 864c69928fe6a5941ae7e159e62f729c866e799c..47b08f4162f53b3e3a31cbed494f34494e813de0 100644 (file)
--- a/src/storage/storage_source.c
+++ b/src/storage/storage_source.c
@@ -490,7 +490,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
     ret = 0;
 
  cleanup:
-    if (src->backingStore)
+    if (virStorageSourceHasBacking(src))
         src->backingStore->id = depth;
     VIR_FREE(buf);
     virStorageFileDeinit(src);

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index a10e4590f3c4d4c185936444bcc4cdae9611b42f..5252e429fd603b3f51a8055129d3e93b99941335 100644 (file)
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -415,7 +415,7 @@ storageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
         goto cleanup;
     }
 
-    if (vol->target.backingStore) {
+    if (virStorageSourceHasBacking(&vol->target)) {
         virReportError(VIR_ERR_NO_SUPPORT, "%s",
                        _("backing storage not supported for raw volumes"));
         goto cleanup;
@@ -722,7 +722,7 @@ storageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED,
         return -1;
     }
 
-    if (vol->target.backingStore != NULL) {
+    if (virStorageSourceHasBacking(&vol->target)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("copy-on-write ploop volumes are not yet supported"));
         return -1;
@@ -1055,7 +1055,7 @@ storageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool,
      * backing store, not really sure what use it serves though, and it
      * may cause issues with lvm. Untested essentially.
      */
-    if (inputvol && inputvol->target.backingStore &&
+    if (inputvol && virStorageSourceHasBacking(&inputvol->target) &&
         STRNEQ_NULLABLE(inputvol->target.backingStore->path,
                         info->backingPath)) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -1230,7 +1230,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
         storageBackendCreateQemuImgSetInput(inputvol, &info) < 0)
         return NULL;
 
-    if (vol->target.backingStore &&
+    if (virStorageSourceHasBacking(&vol->target) &&
         storageBackendCreateQemuImgSetBacking(pool, vol, inputvol, &info) < 0)
         return NULL;
 
@@ -1840,7 +1840,7 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
                                                  openflags, readflags)) < 0)
         return ret;
 
-    if (vol->target.backingStore &&
+    if (virStorageSourceHasBacking(&vol->target) &&
         (ret = storageBackendUpdateVolTargetInfo(VIR_STORAGE_VOL_FILE,
                                                  vol->target.backingStore,
                                                  withBlockVolFormat,
@@ -2035,7 +2035,7 @@ createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED,
         return -1;
     }
 
-    if (vol->target.backingStore) {
+    if (virStorageSourceHasBacking(&vol->target)) {
         virReportError(VIR_ERR_NO_SUPPORT, "%s",
                        _("backing storage not supported for directories volumes"));
         return -1;
@@ -3561,7 +3561,7 @@ virStorageBackendRefreshVolTargetUpdate(virStorageVolDefPtr vol)
     if (vol->target.format == VIR_STORAGE_FILE_PLOOP)
         vol->type = VIR_STORAGE_VOL_PLOOP;
 
-    if (vol->target.backingStore) {
+    if (virStorageSourceHasBacking(&vol->target)) {
         ignore_value(storageBackendUpdateVolTargetInfo(VIR_STORAGE_VOL_FILE,
                                                        vol->target.backingStore,
                                                        false,

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index a9d8e59405f3df8456a87b3c4261b21a2a0f9538..7335ae20f3ea8689c9fe9c1be1107e464e450fb2 100644 (file)
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1292,7 +1292,7 @@ virStorageFileChainGetBroken(virStorageSourcePtr chain,
     if (!chain)
         return 0;
 
-    for (tmp = chain; tmp; tmp = tmp->backingStore) {
+    for (tmp = chain; virStorageSourceIsBacking(tmp); tmp = tmp->backingStore) {
         /* Break when we hit end of chain; report error if we detected
          * a missing backing file, infinite loop, or other error */
         if (!tmp->backingStore && tmp->backingStoreRaw) {
@@ -1566,6 +1566,33 @@ virStorageFileParseChainIndex(const char *diskTarget,
     return ret;
 }
 
+
+/**
+ * virStorageSourceIsBacking:
+ * @src: storage source
+ *
+ * Returns true if @src is a eligible backing store structure. Useful
+ * for iterators.
+ */
+bool
+virStorageSourceIsBacking(const virStorageSource *src)
+{
+    return !!src;
+}
+
+/**
+ * virStorageSourceHasBacking:
+ * @src: storage source
+ *
+ * Returns true if @src has backing store/chain.
+ */
+bool
+virStorageSourceHasBacking(const virStorageSource *src)
+{
+    return virStorageSourceIsBacking(src) && src->backingStore;
+}
+
+
 /* Given a @chain, look for the backing store @name that is a backing file
  * of @startFrom (or any member of @chain if @startFrom is NULL) and return
  * that location within the chain.  @chain must always point to the top of
@@ -1594,15 +1621,16 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
     *parent = NULL;
 
     if (startFrom) {
-        while (chain && chain != startFrom->backingStore)
+        while (virStorageSourceIsBacking(chain) &&
+               chain != startFrom->backingStore)
             chain = chain->backingStore;
 
         *parent = startFrom;
     }
 
-    while (chain) {
+    while (virStorageSourceIsBacking(chain)) {
         if (!name && !idx) {
-            if (!chain->backingStore)
+            if (!virStorageSourceHasBacking(chain))
                 break;
         } else if (idx) {
             VIR_DEBUG("%u: %s", chain->id, chain->path);
@@ -1640,7 +1668,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
         chain = chain->backingStore;
     }
 
-    if (!chain)
+    if (!virStorageSourceIsBacking(chain))
         goto error;
 
     return chain;
@@ -3854,7 +3882,7 @@ virStorageFileGetRelativeBackingPath(virStorageSourcePtr top,
 
     *relpath = NULL;
 
-    for (next = top; next; next = next->backingStore) {
+    for (next = top; virStorageSourceIsBacking(next); next = next->backingStore) {
         if (!next->relPath) {
             ret = 1;
             goto cleanup;
@@ -3973,7 +4001,7 @@ virStorageSourceFindByNodeName(virStorageSourcePtr top,
     if (idx)
         *idx = 0;
 
-    for (tmp = top; tmp; tmp = tmp->backingStore) {
+    for (tmp = top; virStorageSourceIsBacking(tmp); tmp = tmp->backingStore) {
         if ((tmp->nodeformat && STREQ(tmp->nodeformat, nodeName)) ||
             (tmp->nodestorage && STREQ(tmp->nodestorage, nodeName)))
             return tmp;

diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index d3bafefc33b346b197e2d167fff0e0e273529854..86e60de2ad6a22ade1440f97ec0ca270f3f7d565 100644 (file)
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -425,4 +425,10 @@ void
 virStorageSourceNetworkAssignDefaultPorts(virStorageSourcePtr src)
     ATTRIBUTE_NONNULL(1);
 
+bool
+virStorageSourceIsBacking(const virStorageSource *src);
+bool
+virStorageSourceHasBacking(const virStorageSource *src);
+
+
 #endif /* __VIR_STORAGE_FILE_H__ */

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index ad4514871aae5ddd816246a2ade9bfc289e25991..35e97ff2630ebc3361e2e217146465d814569094 100644 (file)
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -356,7 +356,7 @@ testStorageChain(const void *args)
     }
 
     elt = meta;
-    while (elt) {
+    while (virStorageSourceIsBacking(elt)) {
         char *expect = NULL;
         char *actual = NULL;