[PATCH 01/13] virStorageSourceNew: Abort on failure
Peter Krempa
pkrempa at redhat.com
Wed Sep 23 13:33:32 UTC 2020
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 at 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
More information about the libvir-list
mailing list