[libvirt] [PATCH 00/14] qemu: Cdrom media change fixes and support for networked storage
Peter Krempa (14): qemu: Explicitly state that hotplugging cdroms and floppies doesn't work conf: Pass virStorageSource into virDomainDiskSourceIsBlockType qemu: hotplug: Untangle cleanup paths in qemuDomainChangeEjectableMedia qemu: hotplug: Add helper to initialize/teardown new disks for VMs qemu: hotplug: Change arguments for qemuDomainChangeEjectableMedia qemu: hotplug: Format proper source string for cdrom media change qemu: shared: Split out insertion code to the shared device list qemu: shared: Split out shared device list remove code qemu: conf: rename qemuCheckSharedDevice to qemuCheckSharedDisk qemu: conf: Split up qemuAddSharedDevice into per-device-type functions qemu: conf: Split up qemuRemoveSharedDevice into per-device-type functions qemu: conf: Split out code to retrieve hostdev key and reuse it qemu: hotplug: Sanitize shared device removal on media change qemu: hotplug: Reject media change when XML contains disk ABI change src/conf/domain_conf.c | 21 +-- src/conf/domain_conf.h | 5 +- src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_conf.c | 445 ++++++++++++++++++++++++----------------------- src/qemu/qemu_conf.h | 5 + src/qemu/qemu_driver.c | 43 ++--- src/qemu/qemu_hotplug.c | 269 +++++++++++++++------------- src/qemu/qemu_hotplug.h | 3 +- 11 files changed, 414 insertions(+), 384 deletions(-) -- 2.0.2
--- src/qemu/qemu_hotplug.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 004b6a4..f6e98b4 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -743,7 +743,9 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def, disk->bus, disk->dst))) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("No device with bus '%s' and target '%s'"), + _("No device with bus '%s' and target '%s'. " + "cdrom and floppy device hotplug isn't supported " + "by libvirt"), virDomainDiskBusTypeToString(disk->bus), disk->dst); goto end; -- 2.0.2
All checks are based on the storage source, thus there's no need to pass the complete disk def. --- src/conf/domain_conf.c | 19 ++++++++----------- src/conf/domain_conf.h | 2 +- src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_conf.c | 6 +++--- 6 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c7016f3..34c1a8c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20011,29 +20011,26 @@ virDomainDefFindDevice(virDomainDefPtr def, * Return true if its source is block type, or false otherwise. */ bool -virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def) +virDomainDiskSourceIsBlockType(virStorageSourcePtr src) { - /* No reason to think the disk source is block type if - * the source is empty - */ - if (!virDomainDiskGetSource(def)) + if (!src->path) return false; - if (virDomainDiskGetType(def) == VIR_STORAGE_TYPE_BLOCK) + if (src->type == VIR_STORAGE_TYPE_BLOCK) return true; /* For volume types, check the srcpool. * If it's a block type source pool, then it's possible */ - if (virDomainDiskGetType(def) == VIR_STORAGE_TYPE_VOLUME && - def->src->srcpool && - def->src->srcpool->voltype == VIR_STORAGE_VOL_BLOCK) { + if (src->type == VIR_STORAGE_TYPE_VOLUME && + src->srcpool && + src->srcpool->voltype == VIR_STORAGE_VOL_BLOCK) { /* We don't think the volume accessed by remote URI is * block type source, since we can't/shouldn't manage it * (e.g. set sgio=filtered|unfiltered for it) in libvirt. */ - if (def->src->srcpool->pooltype == VIR_STORAGE_POOL_ISCSI && - def->src->srcpool->mode == VIR_STORAGE_SOURCE_POOL_MODE_DIRECT) + if (src->srcpool->pooltype == VIR_STORAGE_POOL_ISCSI && + src->srcpool->mode == VIR_STORAGE_SOURCE_POOL_MODE_DIRECT) return false; return true; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ff7d640..21cfba2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2713,7 +2713,7 @@ int virDomainDefFindDevice(virDomainDefPtr def, virDomainDeviceDefPtr dev, bool reportError); -bool virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def) +bool virDomainDiskSourceIsBlockType(virStorageSourcePtr src) ATTRIBUTE_NONNULL(1); void virDomainChrSourceDefClear(virDomainChrSourceDefPtr def); diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index ff88e4f..f9af31c 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -373,7 +373,7 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, VIR_DEBUG("Allowing any disk block devs"); for (i = 0; i < def->ndisks; i++) { - if (!virDomainDiskSourceIsBlockType(def->disks[i])) + if (!virDomainDiskSourceIsBlockType(def->disks[i]->src)) continue; if (virCgroupAllowDevicePath(cgroup, diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 9e12ecc..6eff8b3 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4047,7 +4047,7 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver, goto cleanup; } - if (!virDomainDiskSourceIsBlockType(def)) { + if (!virDomainDiskSourceIsBlockType(def->src)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Can't setup disk for non-block device")); goto cleanup; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8a69976..b8af9e0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3660,7 +3660,7 @@ qemuBuildDriveDevStr(virDomainDefPtr def, virStorageNetProtocolTypeToString(disk->src->protocol)); goto error; } - } else if (!virDomainDiskSourceIsBlockType(disk)) { + } else if (!virDomainDiskSourceIsBlockType(disk->src)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("disk device='lun' is only valid for block type disk source")); goto error; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 6bfa48e..ec665d6 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -979,7 +979,7 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, if (dev->type == VIR_DOMAIN_DEVICE_DISK) { disk = dev->data.disk; - if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk)) + if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src)) return 0; } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev; @@ -1088,7 +1088,7 @@ qemuRemoveSharedDevice(virQEMUDriverPtr driver, if (dev->type == VIR_DOMAIN_DEVICE_DISK) { disk = dev->data.disk; - if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk)) + if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src)) return 0; } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev; @@ -1174,7 +1174,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) disk = dev->data.disk; if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN || - !virDomainDiskSourceIsBlockType(disk)) + !virDomainDiskSourceIsBlockType(disk->src)) return 0; path = virDomainDiskGetSource(disk); -- 2.0.2
Avoid the "audit" label to simplify control flow. --- src/qemu/qemu_hotplug.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f6e98b4..0ad467e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -107,7 +107,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, qemuDomainObjExitMonitor(driver, vm); if (ret < 0) - goto audit; + goto error; virObjectRef(vm); /* we don't want to report errors from media tray_open polling */ @@ -127,7 +127,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Unable to eject media")); ret = -1; - goto audit; + goto error; } src = virDomainDiskGetSource(disk); @@ -153,7 +153,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, src, format); qemuDomainObjExitMonitor(driver, vm); } - audit: + virDomainAuditDisk(vm, origdisk->src, disk->src, "update", ret >= 0); if (ret < 0) @@ -180,6 +180,8 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, return ret; error: + virDomainAuditDisk(vm, origdisk->src, disk->src, "update", false); + if (virSecurityManagerRestoreDiskLabel(driver->securityManager, vm->def, disk) < 0) VIR_WARN("Unable to restore security label on new media %s", src); -- 2.0.2
When we are changing media (or doing other hotplug operations) we need to setup cgroups, locking and seclabels on the new disk. This is a multi-step process where every piece can fail. To simplify dealing with this introduce qemuDomainPrepareDisk that similarly to qemuDomainPrepareDiskChainElement initializes/tears down a whole new disk to be used with the domain. Additionally the function supports passing a different source struct for media changes of cdroms that will be refactored later. --- src/qemu/qemu_driver.c | 8 --- src/qemu/qemu_hotplug.c | 144 ++++++++++++++++++++++++++++-------------------- 2 files changed, 85 insertions(+), 67 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82a82aa..03fe96f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6632,9 +6632,6 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0) goto end; - if (qemuSetupDiskCgroup(vm, disk) < 0) - goto end; - switch (disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: case VIR_DOMAIN_DISK_DEVICE_FLOPPY: @@ -6685,11 +6682,6 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, break; } - if (ret != 0 && - qemuTeardownDiskCgroup(vm, disk) < 0) - VIR_WARN("Failed to teardown cgroup for disk path %s", - NULLSTR(virDomainDiskGetSource(disk))); - end: virObjectUnref(caps); virDomainDeviceDefFree(dev_copy); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 0ad467e..5894f43 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -60,6 +60,83 @@ VIR_LOG_INIT("qemu.qemu_hotplug"); unsigned long long qemuDomainRemoveDeviceWaitTime = 1000ull * 5; +/** + * qemuDomainPrepareDisk: + * @driver: qemu driver struct + * @vm: domain object + * @disk: disk to prepare + * @overridesrc: Source different than @disk->src when necessary + * @teardown: Teardown the disk instead of adding it to a vm + * + * Setup the locks, cgroups and security permissions on a disk of a VM. + * If @overridesrc is specified the source struct is used instead of the + * one present in @disk. If @teardown is true, then the labels and cgroups + * are removed instead. + * + * Returns 0 on success and -1 on error. Reports libvirt error. + */ +static int +qemuDomainPrepareDisk(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virStorageSourcePtr overridesrc, + bool teardown) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + int ret = -1; + virStorageSourcePtr origsrc = NULL; + + if (overridesrc) { + origsrc = disk->src; + disk->src = overridesrc; + } + + /* just tear down the disk access */ + if (teardown) { + ret = 0; + goto rollback_cgroup; + } + + if (virDomainLockDiskAttach(driver->lockManager, cfg->uri, + vm, disk) < 0) + goto cleanup; + + if (virSecurityManagerSetDiskLabel(driver->securityManager, + vm->def, disk) < 0) + goto rollback_lock; + + if (qemuSetupDiskCgroup(vm, disk) < 0) + goto rollback_label; + + ret = 0; + goto cleanup; + + rollback_cgroup: + if (qemuTeardownDiskCgroup(vm, disk) < 0) + VIR_WARN("Unable to tear down cgroup access on %s", + virDomainDiskGetSource(disk)); + + rollback_label: + if (virSecurityManagerRestoreDiskLabel(driver->securityManager, + vm->def, disk) < 0) + VIR_WARN("Unable to restore security label on %s", + virDomainDiskGetSource(disk)); + + rollback_lock: + if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) + VIR_WARN("Unable to release lock on %s", + virDomainDiskGetSource(disk)); + + cleanup: + if (origsrc) + disk->src = origsrc; + + virObjectUnref(cfg); + + return ret; +} + + int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, @@ -87,17 +164,8 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, goto cleanup; } - if (virDomainLockDiskAttach(driver->lockManager, cfg->uri, - vm, disk) < 0) - goto cleanup; - - if (virSecurityManagerSetDiskLabel(driver->securityManager, - vm->def, disk) < 0) { - if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) - VIR_WARN("Unable to release lock on %s", - virDomainDiskGetSource(disk)); + if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0) goto cleanup; - } if (!(driveAlias = qemuDeviceDriveHostAlias(origdisk, priv->qemuCaps))) goto error; @@ -159,14 +227,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, if (ret < 0) goto error; - if (virSecurityManagerRestoreDiskLabel(driver->securityManager, - vm->def, origdisk) < 0) - VIR_WARN("Unable to restore security label on ejected image %s", - virDomainDiskGetSource(origdisk)); - - if (virDomainLockDiskDetach(driver->lockManager, vm, origdisk) < 0) - VIR_WARN("Unable to release lock on disk %s", - virDomainDiskGetSource(origdisk)); + ignore_value(qemuDomainPrepareDisk(driver, vm, origdisk, NULL, true)); if (virDomainDiskSetSource(origdisk, src) < 0) goto error; @@ -182,12 +243,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, error: virDomainAuditDisk(vm, origdisk->src, disk->src, "update", false); - if (virSecurityManagerRestoreDiskLabel(driver->securityManager, - vm->def, disk) < 0) - VIR_WARN("Unable to restore security label on new media %s", src); - - if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) - VIR_WARN("Unable to release lock on %s", src); + ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true)); goto cleanup; } @@ -266,17 +322,9 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, } } - if (virDomainLockDiskAttach(driver->lockManager, cfg->uri, - vm, disk) < 0) + if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0) goto cleanup; - if (virSecurityManagerSetDiskLabel(driver->securityManager, - vm->def, disk) < 0) { - if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) - VIR_WARN("Unable to release lock on %s", src); - goto cleanup; - } - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { if (virDomainCCWAddressAssign(&disk->info, priv->ccwaddrs, @@ -347,6 +395,8 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (releaseaddr) qemuDomainReleaseDeviceAddress(vm, &disk->info, src); + ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true)); + if (virSecurityManagerRestoreDiskLabel(driver->securityManager, vm->def, disk) < 0) VIR_WARN("Unable to restore security label on %s", src); @@ -495,7 +545,6 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, char *devstr = NULL; int ret = -1; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - const char *src = virDomainDiskGetSource(disk); for (i = 0; i < vm->def->ndisks; i++) { if (STREQ(vm->def->disks[i]->dst, disk->dst)) { @@ -505,17 +554,9 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, } } - if (virDomainLockDiskAttach(driver->lockManager, cfg->uri, - vm, disk) < 0) + if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0) goto cleanup; - if (virSecurityManagerSetDiskLabel(driver->securityManager, - vm->def, disk) < 0) { - if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) - VIR_WARN("Unable to release lock on %s", src); - goto cleanup; - } - /* We should have an address already, so make sure */ if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -597,13 +638,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, return ret; error: - if (virSecurityManagerRestoreDiskLabel(driver->securityManager, - vm->def, disk) < 0) - VIR_WARN("Unable to restore security label on %s", src); - - if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) - VIR_WARN("Unable to release lock on %s", src); - + ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true)); goto cleanup; } @@ -736,9 +771,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0) goto end; - if (qemuSetupDiskCgroup(vm, disk) < 0) - goto end; - switch (disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: case VIR_DOMAIN_DISK_DEVICE_FLOPPY: @@ -805,12 +837,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, break; } - if (ret != 0 && - qemuTeardownDiskCgroup(vm, disk) < 0) { - VIR_WARN("Failed to teardown cgroup for disk path %s", - NULLSTR(src)); - } - end: if (ret != 0) ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name)); -- 2.0.2
Pass the source of the changed media instead of a complete disk definition. Note that the @disk argument now contains what @olddisk would contain. The new source is passed as a virStorageSource struct. --- src/qemu/qemu_driver.c | 6 ++++- src/qemu/qemu_hotplug.c | 71 ++++++++++++++++++++++--------------------------- src/qemu/qemu_hotplug.h | 2 +- 3 files changed, 38 insertions(+), 41 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 03fe96f..fc345d5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6623,6 +6623,7 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, virDomainDiskDefPtr orig_disk = NULL; virDomainDiskDefPtr tmp = NULL; virDomainDeviceDefPtr dev_copy = NULL; + virStorageSourcePtr newsrc; virCapsPtr caps = NULL; int ret = -1; @@ -6661,7 +6662,10 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0) goto end; - ret = qemuDomainChangeEjectableMedia(driver, vm, disk, orig_disk, force); + newsrc = disk->src; + disk->src = NULL; + + ret = qemuDomainChangeEjectableMedia(driver, vm, orig_disk, newsrc, force); /* 'disk' must not be accessed now - it has been freed. * 'orig_disk' now points to the new disk, while 'dev_copy' * now points to the old disk */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5894f43..9ad06be 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -140,34 +140,33 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, - virDomainDiskDefPtr origdisk, + virStorageSourcePtr newsrc, bool force) { int ret = -1; char *driveAlias = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; int retries = CHANGE_MEDIA_RETRIES; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - const char *src = NULL; + const char *format = NULL; - if (!origdisk->info.alias) { + if (!disk->info.alias) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("missing disk device alias name for %s"), origdisk->dst); + _("missing disk device alias name for %s"), disk->dst); goto cleanup; } - if (origdisk->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY && - origdisk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) { + if (disk->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY && + disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Removable media not supported for %s device"), virDomainDiskDeviceTypeToString(disk->device)); goto cleanup; } - if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0) + if (qemuDomainPrepareDisk(driver, vm, disk, newsrc, false) < 0) goto cleanup; - if (!(driveAlias = qemuDeviceDriveHostAlias(origdisk, priv->qemuCaps))) + if (!(driveAlias = qemuDeviceDriveHostAlias(disk, priv->qemuCaps))) goto error; qemuDomainObjEnterMonitor(driver, vm); @@ -180,7 +179,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virObjectRef(vm); /* we don't want to report errors from media tray_open polling */ while (retries) { - if (origdisk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) + if (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) break; retries--; @@ -198,53 +197,42 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, goto error; } - src = virDomainDiskGetSource(disk); - if (src) { - /* deliberately don't depend on 'ret' as 'eject' may have failed the - * first time and we are going to check the drive state anyway */ - const char *format = NULL; - int type = virDomainDiskGetType(disk); - int diskFormat = virDomainDiskGetFormat(disk); - - if (type != VIR_STORAGE_TYPE_DIR) { - if (diskFormat > 0) { - format = virStorageFileFormatTypeToString(diskFormat); + if (newsrc->path) { + if (virStorageSourceGetActualType(newsrc) != VIR_STORAGE_TYPE_DIR) { + if (newsrc->format > 0) { + format = virStorageFileFormatTypeToString(newsrc->format); } else { - diskFormat = virDomainDiskGetFormat(origdisk); - if (diskFormat > 0) - format = virStorageFileFormatTypeToString(diskFormat); + if (disk->src->format > 0) + format = virStorageFileFormatTypeToString(disk->src->format); } } qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorChangeMedia(priv->mon, driveAlias, - src, format); + newsrc->path, + format); qemuDomainObjExitMonitor(driver, vm); } - virDomainAuditDisk(vm, origdisk->src, disk->src, "update", ret >= 0); + virDomainAuditDisk(vm, disk->src, newsrc, "update", ret >= 0); if (ret < 0) goto error; - ignore_value(qemuDomainPrepareDisk(driver, vm, origdisk, NULL, true)); - - if (virDomainDiskSetSource(origdisk, src) < 0) - goto error; - virDomainDiskSetType(origdisk, virDomainDiskGetType(disk)); + ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true)); - virDomainDiskDefFree(disk); + virStorageSourceFree(disk->src); + disk->src = newsrc; + newsrc = NULL; cleanup: + virStorageSourceFree(newsrc); VIR_FREE(driveAlias); - virObjectUnref(cfg); return ret; error: - virDomainAuditDisk(vm, origdisk->src, disk->src, "update", false); - - ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true)); - + virDomainAuditDisk(vm, disk->src, newsrc, "update", false); + ignore_value(qemuDomainPrepareDisk(driver, vm, disk, newsrc, true)); goto cleanup; } @@ -746,6 +734,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, virDomainDiskDefPtr disk = dev->data.disk; virDomainDiskDefPtr orig_disk = NULL; virDomainDeviceDefPtr dev_copy = NULL; + virStorageSourcePtr newsrc; virDomainDiskDefPtr tmp = NULL; virCapsPtr caps = NULL; int ret = -1; @@ -788,6 +777,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto end; + tmp = dev->data.disk; dev->data.disk = orig_disk; @@ -798,8 +788,11 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, } dev->data.disk = tmp; - ret = qemuDomainChangeEjectableMedia(driver, vm, disk, orig_disk, false); - /* 'disk' must not be accessed now - it has been free'd. + newsrc = disk->src; + disk->src = NULL; + + ret = qemuDomainChangeEjectableMedia(driver, vm, orig_disk, newsrc, false); + /* 'newsrc' must not be accessed now - it has been free'd. * 'orig_disk' now points to the new disk, while 'dev_copy' * now points to the old disk */ diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 6192973..5ce8f0a 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -31,7 +31,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, - virDomainDiskDefPtr orig_disk, + virStorageSourcePtr newsrc, bool force); int qemuDomainCheckEjectableMedia(virQEMUDriverPtr driver, virDomainObjPtr vm, -- 2.0.2
Use the qemu source string formatter to format the source string correctly for remote and other storage instead of passing source->path blindly. --- src/qemu/qemu_driver.c | 3 ++- src/qemu/qemu_hotplug.c | 12 +++++++++--- src/qemu/qemu_hotplug.h | 1 + 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fc345d5..7088f19 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6665,7 +6665,8 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, newsrc = disk->src; disk->src = NULL; - ret = qemuDomainChangeEjectableMedia(driver, vm, orig_disk, newsrc, force); + ret = qemuDomainChangeEjectableMedia(driver, conn, vm, + orig_disk, newsrc, force); /* 'disk' must not be accessed now - it has been freed. * 'orig_disk' now points to the new disk, while 'dev_copy' * now points to the old disk */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9ad06be..39907ab 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -138,6 +138,7 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, + virConnectPtr conn, virDomainObjPtr vm, virDomainDiskDefPtr disk, virStorageSourcePtr newsrc, @@ -148,6 +149,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; int retries = CHANGE_MEDIA_RETRIES; const char *format = NULL; + char *sourcestr = NULL; if (!disk->info.alias) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -197,7 +199,10 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, goto error; } - if (newsrc->path) { + if (!virStorageSourceIsLocalStorage(newsrc) || newsrc->path) { + if (qemuGetDriveSourceString(newsrc, conn, &sourcestr) < 0) + goto error; + if (virStorageSourceGetActualType(newsrc) != VIR_STORAGE_TYPE_DIR) { if (newsrc->format > 0) { format = virStorageFileFormatTypeToString(newsrc->format); @@ -209,7 +214,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorChangeMedia(priv->mon, driveAlias, - newsrc->path, + sourcestr, format); qemuDomainObjExitMonitor(driver, vm); } @@ -228,6 +233,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, cleanup: virStorageSourceFree(newsrc); VIR_FREE(driveAlias); + VIR_FREE(sourcestr); return ret; error: @@ -791,7 +797,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, newsrc = disk->src; disk->src = NULL; - ret = qemuDomainChangeEjectableMedia(driver, vm, orig_disk, newsrc, false); + ret = qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk, newsrc, false); /* 'newsrc' must not be accessed now - it has been free'd. * 'orig_disk' now points to the new disk, while 'dev_copy' * now points to the old disk */ diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 5ce8f0a..1200e44 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -29,6 +29,7 @@ # include "domain_conf.h" int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, + virConnectPtr conn, virDomainObjPtr vm, virDomainDiskDefPtr disk, virStorageSourcePtr newsrc, -- 2.0.2
To allow reuse split the code into a separate function and refactor it. To update an existing entry there's no need to copy it first, just update it inplace. --- src/qemu/qemu_conf.c | 81 ++++++++++++++++++++++++++++------------------------ 1 file changed, 44 insertions(+), 37 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index ec665d6..ef5d0a5 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -949,6 +949,47 @@ qemuSharedDeviceEntryCopy(const qemuSharedDeviceEntry *entry) return NULL; } + +static int +qemuSharedDeviceEntryInsert(virQEMUDriverPtr driver, + const char *key, + const char *name) +{ + qemuSharedDeviceEntry *entry = NULL; + int ret = -1; + + if ((entry = virHashLookup(driver->sharedDevices, key))) { + /* Nothing to do if the shared scsi host device is already + * recorded in the table. + */ + if (qemuSharedDeviceEntryDomainExists(entry, name, NULL)) { + ret = 0; + goto cleanup; + } + + if (VIR_EXPAND_N(entry->domains, entry->ref, 1) < 0 || + VIR_STRDUP(entry->domains[entry->ref - 1], name) < 0) + goto cleanup; + } else { + if (VIR_ALLOC(entry) < 0 || + VIR_ALLOC_N(entry->domains, 1) < 0 || + VIR_STRDUP(entry->domains[0], name) < 0) + goto cleanup; + + entry->ref = 1; + + if (virHashAddEntry(driver->sharedDevices, key, entry)) + goto cleanup; + } + + ret = 0; + + cleanup: + qemuSharedDeviceEntryFree(entry, NULL); + + return ret; +} + /* qemuAddSharedDevice: * @driver: Pointer to qemu driver struct * @dev: The device def @@ -963,8 +1004,6 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, virDomainDeviceDefPtr dev, const char *name) { - qemuSharedDeviceEntry *entry = NULL; - qemuSharedDeviceEntry *new_entry = NULL; virDomainDiskDefPtr disk = NULL; virDomainHostdevDefPtr hostdev = NULL; char *dev_name = NULL; @@ -1016,43 +1055,11 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, goto cleanup; } - if ((entry = virHashLookup(driver->sharedDevices, key))) { - /* Nothing to do if the shared scsi host device is already - * recorded in the table. - */ - if (qemuSharedDeviceEntryDomainExists(entry, name, NULL)) { - ret = 0; - goto cleanup; - } - - if (!(new_entry = qemuSharedDeviceEntryCopy(entry))) - goto cleanup; - - if (VIR_EXPAND_N(new_entry->domains, new_entry->ref, 1) < 0 || - VIR_STRDUP(new_entry->domains[new_entry->ref - 1], name) < 0) { - qemuSharedDeviceEntryFree(new_entry, NULL); - goto cleanup; - } - - if (virHashUpdateEntry(driver->sharedDevices, key, new_entry) < 0) { - qemuSharedDeviceEntryFree(new_entry, NULL); - goto cleanup; - } - } else { - if (VIR_ALLOC(entry) < 0 || - VIR_ALLOC_N(entry->domains, 1) < 0 || - VIR_STRDUP(entry->domains[0], name) < 0) { - qemuSharedDeviceEntryFree(entry, NULL); - goto cleanup; - } - - entry->ref = 1; - - if (virHashAddEntry(driver->sharedDevices, key, entry)) - goto cleanup; - } + if (qemuSharedDeviceEntryInsert(driver, key, name) < 0) + goto cleanup; ret = 0; + cleanup: qemuDriverUnlock(driver); VIR_FREE(dev_name); -- 2.0.2
Split it out into a separate function and simplify the code. There's no need to copy the entry to update it as the hash returns pointer to the existing item. Also remove the now unused qemuSharedDeviceEntryCopy function. --- src/qemu/qemu_conf.c | 77 ++++++++++++++++++---------------------------------- 1 file changed, 26 insertions(+), 51 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index ef5d0a5..79f8df8 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -924,31 +924,6 @@ qemuSharedDeviceEntryFree(void *payload, const void *name ATTRIBUTE_UNUSED) VIR_FREE(entry); } -static qemuSharedDeviceEntryPtr -qemuSharedDeviceEntryCopy(const qemuSharedDeviceEntry *entry) -{ - qemuSharedDeviceEntryPtr ret = NULL; - size_t i; - - if (VIR_ALLOC(ret) < 0) - return NULL; - - if (VIR_ALLOC_N(ret->domains, entry->ref) < 0) - goto cleanup; - - for (i = 0; i < entry->ref; i++) { - if (VIR_STRDUP(ret->domains[i], entry->domains[i]) < 0) - goto cleanup; - ret->ref++; - } - - return ret; - - cleanup: - qemuSharedDeviceEntryFree(ret, NULL); - return NULL; -} - static int qemuSharedDeviceEntryInsert(virQEMUDriverPtr driver, @@ -1068,6 +1043,31 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, return ret; } + +static int +qemuSharedDeviceEntryRemove(virQEMUDriverPtr driver, + const char *key, + const char *name) +{ + qemuSharedDeviceEntryPtr entry = NULL; + int idx; + + if (!(entry = virHashLookup(driver->sharedDevices, key))) + return -1; + + /* Nothing to do if the shared disk is not recored in the table. */ + if (!qemuSharedDeviceEntryDomainExists(entry, name, &idx)) + return 0; + + if (entry->ref != 1) + VIR_DELETE_ELEMENT(entry->domains, idx, entry->ref); + else + ignore_value(virHashRemoveEntry(driver->sharedDevices, key)); + + return 0; +} + + /* qemuRemoveSharedDevice: * @driver: Pointer to qemu driver struct * @device: The device def @@ -1082,15 +1082,12 @@ qemuRemoveSharedDevice(virQEMUDriverPtr driver, virDomainDeviceDefPtr dev, const char *name) { - qemuSharedDeviceEntryPtr entry = NULL; - qemuSharedDeviceEntryPtr new_entry = NULL; virDomainDiskDefPtr disk = NULL; virDomainHostdevDefPtr hostdev = NULL; char *key = NULL; char *dev_name = NULL; char *dev_path = NULL; int ret = -1; - int idx; if (dev->type == VIR_DOMAIN_DEVICE_DISK) { disk = dev->data.disk; @@ -1130,31 +1127,9 @@ qemuRemoveSharedDevice(virQEMUDriverPtr driver, goto cleanup; } - if (!(entry = virHashLookup(driver->sharedDevices, key))) + if (qemuSharedDeviceEntryRemove(driver, key, name) < 0) goto cleanup; - /* Nothing to do if the shared disk is not recored in - * the table. - */ - if (!qemuSharedDeviceEntryDomainExists(entry, name, &idx)) { - ret = 0; - goto cleanup; - } - - if (entry->ref != 1) { - if (!(new_entry = qemuSharedDeviceEntryCopy(entry))) - goto cleanup; - - VIR_DELETE_ELEMENT(new_entry->domains, idx, new_entry->ref); - if (virHashUpdateEntry(driver->sharedDevices, key, new_entry) < 0){ - qemuSharedDeviceEntryFree(new_entry, NULL); - goto cleanup; - } - } else { - if (virHashRemoveEntry(driver->sharedDevices, key) < 0) - goto cleanup; - } - ret = 0; cleanup: qemuDriverUnlock(driver); -- 2.0.2
The qemuCheckSharedDevice function is operating only on disk devices. Rename it and change the arguments to reflect that and refactor some logic for more readability. --- src/qemu/qemu_conf.c | 89 ++++++++++++++++++++++------------------------------ 1 file changed, 38 insertions(+), 51 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 79f8df8..5efabfb 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -814,82 +814,68 @@ qemuGetSharedDeviceKey(const char *device_path) * Returns 0 if no conflicts, otherwise returns -1. */ static int -qemuCheckSharedDevice(virHashTablePtr sharedDevices, - virDomainDeviceDefPtr dev) +qemuCheckSharedDisk(virHashTablePtr sharedDevices, + virDomainDiskDefPtr disk) { - virDomainDiskDefPtr disk = NULL; char *sysfs_path = NULL; char *key = NULL; int val; - int ret = 0; - const char *src; - - if (dev->type == VIR_DOMAIN_DEVICE_DISK) { - disk = dev->data.disk; + int ret = -1; - /* The only conflicts between shared disk we care about now - * is sgio setting, which is only valid for device='lun'. - */ - if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN) - return 0; - } else { + if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN) return 0; - } - - src = virDomainDiskGetSource(disk); - if (!(sysfs_path = virGetUnprivSGIOSysfsPath(src, NULL))) { - ret = -1; - goto cleanup; - } - /* It can't be conflict if unpriv_sgio is not supported - * by kernel. - */ - if (!virFileExists(sysfs_path)) + if (!(sysfs_path = virGetUnprivSGIOSysfsPath(disk->src->path, NULL))) goto cleanup; - if (!(key = qemuGetSharedDeviceKey(src))) { - ret = -1; + /* It can't be conflict if unpriv_sgio is not supported by kernel. */ + if (!virFileExists(sysfs_path)) { + ret = 0; goto cleanup; } - /* It can't be conflict if no other domain is - * is sharing it. - */ - if (!(virHashLookup(sharedDevices, key))) + if (!(key = qemuGetSharedDeviceKey(disk->src->path))) goto cleanup; - if (virGetDeviceUnprivSGIO(src, NULL, &val) < 0) { - ret = -1; + /* It can't be conflict if no other domain is sharing it. */ + if (!(virHashLookup(sharedDevices, key))) { + ret = 0; goto cleanup; } - if ((val == 0 && - (disk->sgio == VIR_DOMAIN_DEVICE_SGIO_FILTERED || - disk->sgio == VIR_DOMAIN_DEVICE_SGIO_DEFAULT)) || - (val == 1 && - disk->sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED)) + if (virGetDeviceUnprivSGIO(disk->src->path, NULL, &val) < 0) goto cleanup; - if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_VOLUME) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("sgio of shared disk 'pool=%s' 'volume=%s' conflicts " - "with other active domains"), - disk->src->srcpool->pool, - disk->src->srcpool->volume); - } else { - virReportError(VIR_ERR_OPERATION_INVALID, - _("sgio of shared disk '%s' conflicts with other " - "active domains"), src); + if (!((val == 0 && + (disk->sgio == VIR_DOMAIN_DEVICE_SGIO_FILTERED || + disk->sgio == VIR_DOMAIN_DEVICE_SGIO_DEFAULT)) || + (val == 1 && + disk->sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED))) { + + if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_VOLUME) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("sgio of shared disk 'pool=%s' 'volume=%s' conflicts " + "with other active domains"), + disk->src->srcpool->pool, + disk->src->srcpool->volume); + } else { + virReportError(VIR_ERR_OPERATION_INVALID, + _("sgio of shared disk '%s' conflicts with other " + "active domains"), disk->src->path); + } + + goto cleanup; } - ret = -1; + ret = 0; + cleanup: VIR_FREE(sysfs_path); VIR_FREE(key); return ret; } + bool qemuSharedDeviceEntryDomainExists(qemuSharedDeviceEntryPtr entry, const char *name, @@ -1007,10 +993,11 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, } qemuDriverLock(driver); - if (qemuCheckSharedDevice(driver->sharedDevices, dev) < 0) - goto cleanup; if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + if (qemuCheckSharedDisk(driver->sharedDevices, disk) < 0) + goto cleanup; + if (!(key = qemuGetSharedDeviceKey(virDomainDiskGetSource(disk)))) goto cleanup; } else { -- 2.0.2
Adding a shared device needs special steps for disks and hostdevs. Instead of having one function dealing this split the code into two separate functions that can be used with better granularity. --- src/qemu/qemu_conf.c | 125 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 79 insertions(+), 46 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 5efabfb..7af3c5c 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -951,71 +951,78 @@ qemuSharedDeviceEntryInsert(virQEMUDriverPtr driver, return ret; } -/* qemuAddSharedDevice: + +/* qemuAddSharedDisk: * @driver: Pointer to qemu driver struct - * @dev: The device def + * @src: disk source * @name: The domain name * * Increase ref count and add the domain name into the list which * records all the domains that use the shared device if the entry * already exists, otherwise add a new entry. */ -int -qemuAddSharedDevice(virQEMUDriverPtr driver, - virDomainDeviceDefPtr dev, - const char *name) +static int +qemuAddSharedDisk(virQEMUDriverPtr driver, + virDomainDiskDefPtr disk, + const char *name) { - virDomainDiskDefPtr disk = NULL; - virDomainHostdevDefPtr hostdev = NULL; - char *dev_name = NULL; - char *dev_path = NULL; char *key = NULL; int ret = -1; - /* Currently the only conflicts we have to care about for - * the shared disk and shared host device is "sgio" setting, - * which is only valid for block disk and scsi host device. - */ - if (dev->type == VIR_DOMAIN_DEVICE_DISK) { - disk = dev->data.disk; + if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src)) + return 0; - if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src)) - return 0; - } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { - hostdev = dev->data.hostdev; + qemuDriverLock(driver); - if (!hostdev->shareable || - !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) - return 0; - } else { + if (qemuCheckSharedDisk(driver->sharedDevices, disk) < 0) + goto cleanup; + + if (!(key = qemuGetSharedDeviceKey(virDomainDiskGetSource(disk)))) + goto cleanup; + + if (qemuSharedDeviceEntryInsert(driver, key, name) < 0) + goto cleanup; + + ret = 0; + + cleanup: + qemuDriverUnlock(driver); + VIR_FREE(key); + return ret; +} + + +static int +qemuAddSharedHostdev(virQEMUDriverPtr driver, + virDomainHostdevDefPtr hostdev, + const char *name) +{ + virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; + char *dev_name = NULL; + char *dev_path = NULL; + char *key = NULL; + int ret = -1; + + if (!hostdev->shareable || + !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) return 0; - } qemuDriverLock(driver); - if (dev->type == VIR_DOMAIN_DEVICE_DISK) { - if (qemuCheckSharedDisk(driver->sharedDevices, disk) < 0) - goto cleanup; - - if (!(key = qemuGetSharedDeviceKey(virDomainDiskGetSource(disk)))) - goto cleanup; - } else { - virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; - virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; - if (!(dev_name = virSCSIDeviceGetDevName(NULL, - scsihostsrc->adapter, - scsihostsrc->bus, - scsihostsrc->target, - scsihostsrc->unit))) - goto cleanup; + if (!(dev_name = virSCSIDeviceGetDevName(NULL, + scsihostsrc->adapter, + scsihostsrc->bus, + scsihostsrc->target, + scsihostsrc->unit))) + goto cleanup; - if (virAsprintf(&dev_path, "/dev/%s", dev_name) < 0) - goto cleanup; + if (virAsprintf(&dev_path, "/dev/%s", dev_name) < 0) + goto cleanup; - if (!(key = qemuGetSharedDeviceKey(dev_path))) - goto cleanup; - } + if (!(key = qemuGetSharedDeviceKey(dev_path))) + goto cleanup; if (qemuSharedDeviceEntryInsert(driver, key, name) < 0) goto cleanup; @@ -1055,6 +1062,32 @@ qemuSharedDeviceEntryRemove(virQEMUDriverPtr driver, } +/* qemuAddSharedDevice: + * @driver: Pointer to qemu driver struct + * @dev: The device def + * @name: The domain name + * + * Increase ref count and add the domain name into the list which + * records all the domains that use the shared device if the entry + * already exists, otherwise add a new entry. + */ +int +qemuAddSharedDevice(virQEMUDriverPtr driver, + virDomainDeviceDefPtr dev, + const char *name) +{ + /* Currently the only conflicts we have to care about for + * the shared disk and shared host device is "sgio" setting, + * which is only valid for block disk and scsi host device. + */ + if (dev->type == VIR_DOMAIN_DEVICE_DISK) + return qemuAddSharedDisk(driver, dev->data.disk, name); + else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) + return qemuAddSharedHostdev(driver, dev->data.hostdev, name); + else + return 0; +} + /* qemuRemoveSharedDevice: * @driver: Pointer to qemu driver struct * @device: The device def -- 2.0.2
Removing a shared device needs special steps for disks and hostdevs. Instead of having one function dealing this split the code into two separate functions that can be used with better granularity. --- src/qemu/qemu_conf.c | 117 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 71 insertions(+), 46 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 7af3c5c..f6a3b8d 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1088,64 +1088,65 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, return 0; } -/* qemuRemoveSharedDevice: - * @driver: Pointer to qemu driver struct - * @device: The device def - * @name: The domain name - * - * Decrease ref count and remove the domain name from the list which - * records all the domains that use the shared device if ref is not - * 1, otherwise remove the entry. - */ -int -qemuRemoveSharedDevice(virQEMUDriverPtr driver, - virDomainDeviceDefPtr dev, - const char *name) + +static int +qemuRemoveSharedDisk(virQEMUDriverPtr driver, + virDomainDiskDefPtr disk, + const char *name) { - virDomainDiskDefPtr disk = NULL; - virDomainHostdevDefPtr hostdev = NULL; char *key = NULL; - char *dev_name = NULL; - char *dev_path = NULL; int ret = -1; - if (dev->type == VIR_DOMAIN_DEVICE_DISK) { - disk = dev->data.disk; + if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src)) + return 0; - if (!disk->src->shared || !virDomainDiskSourceIsBlockType(disk->src)) - return 0; - } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { - hostdev = dev->data.hostdev; + qemuDriverLock(driver); - if (!hostdev->shareable || - !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) - return 0; - } else { + if (!(key = qemuGetSharedDeviceKey(virDomainDiskGetSource(disk)))) + goto cleanup; + + if (qemuSharedDeviceEntryRemove(driver, key, name) < 0) + goto cleanup; + + ret = 0; + cleanup: + qemuDriverUnlock(driver); + VIR_FREE(key); + return ret; +} + + +static int +qemuRemoveSharedHostdev(virQEMUDriverPtr driver, + virDomainHostdevDefPtr hostdev, + const char *name) +{ + virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; + char *key = NULL; + char *dev_name = NULL; + char *dev_path = NULL; + int ret = -1; + + if (!hostdev->shareable || + !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) return 0; - } qemuDriverLock(driver); - if (dev->type == VIR_DOMAIN_DEVICE_DISK) { - if (!(key = qemuGetSharedDeviceKey(virDomainDiskGetSource(disk)))) - goto cleanup; - } else { - virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; - virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; - if (!(dev_name = virSCSIDeviceGetDevName(NULL, - scsihostsrc->adapter, - scsihostsrc->bus, - scsihostsrc->target, - scsihostsrc->unit))) - goto cleanup; + if (!(dev_name = virSCSIDeviceGetDevName(NULL, + scsihostsrc->adapter, + scsihostsrc->bus, + scsihostsrc->target, + scsihostsrc->unit))) + goto cleanup; - if (virAsprintf(&dev_path, "/dev/%s", dev_name) < 0) - goto cleanup; + if (virAsprintf(&dev_path, "/dev/%s", dev_name) < 0) + goto cleanup; - if (!(key = qemuGetSharedDeviceKey(dev_path))) - goto cleanup; - } + if (!(key = qemuGetSharedDeviceKey(dev_path))) + goto cleanup; if (qemuSharedDeviceEntryRemove(driver, key, name) < 0) goto cleanup; @@ -1159,6 +1160,30 @@ qemuRemoveSharedDevice(virQEMUDriverPtr driver, return ret; } + +/* qemuRemoveSharedDevice: + * @driver: Pointer to qemu driver struct + * @device: The device def + * @name: The domain name + * + * Decrease ref count and remove the domain name from the list which + * records all the domains that use the shared device if ref is not + * 1, otherwise remove the entry. + */ +int +qemuRemoveSharedDevice(virQEMUDriverPtr driver, + virDomainDeviceDefPtr dev, + const char *name) +{ + if (dev->type == VIR_DOMAIN_DEVICE_DISK) + return qemuRemoveSharedDisk(driver, dev->data.disk, name); + else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) + return qemuRemoveSharedHostdev(driver, dev->data.hostdev, name); + else + return 0; +} + + int qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) { -- 2.0.2
Both addition and removal of a shared hostdev share the code to generate the hostdev key. Split it out into a separate function and refactor them. --- src/qemu/qemu_conf.c | 76 ++++++++++++++++++++++------------------------------ 1 file changed, 32 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index f6a3b8d..00405cc 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -992,24 +992,14 @@ qemuAddSharedDisk(virQEMUDriverPtr driver, } -static int -qemuAddSharedHostdev(virQEMUDriverPtr driver, - virDomainHostdevDefPtr hostdev, - const char *name) +static char * +qemuGetSharedHostdevKey(virDomainHostdevDefPtr hostdev) { virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; char *dev_name = NULL; char *dev_path = NULL; char *key = NULL; - int ret = -1; - - if (!hostdev->shareable || - !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) - return 0; - - qemuDriverLock(driver); if (!(dev_name = virSCSIDeviceGetDevName(NULL, scsihostsrc->adapter, @@ -1024,15 +1014,33 @@ qemuAddSharedHostdev(virQEMUDriverPtr driver, if (!(key = qemuGetSharedDeviceKey(dev_path))) goto cleanup; - if (qemuSharedDeviceEntryInsert(driver, key, name) < 0) - goto cleanup; - - ret = 0; - cleanup: - qemuDriverUnlock(driver); VIR_FREE(dev_name); VIR_FREE(dev_path); + + return key; +} + +static int +qemuAddSharedHostdev(virQEMUDriverPtr driver, + virDomainHostdevDefPtr hostdev, + const char *name) +{ + char *key = NULL; + int ret = -1; + + if (!hostdev->shareable || + !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) + return 0; + + if (!(key = qemuGetSharedHostdevKey(hostdev))) + return -1; + + qemuDriverLock(driver); + ret = qemuSharedDeviceEntryInsert(driver, key, name); + qemuDriverUnlock(driver); + VIR_FREE(key); return ret; } @@ -1121,41 +1129,21 @@ qemuRemoveSharedHostdev(virQEMUDriverPtr driver, virDomainHostdevDefPtr hostdev, const char *name) { - virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; - virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; char *key = NULL; - char *dev_name = NULL; - char *dev_path = NULL; - int ret = -1; + int ret; if (!hostdev->shareable || !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) return 0; - qemuDriverLock(driver); - - if (!(dev_name = virSCSIDeviceGetDevName(NULL, - scsihostsrc->adapter, - scsihostsrc->bus, - scsihostsrc->target, - scsihostsrc->unit))) - goto cleanup; - - if (virAsprintf(&dev_path, "/dev/%s", dev_name) < 0) - goto cleanup; - - if (!(key = qemuGetSharedDeviceKey(dev_path))) - goto cleanup; - - if (qemuSharedDeviceEntryRemove(driver, key, name) < 0) - goto cleanup; + if (!(key = qemuGetSharedHostdevKey(hostdev))) + return -1; - ret = 0; - cleanup: + qemuDriverLock(driver); + ret = qemuSharedDeviceEntryRemove(driver, key, name); qemuDriverUnlock(driver); - VIR_FREE(dev_name); - VIR_FREE(dev_path); + VIR_FREE(key); return ret; } -- 2.0.2
Instead of tediously copying of the disk source to remove it later ensure that the media change function removes the old device after it succeeds. --- src/qemu/qemu_conf.c | 2 +- src/qemu/qemu_conf.h | 5 ++++ src/qemu/qemu_driver.c | 38 +++++++----------------------- src/qemu/qemu_hotplug.c | 62 +++++++++++++++++++++++-------------------------- 4 files changed, 43 insertions(+), 64 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 00405cc..6b4a497 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1097,7 +1097,7 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, } -static int +int qemuRemoveSharedDisk(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, const char *name) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 90aebef..2f60b6a 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -302,6 +302,11 @@ int qemuRemoveSharedDevice(virQEMUDriverPtr driver, const char *name) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +int qemuRemoveSharedDisk(virQEMUDriverPtr driver, + virDomainDiskDefPtr disk, + const char *name) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + int qemuSetUnprivSGIO(virDomainDeviceDefPtr dev); int qemuDriverAllocateID(virQEMUDriverPtr driver); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7088f19..4b1c69f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6621,9 +6621,6 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, { virDomainDiskDefPtr disk = dev->data.disk; virDomainDiskDefPtr orig_disk = NULL; - virDomainDiskDefPtr tmp = NULL; - virDomainDeviceDefPtr dev_copy = NULL; - virStorageSourcePtr newsrc; virCapsPtr caps = NULL; int ret = -1; @@ -6648,38 +6645,20 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto end; - tmp = dev->data.disk; - dev->data.disk = orig_disk; - - if (!(dev_copy = virDomainDeviceDefCopy(dev, vm->def, - caps, driver->xmlopt))) { - dev->data.disk = tmp; - goto end; - } - dev->data.disk = tmp; - /* Add the new disk src into shared disk hash table */ if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0) goto end; - newsrc = disk->src; - disk->src = NULL; - - ret = qemuDomainChangeEjectableMedia(driver, conn, vm, - orig_disk, newsrc, force); - /* 'disk' must not be accessed now - it has been freed. - * 'orig_disk' now points to the new disk, while 'dev_copy' - * now points to the old disk */ - - /* Need to remove the shared disk entry for the original - * disk src if the operation is either ejecting or updating. - */ - if (ret == 0) { - dev->data.disk = NULL; - ignore_value(qemuRemoveSharedDevice(driver, dev_copy, - vm->def->name)); + if (qemuDomainChangeEjectableMedia(driver, conn, vm, + orig_disk, dev->data.disk->src, force) < 0) { + ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk, vm->def->name)); + goto end; } + + dev->data.disk->src = NULL; + ret = 0; break; + default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk bus '%s' cannot be updated."), @@ -6689,7 +6668,6 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, end: virObjectUnref(caps); - virDomainDeviceDefFree(dev_copy); return ret; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 39907ab..4c69c5e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -137,12 +137,29 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, } -int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, - virConnectPtr conn, - virDomainObjPtr vm, - virDomainDiskDefPtr disk, - virStorageSourcePtr newsrc, - bool force) +/** + * qemuDomainChangeEjectableMedia: + * @driver: qemu driver structure + * @conn: connection structure + * @vm: domain definition + * @disk: disk definition to change the source of + * @newsrc: new disk source to change to + * @force: force the change of media + * + * Change the media in an ejectable device to the one described by + * @newsrc. This function also removes the old source from the + * shared device table if appropriate. Note that newsrc is consumed + * on success and the old source is freed on success. + * + * Returns 0 on success, -1 on error and reports libvirt error + */ +int +qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, + virConnectPtr conn, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virStorageSourcePtr newsrc, + bool force) { int ret = -1; char *driveAlias = NULL; @@ -224,6 +241,8 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, if (ret < 0) goto error; + /* remove the old source from shared device list */ + ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name)); ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true)); virStorageSourceFree(disk->src); @@ -231,7 +250,6 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, newsrc = NULL; cleanup: - virStorageSourceFree(newsrc); VIR_FREE(driveAlias); VIR_FREE(sourcestr); return ret; @@ -739,9 +757,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, { virDomainDiskDefPtr disk = dev->data.disk; virDomainDiskDefPtr orig_disk = NULL; - virDomainDeviceDefPtr dev_copy = NULL; - virStorageSourcePtr newsrc; - virDomainDiskDefPtr tmp = NULL; virCapsPtr caps = NULL; int ret = -1; const char *driverName = virDomainDiskGetDriver(disk); @@ -783,32 +798,14 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto end; - - tmp = dev->data.disk; - dev->data.disk = orig_disk; - - if (!(dev_copy = virDomainDeviceDefCopy(dev, vm->def, - caps, driver->xmlopt))) { - dev->data.disk = tmp; + if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk, + disk->src, false) < 0) goto end; - } - dev->data.disk = tmp; - newsrc = disk->src; disk->src = NULL; - - ret = qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk, newsrc, false); - /* 'newsrc' must not be accessed now - it has been free'd. - * 'orig_disk' now points to the new disk, while 'dev_copy' - * now points to the old disk */ - - /* Need to remove the shared disk entry for the original disk src - * if the operation is either ejecting or updating. - */ - if (ret == 0) - ignore_value(qemuRemoveSharedDevice(driver, dev_copy, - vm->def->name)); + ret = 0; break; + case VIR_DOMAIN_DISK_DEVICE_DISK: case VIR_DOMAIN_DISK_DEVICE_LUN: if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { @@ -840,7 +837,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, if (ret != 0) ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name)); virObjectUnref(caps); - virDomainDeviceDefFree(dev_copy); return ret; } -- 2.0.2
Changing the media doesn't change other (especially ABI based) aspects of the disk. Add verification and reject disk change if ABI would be changed. --- src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 4 ++++ src/qemu/qemu_hotplug.c | 4 ++++ 5 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 34c1a8c..126a489 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13667,7 +13667,7 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src, } -static bool +bool virDomainDiskDefCheckABIStability(virDomainDiskDefPtr src, virDomainDiskDefPtr dst) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 21cfba2..c59ad19 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2304,6 +2304,9 @@ virDomainDefPtr virDomainDefParseNode(xmlDocPtr doc, bool virDomainDefCheckABIStability(virDomainDefPtr src, virDomainDefPtr dst); +bool virDomainDiskDefCheckABIStability(virDomainDiskDefPtr src, + virDomainDiskDefPtr dst); + int virDomainDefAddImplicitControllers(virDomainDefPtr def); char *virDomainDefFormat(virDomainDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 08111d4..e85467e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -219,6 +219,7 @@ virDomainDiskBusTypeToString; virDomainDiskCacheTypeFromString; virDomainDiskCacheTypeToString; virDomainDiskDefAssignAddress; +virDomainDiskDefCheckABIStability; virDomainDiskDefForeachPath; virDomainDiskDefFree; virDomainDiskDefNew; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4b1c69f..ad7cb84 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6642,6 +6642,10 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, goto end; } + /* when changing media, rest of the disk ABI cannot change */ + if (!virDomainDiskDefCheckABIStability(orig_disk, disk)) + goto end; + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto end; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4c69c5e..d937d44 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -795,6 +795,10 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, goto end; } + /* when changing media, rest of the disk ABI cannot change */ + if (!virDomainDiskDefCheckABIStability(orig_disk, disk)) + goto end; + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto end; -- 2.0.2
On 08/08/14 16:52, Peter Krempa wrote:
Peter Krempa (14): qemu: Explicitly state that hotplugging cdroms and floppies doesn't work conf: Pass virStorageSource into virDomainDiskSourceIsBlockType qemu: hotplug: Untangle cleanup paths in qemuDomainChangeEjectableMedia qemu: hotplug: Add helper to initialize/teardown new disks for VMs qemu: hotplug: Change arguments for qemuDomainChangeEjectableMedia qemu: hotplug: Format proper source string for cdrom media change qemu: shared: Split out insertion code to the shared device list qemu: shared: Split out shared device list remove code qemu: conf: rename qemuCheckSharedDevice to qemuCheckSharedDisk qemu: conf: Split up qemuAddSharedDevice into per-device-type functions qemu: conf: Split up qemuRemoveSharedDevice into per-device-type functions qemu: conf: Split out code to retrieve hostdev key and reuse it qemu: hotplug: Sanitize shared device removal on media change qemu: hotplug: Reject media change when XML contains disk ABI change
src/conf/domain_conf.c | 21 +-- src/conf/domain_conf.h | 5 +- src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_conf.c | 445 ++++++++++++++++++++++++----------------------- src/qemu/qemu_conf.h | 5 + src/qemu/qemu_driver.c | 43 ++--- src/qemu/qemu_hotplug.c | 269 +++++++++++++++------------- src/qemu/qemu_hotplug.h | 3 +- 11 files changed, 414 insertions(+), 384 deletions(-)
Ping? Could somebody please have a look? :) Peter
On 08.08.2014 16:52, Peter Krempa wrote:
Peter Krempa (14): qemu: Explicitly state that hotplugging cdroms and floppies doesn't work conf: Pass virStorageSource into virDomainDiskSourceIsBlockType qemu: hotplug: Untangle cleanup paths in qemuDomainChangeEjectableMedia qemu: hotplug: Add helper to initialize/teardown new disks for VMs qemu: hotplug: Change arguments for qemuDomainChangeEjectableMedia qemu: hotplug: Format proper source string for cdrom media change qemu: shared: Split out insertion code to the shared device list qemu: shared: Split out shared device list remove code qemu: conf: rename qemuCheckSharedDevice to qemuCheckSharedDisk qemu: conf: Split up qemuAddSharedDevice into per-device-type functions qemu: conf: Split up qemuRemoveSharedDevice into per-device-type functions qemu: conf: Split out code to retrieve hostdev key and reuse it qemu: hotplug: Sanitize shared device removal on media change qemu: hotplug: Reject media change when XML contains disk ABI change
src/conf/domain_conf.c | 21 +-- src/conf/domain_conf.h | 5 +- src/libvirt_private.syms | 1 + src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_conf.c | 445 ++++++++++++++++++++++++----------------------- src/qemu/qemu_conf.h | 5 + src/qemu/qemu_driver.c | 43 ++--- src/qemu/qemu_hotplug.c | 269 +++++++++++++++------------- src/qemu/qemu_hotplug.h | 3 +- 11 files changed, 414 insertions(+), 384 deletions(-)
ACK series Michal
On 08/19/14 16:44, Michal Privoznik wrote:
On 08.08.2014 16:52, Peter Krempa wrote:
Peter Krempa (14): qemu: Explicitly state that hotplugging cdroms and floppies doesn't work conf: Pass virStorageSource into virDomainDiskSourceIsBlockType qemu: hotplug: Untangle cleanup paths in qemuDomainChangeEjectableMedia qemu: hotplug: Add helper to initialize/teardown new disks for VMs qemu: hotplug: Change arguments for qemuDomainChangeEjectableMedia qemu: hotplug: Format proper source string for cdrom media change qemu: shared: Split out insertion code to the shared device list qemu: shared: Split out shared device list remove code qemu: conf: rename qemuCheckSharedDevice to qemuCheckSharedDisk qemu: conf: Split up qemuAddSharedDevice into per-device-type functions qemu: conf: Split up qemuRemoveSharedDevice into per-device-type functions qemu: conf: Split out code to retrieve hostdev key and reuse it qemu: hotplug: Sanitize shared device removal on media change qemu: hotplug: Reject media change when XML contains disk ABI change
ACK series
I've pushed patches 1 through 13. On a second look, patch 14 might forbid more than we'd expect so I'll think a bit more about it. Thanks. Peter
participants (2)
-
Michal Privoznik -
Peter Krempa