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@xxxxxxxxxx> --- 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