Re: [PATCHv1.5 04/27] qemuxml2argv: Add test for disk type='volume' with iSCSI pools

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

 



On 26.11.2013 17:48, Peter Krempa wrote:
> Tweak the existing file so that it can be tested for command line
> corectness.
> ---
>  src/conf/domain_conf.h                             |   1 +
>  src/libvirt_private.syms                           |   1 +
>  src/qemu/qemu_command.c                            |  76 +-----------
>  src/qemu/qemu_conf.c                               | 129 ++++++++++++++-------
>  src/qemu/qemu_conf.h                               |   2 +
>  .../qemuxml2argv-disk-source-pool-mode.args        |  10 ++
>  .../qemuxml2argv-disk-source-pool-mode.xml         |   4 +-
>  tests/qemuxml2argvtest.c                           |   2 +
>  8 files changed, 112 insertions(+), 113 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args
> 
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 4561ccc..a5ef2ca 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -686,6 +686,7 @@ struct _virDomainDiskSourcePoolDef {
>      char *volume; /* volume name */
>      int voltype; /* enum virStorageVolType, internal only */
>      int pooltype; /* enum virStoragePoolType, internal only */
> +    int actualtype; /* enum virDomainDiskType, internal only */
>      int mode; /* enum virDomainDiskSourcePoolMode */
>  };
>  typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr;
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 205fe56..aeb3568 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -693,6 +693,7 @@ virStoragePoolSourceFree;
>  virStoragePoolSourceListFormat;
>  virStoragePoolSourceListNewSource;
>  virStoragePoolTypeFromString;
> +virStoragePoolTypeToString;
>  virStorageVolDefFindByKey;
>  virStorageVolDefFindByName;
>  virStorageVolDefFindByPath;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 763417f..b8129a3 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3775,69 +3775,6 @@ qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr op
>      return 0;
>  }
> 
> -static int
> -qemuBuildVolumeString(virConnectPtr conn,
> -                      virDomainDiskDefPtr disk,
> -                      virBufferPtr opt)
> -{
> -    int ret = -1;
> -
> -    switch ((virStorageVolType) disk->srcpool->voltype) {
> -    case VIR_STORAGE_VOL_DIR:
> -        if (!disk->readonly) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                           _("cannot create virtual FAT disks in read-write mode"));
> -            goto cleanup;
> -        }
> -        if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
> -            virBufferEscape(opt, ',', ",", "file=fat:floppy:%s,", disk->src);
> -        else
> -            virBufferEscape(opt, ',', ",", "file=fat:%s,", disk->src);
> -        break;
> -    case VIR_STORAGE_VOL_BLOCK:
> -        if (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) {
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                           _("tray status 'open' is invalid for "
> -                             "block type volume"));
> -            goto cleanup;
> -        }
> -        if (disk->srcpool->pooltype == VIR_STORAGE_POOL_ISCSI) {
> -            if (disk->srcpool->mode ==
> -                VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT) {
> -                if (qemuBuildISCSIString(conn, disk, opt) < 0)
> -                    goto cleanup;
> -                virBufferAddChar(opt, ',');
> -            } else if (disk->srcpool->mode ==
> -                       VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) {
> -                virBufferEscape(opt, ',', ",", "file=%s,", disk->src);
> -            }
> -        } else {
> -            virBufferEscape(opt, ',', ",", "file=%s,", disk->src);
> -        }
> -        break;
> -    case VIR_STORAGE_VOL_FILE:
> -        if (disk->auth.username) {
> -            if (qemuBuildISCSIString(conn, disk, opt) < 0)
> -                goto cleanup;
> -            virBufferAddChar(opt, ',');
> -        } else {
> -            virBufferEscape(opt, ',', ",", "file=%s,", disk->src);
> -        }
> -        break;
> -    case VIR_STORAGE_VOL_NETWORK:
> -    case VIR_STORAGE_VOL_NETDIR:
> -    case VIR_STORAGE_VOL_LAST:
> -        /* Keep the compiler quiet, qemuTranslateDiskSourcePool already
> -         * reported the unsupported error.
> -         */
> -        goto cleanup;
> -    }
> -
> -    ret = 0;
> -
> -cleanup:
> -    return ret;
> -}
> 
>  char *
>  qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
> @@ -3851,6 +3788,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
>          virDomainDiskGeometryTransTypeToString(disk->geometry.trans);
>      int idx = virDiskNameToIndex(disk->dst);
>      int busid = -1, unitid = -1;
> +    int actualType = qemuDiskGetActualType(disk);
> 
>      if (idx < 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -3934,12 +3872,13 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
> 
>      /* disk->src is NULL when we use nbd disks */
>      if ((disk->src ||
> -        (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK &&
> +        (actualType == VIR_DOMAIN_DISK_TYPE_NETWORK &&
>           disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_NBD)) &&
>          !((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY ||
>             disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) &&
>            disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) {
> -        if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) {
> +
> +        if (actualType == VIR_DOMAIN_DISK_TYPE_DIR) {
>              /* QEMU only supports magic FAT format for now */
>              if (disk->format > 0 && disk->format != VIR_STORAGE_FILE_FAT) {
>                  virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -3957,7 +3896,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
>                                  disk->src);
>              else
>                  virBufferEscape(&opt, ',', ",", "file=fat:%s,", disk->src);
> -        } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) {
> +        } else if (actualType == VIR_DOMAIN_DISK_TYPE_NETWORK) {
>              switch (disk->protocol) {
>              case VIR_DOMAIN_DISK_PROTOCOL_NBD:
>                  if (qemuBuildNBDString(conn, disk, &opt) < 0)
> @@ -4025,11 +3964,8 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED,
>                  virBufferEscape(&opt, ',', ",", "%s,", disk->src);
>                  break;
>              }
> -        } else if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) {
> -            if (qemuBuildVolumeString(conn, disk, &opt) < 0)
> -                goto error;
>          } else {
> -            if ((disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK) &&
> +            if ((actualType == VIR_DOMAIN_DISK_TYPE_BLOCK) &&
>                  (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) {
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                                 _("tray status 'open' is invalid for "
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 77df370..58a0500 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1298,6 +1298,17 @@ cleanup:
>      return ret;
>  }
> 
> +
> +int
> +qemuDiskGetActualType(virDomainDiskDefPtr def)
> +{
> +    if (def->type == VIR_DOMAIN_DISK_TYPE_VOLUME)
> +        return def->srcpool->actualtype;
> +
> +    return def->type;
> +}
> +
> +
>  int
>  qemuTranslateDiskSourcePool(virConnectPtr conn,
>                              virDomainDiskDefPtr def)
> @@ -1319,72 +1330,108 @@ qemuTranslateDiskSourcePool(virConnectPtr conn,
>      if (!(pool = virStoragePoolLookupByName(conn, def->srcpool->pool)))
>          return -1;
> 
> +    if (virStoragePoolIsActive(pool) != 1) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("storage pool '%s' containing volume '%s' "
> +                         "is not active"),
> +                       def->srcpool->pool, def->srcpool->volume);
> +        goto cleanup;
> +    }
> +
>      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) {
> +    if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0)))
> +        goto cleanup;
> +
> +    if (!(pooldef = virStoragePoolDefParseString(poolxml)))
> +        goto cleanup;
> +
> +    def->srcpool->pooltype = pooldef->type;
> +    def->srcpool->voltype = info.type;
> +
> +    if (def->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) {
>          virReportError(VIR_ERR_XML_ERROR, "%s",
> -                       _("'startupPolicy' is only valid for 'file' type volume"));
> +                       _("disk source mode is only valid when "
> +                         "storage pool is of iscsi type"));
>          goto cleanup;
>      }
> 
> -    switch ((virStorageVolType) info.type) {
> -    case VIR_STORAGE_VOL_FILE:
> -    case VIR_STORAGE_VOL_DIR:
> +    switch ((enum virStoragePoolType) pooldef->type) {
> +    case VIR_STORAGE_POOL_DIR:
> +    case VIR_STORAGE_POOL_FS:
> +    case VIR_STORAGE_POOL_NETFS:
> +    case VIR_STORAGE_POOL_LOGICAL:
> +    case VIR_STORAGE_POOL_DISK:
> +    case VIR_STORAGE_POOL_SCSI:
>          if (!(def->src = virStorageVolGetPath(vol)))
>              goto cleanup;
> -        break;
> -    case VIR_STORAGE_VOL_BLOCK:
> -        if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0)))
> -            goto cleanup;
> -
> -        if (!(pooldef = virStoragePoolDefParseString(poolxml)))
> -            goto cleanup;
> 
> -        if (def->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) {
> -            virReportError(VIR_ERR_XML_ERROR, "%s",
> -                           _("disk source mode is only valid when "
> -                             "storage pool is of iscsi type"));
> +        switch (info.type) {
> +        case VIR_STORAGE_VOL_FILE:
> +            def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_FILE;
> +            break;
> +
> +        case VIR_STORAGE_VOL_DIR:
> +            def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_DIR;
> +            break;
> +
> +        case VIR_STORAGE_VOL_BLOCK:
> +            def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_BLOCK;
> +            break;
> +
> +        case VIR_STORAGE_VOL_NETWORK:
> +        case VIR_STORAGE_VOL_NETDIR:
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("unexpected storage volume type '%s' "
> +                             "for storage pool type '%s'"),
> +                           virStorageVolTypeToString(info.type),
> +                           virStoragePoolTypeToString(pooldef->type));
>              goto cleanup;
>          }
> 
> -        def->srcpool->pooltype = pooldef->type;
> -        if (pooldef->type == VIR_STORAGE_POOL_ISCSI) {
> -            /* Default to use the LUN's path on host */
> -            if (!def->srcpool->mode)
> -                def->srcpool->mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST;
> -
> -            if (def->srcpool->mode ==
> -                VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT) {
> -                if (qemuAddISCSIPoolSourceHost(def, pooldef) < 0)
> -                    goto cleanup;
> -            } else if (def->srcpool->mode ==
> -                       VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) {
> -                if (!(def->src = virStorageVolGetPath(vol)))
> -                    goto cleanup;
> -            }
> +        break;
> +
> +    case VIR_STORAGE_POOL_ISCSI:
> +       switch (def->srcpool->mode) {
> +        case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DEFAULT:
> +        case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_LAST:
> +            def->srcpool->mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST;
> +            def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_BLOCK;
> +            /* fallthrough */
> +        case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST:
> +            if (!(def->src = virStorageVolGetPath(vol)))
> +                goto cleanup;
> +            break;
> +
> +        case VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT:
> +            def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_NETWORK;
> +            def->protocol = VIR_DOMAIN_DISK_PROTOCOL_ISCSI;
> 
>              if (qemuTranslateDiskSourcePoolAuth(def, pooldef) < 0)
>                  goto cleanup;
> -        } else {
> -            if (!(def->src = virStorageVolGetPath(vol)))
> +
> +            if (qemuAddISCSIPoolSourceHost(def, pooldef) < 0)
>                  goto cleanup;
> +            break;
>          }
> -
>          break;
> -    case VIR_STORAGE_VOL_NETWORK:
> -    case VIR_STORAGE_VOL_NETDIR:
> -    case VIR_STORAGE_VOL_LAST:
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("Using network volume as disk source is not supported"));
> +
> +    case VIR_STORAGE_POOL_MPATH:
> +    case VIR_STORAGE_POOL_RBD:
> +    case VIR_STORAGE_POOL_SHEEPDOG:
> +    case VIR_STORAGE_POOL_GLUSTER:
> +    case VIR_STORAGE_POOL_LAST:
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("using '%s' pools for backing 'volume' disks "
> +                         "isn't yet supported"),
> +                       virStoragePoolTypeToString(pooldef->type));
>          goto cleanup;
>      }
> 
> -    def->srcpool->voltype = info.type;
>      ret = 0;
>  cleanup:
>      if (ret < 0)
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index f9ff7af..b9786b1 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -304,6 +304,8 @@ int qemuSetUnprivSGIO(virDomainDeviceDefPtr dev);
>  int qemuDriverAllocateID(virQEMUDriverPtr driver);
>  virDomainXMLOptionPtr virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver);
> 
> +int qemuDiskGetActualType(virDomainDiskDefPtr def);
> +
>  int qemuTranslateDiskSourcePool(virConnectPtr conn,
>                                  virDomainDiskDefPtr def);
> 
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args
> new file mode 100644
> index 0000000..8f6a3dd
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args
> @@ -0,0 +1,10 @@
> +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \
> +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \
> +file=/some/block/device/unit:0:0:1,if=none,media=cdrom,id=drive-ide0-0-1 -device \
> +ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 -drive \
> +file=iscsi://iscsi.example.com:3260/demo-target/2,if=none,media=cdrom,id=drive-ide0-0-2 \
> +-device ide-drive,bus=ide.0,unit=2,drive=drive-ide0-0-2,id=ide0-0-2 -drive \
> +file=/tmp/idedisk.img,if=none,id=drive-ide0-0-3 -device \
> +ide-drive,bus=ide.0,unit=3,drive=drive-ide0-0-3,id=ide0-0-3 -device \
> +virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml
> index b907633..9f90293 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml
> @@ -15,7 +15,7 @@
>    <devices>
>      <emulator>/usr/bin/qemu</emulator>
>      <disk type='volume' device='cdrom'>
> -      <source pool='blk-pool0' volume='blk-pool0-vol0' mode='host' startupPolicy='optional'>
> +      <source pool='pool-iscsi-auth' volume='unit:0:0:1' mode='host'>
>          <seclabel model='selinux' relabel='yes'>
>            <label>system_u:system_r:public_content_t:s0</label>
>          </seclabel>
> @@ -25,7 +25,7 @@
>        <address type='drive' controller='0' bus='0' target='0' unit='1'/>
>      </disk>
>      <disk type='volume' device='cdrom'>
> -      <source pool='blk-pool0' volume='blk-pool0-vol1' mode='direct' startupPolicy='optional'>

Just curious, I see you're removing startupPolicy here and in other
patches. Would it hurt to leave it there?

At any rate, ACK.

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]