[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[PATCH 01/13] virStorageSourceNew: Abort on failure



Add an abort() on the class/object allocation failures so that
virStorageSourceNew() always returns a virStorageSource and remove
checks from all callers.

Signed-off-by: Peter Krempa <pkrempa redhat com>
---
 src/conf/backup_conf.c                |  7 ++----
 src/conf/domain_conf.c                | 21 ++++++------------
 src/conf/snapshot_conf.c              |  7 ++----
 src/conf/storage_conf.c               |  4 +---
 src/qemu/qemu_domain.c                | 21 ++++++------------
 src/qemu/qemu_driver.c                | 12 +++-------
 src/qemu/qemu_migration.c             |  7 ++----
 src/qemu/qemu_snapshot.c              |  3 +--
 src/storage/storage_backend_gluster.c |  5 +----
 src/storage/storage_backend_logical.c |  4 +---
 src/storage/storage_util.c            | 10 +++------
 src/util/virstoragefile.c             | 32 ++++++++++-----------------
 tests/qemublocktest.c                 | 16 +++-----------
 tests/virstoragetest.c                |  5 +----
 14 files changed, 46 insertions(+), 108 deletions(-)

diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
index 5caba621d8..5c475239ad 100644
--- a/src/conf/backup_conf.c
+++ b/src/conf/backup_conf.c
@@ -169,8 +169,7 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node,
         def->state = tmp;
     }

-    if (!(def->store = virStorageSourceNew()))
-        return -1;
+    def->store = virStorageSourceNew();

     if ((type = virXMLPropString(node, "type"))) {
         if ((def->store->type = virStorageTypeFromString(type)) <= 0) {
@@ -500,9 +499,7 @@ virDomainBackupDefAssignStore(virDomainBackupDiskDefPtr disk,
         }
     } else if (!disk->store) {
         if (virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_FILE) {
-            if (!(disk->store = virStorageSourceNew()))
-                return -1;
-
+            disk->store = virStorageSourceNew();
             disk->store->type = VIR_STORAGE_TYPE_FILE;
             disk->store->path = g_strdup_printf("%s.%s", src->path, suffix);
         } else {
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9289c147fe..bbe3cddadf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2187,8 +2187,7 @@ virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt)
     if (VIR_ALLOC(ret) < 0)
         return NULL;

-    if (!(ret->src = virStorageSourceNew()))
-        goto error;
+    ret->src = virStorageSourceNew();

     if (xmlopt &&
         xmlopt->privateData.diskNew &&
@@ -2400,8 +2399,7 @@ virDomainFSDefNew(virDomainXMLOptionPtr xmlopt)
     if (VIR_ALLOC(ret) < 0)
         return NULL;

-    if (!(ret->src = virStorageSourceNew()))
-        goto cleanup;
+    ret->src = virStorageSourceNew();

     if (xmlopt &&
         xmlopt->privateData.fsNew &&
@@ -8325,9 +8323,8 @@ virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode,
     if (flags & VIR_DOMAIN_DEF_PARSE_STATUS &&
         xmlopt && xmlopt->privateData.storageParse) {
         if ((ctxt->node = virXPathNode("./privateData", ctxt))) {
-            if (!scsihostsrc->src &&
-                !(scsihostsrc->src = virStorageSourceNew()))
-                return -1;
+            if (!scsihostsrc->src)
+                scsihostsrc->src = virStorageSourceNew();
             if (xmlopt->privateData.storageParse(ctxt, scsihostsrc->src) < 0)
                 return -1;
         }
@@ -8353,8 +8350,7 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode,

     /* For the purposes of command line creation, this needs to look
      * like a disk storage source */
-    if (!(iscsisrc->src = virStorageSourceNew()))
-        return -1;
+    iscsisrc->src = virStorageSourceNew();
     iscsisrc->src->type = VIR_STORAGE_TYPE_NETWORK;
     iscsisrc->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;

@@ -9770,9 +9766,7 @@ virDomainStorageSourceParseBase(const char *type,
 {
     g_autoptr(virStorageSource) src = NULL;

-    if (!(src = virStorageSourceNew()))
-        return NULL;
-
+    src = virStorageSourceNew();
     src->type = VIR_STORAGE_TYPE_FILE;

     if (type &&
@@ -9960,8 +9954,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,

     /* terminator does not have a type */
     if (!(type = virXMLPropString(ctxt->node, "type"))) {
-        if (!(src->backingStore = virStorageSourceNew()))
-            return -1;
+        src->backingStore = virStorageSourceNew();
         return 0;
     }

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 1ee63b9141..87a00082ef 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -148,9 +148,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,

     ctxt->node = node;

-    if (!(def->src = virStorageSourceNew()))
-        goto cleanup;
-
+    def->src = virStorageSourceNew();
     def->name = virXMLPropString(node, "name");
     if (!def->name) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -744,8 +742,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
         if (virBitmapIsBitSet(map, i))
             continue;
         disk = &def->disks[ndisks++];
-        if (!(disk->src = virStorageSourceNew()))
-            goto cleanup;
+        disk->src = virStorageSourceNew();
         disk->name = g_strdup(def->parent.dom->disks[i]->dst);
         disk->idx = i;

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 001f4f2bdd..2b00f09d73 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1350,9 +1350,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
     }

     if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) {
-        if (!(def->target.backingStore = virStorageSourceNew()))
-            return NULL;
-
+        def->target.backingStore = virStorageSourceNew();
         def->target.backingStore->type = VIR_STORAGE_TYPE_FILE;

         def->target.backingStore->path = backingStore;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9e0d3a15b2..c7a3e4f7cc 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5273,9 +5273,8 @@ qemuDomainDeviceHostdevDefPostParseRestoreBackendAlias(virDomainHostdevDefPtr ho

     switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) {
     case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE:
-        if (!scsisrc->u.host.src &&
-            !(scsisrc->u.host.src = virStorageSourceNew()))
-            return -1;
+        if (!scsisrc->u.host.src)
+            scsisrc->u.host.src = virStorageSourceNew();

         src = scsisrc->u.host.src;
         break;
@@ -7167,9 +7166,8 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
         }

         /* terminate the chain for such images as the code below would do */
-        if (!disksrc->backingStore &&
-            !(disksrc->backingStore = virStorageSourceNew()))
-            return -1;
+        if (!disksrc->backingStore)
+            disksrc->backingStore = virStorageSourceNew();

         /* host cdrom requires special treatment in qemu, so we need to check
          * whether a block device is a cdrom */
@@ -10456,8 +10454,7 @@ qemuDomainPrepareHostdev(virDomainHostdevDefPtr hostdev,
         switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) {
         case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE:
             virObjectUnref(scsisrc->u.host.src);
-            if (!(scsisrc->u.host.src = virStorageSourceNew()))
-                return -1;
+            scsisrc->u.host.src = virStorageSourceNew();
             src = scsisrc->u.host.src;
             break;

@@ -10850,9 +10847,7 @@ qemuDomainInitializePflashStorageSource(virDomainObjPtr vm)
     if (!virDomainDefHasOldStyleUEFI(def))
         return 0;

-    if (!(pflash0 = virStorageSourceNew()))
-        return -1;
-
+    pflash0 = virStorageSourceNew();
     pflash0->type = VIR_STORAGE_TYPE_FILE;
     pflash0->format = VIR_STORAGE_FILE_RAW;
     pflash0->path = g_strdup(def->os.loader->path);
@@ -10862,9 +10857,7 @@ qemuDomainInitializePflashStorageSource(virDomainObjPtr vm)


     if (def->os.loader->nvram) {
-        if (!(pflash1 = virStorageSourceNew()))
-            return -1;
-
+        pflash1 = virStorageSourceNew();
         pflash1->type = VIR_STORAGE_TYPE_FILE;
         pflash1->format = VIR_STORAGE_FILE_RAW;
         pflash1->path = g_strdup(def->os.loader->nvram);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ae715c01d7..9b5ff3a65c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14895,10 +14895,8 @@ qemuDomainBlockCopyCommonValidateUserMirrorBackingStore(virStorageSourcePtr mirr
 {
     if (!virStorageSourceHasBacking(mirror)) {
         /* for deep copy there won't be backing chain so we can terminate it */
-        if (!mirror->backingStore &&
-            !shallow &&
-            !(mirror->backingStore = virStorageSourceNew()))
-            return -1;
+        if (!mirror->backingStore && !shallow)
+            mirror->backingStore = virStorageSourceNew();

         /* When reusing an external image we document that the user must ensure
          * that the <mirror> image must expose data as the original image did
@@ -15149,9 +15147,6 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
             if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV_SNAPSHOT_ALLOW_WRITE_ONLY)) {
                 g_autoptr(virStorageSource) terminator = virStorageSourceNew();

-                if (!terminator)
-                    goto endjob;
-
                 if (!(data = qemuBuildStorageSourceChainAttachPrepareBlockdevTop(mirror,
                                                                                  terminator,
                                                                                  priv->qemuCaps)))
@@ -15296,8 +15291,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
         return qemuDomainBlockPullCommon(vm, path, base, bandwidth, flags);

     /* If we got here, we are doing a block copy rebase. */
-    if (!(dest = virStorageSourceNew()))
-        goto cleanup;
+    dest = virStorageSourceNew();
     dest->type = (flags & VIR_DOMAIN_BLOCK_REBASE_COPY_DEV) ?
         VIR_STORAGE_TYPE_BLOCK : VIR_STORAGE_TYPE_FILE;
     dest->path = g_strdup(base);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index a530c17582..5708334d2f 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -837,15 +837,12 @@ qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(virDomainDiskDefPtr disk,
 {
     g_autoptr(virStorageSource) copysrc = NULL;

-    if (!(copysrc = virStorageSourceNew()))
-        return NULL;
-
+    copysrc = virStorageSourceNew();
     copysrc->type = VIR_STORAGE_TYPE_NETWORK;
     copysrc->protocol = VIR_STORAGE_NET_PROTOCOL_NBD;
     copysrc->format = VIR_STORAGE_FILE_RAW;

-    if (!(copysrc->backingStore = virStorageSourceNew()))
-        return NULL;
+    copysrc->backingStore = virStorageSourceNew();

     if (!(copysrc->path = qemuAliasDiskDriveFromDisk(disk)))
         return NULL;
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 5f49fd17a4..a6e091dd09 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -894,8 +894,7 @@ qemuSnapshotDiskPrepareOneBlockdev(virQEMUDriverPtr driver,

     /* create a terminator for the snapshot disks so that qemu does not try
      * to open them at first */
-    if (!(terminator = virStorageSourceNew()))
-        return -1;
+    terminator = virStorageSourceNew();

     if (qemuDomainPrepareStorageSourceBlockdev(dd->disk, dd->src,
                                                priv, cfg) < 0)
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
index 70e6f2b086..4763a569e3 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -279,11 +279,8 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
         goto cleanup;

     if (meta->backingStoreRaw) {
-        if (!(vol->target.backingStore = virStorageSourceNew()))
-            goto cleanup;
-
+        vol->target.backingStore = virStorageSourceNew();
         vol->target.backingStore->type = VIR_STORAGE_TYPE_NETWORK;
-
         vol->target.backingStore->path = g_steal_pointer(&meta->backingStoreRaw);
         vol->target.backingStore->format = meta->backingStoreRawFormat;

diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
index 43cf3a1598..e3debc390c 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -282,9 +282,7 @@ virStorageBackendLogicalMakeVol(char **const groups,
      *  lv is created with "--virtualsize").
      */
     if (groups[1] && STRNEQ(groups[1], "") && (groups[1][0] != '[')) {
-        if (!(vol->target.backingStore = virStorageSourceNew()))
-            goto cleanup;
-
+        vol->target.backingStore = virStorageSourceNew();
         vol->target.backingStore->path = g_strdup_printf("%s/%s",
                                                          def->target.path, groups[1]);

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 9171cb084f..08d9bd4172 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -3389,8 +3389,7 @@ storageBackendProbeTarget(virStorageSourcePtr target,
         return -1;

     if (meta->backingStoreRaw) {
-        if (virStorageSourceNewFromBacking(meta, &target->backingStore) < 0)
-            return -1;
+        virStorageSourceNewFromBacking(meta, &target->backingStore);

         /* XXX: Remote storage doesn't play nicely with volumes backed by
          * remote storage. To avoid trouble, just fake the backing store is RAW
@@ -3398,9 +3397,7 @@ storageBackendProbeTarget(virStorageSourcePtr target,
         if (!virStorageSourceIsLocalStorage(target->backingStore)) {
             virObjectUnref(target->backingStore);

-            if (!(target->backingStore = virStorageSourceNew()))
-                return -1;
-
+            target->backingStore = virStorageSourceNew();
             target->backingStore->type = VIR_STORAGE_TYPE_NETWORK;
             target->backingStore->path = meta->backingStoreRaw;
             meta->backingStoreRaw = NULL;
@@ -3568,8 +3565,7 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool)
         goto cleanup;
     VIR_DIR_CLOSE(dir);

-    if (!(target = virStorageSourceNew()))
-        goto cleanup;
+    target = virStorageSourceNew();

     if ((fd = open(def->target.path, O_RDONLY)) < 0) {
         virReportSystemError(errno,
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 42341150e5..229de14935 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1084,10 +1084,7 @@ static virStorageSourcePtr
 virStorageFileMetadataNew(const char *path,
                           int format)
 {
-    g_autoptr(virStorageSource) def = NULL;
-
-    if (!(def = virStorageSourceNew()))
-        return NULL;
+    g_autoptr(virStorageSource) def = virStorageSourceNew();

     def->format = format;
     def->type = VIR_STORAGE_TYPE_FILE;
@@ -2368,10 +2365,7 @@ virStorageSourcePtr
 virStorageSourceCopy(const virStorageSource *src,
                      bool backingChain)
 {
-    g_autoptr(virStorageSource) def = NULL;
-
-    if (!(def = virStorageSourceNew()))
-        return NULL;
+    g_autoptr(virStorageSource) def = virStorageSourceNew();

     def->id = src->id;
     def->type = src->type;
@@ -2746,10 +2740,15 @@ VIR_ONCE_GLOBAL_INIT(virStorageSource);
 virStorageSourcePtr
 virStorageSourceNew(void)
 {
+    virStorageSourcePtr ret;
+
     if (virStorageSourceInitialize() < 0)
-        return NULL;
+        abort();
+
+    if (!(ret = virObjectNew(virStorageSourceClass)))
+        abort();

-    return virObjectNew(virStorageSourceClass);
+    return ret;
 }


@@ -2758,10 +2757,7 @@ virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
                                        const char *rel)
 {
     g_autofree char *dirname = NULL;
-    g_autoptr(virStorageSource) def = NULL;
-
-    if (!(def = virStorageSourceNew()))
-        return NULL;
+    g_autoptr(virStorageSource) def = virStorageSourceNew();

     /* store relative name */
     def->relPath = g_strdup(rel);
@@ -3980,13 +3976,10 @@ virStorageSourceNewFromBackingAbsolute(const char *path,
     const char *json;
     const char *dirpath;
     int rc = 0;
-    g_autoptr(virStorageSource) def = NULL;
+    g_autoptr(virStorageSource) def = virStorageSourceNew();

     *src = NULL;

-    if (!(def = virStorageSourceNew()))
-        return -1;
-
     if (virStorageIsFile(path)) {
         def->type = VIR_STORAGE_TYPE_FILE;

@@ -5317,8 +5310,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
         src->backingStore = g_steal_pointer(&backingStore);
     } else {
         /* add terminator */
-        if (!(src->backingStore = virStorageSourceNew()))
-            return -1;
+        src->backingStore = virStorageSourceNew();
     }

     return 0;
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 0685b703a1..7bde613843 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -67,9 +67,7 @@ testBackingXMLjsonXML(const void *args)
     if (data->legacy)
         backendpropsflags |= QEMU_BLOCK_STORAGE_SOURCE_BACKEND_PROPS_LEGACY;

-    if (!(xmlsrc = virStorageSourceNew()))
-        return -1;
-
+    xmlsrc = virStorageSourceNew();
     xmlsrc->type = data->type;

     if (!(xml = virXMLParseStringCtxt(data->xml, "(test storage source XML)", &ctxt)))
@@ -673,10 +671,7 @@ testQemuBitmapListPrint(const char *title,
 static virStorageSourcePtr
 testQemuBackupIncrementalBitmapCalculateGetFakeImage(size_t idx)
 {
-   virStorageSourcePtr ret;
-
-   if (!(ret = virStorageSourceNew()))
-       abort();
+   virStorageSourcePtr ret = virStorageSourceNew();

    ret->id = idx;
    ret->type = VIR_STORAGE_TYPE_FILE;
@@ -755,9 +750,7 @@ testQemuBackupIncrementalBitmapCalculate(const void *opaque)
         return -1;
     }

-    if (!(target = virStorageSourceNew()))
-        return -1;
-
+    target = virStorageSourceNew();
     target->nodeformat = g_strdup_printf("target_node");

     if (qemuBackupDiskPrepareOneBitmapsChain(data->chain,
@@ -889,9 +882,6 @@ testQemuBlockBitmapBlockcopy(const void *opaque)
     g_autoptr(virStorageSource) fakemirror = virStorageSourceNew();
     g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;

-    if (!fakemirror)
-        return -1;
-
     fakemirror->nodeformat = g_strdup("mirror-format-node");

     expectpath = g_strdup_printf("%s/%s%s-out.json", abs_srcdir,
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index e0f8b6d621..2e466ecb99 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -85,10 +85,7 @@ testStorageFileGetMetadata(const char *path,
                            uid_t uid, gid_t gid)
 {
     struct stat st;
-    g_autoptr(virStorageSource) def = NULL;
-
-    if (!(def = virStorageSourceNew()))
-        return NULL;
+    g_autoptr(virStorageSource) def = virStorageSourceNew();

     def->type = VIR_STORAGE_TYPE_FILE;
     def->format = format;
-- 
2.26.2


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]