[PATCH 00/28] a bunch of domain_conf cleanup
Most of this is making functions void that unnecessarily return an int. It also includes some conversion to GLib. Feel free to squash related commits, if you'd like. I left them separate to make it easier to review. Matt Coleman (28): domain_conf: make virDomainDiskSetDriver() void domain_conf: make virDomainPostParseCheckISCSIPath() void domain_conf: use g_free() in virDomainPostParseCheckISCSIPath() domain_conf: make virDomainHostdevAssignAddress() void domain_conf: make virDomainChr/RNG/Video/VsockDefPostParse() and virDomainNVRAMDefFormat() void domain_conf: make virDomainDeviceInfoFormat() void domain_conf: make virDomainGraphicsDefParseXMLEGLHeadless() void domain_conf: make virDomainLeaseDefFormat() void domain_conf: make virDomainDiskSourceFormatNetwork() void domain_conf: make virDomainDiskDefFormatIotune() void domain_conf: make virDomainDiskDefFormatDriver() void domain_conf: make virDomainControllerDriverFormat() void domain_conf: make virDomainVirtioNetGuestOpts/HostOpts/DriverFormat() void domain_conf: make virDomainRedirFilterDefFormat() void domain_conf: make virDomainIOMMUDefFormat() void domain_conf: make virDomainDefFormatBlkiotune() void domain_conf: make virDomainChrSourceDefFormat() void domain_conf: make virDomainDiskSetBlockIOTune() void domain_conf: use g_free in virDomainDiskSetBlockIOTune() domain_conf: use g_renew in virDomainDiskInsert() and virDomainControllerInsert() domain_conf: make virDomainDiskInsert() void domain_conf: make virDomainControllerInsert() void domain_conf: use g_renew in virDomainLeaseInsertPreAlloc() domain_conf: make virDomainLeaseInsertPreAlloc() void domain_conf: make virDomainLeaseInsert() void domain_conf: make virDomainPanicDefFormat() void domain_conf: make virDomainShmemDefFormat() void domain_conf: make virDomainVsockDefFormat() void src/conf/domain_conf.c | 349 ++++++++++++++------------------------- src/conf/domain_conf.h | 21 +-- src/libxl/libxl_conf.c | 5 +- src/libxl/libxl_domain.c | 5 +- src/libxl/libxl_driver.c | 9 +- src/libxl/xen_xl.c | 12 +- src/libxl/xen_xm.c | 10 +- src/lxc/lxc_driver.c | 3 +- src/qemu/qemu_domain.c | 5 +- src/qemu/qemu_driver.c | 15 +- src/qemu/qemu_hotplug.c | 3 +- src/test/test_driver.c | 3 +- src/vz/vz_sdk.c | 9 +- tests/qemublocktest.c | 5 +- 14 files changed, 158 insertions(+), 296 deletions(-) -- 2.27.0
The function only returns zero or aborts, so it might as well be void. This has the added benefit of simplifying the code that calls it. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 3 +-- src/conf/domain_conf.h | 3 +-- src/libxl/libxl_conf.c | 2 +- src/libxl/libxl_domain.c | 5 ++--- src/libxl/libxl_driver.c | 3 +-- src/libxl/xen_xl.c | 12 ++++-------- src/libxl/xen_xm.c | 10 +++------- src/qemu/qemu_domain.c | 5 ++--- src/vz/vz_sdk.c | 3 +-- 9 files changed, 16 insertions(+), 30 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5c8ec19da8..eed11b4793 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2270,13 +2270,12 @@ virDomainDiskGetDriver(const virDomainDiskDef *def) } -int +void virDomainDiskSetDriver(virDomainDiskDefPtr def, const char *name) { char *tmp = g_strdup(name); g_free(def->driverName); def->driverName = tmp; - return 0; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c164b28ea1..4608c1fafb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3043,8 +3043,7 @@ const char *virDomainDiskGetSource(virDomainDiskDef const *def); void virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src); void virDomainDiskEmptySource(virDomainDiskDefPtr def); const char *virDomainDiskGetDriver(const virDomainDiskDef *def); -int virDomainDiskSetDriver(virDomainDiskDefPtr def, const char *name) - G_GNUC_WARN_UNUSED_RESULT; +void virDomainDiskSetDriver(virDomainDiskDefPtr def, const char *name); int virDomainDiskGetFormat(virDomainDiskDefPtr def); void virDomainDiskSetFormat(virDomainDiskDefPtr def, int format); virDomainControllerDefPtr diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 43d23565f1..383f2295a2 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1237,7 +1237,7 @@ libxlUpdateDiskDef(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) break; } if (driver) - ignore_value(virDomainDiskSetDriver(l_disk, driver)); + virDomainDiskSetDriver(l_disk, driver); } int diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index a3f362a0c8..3669b358f6 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -361,9 +361,8 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, /* for network-based disks, set 'qemu' as the default driver */ if (actual_type == VIR_STORAGE_TYPE_NETWORK) { - if (!virDomainDiskGetDriver(disk) && - virDomainDiskSetDriver(disk, "qemu") < 0) - return -1; + if (!virDomainDiskGetDriver(disk)) + virDomainDiskSetDriver(disk, "qemu"); } /* xl.cfg default format is raw. See xl-disk-configuration(5) */ diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 00a74dcb49..2195ecf47b 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -4086,8 +4086,7 @@ libxlDomainUpdateDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) virDomainDiskSetSource(orig, virDomainDiskGetSource(disk)); virDomainDiskSetType(orig, virDomainDiskGetType(disk)); virDomainDiskSetFormat(orig, virDomainDiskGetFormat(disk)); - if (virDomainDiskSetDriver(orig, virDomainDiskGetDriver(disk)) < 0) - return -1; + virDomainDiskSetDriver(orig, virDomainDiskGetDriver(disk)); break; default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c index d195f866c5..ed0ce77db4 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -725,8 +725,7 @@ xenParseXLDisk(virConfPtr conf, virDomainDefPtr def) disk->removable = libxldisk->removable; if (libxldisk->is_cdrom) { - if (virDomainDiskSetDriver(disk, "qemu") < 0) - goto fail; + virDomainDiskSetDriver(disk, "qemu"); virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE); disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; @@ -772,21 +771,18 @@ xenParseXLDisk(virConfPtr conf, virDomainDefPtr def) switch (libxldisk->backend) { case LIBXL_DISK_BACKEND_QDISK: case LIBXL_DISK_BACKEND_UNKNOWN: - if (virDomainDiskSetDriver(disk, "qemu") < 0) - goto fail; + virDomainDiskSetDriver(disk, "qemu"); if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_NONE) virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE); break; case LIBXL_DISK_BACKEND_TAP: - if (virDomainDiskSetDriver(disk, "tap") < 0) - goto fail; + virDomainDiskSetDriver(disk, "tap"); virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE); break; case LIBXL_DISK_BACKEND_PHY: - if (virDomainDiskSetDriver(disk, "phy") < 0) - goto fail; + virDomainDiskSetDriver(disk, "phy"); virDomainDiskSetType(disk, VIR_STORAGE_TYPE_BLOCK); break; default: diff --git a/src/libxl/xen_xm.c b/src/libxl/xen_xm.c index 283ed1ee8b..3e81c9ef0d 100644 --- a/src/libxl/xen_xm.c +++ b/src/libxl/xen_xm.c @@ -165,10 +165,7 @@ xenParseXMDisk(char *entry, int hvm) len = tmp - src; tmp = g_strndup(src, len); - if (virDomainDiskSetDriver(disk, tmp) < 0) { - VIR_FREE(tmp); - goto error; - } + virDomainDiskSetDriver(disk, tmp); VIR_FREE(tmp); /* Strip the prefix we found off the source file name */ @@ -208,9 +205,8 @@ xenParseXMDisk(char *entry, int hvm) } /* No source, or driver name, so fix to phy: */ - if (!virDomainDiskGetDriver(disk) && - virDomainDiskSetDriver(disk, "phy") < 0) - goto error; + if (!virDomainDiskGetDriver(disk)) + virDomainDiskSetDriver(disk, "phy"); /* phy: type indicates a block device */ virDomainDiskSetType(disk, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d7dbca487a..2a7316801b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5086,9 +5086,8 @@ qemuDomainDeviceDiskDefPostParse(virDomainDiskDefPtr disk, unsigned int parseFlags) { /* set default disk types and drivers */ - if (!virDomainDiskGetDriver(disk) && - virDomainDiskSetDriver(disk, "qemu") < 0) - return -1; + if (!virDomainDiskGetDriver(disk)) + virDomainDiskSetDriver(disk, "qemu"); /* default disk format for drives */ if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_NONE && diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 6a0ab5c862..cdd3f61ee1 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -698,8 +698,7 @@ prlsdkGetDiskInfo(vzDriverPtr driver, VIR_FREE(disk->serial); } - if (virDomainDiskSetDriver(disk, "vz") < 0) - goto cleanup; + virDomainDiskSetDriver(disk, "vz"); if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { pret = PrlVmDevHd_GetDiskSize(prldisk, &size); -- 2.27.0
The function only returns zero or aborts, so it might as well be void. This has the added benefit of simplifying the code that calls it. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index eed11b4793..ce49905360 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5068,21 +5068,18 @@ virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt, * provided, then default to zero. For an ISCSI LUN that is * is provided by /dev/disk/by-path/... , then that path will * have the specific lun requested. - * - * Returns 0 on success, -1 on failure */ -static int +static void virDomainPostParseCheckISCSIPath(char **srcpath) { char *path = NULL; if (strchr(*srcpath, '/')) - return 0; + return; path = g_strdup_printf("%s/0", *srcpath); VIR_FREE(*srcpath); *srcpath = g_steal_pointer(&path); - return 0; } @@ -5101,9 +5098,7 @@ virDomainHostdevDefPostParse(virDomainHostdevDefPtr dev, scsisrc = &dev->source.subsys.u.scsi; if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; - - if (virDomainPostParseCheckISCSIPath(&iscsisrc->src->path) < 0) - return -1; + virDomainPostParseCheckISCSIPath(&iscsisrc->src->path); } if (dev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && @@ -5283,9 +5278,8 @@ virDomainDiskDefPostParse(virDomainDiskDefPtr disk, } if (disk->src->type == VIR_STORAGE_TYPE_NETWORK && - disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI && - virDomainPostParseCheckISCSIPath(&disk->src->path) < 0) { - return -1; + disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI) { + virDomainPostParseCheckISCSIPath(&disk->src->path); } if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO && -- 2.27.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ce49905360..a64dec8df4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5078,7 +5078,7 @@ virDomainPostParseCheckISCSIPath(char **srcpath) return; path = g_strdup_printf("%s/0", *srcpath); - VIR_FREE(*srcpath); + g_free(*srcpath); *srcpath = g_steal_pointer(&path); } -- 2.27.0
On 11/6/20 4:32 AM, Matt Coleman wrote:
Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ce49905360..a64dec8df4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5078,7 +5078,7 @@ virDomainPostParseCheckISCSIPath(char **srcpath) return;
path = g_strdup_printf("%s/0", *srcpath); - VIR_FREE(*srcpath); + g_free(*srcpath); *srcpath = g_steal_pointer(&path); }
Can't we do this for other places too? I mean, this boils down to discussions we had when starting to adopt glib, but rather than doing this change per function I think we want bigger blocks. The same applies for 20/28 where you're switching to g_renew() from VIR_REALLOC_N(). I understand that you want to touch only some functions because later you are turning their return type into void, but I'd rather see bulk glib conversions. Michal
On Mon, Nov 09, 2020 at 15:37:39 +0100, Michal Privoznik wrote:
On 11/6/20 4:32 AM, Matt Coleman wrote:
Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ce49905360..a64dec8df4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5078,7 +5078,7 @@ virDomainPostParseCheckISCSIPath(char **srcpath) return; path = g_strdup_printf("%s/0", *srcpath); - VIR_FREE(*srcpath); + g_free(*srcpath); *srcpath = g_steal_pointer(&path); }
Can't we do this for other places too? I mean, this boils down to discussions we had when starting to adopt glib, but rather than doing this change per function I think we want bigger blocks. The same applies for 20/28 where you're switching to g_renew() from VIR_REALLOC_N(). I understand that you want to touch only some functions because later you are turning their return type into void, but I'd rather see bulk glib conversions.
Specifically you can only replace VIR_FREE with g_free if you immediately overwrite it. Our semantics of VIR_FREE lead in many cases to a code pattern of using VIR_FREE and then reusing the variable and having another VIR_FREE in the cleanup path. Since g_free doesn't clear the pointer this would lead to double frees when blindly replaced. Non-blind replace is obviously very expensive both for the author and for the reviewer. The only blind replacement we can do is to replace VIR_FREE with g_clear_pointer(&ptr, g_free); This is in the end what VIR_FREE exactly does. Now it's only necessary to justify the churn. To me it's borderline not worth it.
On a Thursday in 2020, Matt Coleman wrote:
Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 41 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a64dec8df4..a55079300c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5016,7 +5016,7 @@ virDomainControllerSCSINextUnit(const virDomainDef *def, #define SCSI_WIDE_BUS_MAX_CONT_UNIT 16 #define SCSI_NARROW_BUS_MAX_CONT_UNIT 7 -static int +static void virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt, const virDomainDef *def, virDomainHostdevDefPtr hostdev) @@ -5054,8 +5054,6 @@ virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt, hostdev->info->addr.drive.bus = 0; hostdev->info->addr.drive.target = 0; hostdev->info->addr.drive.unit = next_unit; - - return 0; } @@ -5089,6 +5087,7 @@ virDomainHostdevDefPostParse(virDomainHostdevDefPtr dev, virDomainXMLOptionPtr xmlopt) { virDomainHostdevSubsysSCSIPtr scsisrc; + virDomainDeviceDriveAddressPtr addr = NULL; if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) return 0; @@ -5101,27 +5100,23 @@ virDomainHostdevDefPostParse(virDomainHostdevDefPtr dev, virDomainPostParseCheckISCSIPath(&iscsisrc->src->path); } - if (dev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && - virDomainHostdevAssignAddress(xmlopt, def, dev) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Cannot assign SCSI host device address")); + if (dev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + virDomainHostdevAssignAddress(xmlopt, def, dev); + + /* Ensure provided address doesn't conflict with existing + * scsi disk drive address + */ + addr = &dev->info->addr.drive; + if (virDomainDriveAddressIsUsedByDisk(def, + VIR_DOMAIN_DISK_BUS_SCSI, + addr)) { + virReportError(VIR_ERR_XML_ERROR, + _("SCSI host address controller='%u' " + "bus='%u' target='%u' unit='%u' in " + "use by a SCSI disk"), + addr->controller, addr->bus, + addr->target, addr->unit); return -1; - } else { - /* Ensure provided address doesn't conflict with existing - * scsi disk drive address - */ - virDomainDeviceDriveAddressPtr addr = &dev->info->addr.drive; - if (virDomainDriveAddressIsUsedByDisk(def, - VIR_DOMAIN_DISK_BUS_SCSI, - addr)) { - virReportError(VIR_ERR_XML_ERROR, - _("SCSI host address controller='%u' " - "bus='%u' target='%u' unit='%u' in " - "use by a SCSI disk"), - addr->controller, addr->bus, - addr->target, addr->unit); - return -1; - } } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: { -- 2.27.0
These functions always return zero, so they might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a55079300c..26630884ac 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5177,7 +5177,7 @@ virDomainCheckVirtioOptions(virDomainVirtioOptionsPtr virtio) } -static int +static void virDomainChrDefPostParse(virDomainChrDefPtr chr, const virDomainDef *def) { @@ -5204,12 +5204,10 @@ virDomainChrDefPostParse(virDomainChrDefPtr chr, chr->target.port = maxport + 1; } - - return 0; } -static int +static void virDomainRNGDefPostParse(virDomainRNGDefPtr rng) { /* set default path for virtio-rng "random" backend to /dev/random */ @@ -5217,8 +5215,6 @@ virDomainRNGDefPostParse(virDomainRNGDefPtr rng) !rng->source.file) { rng->source.file = g_strdup("/dev/random"); } - - return 0; } @@ -5298,7 +5294,7 @@ virDomainDiskDefPostParse(virDomainDiskDefPtr disk, } -static int +static void virDomainVideoDefPostParse(virDomainVideoDefPtr video, const virDomainDef *def) { @@ -5310,8 +5306,6 @@ virDomainVideoDefPostParse(virDomainVideoDefPtr video, video->ram = VIR_ROUND_UP_POWER_OF_TWO(video->ram); video->vram = VIR_ROUND_UP_POWER_OF_TWO(video->vram); - - return 0; } @@ -5344,7 +5338,7 @@ virDomainNetDefPostParse(virDomainNetDefPtr net) } -static int +static void virDomainVsockDefPostParse(virDomainVsockDefPtr vsock) { if (vsock->auto_cid == VIR_TRISTATE_BOOL_ABSENT) { @@ -5353,8 +5347,6 @@ virDomainVsockDefPostParse(virDomainVsockDefPtr vsock) else vsock->auto_cid = VIR_TRISTATE_BOOL_YES; } - - return 0; } @@ -5386,11 +5378,13 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev, switch ((virDomainDeviceType)dev->type) { case VIR_DOMAIN_DEVICE_CHR: - ret = virDomainChrDefPostParse(dev->data.chr, def); + virDomainChrDefPostParse(dev->data.chr, def); + ret = 0; break; case VIR_DOMAIN_DEVICE_RNG: - ret = virDomainRNGDefPostParse(dev->data.rng); + virDomainRNGDefPostParse(dev->data.rng); + ret = 0; break; case VIR_DOMAIN_DEVICE_DISK: @@ -5398,7 +5392,8 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev, break; case VIR_DOMAIN_DEVICE_VIDEO: - ret = virDomainVideoDefPostParse(dev->data.video, def); + virDomainVideoDefPostParse(dev->data.video, def); + ret = 0; break; case VIR_DOMAIN_DEVICE_HOSTDEV: @@ -5414,7 +5409,8 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev, break; case VIR_DOMAIN_DEVICE_VSOCK: - ret = virDomainVsockDefPostParse(dev->data.vsock); + virDomainVsockDefPostParse(dev->data.vsock); + ret = 0; break; case VIR_DOMAIN_DEVICE_MEMORY: @@ -27734,7 +27730,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf, return 0; } -static int +static void virDomainNVRAMDefFormat(virBufferPtr buf, virDomainNVRAMDefPtr def, unsigned int flags) @@ -27746,8 +27742,6 @@ virDomainNVRAMDefFormat(virBufferPtr buf, virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</nvram>\n"); - - return 0; } -- 2.27.0
On 11/6/20 4:32 AM, Matt Coleman wrote:
These functions always return zero, so they might as well be void.
Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-)
@@ -27734,7 +27730,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf, return 0; }
-static int +static void virDomainNVRAMDefFormat(virBufferPtr buf, virDomainNVRAMDefPtr def, unsigned int flags) @@ -27746,8 +27742,6 @@ virDomainNVRAMDefFormat(virBufferPtr buf,
virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</nvram>\n"); - - return 0; }
In this function there is a check that can return -1; hence this hunk should not go in. Or the order of patches needs to be fixed since you are dropping the check in the next patch. Michal
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 81 +++++++++++++----------------------------- 1 file changed, 25 insertions(+), 56 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 26630884ac..04f4cd5114 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7488,7 +7488,7 @@ virDomainVirtioOptionsFormat(virBufferPtr buf, } -static int ATTRIBUTE_NONNULL(2) +static void ATTRIBUTE_NONNULL(2) virDomainDeviceInfoFormat(virBufferPtr buf, virDomainDeviceInfoPtr info, unsigned int flags) @@ -7539,7 +7539,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf, if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE || info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) /* We're done here */ - return 0; + return; virBufferAsprintf(&attrBuf, " type='%s'", virDomainDeviceAddressTypeToString(info->type)); @@ -7634,8 +7634,6 @@ virDomainDeviceInfoFormat(virBufferPtr buf, } virXMLFormatElement(buf, "address", &attrBuf, &childBuf); - - return 0; } static int @@ -25909,10 +25907,7 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->src->encryption && def->diskElementEnc && virStorageEncryptionFormat(buf, def->src->encryption) < 0) return -1; - if (virDomainDeviceInfoFormat(buf, &def->info, - flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) < 0) { - return -1; - } + virDomainDeviceInfoFormat(buf, &def->info, flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT); if (virDomainDiskDefFormatPrivateData(buf, def, flags, xmlopt) < 0) return -1; @@ -26113,8 +26108,7 @@ virDomainControllerDefFormat(virBufferPtr buf, if (virDomainControllerDriverFormat(&childBuf, def) < 0) return -1; - if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) - return -1; + virDomainDeviceInfoFormat(&childBuf, &def->info, flags); if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && def->opts.pciopts.pcihole64) { @@ -26287,8 +26281,7 @@ virDomainFSDefFormat(virBufferPtr buf, if (def->readonly) virBufferAddLit(buf, "<readonly/>\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) - return -1; + virDomainDeviceInfoFormat(buf, &def->info, flags); if (def->space_hard_limit) virBufferAsprintf(buf, "<space_hard_limit unit='bytes'>" @@ -27179,11 +27172,8 @@ virDomainNetDefFormat(virBufferPtr buf, virDomainNetDefCoalesceFormatXML(buf, def->coalesce); - if (virDomainDeviceInfoFormat(buf, &def->info, - flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT - | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) < 0) { - return -1; - } + virDomainDeviceInfoFormat(buf, &def->info, flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT + | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</interface>\n"); @@ -27482,8 +27472,7 @@ virDomainChrDefFormat(virBufferPtr buf, if (virDomainChrTargetDefFormat(buf, def, flags) < 0) return -1; - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) - return -1; + virDomainDeviceInfoFormat(buf, &def->info, flags); virBufferAdjustIndent(buf, -2); virBufferAsprintf(buf, "</%s>\n", elementName); @@ -27529,8 +27518,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf, _("unexpected smartcard type %d"), def->type); return -1; } - if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) - return -1; + virDomainDeviceInfoFormat(&childBuf, &def->info, flags); virBufferAsprintf(buf, "<smartcard mode='%s'", mode); if (def->type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH && @@ -27605,8 +27593,7 @@ virDomainTPMDefFormat(virBufferPtr buf, break; } - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) - return -1; + virDomainDeviceInfoFormat(buf, &def->info, flags); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</tpm>\n"); @@ -27636,8 +27623,7 @@ virDomainSoundDefFormat(virBufferPtr buf, if (def->audioId > 0) virBufferAsprintf(&childBuf, "<audio id='%d'/>\n", def->audioId); - if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) - return -1; + virDomainDeviceInfoFormat(&childBuf, &def->info, flags); virBufferAsprintf(buf, "<sound model='%s'", model); if (virBufferUse(&childBuf)) { @@ -27719,8 +27705,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf, if (def->period) virBufferAsprintf(&childrenBuf, "<stats period='%i'/>\n", def->period); - if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags) < 0) - return -1; + virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags); virDomainVirtioOptionsFormat(&driverAttrBuf, def->virtio); @@ -27737,8 +27722,7 @@ virDomainNVRAMDefFormat(virBufferPtr buf, { virBufferAddLit(buf, "<nvram>\n"); virBufferAdjustIndent(buf, 2); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) - return -1; + virDomainDeviceInfoFormat(buf, &def->info, flags); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</nvram>\n"); @@ -27769,8 +27753,7 @@ virDomainWatchdogDefFormat(virBufferPtr buf, virBufferAsprintf(&attrBuf, " model='%s' action='%s'", model, action); - if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) - return -1; + virDomainDeviceInfoFormat(&childBuf, &def->info, flags); virXMLFormatElement(buf, "watchdog", &attrBuf, &childBuf); @@ -27787,8 +27770,7 @@ static int virDomainPanicDefFormat(virBufferPtr buf, virBufferAsprintf(&attrBuf, " model='%s'", virDomainPanicModelTypeToString(def->model)); - if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0) < 0) - return -1; + virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0); virXMLFormatElement(buf, "panic", &attrBuf, &childrenBuf); @@ -27830,8 +27812,7 @@ virDomainShmemDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) - return -1; + virDomainDeviceInfoFormat(buf, &def->info, flags); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</shmem>\n"); @@ -27886,8 +27867,7 @@ virDomainRNGDefFormat(virBufferPtr buf, virXMLFormatElement(buf, "driver", &driverAttrBuf, NULL); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) - return -1; + virDomainDeviceInfoFormat(buf, &def->info, flags); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</rng>\n"); @@ -28023,8 +28003,7 @@ virDomainMemoryDefFormat(virBufferPtr buf, virDomainMemoryTargetDefFormat(buf, def); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) - return -1; + virDomainDeviceInfoFormat(buf, &def->info, flags); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</memory>\n"); @@ -28117,8 +28096,7 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) - return -1; + virDomainDeviceInfoFormat(buf, &def->info, flags); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</video>\n"); @@ -28174,8 +28152,7 @@ virDomainInputDefFormat(virBufferPtr buf, virXMLFormatElement(&childBuf, "driver", &driverAttrBuf, NULL); virBufferEscapeString(&childBuf, "<source evdev='%s'/>\n", def->source.evdev); - if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) - return -1; + virDomainDeviceInfoFormat(&childBuf, &def->info, flags); virXMLFormatElement(buf, "input", &attrBuf, &childBuf); @@ -28786,11 +28763,8 @@ virDomainHostdevDefFormat(virBufferPtr buf, if (def->shareable) virBufferAddLit(buf, "<shareable/>\n"); - if (virDomainDeviceInfoFormat(buf, def->info, - flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT - | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) < 0) { - return -1; - } + virDomainDeviceInfoFormat(buf, def->info, flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT + | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</hostdev>\n"); @@ -28816,10 +28790,7 @@ virDomainRedirdevDefFormat(virBufferPtr buf, if (virDomainChrSourceDefFormat(buf, def->source, flags) < 0) return -1; - if (virDomainDeviceInfoFormat(buf, &def->info, - flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) < 0) { - return -1; - } + virDomainDeviceInfoFormat(buf, &def->info, flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</redirdev>\n"); return 0; @@ -28879,8 +28850,7 @@ virDomainHubDefFormat(virBufferPtr buf, return -1; } - if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) - return -1; + virDomainDeviceInfoFormat(&childBuf, &def->info, flags); virBufferAsprintf(&attrBuf, " type='%s'", type); @@ -29543,8 +29513,7 @@ virDomainVsockDefFormat(virBufferPtr buf, virBufferAsprintf(&cidAttrBuf, " address='%u'", vsock->guest_cid); virXMLFormatElement(&childBuf, "cid", &cidAttrBuf, NULL); - if (virDomainDeviceInfoFormat(&childBuf, &vsock->info, 0) < 0) - return -1; + virDomainDeviceInfoFormat(&childBuf, &vsock->info, 0); virXMLFormatElement(buf, "vsock", &attrBuf, &childBuf); -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 04f4cd5114..8476be6bf8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14883,7 +14883,7 @@ virDomainGraphicsDefParseXMLSpice(virDomainGraphicsDefPtr def, } -static int +static void virDomainGraphicsDefParseXMLEGLHeadless(virDomainGraphicsDefPtr def, xmlNodePtr node, xmlXPathContextPtr ctxt) @@ -14896,7 +14896,6 @@ virDomainGraphicsDefParseXMLEGLHeadless(virDomainGraphicsDefPtr def, if ((glNode = virXPathNode("./gl", ctxt))) def->data.egl_headless.rendernode = virXMLPropString(glNode, "rendernode"); - return 0; } @@ -14984,8 +14983,7 @@ virDomainGraphicsDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; break; case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: - if (virDomainGraphicsDefParseXMLEGLHeadless(def, node, ctxt) < 0) - goto error; + virDomainGraphicsDefParseXMLEGLHeadless(def, node, ctxt); break; case VIR_DOMAIN_GRAPHICS_TYPE_LAST: break; -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8476be6bf8..30aee44445 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -25223,7 +25223,7 @@ virSecurityDeviceLabelDefFormat(virBufferPtr buf, } -static int +static void virDomainLeaseDefFormat(virBufferPtr buf, virDomainLeaseDefPtr def) { @@ -25237,8 +25237,6 @@ virDomainLeaseDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</lease>\n"); - - return 0; } static void @@ -30275,8 +30273,7 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def, return -1; for (n = 0; n < def->nleases; n++) - if (virDomainLeaseDefFormat(buf, def->leases[n]) < 0) - return -1; + virDomainLeaseDefFormat(buf, def->leases[n]); for (n = 0; n < def->nfss; n++) if (virDomainFSDefFormat(buf, def->fss[n], flags) < 0) @@ -31463,7 +31460,8 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, rc = virDomainDiskDefFormat(&buf, src->data.disk, flags, xmlopt); break; case VIR_DOMAIN_DEVICE_LEASE: - rc = virDomainLeaseDefFormat(&buf, src->data.lease); + virDomainLeaseDefFormat(&buf, src->data.lease); + rc = 0; break; case VIR_DOMAIN_DEVICE_FS: rc = virDomainFSDefFormat(&buf, src->data.fs, flags); -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 30aee44445..c106de7f81 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -25317,7 +25317,7 @@ virDomainDiskSourceFormatNetworkCookies(virBufferPtr buf, } -static int +static void virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf, virBufferPtr childBuf, virStorageSourcePtr src, @@ -25375,8 +25375,6 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf, if (src->timeout) virBufferAsprintf(childBuf, "<timeout seconds='%llu'/>\n", src->timeout); - - return 0; } @@ -25488,9 +25486,7 @@ virDomainDiskSourceFormat(virBufferPtr buf, break; case VIR_STORAGE_TYPE_NETWORK: - if (virDomainDiskSourceFormatNetwork(&attrBuf, &childBuf, - src, flags) < 0) - return -1; + virDomainDiskSourceFormatNetwork(&attrBuf, &childBuf, src, flags); break; case VIR_STORAGE_TYPE_VOLUME: -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c106de7f81..a3bfa11817 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -25605,7 +25605,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, disk->blkdeviotune.val); \ } -static int +static void virDomainDiskDefFormatIotune(virBufferPtr buf, virDomainDiskDefPtr disk) { @@ -25643,8 +25643,6 @@ virDomainDiskDefFormatIotune(virBufferPtr buf, FORMAT_IOTUNE(write_iops_sec_max_length); virXMLFormatElement(buf, "iotune", NULL, &childBuf); - - return 0; } #undef FORMAT_IOTUNE @@ -25880,8 +25878,7 @@ virDomainDiskDefFormat(virBufferPtr buf, } virBufferAddLit(buf, "/>\n"); - if (virDomainDiskDefFormatIotune(buf, def) < 0) - return -1; + virDomainDiskDefFormatIotune(buf, def); if (def->src->readonly) virBufferAddLit(buf, "<readonly/>\n"); -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a3bfa11817..88125683d1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -25648,7 +25648,7 @@ virDomainDiskDefFormatIotune(virBufferPtr buf, #undef FORMAT_IOTUNE -static int +static void virDomainDiskDefFormatDriver(virBufferPtr buf, virDomainDiskDefPtr disk) { @@ -25705,7 +25705,6 @@ virDomainDiskDefFormatDriver(virBufferPtr buf, virDomainVirtioOptionsFormat(&driverBuf, disk->virtio); virXMLFormatElement(buf, "driver", &driverBuf, NULL); - return 0; } @@ -25837,8 +25836,7 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); - if (virDomainDiskDefFormatDriver(buf, def) < 0) - return -1; + virDomainDiskDefFormatDriver(buf, def); /* Format as child of <disk> if defined there; otherwise, * if defined as child of <source>, then format later */ -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 88125683d1..d8a6f89695 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -25918,7 +25918,7 @@ virDomainDiskDefFormat(virBufferPtr buf, } -static int +static void virDomainControllerDriverFormat(virBufferPtr buf, virDomainControllerDefPtr def) { @@ -25944,8 +25944,6 @@ virDomainControllerDriverFormat(virBufferPtr buf, virDomainVirtioOptionsFormat(&driverBuf, def->virtio); virXMLFormatElement(buf, "driver", &driverBuf, NULL); - - return 0; } @@ -26092,8 +26090,7 @@ virDomainControllerDefFormat(virBufferPtr buf, } } - if (virDomainControllerDriverFormat(&childBuf, def) < 0) - return -1; + virDomainControllerDriverFormat(&childBuf, def); virDomainDeviceInfoFormat(&childBuf, &def->info, flags); -- 2.27.0
These functions always return zero, so they might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d8a6f89695..bcd5cc1313 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26692,7 +26692,7 @@ virDomainActualNetDefFormat(virBufferPtr buf, } -static int +static void virDomainVirtioNetGuestOptsFormat(char **outstr, virDomainNetDefPtr def) { @@ -26720,11 +26720,10 @@ virDomainVirtioNetGuestOptsFormat(char **outstr, virBufferTrim(&buf, " "); *outstr = virBufferContentAndReset(&buf); - return 0; } -static int +static void virDomainVirtioNetHostOptsFormat(char **outstr, virDomainNetDefPtr def) { @@ -26760,11 +26759,10 @@ virDomainVirtioNetHostOptsFormat(char **outstr, virBufferTrim(&buf, " "); *outstr = virBufferContentAndReset(&buf); - return 0; } -static int +static void virDomainVirtioNetDriverFormat(char **outstr, virDomainNetDefPtr def) { @@ -26797,7 +26795,6 @@ virDomainVirtioNetDriverFormat(char **outstr, virDomainVirtioOptionsFormat(&buf, def->virtio); *outstr = virBufferContentAndReset(&buf); - return 0; } @@ -27094,10 +27091,9 @@ virDomainNetDefFormat(virBufferPtr buf, g_autofree char *gueststr = NULL; g_autofree char *hoststr = NULL; - if (virDomainVirtioNetDriverFormat(&str, def) < 0 || - virDomainVirtioNetGuestOptsFormat(&gueststr, def) < 0 || - virDomainVirtioNetHostOptsFormat(&hoststr, def) < 0) - rc = -1; + virDomainVirtioNetDriverFormat(&str, def); + virDomainVirtioNetGuestOptsFormat(&gueststr, def); + virDomainVirtioNetHostOptsFormat(&hoststr, def); if (!gueststr && !hoststr) { if (str) -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bcd5cc1313..d716db596a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -28776,7 +28776,7 @@ virDomainRedirdevDefFormat(virBufferPtr buf, return 0; } -static int +static void virDomainRedirFilterDefFormat(virBufferPtr buf, virDomainRedirFilterDefPtr filter) { @@ -28784,7 +28784,7 @@ virDomainRedirFilterDefFormat(virBufferPtr buf, /* no need format an empty redirfilter */ if (filter->nusbdevs == 0) - return 0; + return; virBufferAddLit(buf, "<redirfilter>\n"); virBufferAdjustIndent(buf, 2); @@ -28812,7 +28812,6 @@ virDomainRedirFilterDefFormat(virBufferPtr buf, } virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</redirfilter>\n"); - return 0; } static int -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d716db596a..edfaf7c8af 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29372,7 +29372,7 @@ virDomainDefIothreadShouldFormat(virDomainDefPtr def) } -static int +static void virDomainIOMMUDefFormat(virBufferPtr buf, const virDomainIOMMUDef *iommu) { @@ -29407,8 +29407,6 @@ virDomainIOMMUDefFormat(virBufferPtr buf, virDomainIOMMUModelTypeToString(iommu->model)); virXMLFormatElement(buf, "iommu", &attrBuf, &childBuf); - - return 0; } @@ -30384,9 +30382,8 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def, return -1; } - if (def->iommu && - virDomainIOMMUDefFormat(buf, def->iommu) < 0) - return -1; + if (def->iommu) + virDomainIOMMUDefFormat(buf, def->iommu); if (def->vsock && virDomainVsockDefFormat(buf, def->vsock) < 0) -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index edfaf7c8af..6c7b386551 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29498,7 +29498,7 @@ virDomainVsockDefFormat(virBufferPtr buf, } -static int +static void virDomainDefFormatBlkiotune(virBufferPtr buf, virDomainDefPtr def) { @@ -29539,8 +29539,6 @@ virDomainDefFormatBlkiotune(virBufferPtr buf, } virXMLFormatElement(buf, "blkiotune", NULL, &childrenBuf); - - return 0; } @@ -29986,8 +29984,7 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def, virBufferAsprintf(buf, "<currentMemory unit='KiB'>%llu</currentMemory>\n", def->mem.cur_balloon); - if (virDomainDefFormatBlkiotune(buf, def) < 0) - return -1; + virDomainDefFormatBlkiotune(buf, def); virDomainMemtuneFormat(buf, &def->mem); virDomainMemorybackingFormat(buf, &def->mem); -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6c7b386551..7c4e34a2e6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27185,7 +27185,7 @@ virDomainChrAttrsDefFormat(virBufferPtr buf, return 0; } -static int +static void virDomainChrSourceDefFormat(virBufferPtr buf, virDomainChrSourceDefPtr def, unsigned int flags) @@ -27298,8 +27298,6 @@ virDomainChrSourceDefFormat(virBufferPtr buf, } virBufferAddLit(buf, "/>\n"); } - - return 0; } @@ -27446,8 +27444,7 @@ virDomainChrDefFormat(virBufferPtr buf, return -1; virBufferAddLit(buf, ">\n"); - if (virDomainChrSourceDefFormat(buf, def->source, flags) < 0) - return -1; + virDomainChrSourceDefFormat(buf, def->source, flags); if (virDomainChrTargetDefFormat(buf, def, flags) < 0) return -1; @@ -27489,8 +27486,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: - if (virDomainChrSourceDefFormat(&childBuf, def->data.passthru, flags) < 0) - return -1; + virDomainChrSourceDefFormat(&childBuf, def->data.passthru, flags); break; default: @@ -27829,8 +27825,7 @@ virDomainRNGDefFormat(virBufferPtr buf, return -1; virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); - if (virDomainChrSourceDefFormat(buf, def->source.chardev, flags) < 0) - return -1; + virDomainChrSourceDefFormat(buf, def->source.chardev, flags); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</backend>\n"); break; @@ -28767,8 +28762,7 @@ virDomainRedirdevDefFormat(virBufferPtr buf, virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); - if (virDomainChrSourceDefFormat(buf, def->source, flags) < 0) - return -1; + virDomainChrSourceDefFormat(buf, def->source, flags); virDomainDeviceInfoFormat(buf, &def->info, flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT); virBufferAdjustIndent(buf, -2); -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 6 +----- src/conf/domain_conf.h | 4 ++-- src/qemu/qemu_driver.c | 6 ++---- src/test/test_driver.c | 3 +-- 4 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7c4e34a2e6..25baa44a92 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31963,10 +31963,8 @@ virDomainDefVcpuOrderClear(virDomainDefPtr def) * * Set the block I/O tune settings from @info on the @disk, but error out early * in case of any error. That is to make sure nothing will fail half-way. - * - * Returns: 0 on success, -1 otherwise */ -int +void virDomainDiskSetBlockIOTune(virDomainDiskDefPtr disk, virDomainBlockIoTuneInfo *info) { @@ -31977,8 +31975,6 @@ virDomainDiskSetBlockIOTune(virDomainDiskDefPtr disk, VIR_FREE(disk->blkdeviotune.group_name); disk->blkdeviotune = *info; disk->blkdeviotune.group_name = g_steal_pointer(&tmp_group); - - return 0; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4608c1fafb..c1f182af1b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3804,8 +3804,8 @@ virDomainGetBlkioParametersAssignFromDef(virDomainDefPtr def, int *nparams, int maxparams); -int virDomainDiskSetBlockIOTune(virDomainDiskDefPtr disk, - virDomainBlockIoTuneInfo *info); +void virDomainDiskSetBlockIOTune(virDomainDiskDefPtr disk, + virDomainBlockIoTuneInfo *info); bool virDomainNetTypeSharesHostView(const virDomainNetDef *net); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cb56fbbfcf..591e3ec706 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16324,8 +16324,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, goto endjob; ret = -1; - if (virDomainDiskSetBlockIOTune(disk, &info) < 0) - goto endjob; + virDomainDiskSetBlockIOTune(disk, &info); qemuDomainSetGroupBlockIoTune(def, &info); @@ -16356,8 +16355,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, if (qemuDomainCheckBlockIoTuneReset(conf_disk, &conf_info) < 0) goto endjob; - if (virDomainDiskSetBlockIOTune(conf_disk, &conf_info) < 0) - goto endjob; + virDomainDiskSetBlockIOTune(conf_disk, &conf_info); qemuDomainSetGroupBlockIoTune(persistentDef, &conf_info); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 5c02a8ebb0..0f3d414a6a 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3832,8 +3832,7 @@ testDomainSetBlockIoTune(virDomainPtr dom, #undef TEST_BLOCK_IOTUNE_MAX_CHECK - if (virDomainDiskSetBlockIOTune(conf_disk, &info) < 0) - goto cleanup; + virDomainDiskSetBlockIOTune(conf_disk, &info); info.group_name = NULL; ret = 0; -- 2.27.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 25baa44a92..d71bd68682 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31972,7 +31972,7 @@ virDomainDiskSetBlockIOTune(virDomainDiskDefPtr disk, tmp_group = g_strdup(info->group_name); - VIR_FREE(disk->blkdeviotune.group_name); + g_free(disk->blkdeviotune.group_name); disk->blkdeviotune = *info; disk->blkdeviotune.group_name = g_steal_pointer(&tmp_group); } -- 2.27.0
On a Thursday in 2020, Matt Coleman wrote:
Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 25baa44a92..d71bd68682 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31972,7 +31972,7 @@ virDomainDiskSetBlockIOTune(virDomainDiskDefPtr disk,
tmp_group = g_strdup(info->group_name);
The tmp_group variable is not needed here. It's an artifact from back when strdup could fail, to avoid the situation of freeing the old group name without being able to copy the new one. Jano
- VIR_FREE(disk->blkdeviotune.group_name); + g_free(disk->blkdeviotune.group_name); disk->blkdeviotune = *info; disk->blkdeviotune.group_name = g_steal_pointer(&tmp_group); } -- 2.27.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d71bd68682..54d6f890ef 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17685,8 +17685,7 @@ virDomainDiskByTarget(virDomainDefPtr def, int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk) { - if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) - return -1; + def->disks = g_renew(virDomainDiskDefPtr, def->disks, def->ndisks + 1); virDomainDiskInsertPreAlloced(def, disk); @@ -18093,8 +18092,7 @@ virDomainNetARPInterfaces(virDomainDefPtr def, int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller) { - if (VIR_REALLOC_N(def->controllers, def->ncontrollers+1) < 0) - return -1; + def->controllers = g_renew(virDomainControllerDefPtr, def->controllers, def->ncontrollers + 1); virDomainControllerInsertPreAlloced(def, controller); -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 7 ++----- src/conf/domain_conf.h | 4 +--- src/libxl/libxl_driver.c | 3 +-- src/lxc/lxc_driver.c | 3 +-- src/qemu/qemu_driver.c | 3 +-- src/vz/vz_sdk.c | 6 ++---- tests/qemublocktest.c | 5 +++-- 7 files changed, 11 insertions(+), 20 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 54d6f890ef..6d0403b612 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17682,14 +17682,11 @@ virDomainDiskByTarget(virDomainDefPtr def, } -int virDomainDiskInsert(virDomainDefPtr def, - virDomainDiskDefPtr disk) +void virDomainDiskInsert(virDomainDefPtr def, + virDomainDiskDefPtr disk) { def->disks = g_renew(virDomainDiskDefPtr, def->disks, def->ndisks + 1); - virDomainDiskInsertPreAlloced(def, disk); - - return 0; } void virDomainDiskInsertPreAlloced(virDomainDefPtr def, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c1f182af1b..92155ec305 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3354,9 +3354,7 @@ virDomainDiskDefPtr virDomainDiskByTarget(virDomainDefPtr def, const char *dst); -int virDomainDiskInsert(virDomainDefPtr def, - virDomainDiskDefPtr disk) - G_GNUC_WARN_UNUSED_RESULT; +void virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk); void virDomainDiskInsertPreAlloced(virDomainDefPtr def, virDomainDiskDefPtr disk); int virDomainStorageNetworkParseHost(xmlNodePtr hostnode, diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 2195ecf47b..176516f5cb 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3548,8 +3548,7 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) _("target %s already exists."), disk->dst); return -1; } - if (virDomainDiskInsert(vmdef, disk) < 0) - return -1; + virDomainDiskInsert(vmdef, disk); /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ dev->data.disk = NULL; break; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index a6905b5a54..d0503ef2ea 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3045,8 +3045,7 @@ lxcDomainAttachDeviceConfig(virDomainDefPtr vmdef, _("target %s already exists."), disk->dst); return -1; } - if (virDomainDiskInsert(vmdef, disk) < 0) - return -1; + virDomainDiskInsert(vmdef, disk); /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ dev->data.disk = NULL; ret = 0; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 591e3ec706..58c34cd472 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7233,8 +7233,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, return -1; if (qemuCheckDiskConfigAgainstDomain(vmdef, disk) < 0) return -1; - if (virDomainDiskInsert(vmdef, disk) < 0) - return -1; + virDomainDiskInsert(vmdef, disk); /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ dev->data.disk = NULL; break; diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index cdd3f61ee1..00891dc16a 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -833,8 +833,7 @@ prlsdkAddDomainHardDisksInfo(vzDriverPtr driver, PRL_HANDLE sdkdom, virDomainDef if (prlsdkGetDiskInfo(driver, hdd, disk, false, IS_CT(def)) < 0) goto error; - if (virDomainDiskInsert(def, disk) < 0) - goto error; + virDomainDiskInsert(def, disk); disk = NULL; PrlHandle_Free(hdd); @@ -876,8 +875,7 @@ prlsdkAddDomainOpticalDisksInfo(vzDriverPtr driver, PRL_HANDLE sdkdom, virDomain PrlHandle_Free(cdrom); cdrom = PRL_INVALID_HANDLE; - if (virDomainDiskInsert(def, disk) < 0) - goto error; + virDomainDiskInsert(def, disk); } return 0; diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index c39f96716f..14212daadf 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -278,10 +278,11 @@ testQemuDiskXMLToProps(const void *opaque) VIR_DOMAIN_DEF_PARSE_STATUS))) return -1; - if (!(vmdef = virDomainDefNew()) || - virDomainDiskInsert(vmdef, disk) < 0) + if (!(vmdef = virDomainDefNew())) return -1; + virDomainDiskInsert(vmdef, disk); + if (qemuValidateDomainDeviceDefDisk(disk, vmdef, data->qemuCaps) < 0) { VIR_TEST_VERBOSE("invalid configuration for disk"); return -1; -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 5 +---- src/conf/domain_conf.h | 4 +--- src/libxl/libxl_conf.c | 3 +-- src/libxl/libxl_driver.c | 3 +-- src/qemu/qemu_driver.c | 3 +-- 5 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6d0403b612..4eace66ce7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18086,14 +18086,11 @@ virDomainNetARPInterfaces(virDomainDefPtr def, } -int virDomainControllerInsert(virDomainDefPtr def, +void virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller) { def->controllers = g_renew(virDomainControllerDefPtr, def->controllers, def->ncontrollers + 1); - virDomainControllerInsertPreAlloced(def, controller); - - return 0; } void virDomainControllerInsertPreAlloced(virDomainDefPtr def, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 92155ec305..83dfd6ac3e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3417,9 +3417,7 @@ int virDomainNetAppendIPAddress(virDomainNetDefPtr def, int family, unsigned int prefix); -int virDomainControllerInsert(virDomainDefPtr def, - virDomainControllerDefPtr controller) - G_GNUC_WARN_UNUSED_RESULT; +void virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); void virDomainControllerInsertPreAlloced(virDomainDefPtr def, virDomainControllerDefPtr controller); int virDomainControllerFind(const virDomainDef *def, int type, int idx); diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 383f2295a2..00748e21e8 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -2127,8 +2127,7 @@ libxlMakeDefaultUSBControllers(virDomainDefPtr def, if (libxlMakeUSBController(l_controller, &x_controllers[i]) < 0) goto error; - if (virDomainControllerInsert(def, l_controller) < 0) - goto error; + virDomainControllerInsert(def, l_controller); l_controller = NULL; } diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 176516f5cb..6af274cb1b 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3563,8 +3563,7 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) return -1; } - if (virDomainControllerInsert(vmdef, controller) < 0) - return -1; + virDomainControllerInsert(vmdef, controller); dev->data.controller = NULL; break; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 58c34cd472..30fe3f4ad6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7290,8 +7290,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, return -1; } - if (virDomainControllerInsert(vmdef, controller) < 0) - return -1; + virDomainControllerInsert(vmdef, controller); dev->data.controller = NULL; break; -- 2.27.0
Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4eace66ce7..987992efa6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18274,7 +18274,8 @@ int virDomainLeaseIndex(virDomainDefPtr def, int virDomainLeaseInsertPreAlloc(virDomainDefPtr def) { - return VIR_EXPAND_N(def->leases, def->nleases, 1); + def->leases = g_renew(virDomainLeaseDefPtr, def->leases, def->nleases + 1); + return 0; } int virDomainLeaseInsert(virDomainDefPtr def, -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 6 ++---- src/conf/domain_conf.h | 3 +-- src/qemu/qemu_hotplug.c | 3 +-- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 987992efa6..355bcca66d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18272,17 +18272,15 @@ int virDomainLeaseIndex(virDomainDefPtr def, } -int virDomainLeaseInsertPreAlloc(virDomainDefPtr def) +void virDomainLeaseInsertPreAlloc(virDomainDefPtr def) { def->leases = g_renew(virDomainLeaseDefPtr, def->leases, def->nleases + 1); - return 0; } int virDomainLeaseInsert(virDomainDefPtr def, virDomainLeaseDefPtr lease) { - if (virDomainLeaseInsertPreAlloc(def) < 0) - return -1; + virDomainLeaseInsertPreAlloc(def); virDomainLeaseInsertPreAlloced(def, lease); return 0; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 83dfd6ac3e..1b5dbc57e2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3434,8 +3434,7 @@ int virDomainLeaseIndex(virDomainDefPtr def, virDomainLeaseDefPtr lease); int virDomainLeaseInsert(virDomainDefPtr def, virDomainLeaseDefPtr lease); -int virDomainLeaseInsertPreAlloc(virDomainDefPtr def) - G_GNUC_WARN_UNUSED_RESULT; +void virDomainLeaseInsertPreAlloc(virDomainDefPtr def); void virDomainLeaseInsertPreAlloced(virDomainDefPtr def, virDomainLeaseDefPtr lease); virDomainLeaseDefPtr diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 81bbe178a9..c1461ac621 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3321,8 +3321,7 @@ qemuDomainAttachLease(virQEMUDriverPtr driver, { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - if (virDomainLeaseInsertPreAlloc(vm->def) < 0) - return -1; + virDomainLeaseInsertPreAlloc(vm->def); if (virDomainLockLeaseAttach(driver->lockManager, cfg->uri, vm, lease) < 0) { -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 5 +---- src/conf/domain_conf.h | 3 +-- src/qemu/qemu_driver.c | 3 +-- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 355bcca66d..090889889a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18277,13 +18277,10 @@ void virDomainLeaseInsertPreAlloc(virDomainDefPtr def) def->leases = g_renew(virDomainLeaseDefPtr, def->leases, def->nleases + 1); } -int virDomainLeaseInsert(virDomainDefPtr def, - virDomainLeaseDefPtr lease) +void virDomainLeaseInsert(virDomainDefPtr def, virDomainLeaseDefPtr lease) { virDomainLeaseInsertPreAlloc(def); - virDomainLeaseInsertPreAlloced(def, lease); - return 0; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1b5dbc57e2..42c36d2a37 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3432,8 +3432,7 @@ const char *virDomainControllerAliasFind(const virDomainDef *def, int virDomainLeaseIndex(virDomainDefPtr def, virDomainLeaseDefPtr lease); -int virDomainLeaseInsert(virDomainDefPtr def, - virDomainLeaseDefPtr lease); +void virDomainLeaseInsert(virDomainDefPtr def, virDomainLeaseDefPtr lease); void virDomainLeaseInsertPreAlloc(virDomainDefPtr def); void virDomainLeaseInsertPreAlloced(virDomainDefPtr def, virDomainLeaseDefPtr lease); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 30fe3f4ad6..78d1574206 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7272,8 +7272,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, lease->key, NULLSTR(lease->lockspace)); return -1; } - if (virDomainLeaseInsert(vmdef, lease) < 0) - return -1; + virDomainLeaseInsert(vmdef, lease); /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ dev->data.lease = NULL; -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 090889889a..e3d1922943 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27724,8 +27724,7 @@ virDomainWatchdogDefFormat(virBufferPtr buf, return 0; } -static int virDomainPanicDefFormat(virBufferPtr buf, - virDomainPanicDefPtr def) +static void virDomainPanicDefFormat(virBufferPtr buf, virDomainPanicDefPtr def) { g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) childrenBuf = VIR_BUFFER_INIT_CHILD(buf); @@ -27737,8 +27736,6 @@ static int virDomainPanicDefFormat(virBufferPtr buf, virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0); virXMLFormatElement(buf, "panic", &attrBuf, &childrenBuf); - - return 0; } static int @@ -30346,10 +30343,8 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def, if (def->nvram) virDomainNVRAMDefFormat(buf, def->nvram, flags); - for (n = 0; n < def->npanics; n++) { - if (virDomainPanicDefFormat(buf, def->panics[n]) < 0) - return -1; - } + for (n = 0; n < def->npanics; n++) + virDomainPanicDefFormat(buf, def->panics[n]); for (n = 0; n < def->nshmems; n++) { if (virDomainShmemDefFormat(buf, def->shmems[n], flags) < 0) @@ -31465,7 +31460,8 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, rc = virDomainTPMDefFormat(&buf, src->data.tpm, flags); break; case VIR_DOMAIN_DEVICE_PANIC: - rc = virDomainPanicDefFormat(&buf, src->data.panic); + virDomainPanicDefFormat(&buf, src->data.panic); + rc = 0; break; case VIR_DOMAIN_DEVICE_MEMORY: rc = virDomainMemoryDefFormat(&buf, src->data.memory, def, flags); -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e3d1922943..df876ae8c3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27738,7 +27738,7 @@ static void virDomainPanicDefFormat(virBufferPtr buf, virDomainPanicDefPtr def) virXMLFormatElement(buf, "panic", &attrBuf, &childrenBuf); } -static int +static void virDomainShmemDefFormat(virBufferPtr buf, virDomainShmemDefPtr def, unsigned int flags) @@ -27777,8 +27777,6 @@ virDomainShmemDefFormat(virBufferPtr buf, virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</shmem>\n"); - - return 0; } static int @@ -30346,10 +30344,8 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def, for (n = 0; n < def->npanics; n++) virDomainPanicDefFormat(buf, def->panics[n]); - for (n = 0; n < def->nshmems; n++) { - if (virDomainShmemDefFormat(buf, def->shmems[n], flags) < 0) - return -1; - } + for (n = 0; n < def->nshmems; n++) + virDomainShmemDefFormat(buf, def->shmems[n], flags); for (n = 0; n < def->nmems; n++) { if (virDomainMemoryDefFormat(buf, def->mems[n], def, flags) < 0) @@ -31467,7 +31463,8 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, rc = virDomainMemoryDefFormat(&buf, src->data.memory, def, flags); break; case VIR_DOMAIN_DEVICE_SHMEM: - rc = virDomainShmemDefFormat(&buf, src->data.shmem, flags); + virDomainShmemDefFormat(&buf, src->data.shmem, flags); + rc = 0; break; case VIR_DOMAIN_DEVICE_VSOCK: rc = virDomainVsockDefFormat(&buf, src->data.vsock); -- 2.27.0
This function always returns zero, so it might as well be void. Signed-off-by: Matt Coleman <matt@datto.com> --- src/conf/domain_conf.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index df876ae8c3..54ea7664d8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29446,7 +29446,7 @@ virDomainMemorybackingFormat(virBufferPtr buf, } -static int +static void virDomainVsockDefFormat(virBufferPtr buf, virDomainVsockDefPtr vsock) { @@ -29470,8 +29470,6 @@ virDomainVsockDefFormat(virBufferPtr buf, virDomainDeviceInfoFormat(&childBuf, &vsock->info, 0); virXMLFormatElement(buf, "vsock", &attrBuf, &childBuf); - - return 0; } @@ -30355,9 +30353,8 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def, if (def->iommu) virDomainIOMMUDefFormat(buf, def->iommu); - if (def->vsock && - virDomainVsockDefFormat(buf, def->vsock) < 0) - return -1; + if (def->vsock) + virDomainVsockDefFormat(buf, def->vsock); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</devices>\n"); @@ -31467,7 +31464,8 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, rc = 0; break; case VIR_DOMAIN_DEVICE_VSOCK: - rc = virDomainVsockDefFormat(&buf, src->data.vsock); + virDomainVsockDefFormat(&buf, src->data.vsock); + rc = 0; break; case VIR_DOMAIN_DEVICE_AUDIO: rc = virDomainAudioDefFormat(&buf, src->data.audio); -- 2.27.0
On Thu, Nov 5, 2020 at 10:33 PM Matt Coleman <mcoleman@datto.com> wrote:
Most of this is making functions void that unnecessarily return an int. It also includes some conversion to GLib.
Feel free to squash related commits, if you'd like. I left them separate to make it easier to review.
Matt Coleman (28): domain_conf: make virDomainDiskSetDriver() void domain_conf: make virDomainPostParseCheckISCSIPath() void domain_conf: use g_free() in virDomainPostParseCheckISCSIPath() domain_conf: make virDomainHostdevAssignAddress() void domain_conf: make virDomainChr/RNG/Video/VsockDefPostParse() and virDomainNVRAMDefFormat() void domain_conf: make virDomainDeviceInfoFormat() void domain_conf: make virDomainGraphicsDefParseXMLEGLHeadless() void domain_conf: make virDomainLeaseDefFormat() void domain_conf: make virDomainDiskSourceFormatNetwork() void domain_conf: make virDomainDiskDefFormatIotune() void domain_conf: make virDomainDiskDefFormatDriver() void domain_conf: make virDomainControllerDriverFormat() void domain_conf: make virDomainVirtioNetGuestOpts/HostOpts/DriverFormat() void domain_conf: make virDomainRedirFilterDefFormat() void domain_conf: make virDomainIOMMUDefFormat() void domain_conf: make virDomainDefFormatBlkiotune() void domain_conf: make virDomainChrSourceDefFormat() void domain_conf: make virDomainDiskSetBlockIOTune() void domain_conf: use g_free in virDomainDiskSetBlockIOTune() domain_conf: use g_renew in virDomainDiskInsert() and virDomainControllerInsert() domain_conf: make virDomainDiskInsert() void domain_conf: make virDomainControllerInsert() void domain_conf: use g_renew in virDomainLeaseInsertPreAlloc() domain_conf: make virDomainLeaseInsertPreAlloc() void domain_conf: make virDomainLeaseInsert() void domain_conf: make virDomainPanicDefFormat() void domain_conf: make virDomainShmemDefFormat() void domain_conf: make virDomainVsockDefFormat() void
src/conf/domain_conf.c | 349 ++++++++++++++------------------------- src/conf/domain_conf.h | 21 +-- src/libxl/libxl_conf.c | 5 +- src/libxl/libxl_domain.c | 5 +- src/libxl/libxl_driver.c | 9 +- src/libxl/xen_xl.c | 12 +- src/libxl/xen_xm.c | 10 +- src/lxc/lxc_driver.c | 3 +- src/qemu/qemu_domain.c | 5 +- src/qemu/qemu_driver.c | 15 +- src/qemu/qemu_hotplug.c | 3 +- src/test/test_driver.c | 3 +- src/vz/vz_sdk.c | 9 +- tests/qemublocktest.c | 5 +- 14 files changed, 158 insertions(+), 296 deletions(-)
-- 2.27.0
Series LGTM. Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ！/ Always, there's only one truth!
On 11/6/20 4:32 AM, Matt Coleman wrote:
Most of this is making functions void that unnecessarily return an int. It also includes some conversion to GLib.
Feel free to squash related commits, if you'd like. I left them separate to make it easier to review.
Yeah, some might be squashed.
14 files changed, 158 insertions(+), 296 deletions(-)
Nice cleanup. But as I say in 03/28 I think we might want glib adoption to be done in bigger chunks. Usually we rewrite VIR_ALLOC/VIR_REALLOC_N -> g_new0()/g_renew() in one patch (might be coupled with g_free() except VIR_FREE() resets the pointer to NULL and g_free() doesn't do that so I'm not really a fan of g_free()), then g_strdup() in another path, and so on. Michal
On a Monday in 2020, Michal Privoznik wrote:
On 11/6/20 4:32 AM, Matt Coleman wrote:
Most of this is making functions void that unnecessarily return an int. It also includes some conversion to GLib.
Feel free to squash related commits, if you'd like. I left them separate to make it easier to review.
Yeah, some might be squashed.
14 files changed, 158 insertions(+), 296 deletions(-)
Nice cleanup. But as I say in 03/28 I think we might want glib adoption to be done in bigger chunks.
Of course a bulk rewrite to g_free would get rid of VIR_FREE faster. But this series is about int -> void conversion. I think it's perfectly fine to include minor fixes you notice while doing that. No need to shave the whole yak [0] :) Jano [0] https://en.wiktionary.org/wiki/yak_shaving#Noun
Usually we rewrite VIR_ALLOC/VIR_REALLOC_N -> g_new0()/g_renew() in one patch (might be coupled with g_free() except VIR_FREE() resets the pointer to NULL and g_free() doesn't do that so I'm not really a fan of g_free()), then g_strdup() in another path, and so on.
Michal
On Nov 9, 2020, at 9:43 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
Nice cleanup. But as I say in 03/28 I think we might want glib adoption to be done in bigger chunks. Usually we rewrite VIR_ALLOC/VIR_REALLOC_N -> g_new0()/g_renew() in one patch (might be coupled with g_free() except VIR_FREE() resets the pointer to NULL and g_free() doesn't do that so I'm not really a fan of g_free()), then g_strdup() in another path, and so on.
I had thought that g_free() was a drop-in replacement for VIR_FREE() based on other earlier commits that I had seen. You can drop the g_free() commits from this series if everything else looks good. -- Matt
On 11/11/20 8:18 AM, Matt Coleman wrote:
On Nov 9, 2020, at 9:43 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
Nice cleanup. But as I say in 03/28 I think we might want glib adoption to be done in bigger chunks. Usually we rewrite VIR_ALLOC/VIR_REALLOC_N -> g_new0()/g_renew() in one patch (might be coupled with g_free() except VIR_FREE() resets the pointer to NULL and g_free() doesn't do that so I'm not really a fan of g_free()), then g_strdup() in another path, and so on.
I had thought that g_free() was a drop-in replacement for VIR_FREE() based on other earlier commits that I had seen. You can drop the g_free() commits from this series if everything else looks good.
Yes, please. VIR_FREE() is perfect as is :-) g_free() doesn't clear the pointer and g_clear_pointer(&ptr, g_free); is just too long. Michal
On 11/6/20 4:32 AM, Matt Coleman wrote:
Most of this is making functions void that unnecessarily return an int. It also includes some conversion to GLib.
Feel free to squash related commits, if you'd like. I left them separate to make it easier to review.
Matt Coleman (28): domain_conf: make virDomainDiskSetDriver() void domain_conf: make virDomainPostParseCheckISCSIPath() void domain_conf: use g_free() in virDomainPostParseCheckISCSIPath() domain_conf: make virDomainHostdevAssignAddress() void domain_conf: make virDomainChr/RNG/Video/VsockDefPostParse() and virDomainNVRAMDefFormat() void domain_conf: make virDomainDeviceInfoFormat() void domain_conf: make virDomainGraphicsDefParseXMLEGLHeadless() void domain_conf: make virDomainLeaseDefFormat() void domain_conf: make virDomainDiskSourceFormatNetwork() void domain_conf: make virDomainDiskDefFormatIotune() void domain_conf: make virDomainDiskDefFormatDriver() void domain_conf: make virDomainControllerDriverFormat() void domain_conf: make virDomainVirtioNetGuestOpts/HostOpts/DriverFormat() void domain_conf: make virDomainRedirFilterDefFormat() void domain_conf: make virDomainIOMMUDefFormat() void domain_conf: make virDomainDefFormatBlkiotune() void domain_conf: make virDomainChrSourceDefFormat() void domain_conf: make virDomainDiskSetBlockIOTune() void domain_conf: use g_free in virDomainDiskSetBlockIOTune() domain_conf: use g_renew in virDomainDiskInsert() and virDomainControllerInsert() domain_conf: make virDomainDiskInsert() void domain_conf: make virDomainControllerInsert() void domain_conf: use g_renew in virDomainLeaseInsertPreAlloc() domain_conf: make virDomainLeaseInsertPreAlloc() void domain_conf: make virDomainLeaseInsert() void domain_conf: make virDomainPanicDefFormat() void domain_conf: make virDomainShmemDefFormat() void domain_conf: make virDomainVsockDefFormat() void
src/conf/domain_conf.c | 349 ++++++++++++++------------------------- src/conf/domain_conf.h | 21 +-- src/libxl/libxl_conf.c | 5 +- src/libxl/libxl_domain.c | 5 +- src/libxl/libxl_driver.c | 9 +- src/libxl/xen_xl.c | 12 +- src/libxl/xen_xm.c | 10 +- src/lxc/lxc_driver.c | 3 +- src/qemu/qemu_domain.c | 5 +- src/qemu/qemu_driver.c | 15 +- src/qemu/qemu_hotplug.c | 3 +- src/test/test_driver.c | 3 +- src/vz/vz_sdk.c | 9 +- tests/qemublocktest.c | 5 +- 14 files changed, 158 insertions(+), 296 deletions(-)
I've dropped the g_free() patches (even though in this specific case they free a pointer which is set right after), Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and pushed. Michal
participants (5)
-
Ján Tomko -
Matt Coleman -
Michal Privoznik -
Neal Gompa -
Peter Krempa