From e84ab7938d5a794e9c3bf2f6b01356c4da201bfa Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Thu, 4 Feb 2016 14:24:53 +0100 Subject: [PATCH] conf: Move and optimize disk target duplicity checking Move the logic from virDomainDiskDefDstDuplicates into virDomainDiskDefCheckDuplicateInfo so that we don't have to loop multiple times through the array of disks. Since the original function was called in qemuBuildDriveDevStr, it was actually called for every single disk which was quite wasteful. Additionally the target uniqueness check needed to be duplicated in the disk hotplug case, since the disk was inserted into the domain definition after the device string was formatted and thus virDomainDiskDefDstDuplicates didn't do anything in that case. --- src/conf/domain_conf.c | 44 ++++++++++++---------------------------- src/conf/domain_conf.h | 1 - src/libvirt_private.syms | 1 - src/qemu/qemu_command.c | 3 --- src/qemu/qemu_driver.c | 3 +++ src/qemu/qemu_hotplug.c | 6 ------ 6 files changed, 16 insertions(+), 42 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e7d4ac90fa..e4eba1a957 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12992,31 +12992,6 @@ virDomainDiskControllerMatch(int controller_type, int disk_bus) return false; } -/* Return true if there's a duplicate disk[]->dst name for the same bus */ -bool -virDomainDiskDefDstDuplicates(virDomainDefPtr def) -{ - size_t i, j; - - /* optimization */ - if (def->ndisks <= 1) - return false; - - for (i = 1; i < def->ndisks; i++) { - for (j = 0; j < i; j++) { - if (STREQ(def->disks[i]->dst, def->disks[j]->dst)) { - virReportError(VIR_ERR_XML_ERROR, - _("target '%s' duplicated for disk sources " - "'%s' and '%s'"), - def->disks[i]->dst, - NULLSTR(virDomainDiskGetSource(def->disks[i])), - NULLSTR(virDomainDiskGetSource(def->disks[j]))); - return true; - } - } - } - return false; -} int virDomainDiskIndexByAddress(virDomainDefPtr def, @@ -23992,6 +23967,15 @@ int virDomainDiskDefCheckDuplicateInfo(virDomainDiskDefPtr a, virDomainDiskDefPtr b) { + if (STREQ(a->dst, b->dst)) { + virReportError(VIR_ERR_XML_ERROR, + _("target '%s' duplicated for disk sources '%s' and '%s'"), + a->dst, + NULLSTR(virDomainDiskGetSource(a)), + NULLSTR(virDomainDiskGetSource(b))); + return -1; + } + if (a->wwn && b->wwn && STREQ(a->wwn, b->wwn)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Disks '%s' and '%s' have identical WWN"), @@ -24017,12 +24001,10 @@ virDomainDefCheckDuplicateDiskInfo(virDomainDefPtr def) size_t j; for (i = 0; i < def->ndisks; i++) { - if (def->disks[i]->wwn || def->disks[i]->serial) { - for (j = i + 1; j < def->ndisks; j++) { - if (virDomainDiskDefCheckDuplicateInfo(def->disks[i], - def->disks[j]) < 0) - return -1; - } + for (j = i + 1; j < def->ndisks; j++) { + if (virDomainDiskDefCheckDuplicateInfo(def->disks[i], + def->disks[j]) < 0) + return -1; } } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index aa1908efc4..4afae59c78 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2770,7 +2770,6 @@ int virDomainDefCompatibleDevice(virDomainDefPtr def, void virDomainRNGDefFree(virDomainRNGDefPtr def); -bool virDomainDiskDefDstDuplicates(virDomainDefPtr def); int virDomainDiskIndexByAddress(virDomainDefPtr def, virDevicePCIAddressPtr pci_controller, unsigned int bus, unsigned int target, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 23a15a4e25..516e07375f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -258,7 +258,6 @@ virDomainDiskCacheTypeFromString; virDomainDiskCacheTypeToString; virDomainDiskDefAssignAddress; virDomainDiskDefCheckDuplicateInfo; -virDomainDiskDefDstDuplicates; virDomainDiskDefForeachPath; virDomainDiskDefFree; virDomainDiskDefNew; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8943270176..d7f19f37e6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4228,9 +4228,6 @@ qemuBuildDriveDevStr(virDomainDefPtr def, const char *contAlias; int controllerModel; - if (virDomainDiskDefDstDuplicates(def)) - goto error; - if (qemuCheckDiskConfig(disk) < 0) goto error; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6fc11c6a08..d10808b56a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7278,6 +7278,9 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup; + if (qemuProcessStartValidate(def, qemuCaps, false, false) < 0) + goto cleanup; + /* Since we're just exporting args, we can't do bridge/network/direct * setups, since libvirt will normally create TAP/macvtap devices * directly. We convert those configs into generic 'ethernet' diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 18a5a12d2a..87717804bf 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -794,12 +794,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, case VIR_DOMAIN_DISK_DEVICE_DISK: case VIR_DOMAIN_DISK_DEVICE_LUN: for (i = 0; i < vm->def->ndisks; i++) { - if (STREQ(vm->def->disks[i]->dst, disk->dst)) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("target %s already exists"), disk->dst); - goto cleanup; - } - if (virDomainDiskDefCheckDuplicateInfo(vm->def->disks[i], disk) < 0) goto cleanup; }