On 02/19/2013 07:27 AM, Osier Yang wrote:
> For both qemuDomainAttachDeviceDiskLive and qemuDomainChangeDiskMediaLive,
> remove the shared disk entry of original disk src.
> ---
> src/conf/domain_conf.c | 20 ++++++++++++
> src/conf/domain_conf.h | 3 ++
> src/libvirt_private.syms | 1 +
> src/qemu/qemu_driver.c | 76 ++++++++++++++++++++++++++++++++++++++++++++--
> src/qemu/qemu_hotplug.c | 19 +-----------
> src/qemu/qemu_hotplug.h | 1 +
> 6 files changed, 99 insertions(+), 21 deletions(-)
>
Soft ACK - it looks OK, but I'm not too familiar with the code and
what's being done here. I do think you need to make a better git
description at least with respect to what's going on!
John
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7a2b012..f2887c6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3359,6 +3359,26 @@ virDomainDiskFindControllerModel(virDomainDefPtr def,
> return model;
> }
>
> +virDomainDiskDefPtr
> +virDomainDiskFindByBusAndDst(virDomainDefPtr def,
> + int bus,
> + char *dst)
> +{
> + int i;
> +
> + if (!dst)
> + return NULL;
> +
> + for (i = 0 ; i < def->ndisks ; i++) {
> + if (def->disks[i]->bus == bus &&
> + STREQ(def->disks[i]->dst, dst)) {
> + return def->disks[i];
> + }
> + }
> +
> + return NULL;
> +}
> +
> int
> virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def)
> {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 9232ff9..4ffa4aa 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1936,6 +1936,9 @@ void virDomainDiskHostDefFree(virDomainDiskHostDefPtr def);
> int virDomainDiskFindControllerModel(virDomainDefPtr def,
> virDomainDiskDefPtr disk,
> int controllerType);
> +virDomainDiskDefPtr virDomainDiskFindByBusAndDst(virDomainDefPtr def,
> + int bus,
> + char *dst);
> void virDomainControllerDefFree(virDomainControllerDefPtr def);
> void virDomainFSDefFree(virDomainFSDefPtr def);
> void virDomainActualNetDefFree(virDomainActualNetDefPtr def);
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index d8d5877..54ccf95 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -360,6 +360,7 @@ virDomainDiskDefGetSecurityLabelDef;
> virDomainDiskDeviceTypeToString;
> virDomainDiskErrorPolicyTypeFromString;
> virDomainDiskErrorPolicyTypeToString;
> +virDomainDiskFindByBusAndDst;
> virDomainDiskFindControllerModel;
> virDomainDiskGeometryTransTypeFromString;
> virDomainDiskGeometryTransTypeToString;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5a3550d..18302e7 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5700,7 +5700,11 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
> virDomainDeviceDefPtr dev)
> {
> virDomainDiskDefPtr disk = dev->data.disk;
> + virDomainDiskDefPtr orig_disk = NULL;
> + virDomainDeviceDefPtr dev_copy = NULL;
> + virDomainDiskDefPtr tmp = NULL;
> virCgroupPtr cgroup = NULL;
> + virCapsPtr caps = NULL;
> int ret = -1;
>
> if (disk->driverName != NULL && !STREQ(disk->driverName, "qemu")) {
> @@ -5733,7 +5737,37 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
> switch (disk->device) {
> case VIR_DOMAIN_DISK_DEVICE_CDROM:
> case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
> - ret = qemuDomainChangeEjectableMedia(driver, vm, disk, false);
> + if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
> + disk->bus, disk->dst))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("No device with bus '%s' and target '%s'"),
> + virDomainDiskBusTypeToString(disk->bus),
> + disk->dst);
> + goto end;
> + }
> +
> + if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> + goto end;
> +
> + tmp = dev->data.disk;
> + dev->data.disk = orig_disk;
> +
> + if (!(dev_copy = virDomainDeviceDefCopy(caps, vm->def, dev))) {
> + dev->data.disk = tmp;
> + goto end;
> + }
> + dev->data.disk = tmp;
> +
> + ret = qemuDomainChangeEjectableMedia(driver, vm, disk, orig_disk, false);
> +
> + /* Need to remove the shared disk entry for the original disk src
> + * if the operation is either ejecting or updating.
> + */
> + if (ret == 0 &&
> + orig_disk->src &&
> + STRNEQ_NULLABLE(orig_disk->src, disk->src))
> + ignore_value(qemuRemoveSharedDisk(driver, dev_copy->data.disk,
> + vm->def->name));
> break;
> case VIR_DOMAIN_DISK_DEVICE_DISK:
> case VIR_DOMAIN_DISK_DEVICE_LUN:
> @@ -5773,6 +5807,8 @@ end:
> ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
> if (cgroup)
> virCgroupFree(&cgroup);
> + virObjectUnref(caps);
> + virDomainDeviceDefFree(dev_copy);
> return ret;
> }
>
> @@ -5951,7 +5987,11 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm,
> bool force)
> {
> virDomainDiskDefPtr disk = dev->data.disk;
> + virDomainDiskDefPtr orig_disk = NULL;
> + virDomainDiskDefPtr tmp = NULL;
> virCgroupPtr cgroup = NULL;
> + virDomainDeviceDefPtr dev_copy = NULL;
> + virCapsPtr caps = NULL;
> int ret = -1;
>
> if (qemuDomainDetermineDiskChain(driver, disk, false) < 0)
> @@ -5972,9 +6012,37 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm,
> switch (disk->device) {
> case VIR_DOMAIN_DISK_DEVICE_CDROM:
> case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
> - ret = qemuDomainChangeEjectableMedia(driver, vm, disk, force);
> - if (ret == 0)
> + if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
> + disk->bus, disk->dst))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("No device with bus '%s' and target '%s'"),
> + virDomainDiskBusTypeToString(disk->bus),
> + disk->dst);
> + goto end;
> + }
> +
> + if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> + goto end;
> +
> + tmp = dev->data.disk;
> + dev->data.disk = orig_disk;
> +
> + if (!(dev_copy = virDomainDeviceDefCopy(caps, vm->def, dev))) {
> + dev->data.disk = tmp;
> + goto end;
> + }
> + dev->data.disk = tmp;
> +
> + ret = qemuDomainChangeEjectableMedia(driver, vm, disk, orig_disk, force);
> + if (ret == 0) {
> dev->data.disk = NULL;
> + /* Need to remove the shared disk entry for the original
> + * disk src if the operation is either ejecting or updating.
> + */
> + if (orig_disk->src && STRNEQ_NULLABLE(orig_disk->src, disk->src))
> + ignore_value(qemuRemoveSharedDisk(driver, dev_copy->data.disk,
> + vm->def->name));
> + }
> break;
> default:
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -5991,6 +6059,8 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm,
> end:
> if (cgroup)
> virCgroupFree(&cgroup);
> + virObjectUnref(caps);
> + virDomainDeviceDefFree(dev_copy);
> return ret;
> }
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 488a440..e631587 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -53,32 +53,15 @@
> int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> virDomainDiskDefPtr disk,
> + virDomainDiskDefPtr origdisk,
> bool force)
> {
> - virDomainDiskDefPtr origdisk = NULL;
> - int i;
> int ret = -1;
> char *driveAlias = NULL;
> qemuDomainObjPrivatePtr priv = vm->privateData;
> int retries = CHANGE_MEDIA_RETRIES;
> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>
> - for (i = 0 ; i < vm->def->ndisks ; i++) {
> - if (vm->def->disks[i]->bus == disk->bus &&
> - STREQ(vm->def->disks[i]->dst, disk->dst)) {
> - origdisk = vm->def->disks[i];
> - break;
> - }
> - }
> -
> - if (!origdisk) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("No device with bus '%s' and target '%s'"),
> - virDomainDiskBusTypeToString(disk->bus),
> - disk->dst);
> - goto cleanup;
> - }
> -
> if (!origdisk->info.alias) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("missing disk device alias name for %s"), origdisk->dst);
> diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
> index 8f01d23..2a146fe 100644
> --- a/src/qemu/qemu_hotplug.h
> +++ b/src/qemu/qemu_hotplug.h
> @@ -31,6 +31,7 @@
> int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> virDomainDiskDefPtr disk,
> + virDomainDiskDefPtr orig_disk,
> bool force);
> int qemuDomainCheckEjectableMedia(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list