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.

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

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)

src/libvirt_private.syms

@@ -2694,7 +2694,9 @@ virStorageSourceFindByNodeName;
 virStorageSourceFree;
 virStorageSourceGetActualType;
 virStorageSourceGetSecurityLabelDef;
+virStorageSourceHasBacking;
 virStorageSourceInitChainElement;
+virStorageSourceIsBacking;
 virStorageSourceIsBlockLocal;
 virStorageSourceIsEmpty;
 virStorageSourceIsLocalStorage;

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

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;
     }

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. */

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)

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;
     }

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;
 

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);
         }

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

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

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,

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;

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__ */

tests/virstoragetest.c

@@ -356,7 +356,7 @@ testStorageChain(const void *args)
     }
 
     elt = meta;
-    while (elt) {
+    while (virStorageSourceIsBacking(elt)) {
         char *expect = NULL;
         char *actual = NULL;