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

Re: [libvirt] [PATCH 6/8] qemu: Translate the pool disk source earlier



On 04/04/2013 03:38 PM, Osier Yang wrote:
> To support "shareable" for volume type disk, we have to translate
> the source before trying to add the shared disk entry. To archieve

s/archeive/achieve/

> the goal, this moves the helper qemuTranslateDiskSourcePool into
> src/qemu/qemu_conf.c, and introduce a internal only member (voltype)

s/a internal/an internal/

> for struct _virDomainDiskSourcePoolDef, to record the underlying
> volume type, for use when building the drive string.

s/type,/type/

> 
> Later patch will support "shareable" volume type disk.
> ---
>  src/conf/domain_conf.h  |  1 +
>  src/qemu/qemu_command.c | 56 +------------------------------------------------
>  src/qemu/qemu_command.h |  1 -
>  src/qemu/qemu_conf.c    | 52 +++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_conf.h    |  3 +++
>  src/qemu/qemu_driver.c  | 16 ++++++++++----
>  src/qemu/qemu_process.c |  7 +++++++
>  7 files changed, 76 insertions(+), 60 deletions(-)
> 
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 988636e..13d6d89 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -611,6 +611,7 @@ typedef struct _virDomainDiskSourcePoolDef virDomainDiskSourcePoolDef;
>  struct _virDomainDiskSourcePoolDef {
>      char *pool; /* pool name */
>      char *volume; /* volume name */
> +    int voltype; /* enum virStorageVolType, internal only */
>  };
>  typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr;
>  
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index cdfe801..c29796d 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2681,56 +2681,6 @@ qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr op
>      return 0;
>  }
>  
> -static int
> -qemuTranslateDiskSourcePool(virConnectPtr conn,
> -                            virDomainDiskDefPtr def,
> -                            int *voltype)
> -{
> -    virStoragePoolPtr pool = NULL;
> -    virStorageVolPtr vol = NULL;
> -    virStorageVolInfo info;
> -    int ret = -1;
> -
> -    if (def->type != VIR_DOMAIN_DISK_TYPE_VOLUME)
> -        return 0;
> -
> -    if (!(pool = virStoragePoolLookupByName(conn, def->srcpool->pool)))
> -        return -1;
> -
> -    if (!(vol = virStorageVolLookupByName(pool, def->srcpool->volume)))
> -        goto cleanup;
> -
> -    if (virStorageVolGetInfo(vol, &info) < 0)
> -        goto cleanup;
> -
> -    if (def->startupPolicy &&
> -        info.type != VIR_STORAGE_VOL_FILE) {
> -        virReportError(VIR_ERR_XML_ERROR, "%s",
> -                       _("'startupPolicy' is only valid for 'file' type volume"));
> -        goto cleanup;
> -    }
> -
> -    switch (info.type) {
> -    case VIR_STORAGE_VOL_FILE:
> -    case VIR_STORAGE_VOL_BLOCK:
> -    case VIR_STORAGE_VOL_DIR:
> -        if (!(def->src = virStorageVolGetPath(vol)))
> -            goto cleanup;
> -        break;
> -    case VIR_STORAGE_VOL_NETWORK:
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("Using network volume as disk source is not supported"));
> -        goto cleanup;
> -    }
> -
> -    *voltype = info.type;
> -    ret = 0;
> -cleanup:
> -    virStoragePoolFree(pool);
> -    virStorageVolFree(vol);
> -    return ret;
> -}
> -
>  char *
>  qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
>                    virDomainDiskDefPtr disk,
> @@ -2743,7 +2693,6 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
>          virDomainDiskGeometryTransTypeToString(disk->geometry.trans);
>      int idx = virDiskNameToIndex(disk->dst);
>      int busid = -1, unitid = -1;
> -    int voltype = -1;
>  
>      if (idx < 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -2751,9 +2700,6 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
>          goto error;
>      }
>  
> -    if (qemuTranslateDiskSourcePool(conn, disk, &voltype) < 0)
> -        goto error;
> -
>      switch (disk->bus) {
>      case VIR_DOMAIN_DISK_BUS_SCSI:
>          if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
> @@ -2889,7 +2835,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
>                  break;
>              }
>          } else if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) {
> -            switch (voltype) {
> +            switch (disk->srcpool->voltype) {
>              case VIR_STORAGE_VOL_DIR:
>                  if (!disk->readonly) {
>                      virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 17687f4..20e3066 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -237,5 +237,4 @@ qemuParseKeywords(const char *str,
>                    char ***retvalues,
>                    int allowEmptyValue);
>  
> -
>  #endif /* __QEMU_COMMAND_H__*/
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index c2e2e10..8ae690f 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1240,3 +1240,55 @@ int qemuDriverAllocateID(virQEMUDriverPtr driver)
>  {
>      return virAtomicIntInc(&driver->nextvmid);
>  }
> +
> +int
> +qemuTranslateDiskSourcePool(virConnectPtr conn,
> +                            virDomainDiskDefPtr def)
> +{
> +    virStoragePoolPtr pool = NULL;
> +    virStorageVolPtr vol = NULL;
> +    virStorageVolInfo info;
> +    int ret = -1;
> +
> +    if (def->type != VIR_DOMAIN_DISK_TYPE_VOLUME)
> +        return 0;
> +
> +    if (!def->srcpool)
> +        return 0;
> +
> +    if (!(pool = virStoragePoolLookupByName(conn, def->srcpool->pool)))
> +        return -1;
> +
> +    if (!(vol = virStorageVolLookupByName(pool, def->srcpool->volume)))
> +        goto cleanup;
> +
> +    if (virStorageVolGetInfo(vol, &info) < 0)
> +        goto cleanup;
> +
> +    if (def->startupPolicy &&
> +        info.type != VIR_STORAGE_VOL_FILE) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("'startupPolicy' is only valid for 'file' type volume"));
> +        goto cleanup;
> +    }
> +
> +    switch (info.type) {
> +    case VIR_STORAGE_VOL_FILE:
> +    case VIR_STORAGE_VOL_BLOCK:
> +    case VIR_STORAGE_VOL_DIR:
> +        if (!(def->src = virStorageVolGetPath(vol)))
> +            goto cleanup;
> +        break;
> +    case VIR_STORAGE_VOL_NETWORK:
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("Using network volume as disk source is not supported"));
> +        goto cleanup;
> +    }
> +
> +    def->srcpool->voltype = info.type;
> +    ret = 0;
> +cleanup:
> +    virStoragePoolFree(pool);
> +    virStorageVolFree(vol);
> +    return ret;
> +}
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index c5ddaad..9f58454 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -303,4 +303,7 @@ void qemuSharedDiskEntryFree(void *payload, const void *name)
>  int qemuDriverAllocateID(virQEMUDriverPtr driver);
>  virDomainXMLConfPtr virQEMUDriverCreateXMLConf(void);
>  
> +int qemuTranslateDiskSourcePool(virConnectPtr conn,
> +                                virDomainDiskDefPtr def);
> +
>  #endif /* __QEMUD_CONF_H */
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 552a81b..4e8c666 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5748,6 +5748,9 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
>          goto end;
>      }
>  
> +    if (qemuTranslateDiskSourcePool(conn, disk) < 0)
> +        goto end;
> +
>      if (qemuAddSharedDisk(driver, disk, vm->def->name) < 0)
>          goto end;
>  
> @@ -6016,7 +6019,8 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
>  }
>  
>  static int
> -qemuDomainChangeDiskMediaLive(virDomainObjPtr vm,
> +qemuDomainChangeDiskMediaLive(virConnectPtr conn,
> +                              virDomainObjPtr vm,
>                                virDomainDeviceDefPtr dev,
>                                virQEMUDriverPtr driver,
>                                bool force)
> @@ -6029,6 +6033,9 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm,
>      virCapsPtr caps = NULL;
>      int ret = -1;
>  
> +    if (qemuTranslateDiskSourcePool(conn, disk) < 0)
> +        goto end;
> +
>      if (qemuDomainDetermineDiskChain(driver, disk, false) < 0)
>          goto end;
>  
> @@ -6107,7 +6114,8 @@ end:
>  }
>  
>  static int
> -qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
> +qemuDomainUpdateDeviceLive(virConnectPtr conn,
> +                           virDomainObjPtr vm,
>                             virDomainDeviceDefPtr dev,
>                             virDomainPtr dom,
>                             bool force)
> @@ -6117,7 +6125,7 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
>  
>      switch (dev->type) {
>      case VIR_DOMAIN_DEVICE_DISK:
> -        ret = qemuDomainChangeDiskMediaLive(vm, dev, driver, force);
> +        ret = qemuDomainChangeDiskMediaLive(conn, vm, dev, driver, force);
>          break;
>      case VIR_DOMAIN_DEVICE_GRAPHICS:
>          ret = qemuDomainChangeGraphics(driver, vm, dev->data.graphics);
> @@ -6529,7 +6537,7 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
>              ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom);
>              break;
>          case QEMU_DEVICE_UPDATE:
> -            ret = qemuDomainUpdateDeviceLive(vm, dev_copy, dom, force);
> +            ret = qemuDomainUpdateDeviceLive(dom->conn, vm, dev_copy, dom, force);
>              break;
>          default:
>              virReportError(VIR_ERR_INTERNAL_ERROR,
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 8c4bfb7..3ac0c70 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3008,6 +3008,8 @@ qemuProcessReconnect(void *opaque)
>       * qemu_driver->sharedDisks.
>       */
>      for (i = 0; i < obj->def->ndisks; i++) {
> +        if (qemuTranslateDiskSourcePool(conn, obj->def->disks[i]) < 0)
> +            goto error;
>          if (qemuAddSharedDisk(driver, obj->def->disks[i],
>                                obj->def->name) < 0)
>              goto error;
> @@ -3556,6 +3558,11 @@ int qemuProcessStart(virConnectPtr conn,
>              goto cleanup;
>      }
>  
> +    for (i = 0; i < vm->def->ndisks; i++) {
> +        if (qemuTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0)
> +            goto cleanup;
> +    }
> +
>      VIR_DEBUG("Building emulator command line");
>      if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig,
>                                       priv->monJSON != 0, priv->qemuCaps,
> 


ACK; however, I see paths to qemuBuildDriveStr() that don't go through
qemuBuildCommandLine(), so just double check that you aren't missing a
place to get that disk->src set for one of these pool/vol's.  The point
being that qemuBuildDriveStr() "had" code that did something before
(albeit in this set of patches). Since you're moving it - just be sure
there's no path that could enter qemuBuildDriveStr() that would need it.

John


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