Re: [libvirt] [PATCH 6/6] qemu: Remove the shared disk entry if the operation is ejecting or updating

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(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list