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

Re: [libvirt] [PATCH 4/7] qemu: process: Fix and improve disk data extraction



On 24.05.2016 15:17, Peter Krempa wrote:
> Extract information for all disks and update tray state and source only
> for removable drives. Additionally store whether a drive is removable
> and whether it has a tray.
> ---
>  src/qemu/qemu_domain.h  |  4 ++++
>  src/qemu/qemu_monitor.c | 18 ------------------
>  src/qemu/qemu_monitor.h |  3 ---
>  src/qemu/qemu_process.c | 28 +++++++++++++++-------------
>  4 files changed, 19 insertions(+), 34 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 825036c..9ac0209 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -299,6 +299,10 @@ struct _qemuDomainDiskPrivate {
>      /* for storage devices using auth/secret
>       * NB: *not* to be written to qemu domain object XML */
>      qemuDomainSecretInfoPtr secinfo;
> +
> +    /* information about the device */
> +    bool tray; /* device has tray */
> +    bool removable; /* device media can be removed/changed */

I wonder whether we should drop these one day and replace them with
struct  qemuDomainDiskInfo which lives right below this struct (not to
be seen in the context though). But that can be done later, not
necessarily now.

>  };
> 
>  # define QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev)	\
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 7d1ca91..a0060cc 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -1826,24 +1826,6 @@ qemuMonitorGetBlockInfo(qemuMonitorPtr mon)
>  }
> 
> 
> -struct qemuDomainDiskInfo *
> -qemuMonitorBlockInfoLookup(virHashTablePtr blockInfo,
> -                           const char *dev)
> -{
> -    struct qemuDomainDiskInfo *info;
> -
> -    VIR_DEBUG("blockInfo=%p dev=%s", blockInfo, NULLSTR(dev));
> -
> -    if (!(info = virHashLookup(blockInfo, dev))) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("cannot find info for device '%s'"),
> -                       NULLSTR(dev));
> -    }
> -
> -    return info;
> -}
> -
> -
>  /**
>   * qemuMonitorGetAllBlockStatsInfo:
>   * @mon: monitor object
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 6359314..a1cbc94 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -404,9 +404,6 @@ int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
> 
>  int qemuMonitorBlockIOStatusToError(const char *status);
>  virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon);
> -struct qemuDomainDiskInfo *
> -qemuMonitorBlockInfoLookup(virHashTablePtr blockInfo,
> -                           const char *dev_name);
> 
>  typedef struct _qemuBlockStats qemuBlockStats;
>  typedef qemuBlockStats *qemuBlockStatsPtr;
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index f2e284f..106ffcd 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -6254,25 +6254,27 @@ qemuProcessRefreshDisks(virQEMUDriverPtr driver,
> 
>      for (i = 0; i < vm->def->ndisks; i++) {
>          virDomainDiskDefPtr disk = vm->def->disks[i];
> +        qemuDomainDiskPrivatePtr diskpriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>          struct qemuDomainDiskInfo *info;
> 
> -        if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK ||
> -            disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
> -                 continue;
> -        }
> -
> -        info = qemuMonitorBlockInfoLookup(table, disk->info.alias);
> -        if (!info)
> -            goto cleanup;
> +        if (!(info = virHashLookup(table, disk->info.alias)))
> +            continue;
> 
> -        if (info->tray_open) {
> -            if (virDomainDiskGetSource(disk))
> +        if (info->removable) {
> +            if (info->empty)
>                  ignore_value(virDomainDiskSetSource(disk, NULL));
> 
> -            disk->tray_status = VIR_DOMAIN_DISK_TRAY_OPEN;
> -        } else {
> -            disk->tray_status = VIR_DOMAIN_DISK_TRAY_CLOSED;
> +            if (info->tray) {
> +                if (info->tray_open)
> +                    disk->tray_status = VIR_DOMAIN_DISK_TRAY_OPEN;
> +                else
> +                    disk->tray_status = VIR_DOMAIN_DISK_TRAY_CLOSED;
> +            }
>          }
> +
> +        /* fill in additional data */
> +        diskpriv->removable = info->removable;
> +        diskpriv->tray = info->tray;

If we go with my suggestion, this code wouldn't be needed.

>      }
> 
>      ret = 0;
> 

Michal


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