[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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,
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]