[PATCH v2 0/2] qemu_alias: Refactor functions
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
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@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
These functions always return success so it seems logical to not return anything and remove unnecessary checks. Signed-off-by: Kristina Hanicova <khanicov@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
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@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
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@redhat.com> Michal
participants (2)
-
Kristina Hanicova -
Michal Prívozník