From df07ccc6e1ceb55a34369f9e65ae921d29c05a29 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Sat, 15 Jun 2019 09:37:18 +0200 Subject: [PATCH] virHostdevPrepareSCSIVHostDevices: Simplify logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Couple of things happening in this patch: 1) We can mark the device we're adding onto active list as used way before - when adding it onto temporary list. 2) When actually moving device from a temporary helper list onto the list of active devices we check if the device isn't already there. The same check is performed by virSCSIVHostDeviceListAdd() later. Drop this duplicity. 3) The 'error' label is renamed to 'rollback' to reflect what it is actually doing. While in the rest of the code we don't allow random label names, this source file is different. Signed-off-by: Michal Privoznik Reviewed-by: Ján Tomko --- src/util/virhostdev.c | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 29e37ee4db..e32c550c2f 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -1645,10 +1645,9 @@ virHostdevPrepareSCSIVHostDevices(virHostdevManagerPtr mgr, virDomainHostdevDefPtr *hostdevs, int nhostdevs) { - size_t i, j; - int count; VIR_AUTOUNREF(virSCSIVHostDeviceListPtr) list = NULL; - virSCSIVHostDevicePtr host, tmp; + virSCSIVHostDevicePtr tmp; + size_t i, j; if (!nhostdevs) return 0; @@ -1665,6 +1664,7 @@ virHostdevPrepareSCSIVHostDevices(virHostdevManagerPtr mgr, for (i = 0; i < nhostdevs; i++) { virDomainHostdevDefPtr hostdev = hostdevs[i]; virDomainHostdevSubsysSCSIVHostPtr hostsrc = &hostdev->source.subsys.u.scsi_host; + VIR_AUTOPTR(virSCSIVHostDevice) host = NULL; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST) @@ -1676,10 +1676,12 @@ virHostdevPrepareSCSIVHostDevices(virHostdevManagerPtr mgr, if (!(host = virSCSIVHostDeviceNew(hostsrc->wwpn))) return -1; - if (virSCSIVHostDeviceListAdd(list, host) < 0) { - virSCSIVHostDeviceFree(host); + if (virSCSIVHostDeviceSetUsedBy(host, drv_name, dom_name) < 0) return -1; - } + + if (virSCSIVHostDeviceListAdd(list, host) < 0) + return -1; + host = NULL; } /* Loop 2: Mark devices in temporary list as used by @name @@ -1687,27 +1689,15 @@ virHostdevPrepareSCSIVHostDevices(virHostdevManagerPtr mgr, * wrong, perform rollback. */ virObjectLock(mgr->activeSCSIVHostHostdevs); - count = virSCSIVHostDeviceListCount(list); - for (i = 0; i < count; i++) { - host = virSCSIVHostDeviceListGet(list, i); - if ((tmp = virSCSIVHostDeviceListFind(mgr->activeSCSIVHostHostdevs, - host))) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("SCSI_host device %s is already in use by " - "another domain"), - virSCSIVHostDeviceGetName(tmp)); - goto error; - } else { - if (virSCSIVHostDeviceSetUsedBy(host, drv_name, dom_name) < 0) - goto error; + for (i = 0; i < virSCSIVHostDeviceListCount(list); i++) { + tmp = virSCSIVHostDeviceListGet(list, i); - VIR_DEBUG("Adding %s to activeSCSIVHostHostdevs", - virSCSIVHostDeviceGetName(host)); + VIR_DEBUG("Adding %s to activeSCSIVHostHostdevs", + virSCSIVHostDeviceGetName(tmp)); - if (virSCSIVHostDeviceListAdd(mgr->activeSCSIVHostHostdevs, host) < 0) - goto error; - } + if (virSCSIVHostDeviceListAdd(mgr->activeSCSIVHostHostdevs, tmp) < 0) + goto rollback; } virObjectUnlock(mgr->activeSCSIVHostHostdevs); @@ -1722,7 +1712,8 @@ virHostdevPrepareSCSIVHostDevices(virHostdevManagerPtr mgr, } return 0; - error: + + rollback: for (j = 0; j < i; j++) { tmp = virSCSIVHostDeviceListGet(list, i); virSCSIVHostDeviceListSteal(mgr->activeSCSIVHostHostdevs, tmp);