10:12 a.m.
This is v2 of:
https://listman.redhat.com/archives/libvir-list/2021-November/msg00748.html
Diff to v1:
* changed return type of functions if possible - when function
always returned success (suggested by Martin)
Kristina Hanicova (2):
qemu_alias: Rewrite of code pattern
qemu_alias: change return type to void if possible
src/qemu/qemu_alias.c | 185 +++++++++++++++-------------------------
src/qemu/qemu_alias.h | 44 +++++-----
src/qemu/qemu_hotplug.c | 43 ++++------
3 files changed, 105 insertions(+), 167 deletions(-)
--
2.31.1
10:12 a.m.
New subject: [PATCH v2 1/2] qemu_alias: Rewrite of code pattern
This patch rewrites the pattern using early return where it is
not needed and changes the return type of the functions to 'void'
if possible.
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/qemu/qemu_alias.c | 107 ++++++++++++++--------------------------
src/qemu/qemu_alias.h | 10 ++--
src/qemu/qemu_hotplug.c | 9 ++--
3 files changed, 44 insertions(+), 82 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 5e35f43614..ff5c7f1bd8 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -357,75 +357,57 @@ qemuAssignDeviceFSAlias(virDomainDef *def,
}
-static int
+static void
qemuAssignDeviceSoundAlias(virDomainSoundDef *sound,
int idx)
{
- if (sound->info.alias)
- return 0;
-
- sound->info.alias = g_strdup_printf("sound%d", idx);
- return 0;
+ if (!sound->info.alias)
+ sound->info.alias = g_strdup_printf("sound%d", idx);
}
-static int
+static void
qemuAssignDeviceVideoAlias(virDomainVideoDef *video,
int idx)
{
- if (video->info.alias)
- return 0;
-
- video->info.alias = g_strdup_printf("video%d", idx);
- return 0;
+ if (!video->info.alias)
+ video->info.alias = g_strdup_printf("video%d", idx);
}
-static int
+static void
qemuAssignDeviceHubAlias(virDomainHubDef *hub,
int idx)
{
- if (hub->info.alias)
- return 0;
-
- hub->info.alias = g_strdup_printf("hub%d", idx);
- return 0;
+ if (!hub->info.alias)
+ hub->info.alias = g_strdup_printf("hub%d", idx);
}
-static int
+static void
qemuAssignDeviceSmartcardAlias(virDomainSmartcardDef *smartcard,
int idx)
{
- if (smartcard->info.alias)
- return 0;
-
- smartcard->info.alias = g_strdup_printf("smartcard%d", idx);
- return 0;
+ if (!smartcard->info.alias)
+ smartcard->info.alias = g_strdup_printf("smartcard%d", idx);
}
-static int
+static void
qemuAssignDeviceMemballoonAlias(virDomainMemballoonDef *memballoon,
int idx)
{
- if (memballoon->info.alias)
- return 0;
-
- memballoon->info.alias = g_strdup_printf("balloon%d", idx);
- return 0;
+ if (!memballoon->info.alias)
+ memballoon->info.alias = g_strdup_printf("balloon%d", idx);
}
-static int
+static void
qemuAssignDeviceTPMAlias(virDomainTPMDef *tpm,
int idx)
{
- if (tpm->info.alias)
- return 0;
-
- tpm->info.alias = g_strdup_printf("tpm%d", idx);
- return 0;
+ if (!tpm->info.alias)
+ tpm->info.alias = g_strdup_printf("tpm%d", idx);
}
@@ -581,26 +563,23 @@ qemuAssignDeviceShmemAlias(virDomainDef *def,
}
-int
+void
qemuAssignDeviceWatchdogAlias(virDomainWatchdogDef *watchdog)
{
/* Currently, there's just one watchdog per domain */
- if (watchdog->info.alias)
- return 0;
-
- watchdog->info.alias = g_strdup("watchdog0");
-
- return 0;
+ if (!watchdog->info.alias)
+ watchdog->info.alias = g_strdup("watchdog0");
}
-int
+
+void
qemuAssignDeviceInputAlias(virDomainDef *def,
virDomainInputDef *input,
int idx)
{
if (input->info.alias)
- return 0;
+ return;
if (idx == -1) {
int thisidx;
@@ -613,19 +592,14 @@ qemuAssignDeviceInputAlias(virDomainDef *def,
}
input->info.alias = g_strdup_printf("input%d", idx);
-
- return 0;
}
-int
+void
qemuAssignDeviceVsockAlias(virDomainVsockDef *vsock)
{
- if (vsock->info.alias)
- return 0;
- vsock->info.alias = g_strdup("vsock0");
-
- return 0;
+ if (!vsock->info.alias)
+ vsock->info.alias = g_strdup("vsock0");
}
@@ -648,8 +622,7 @@ qemuAssignDeviceAliases(virDomainDef *def, virQEMUCaps *qemuCaps)
return -1;
}
for (i = 0; i < def->nsounds; i++) {
- if (qemuAssignDeviceSoundAlias(def->sounds[i], i) < 0)
- return -1;
+ qemuAssignDeviceSoundAlias(def->sounds[i], i);
}
for (i = 0; i < def->nhostdevs; i++) {
/* we can't start assigning at 0, since netdevs may have used
@@ -665,16 +638,14 @@ qemuAssignDeviceAliases(virDomainDef *def, virQEMUCaps *qemuCaps)
return -1;
}
for (i = 0; i < def->nvideos; i++) {
- if (qemuAssignDeviceVideoAlias(def->videos[i], i) < 0)
- return -1;
+ qemuAssignDeviceVideoAlias(def->videos[i], i);
}
for (i = 0; i < def->ncontrollers; i++) {
if (qemuAssignDeviceControllerAlias(def, def->controllers[i]) < 0)
return -1;
}
for (i = 0; i < def->ninputs; i++) {
- if (qemuAssignDeviceInputAlias(def, def->inputs[i], i) < 0)
- return -1;
+ qemuAssignDeviceInputAlias(def, def->inputs[i], i);
}
for (i = 0; i < def->nparallels; i++) {
if (qemuAssignDeviceChrAlias(def, def->parallels[i], i) < 0)
@@ -693,41 +664,35 @@ qemuAssignDeviceAliases(virDomainDef *def, virQEMUCaps *qemuCaps)
return -1;
}
for (i = 0; i < def->nhubs; i++) {
- if (qemuAssignDeviceHubAlias(def->hubs[i], i) < 0)
- return -1;
+ qemuAssignDeviceHubAlias(def->hubs[i], i);
}
for (i = 0; i < def->nshmems; i++) {
if (qemuAssignDeviceShmemAlias(def, def->shmems[i], i) < 0)
return -1;
}
for (i = 0; i < def->nsmartcards; i++) {
- if (qemuAssignDeviceSmartcardAlias(def->smartcards[i], i) < 0)
- return -1;
+ qemuAssignDeviceSmartcardAlias(def->smartcards[i], i);
}
if (def->watchdog) {
- if (qemuAssignDeviceWatchdogAlias(def->watchdog) < 0)
- return -1;
+ qemuAssignDeviceWatchdogAlias(def->watchdog);
}
if (def->memballoon &&
def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_NONE) {
- if (qemuAssignDeviceMemballoonAlias(def->memballoon, 0) < 0)
- return -1;
+ qemuAssignDeviceMemballoonAlias(def->memballoon, 0);
}
for (i = 0; i < def->nrngs; i++) {
if (qemuAssignDeviceRNGAlias(def, def->rngs[i]) < 0)
return -1;
}
for (i = 0; i < def->ntpms; i++) {
- if (qemuAssignDeviceTPMAlias(def->tpms[i], i) < 0)
- return -1;
+ qemuAssignDeviceTPMAlias(def->tpms[i], i);
}
for (i = 0; i < def->nmems; i++) {
if (qemuAssignDeviceMemoryAlias(def, def->mems[i], false) < 0)
return -1;
}
if (def->vsock) {
- if (qemuAssignDeviceVsockAlias(def->vsock) < 0)
- return -1;
+ qemuAssignDeviceVsockAlias(def->vsock);
}
return 0;
diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
index 604e667b9a..35db0dc736 100644
--- a/src/qemu/qemu_alias.h
+++ b/src/qemu/qemu_alias.h
@@ -65,13 +65,13 @@ int qemuAssignDeviceShmemAlias(virDomainDef *def,
virDomainShmemDef *shmem,
int idx);
-int qemuAssignDeviceWatchdogAlias(virDomainWatchdogDef *watchdog);
+void qemuAssignDeviceWatchdogAlias(virDomainWatchdogDef *watchdog);
-int qemuAssignDeviceInputAlias(virDomainDef *def,
- virDomainInputDef *input,
- int idx);
+void qemuAssignDeviceInputAlias(virDomainDef *def,
+ virDomainInputDef *input,
+ int idx);
-int qemuAssignDeviceVsockAlias(virDomainVsockDef *vsock);
+void qemuAssignDeviceVsockAlias(virDomainVsockDef *vsock);
int qemuAssignDeviceAliases(virDomainDef *def, virQEMUCaps *qemuCaps);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 3373ec2cdf..33466c40d6 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3130,8 +3130,7 @@ qemuDomainAttachWatchdog(virQEMUDriver *driver,
return -1;
}
- if (qemuAssignDeviceWatchdogAlias(watchdog) < 0)
- return -1;
+ qemuAssignDeviceWatchdogAlias(watchdog);
if (watchdog->model == VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB) {
if (qemuDomainEnsurePCIAddress(vm, &dev) < 0)
@@ -3239,8 +3238,7 @@ qemuDomainAttachInputDevice(virQEMUDriver *driver,
bool teardownlabel = false;
bool teardowncgroup = false;
- if (qemuAssignDeviceInputAlias(vm->def, input, -1) < 0)
- return -1;
+ qemuAssignDeviceInputAlias(vm->def, input, -1);
switch ((virDomainInputBus) input->bus) {
case VIR_DOMAIN_INPUT_BUS_USB:
@@ -3359,8 +3357,7 @@ qemuDomainAttachVsockDevice(virQEMUDriver *driver,
if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm, &dev) < 0)
return -1;
- if (qemuAssignDeviceVsockAlias(vsock) < 0)
- goto cleanup;
+ qemuAssignDeviceVsockAlias(vsock);
if (qemuProcessOpenVhostVsock(vsock) < 0)
goto cleanup;
--
2.31.1
10:12 a.m.
New subject: [PATCH v2 2/2] qemu_alias: change return type to void if possible
These functions always return success so it seems logical to not
return anything and remove unnecessary checks.
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/qemu/qemu_alias.c | 78 +++++++++++++++++------------------------
src/qemu/qemu_alias.h | 34 +++++++++---------
src/qemu/qemu_hotplug.c | 34 +++++++-----------
3 files changed, 61 insertions(+), 85 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index ff5c7f1bd8..1b80b7559e 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -120,14 +120,14 @@ qemuAssignDeviceChrAlias(virDomainDef *def,
}
-int
+void
qemuAssignDeviceControllerAlias(virDomainDef *domainDef,
virDomainControllerDef *controller)
{
const char *prefix = virDomainControllerTypeToString(controller->type);
if (controller->info.alias)
- return 0;
+ return;
if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
if (!virQEMUCapsHasPCIMultiBus(domainDef)) {
@@ -136,21 +136,21 @@ qemuAssignDeviceControllerAlias(virDomainDef *domainDef,
* "pci".
*/
controller->info.alias = g_strdup("pci");
- return 0;
+ return;
} else if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) {
/* The pcie-root controller on Q35 machinetypes uses a
* different naming convention ("pcie.0"), because it is
* hardcoded that way in qemu.
*/
controller->info.alias = g_strdup_printf("pcie.%d",
controller->idx);
- return 0;
+ return;
}
/* All other PCI controllers use the consistent "pci.%u"
* (including the hardcoded pci-root controller on
* multibus-capable qemus).
*/
controller->info.alias = g_strdup_printf("pci.%d",
controller->idx);
- return 0;
+ return;
} else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE) {
/* for any machine based on e.g. I440FX or G3Beige, the
* first (and currently only) IDE controller is an integrated
@@ -159,7 +159,7 @@ qemuAssignDeviceControllerAlias(virDomainDef *domainDef,
if (qemuDomainHasBuiltinIDE(domainDef) &&
controller->idx == 0) {
controller->info.alias = g_strdup("ide");
- return 0;
+ return;
}
} else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) {
/* for any Q35 machine, the first SATA controller is the
@@ -167,26 +167,25 @@ qemuAssignDeviceControllerAlias(virDomainDef *domainDef,
*/
if (qemuDomainIsQ35(domainDef) && controller->idx == 0) {
controller->info.alias = g_strdup("ide");
- return 0;
+ return;
}
} else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) {
/* first USB device is "usb", others are normal "usb%d" */
if (controller->idx == 0) {
controller->info.alias = g_strdup("usb");
- return 0;
+ return;
}
} else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_NCR53C90 &&
controller->idx == 0) {
controller->info.alias = g_strdup("scsi");
- return 0;
+ return;
}
}
/* all other controllers use the default ${type}${index} naming
* scheme for alias/id.
*/
controller->info.alias = g_strdup_printf("%s%d", prefix,
controller->idx);
- return 0;
}
@@ -264,13 +263,13 @@ qemuAssignDeviceDiskAlias(virDomainDef *def,
}
-int
+void
qemuAssignDeviceHostdevAlias(virDomainDef *def,
char **alias,
int idx)
{
if (*alias)
- return 0;
+ return;
if (idx == -1) {
size_t i;
@@ -296,25 +295,25 @@ qemuAssignDeviceHostdevAlias(virDomainDef *def,
}
*alias = g_strdup_printf("hostdev%d", idx);
-
- return 0;
}
-int
+void
qemuAssignDeviceNetAlias(virDomainDef *def,
virDomainNetDef *net,
int idx)
{
if (net->info.alias)
- return 0;
+ return;
/* <interface type='hostdev'> uses "hostdevN" as the alias
* We must use "-1" as the index because the caller doesn't know
* that we're now looking for a unique hostdevN rather than netN
*/
- if (virDomainNetResolveActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV)
- return qemuAssignDeviceHostdevAlias(def, &net->info.alias, -1);
+ if (virDomainNetResolveActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ qemuAssignDeviceHostdevAlias(def, &net->info.alias, -1);
+ return;
+ }
if (idx == -1) {
size_t i;
@@ -331,11 +330,10 @@ qemuAssignDeviceNetAlias(virDomainDef *def,
}
net->info.alias = g_strdup_printf("net%d", idx);
- return 0;
}
-int
+void
qemuAssignDeviceFSAlias(virDomainDef *def,
virDomainFSDef *fss)
{
@@ -343,7 +341,7 @@ qemuAssignDeviceFSAlias(virDomainDef *def,
int maxidx = 0;
if (fss->info.alias)
- return 0;
+ return;
for (i = 0; i < def->nfss; i++) {
int idx;
@@ -353,7 +351,6 @@ qemuAssignDeviceFSAlias(virDomainDef *def,
}
fss->info.alias = g_strdup_printf("fs%d", maxidx);
- return 0;
}
@@ -411,13 +408,13 @@ qemuAssignDeviceTPMAlias(virDomainTPMDef *tpm,
}
-int
+void
qemuAssignDeviceRedirdevAlias(virDomainDef *def,
virDomainRedirdevDef *redirdev,
int idx)
{
if (redirdev->info.alias)
- return 0;
+ return;
if (idx == -1) {
size_t i;
@@ -432,11 +429,10 @@ qemuAssignDeviceRedirdevAlias(virDomainDef *def,
}
redirdev->info.alias = g_strdup_printf("redir%d", idx);
- return 0;
}
-int
+void
qemuAssignDeviceRNGAlias(virDomainDef *def,
virDomainRNGDef *rng)
{
@@ -445,7 +441,7 @@ qemuAssignDeviceRNGAlias(virDomainDef *def,
int idx;
if (rng->info.alias)
- return 0;
+ return;
for (i = 0; i < def->nrngs; i++) {
if ((idx = qemuDomainDeviceAliasIndex(&def->rngs[i]->info,
"rng")) >= maxidx)
@@ -453,8 +449,6 @@ qemuAssignDeviceRNGAlias(virDomainDef *def,
}
rng->info.alias = g_strdup_printf("rng%d", maxidx);
-
- return 0;
}
@@ -535,13 +529,13 @@ qemuAssignDeviceMemoryAlias(virDomainDef *def,
}
-int
+void
qemuAssignDeviceShmemAlias(virDomainDef *def,
virDomainShmemDef *shmem,
int idx)
{
if (shmem->info.alias)
- return 0;
+ return;
if (idx == -1) {
size_t i;
@@ -559,7 +553,6 @@ qemuAssignDeviceShmemAlias(virDomainDef *def,
}
shmem->info.alias = g_strdup_printf("shmem%d", idx);
- return 0;
}
@@ -613,13 +606,11 @@ qemuAssignDeviceAliases(virDomainDef *def, virQEMUCaps *qemuCaps)
return -1;
}
for (i = 0; i < def->nnets; i++) {
- if (qemuAssignDeviceNetAlias(def, def->nets[i], -1) < 0)
- return -1;
+ qemuAssignDeviceNetAlias(def, def->nets[i], -1);
}
for (i = 0; i < def->nfss; i++) {
- if (qemuAssignDeviceFSAlias(def, def->fss[i]) < 0)
- return -1;
+ qemuAssignDeviceFSAlias(def, def->fss[i]);
}
for (i = 0; i < def->nsounds; i++) {
qemuAssignDeviceSoundAlias(def->sounds[i], i);
@@ -630,19 +621,16 @@ qemuAssignDeviceAliases(virDomainDef *def, virQEMUCaps *qemuCaps)
* linked to a NetDef, they will share an info and the alias
* will already be set, so don't try to set it again.
*/
- if (qemuAssignDeviceHostdevAlias(def,
&def->hostdevs[i]->info->alias, -1) < 0)
- return -1;
+ qemuAssignDeviceHostdevAlias(def, &def->hostdevs[i]->info->alias,
-1);
}
for (i = 0; i < def->nredirdevs; i++) {
- if (qemuAssignDeviceRedirdevAlias(def, def->redirdevs[i], i) < 0)
- return -1;
+ qemuAssignDeviceRedirdevAlias(def, def->redirdevs[i], i);
}
for (i = 0; i < def->nvideos; i++) {
qemuAssignDeviceVideoAlias(def->videos[i], i);
}
for (i = 0; i < def->ncontrollers; i++) {
- if (qemuAssignDeviceControllerAlias(def, def->controllers[i]) < 0)
- return -1;
+ qemuAssignDeviceControllerAlias(def, def->controllers[i]);
}
for (i = 0; i < def->ninputs; i++) {
qemuAssignDeviceInputAlias(def, def->inputs[i], i);
@@ -667,8 +655,7 @@ qemuAssignDeviceAliases(virDomainDef *def, virQEMUCaps *qemuCaps)
qemuAssignDeviceHubAlias(def->hubs[i], i);
}
for (i = 0; i < def->nshmems; i++) {
- if (qemuAssignDeviceShmemAlias(def, def->shmems[i], i) < 0)
- return -1;
+ qemuAssignDeviceShmemAlias(def, def->shmems[i], i);
}
for (i = 0; i < def->nsmartcards; i++) {
qemuAssignDeviceSmartcardAlias(def->smartcards[i], i);
@@ -681,8 +668,7 @@ qemuAssignDeviceAliases(virDomainDef *def, virQEMUCaps *qemuCaps)
qemuAssignDeviceMemballoonAlias(def->memballoon, 0);
}
for (i = 0; i < def->nrngs; i++) {
- if (qemuAssignDeviceRNGAlias(def, def->rngs[i]) < 0)
- return -1;
+ qemuAssignDeviceRNGAlias(def, def->rngs[i]);
}
for (i = 0; i < def->ntpms; i++) {
qemuAssignDeviceTPMAlias(def->tpms[i], i);
diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
index 35db0dc736..8fc27f4696 100644
--- a/src/qemu/qemu_alias.h
+++ b/src/qemu/qemu_alias.h
@@ -31,39 +31,39 @@ int qemuAssignDeviceChrAlias(virDomainDef *def,
virDomainChrDef *chr,
ssize_t idx);
-int qemuAssignDeviceControllerAlias(virDomainDef *domainDef,
- virDomainControllerDef *controller);
+void qemuAssignDeviceControllerAlias(virDomainDef *domainDef,
+ virDomainControllerDef *controller);
int qemuAssignDeviceDiskAlias(virDomainDef *def,
virDomainDiskDef *disk,
virQEMUCaps *qemuCaps);
-int qemuAssignDeviceHostdevAlias(virDomainDef *def,
- char **alias,
- int idx);
+void qemuAssignDeviceHostdevAlias(virDomainDef *def,
+ char **alias,
+ int idx);
-int qemuAssignDeviceNetAlias(virDomainDef *def,
- virDomainNetDef *net,
- int idx);
+void qemuAssignDeviceNetAlias(virDomainDef *def,
+ virDomainNetDef *net,
+ int idx);
-int
+void
qemuAssignDeviceFSAlias(virDomainDef *def,
virDomainFSDef *fss);
-int qemuAssignDeviceRedirdevAlias(virDomainDef *def,
- virDomainRedirdevDef *redirdev,
- int idx);
+void qemuAssignDeviceRedirdevAlias(virDomainDef *def,
+ virDomainRedirdevDef *redirdev,
+ int idx);
-int qemuAssignDeviceRNGAlias(virDomainDef *def,
- virDomainRNGDef *rng);
+void qemuAssignDeviceRNGAlias(virDomainDef *def,
+ virDomainRNGDef *rng);
int qemuAssignDeviceMemoryAlias(virDomainDef *def,
virDomainMemoryDef *mems,
bool oldAlias);
-int qemuAssignDeviceShmemAlias(virDomainDef *def,
- virDomainShmemDef *shmem,
- int idx);
+void qemuAssignDeviceShmemAlias(virDomainDef *def,
+ virDomainShmemDef *shmem,
+ int idx);
void qemuAssignDeviceWatchdogAlias(virDomainWatchdogDef *watchdog);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 33466c40d6..426710786b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -869,8 +869,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriver *driver,
if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm, &dev) < 0)
return -1;
- if (qemuAssignDeviceControllerAlias(vm->def, controller) < 0)
- goto cleanup;
+ qemuAssignDeviceControllerAlias(vm->def, controller);
if (qemuBuildControllerDevProps(vm->def, controller, priv->qemuCaps,
&devprops) < 0)
goto cleanup;
@@ -1221,8 +1220,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
actualType = virDomainNetGetActualType(net);
- if (qemuAssignDeviceNetAlias(vm->def, net, -1) < 0)
- goto cleanup;
+ qemuAssignDeviceNetAlias(vm->def, net, -1);
if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
/* This is really a "smart hostdev", so it should be attached
@@ -1699,8 +1697,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver,
if (backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO)
teardownlabel = true;
- if (qemuAssignDeviceHostdevAlias(vm->def, &info->alias, -1) < 0)
- goto error;
+ qemuAssignDeviceHostdevAlias(vm->def, &info->alias, -1);
if (qemuDomainIsPSeries(vm->def))
/* Isolation groups are only relevant for pSeries guests */
@@ -1960,8 +1957,7 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriver *driver,
const char *secAlias = NULL;
virErrorPtr orig_err;
- if (qemuAssignDeviceRedirdevAlias(def, redirdev, -1) < 0)
- return -1;
+ qemuAssignDeviceRedirdevAlias(def, redirdev, -1);
if (!(charAlias = qemuAliasChardevFromDevAlias(redirdev->info.alias)))
return -1;
@@ -2310,8 +2306,7 @@ qemuDomainAttachRNGDevice(virQEMUDriver *driver,
virJSONValue *props = NULL;
int ret = -1;
- if (qemuAssignDeviceRNGAlias(vm->def, rng) < 0)
- goto cleanup;
+ qemuAssignDeviceRNGAlias(vm->def, rng);
/* preallocate space for the device definition */
VIR_REALLOC_N(vm->def->rngs, vm->def->nrngs + 1);
@@ -2587,8 +2582,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriver *driver,
goto cleanup;
teardownlabel = true;
- if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1)
< 0)
- goto cleanup;
+ qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1);
+
if (!(devprops = qemuBuildUSBHostdevDevProps(vm->def, hostdev,
priv->qemuCaps)))
goto cleanup;
@@ -2667,8 +2662,7 @@ qemuDomainAttachHostSCSIDevice(virQEMUDriver *driver,
goto cleanup;
teardownlabel = true;
- if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1)
< 0)
- goto cleanup;
+ qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1);
if (qemuDomainPrepareHostdev(hostdev, priv) < 0)
goto cleanup;
@@ -2786,8 +2780,7 @@ qemuDomainAttachSCSIVHostDevice(virQEMUDriver *driver,
}
releaseaddr = true;
- if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1)
< 0)
- goto cleanup;
+ qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1);
if (!(devprops = qemuBuildSCSIVHostHostdevDevProps(vm->def,
hostdev,
@@ -2899,8 +2892,7 @@ qemuDomainAttachMediatedDevice(virQEMUDriver *driver,
goto cleanup;
teardownlabel = true;
- if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1)
< 0)
- goto cleanup;
+ qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1);
if (!(devprops = qemuBuildHostdevMediatedDevProps(vm->def, hostdev)))
goto cleanup;
@@ -3028,8 +3020,7 @@ qemuDomainAttachShmemDevice(virQEMUDriver *driver,
return -1;
}
- if (qemuAssignDeviceShmemAlias(vm->def, shmem, -1) < 0)
- return -1;
+ qemuAssignDeviceShmemAlias(vm->def, shmem, -1);
qemuDomainPrepareShmemChardev(shmem);
@@ -3438,8 +3429,7 @@ qemuDomainAttachFSDevice(virQEMUDriver *driver,
if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm, &dev) < 0)
return -1;
- if (qemuAssignDeviceFSAlias(vm->def, fs) < 0)
- goto cleanup;
+ qemuAssignDeviceFSAlias(vm->def, fs);
chardev = virDomainChrSourceDefNew(priv->driver->xmlopt);
chardev->type = VIR_DOMAIN_CHR_TYPE_UNIX;
--
2.31.1
2:20 a.m.
New subject: [PATCH v2 2/2] qemu_alias: change return type to void if possible
On 11/24/21 17:12, Kristina Hanicova wrote:
These functions always return success so it seems logical to not
return anything and remove unnecessary checks.
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/qemu/qemu_alias.c | 78 +++++++++++++++++------------------------
src/qemu/qemu_alias.h | 34 +++++++++---------
src/qemu/qemu_hotplug.c | 34 +++++++-----------
3 files changed, 61 insertions(+), 85 deletions(-)
Ooops, not rebased onto current master :-) But the conflict resolution
was trivial.
I think I see some more functions that can't fail really (can be fixed
in a follow up), for instance: qemuAssignDeviceChrAlias() can fail if
and only if qemuGetNextChrDevIndex() fails, but that can never happen.
This leaves us with qemuAssignDeviceDiskAlias() and
qemuAssignDeviceMemoryAlias(). In the latter, the error is returned only
in enum overflow (which I believe is a dead code). In the former -1 is
returned if qemuDomainFindSCSIControllerModel() fails, but controllers
should have been autofilled at this point.
Michal
2:20 a.m.
On 11/24/21 17:12, Kristina Hanicova wrote:
This is v2 of:
https://listman.redhat.com/archives/libvir-list/2021-November/msg00748.html
Diff to v1:
* changed return type of functions if possible - when function
always returned success (suggested by Martin)
Kristina Hanicova (2):
qemu_alias: Rewrite of code pattern
qemu_alias: change return type to void if possible
src/qemu/qemu_alias.c | 185 +++++++++++++++-------------------------
src/qemu/qemu_alias.h | 44 +++++-----
src/qemu/qemu_hotplug.c | 43 ++++------
3 files changed, 105 insertions(+), 167 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
1128
days inactive
1129
days old
4 comments
2 participants
participants (2)
-
Kristina Hanicova -
Michal Prívozník