On 04/04/2013 03:37 PM, Osier Yang wrote:
> With this patch, one can specify the disk source using libvirt
> storage like:
>
> <disk type='volume' device='disk'>
> <driver name='qemu' type='raw' cache='none'/>
> <source pool='default' volume='fc18.img'/>
> <target dev='vdb' bus='virtio'/>
> </disk>
>
> "seclables" and "startupPolicy" are not supported for this new
> disk type ("volume"). They will be supported in later patches.
>
> docs/formatdomain.html.in:
> * Add documents for new XMLs
> docs/schemas/domaincommon.rng:
> * Add rng for new XMLs;
> src/conf/domain_conf.h:
> * New struct for 'volume' type disk source (virDomainDiskSourcePoolDef)
> * Add VIR_DOMAIN_DISK_TYPE_VOLUME for enum virDomainDiskType
> src/conf/domain_conf.c:
> * New helper virDomainDiskSourcePoolDefParse to parse the 'volume'
> type disk source.
> * New helper virDomainDiskSourcePoolDefFree to free the source def
> if 'volume' type disk.
> tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml:
> tests/qemuxml2xmltest.c:
> * New test
> ---
> docs/formatdomain.html.in | 15 +++-
> docs/schemas/domaincommon.rng | 18 +++++
> src/conf/domain_conf.c | 89 ++++++++++++++++++++--
> src/conf/domain_conf.h | 9 +++
> .../qemuxml2argv-disk-source-pool.xml | 33 ++++++++
> tests/qemuxml2xmltest.c | 1 +
> 6 files changed, 157 insertions(+), 8 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index cf382e8..bdc815f 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1372,6 +1372,11 @@
> <blockio logical_block_size='512' physical_block_size='4096'/>
> <target dev='hda' bus='ide'/>
> </disk>
> + <disk type='volume' device='disk'>
> + <driver name='qemu' type='raw'/>
> + <source pool='blk-pool0' volume='blk-pool0-vol0'/>
> + <target dev='hda' bus='ide'/>
> + </disk>
> </devices>
> ...</pre>
>
> @@ -1452,10 +1457,16 @@
Somewhere up here before this section there is:
"The type attribute is either "file", "block", "dir", or "network" and
refers to the underlying source for the disk."
That list needs to be "updated" to include "volume".
And naturally while not part of your changes - there's no example of
"<disk type='dir'" in the above section... sigh.
John
> <code>iqn.1992-01.com.example/1</code>); the default LUN is zero.
> When the disk <code>type</code> is "network", the <code>source</code>
> may have zero or more <code>host</code> sub-elements used to
> - specify the hosts to connect.
> + specify the hosts to connect. If the disk <code>type</code> is
> + "volume", the underlying disk source is represented by attributes
> + <code>pool</code> and <code>volume</code>. Attribute <code>pool</code>
> + specifies the name of storage pool (managed by libvirt) where the disk
> + source resides, and attribute <code>volume</code> specifies the name of
> + storage volume (managed by libvirt) used as the disk source.
> <span class="since">Since 0.0.3; <code>type='dir'</code> since
> 0.7.5; <code>type='network'</code> since
> - 0.8.7; <code>protocol='iscsi'</code> since 1.0.4</span><br/>
> + 0.8.7; <code>protocol='iscsi'</code> since 1.0.4;
> + <code>type='volume'</code> since 1.0.5;</span><br/>
> For a "file" disk type which represents a cdrom or floppy
> (the <code>device</code> attribute), it is possible to define
> policy what to do with the disk if the source file is not accessible.
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 454ebdb..46cccc4 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1090,6 +1090,24 @@
> <ref name="diskspec"/>
> </interleave>
> </group>
> + <group>
> + <attribute name="type">
> + <value>volume</value>
> + </attribute>
> + <interleave>
> + <optional>
> + <element name="source">
> + <attribute name="pool">
> + <ref name="genericName"/>
> + </attribute>
> + <attribute name="volume">
> + <ref name="volName"/>
> + </attribute>
> + </element>
> + </optional>
> + <ref name="diskspec"/>
> + </interleave>
> + </group>
> <ref name="diskspec"/>
> </choice>
> </element>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 8288959..8538d5f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -207,7 +207,8 @@ VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST,
> "block",
> "file",
> "dir",
> - "network")
> + "network",
> + "volume")
>
> VIR_ENUM_IMPL(virDomainDiskDevice, VIR_DOMAIN_DISK_DEVICE_LAST,
> "disk",
> @@ -1085,6 +1086,18 @@ void virDomainLeaseDefFree(virDomainLeaseDefPtr def)
> VIR_FREE(def);
> }
>
> +static void
> +virDomainDiskSourcePoolDefFree(virDomainDiskSourcePoolDefPtr def)
> +{
> + if (!def)
> + return;
> +
> + VIR_FREE(def->pool);
> + VIR_FREE(def->volume);
> +
> + VIR_FREE(def);
> +}
> +
> void virDomainDiskDefFree(virDomainDiskDefPtr def)
> {
> unsigned int i;
> @@ -1094,6 +1107,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
>
> VIR_FREE(def->serial);
> VIR_FREE(def->src);
> + virDomainDiskSourcePoolDefFree(def->srcpool);
> VIR_FREE(def->dst);
> VIR_FREE(def->driverName);
> virStorageFileFreeMetadata(def->backingChain);
> @@ -3935,6 +3949,46 @@ cleanup:
> goto cleanup;
> }
>
> +static int
> +virDomainDiskSourcePoolDefParse(xmlNodePtr node,
> + virDomainDiskDefPtr def)
> +{
> + char *pool = NULL;
> + char *volume = NULL;
> + int ret = -1;
> +
> + pool = virXMLPropString(node, "pool");
> + volume = virXMLPropString(node, "volume");
> +
> + /* CD-ROM and Floppy allows no source */
> + if (!pool && !volume)
> + return 0;
> +
> + if (!pool || !volume) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("'pool' and 'volume' must be specified together "
> + "for 'pool' type source"));
> + goto cleanup;
> + }
> +
> + if (VIR_ALLOC(def->srcpool) < 0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + def->srcpool->pool = pool;
> + pool = NULL;
> + def->srcpool->volume = volume;
> + volume = NULL;
> +
> + ret = 0;
> +
> +cleanup:
> + VIR_FREE(pool);
> + VIR_FREE(volume);
> + return ret;
> +}
> +
> #define VENDOR_LEN 8
> #define PRODUCT_LEN 16
>
> @@ -4030,7 +4084,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> cur = node->children;
> while (cur != NULL) {
> if (cur->type == XML_ELEMENT_NODE) {
> - if (!source && !hosts &&
> + if (!source && !hosts && !def->srcpool &&
> xmlStrEqual(cur->name, BAD_CAST "source")) {
> sourceNode = cur;
>
> @@ -4123,6 +4177,10 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> child = child->next;
> }
> break;
> + case VIR_DOMAIN_DISK_TYPE_VOLUME:
> + if (virDomainDiskSourcePoolDefParse(cur, def) < 0)
> + goto error;
> + break;
> default:
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("unexpected disk type %s"),
> @@ -4421,7 +4479,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
>
> /* Only CDROM and Floppy devices are allowed missing source path
> * to indicate no media present */
> - if (source == NULL && hosts == NULL &&
> + if (source == NULL && hosts == NULL && !def->srcpool &&
> def->device != VIR_DOMAIN_DISK_DEVICE_CDROM &&
> def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
> virReportError(VIR_ERR_NO_SOURCE,
> @@ -4443,8 +4501,19 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> }
>
> if (target == NULL) {
> - virReportError(VIR_ERR_NO_TARGET,
> - source ? "%s" : NULL, source);
> + if (def->srcpool) {
> + char *tmp;
> + if (virAsprintf(&tmp, "pool = '%s', volume = '%s'",
> + def->srcpool->pool, def->srcpool->volume) < 0) {
> + virReportOOMError();
> + goto error;
> + }
> +
> + virReportError(VIR_ERR_NO_TARGET, "%s", tmp);
> + VIR_FREE(tmp);
> + } else {
> + virReportError(VIR_ERR_NO_TARGET, source ? "%s" : NULL, source);
> + }
> goto error;
> }
>
> @@ -12745,7 +12814,7 @@ virDomainDiskSourceDefFormat(virBufferPtr buf,
> int n;
> const char *startupPolicy = virDomainStartupPolicyTypeToString(def->startupPolicy);
>
> - if (def->src || def->nhosts > 0 ||
> + if (def->src || def->nhosts > 0 || def->srcpool ||
> def->startupPolicy) {
> switch (def->type) {
> case VIR_DOMAIN_DISK_TYPE_FILE:
> @@ -12817,6 +12886,14 @@ virDomainDiskSourceDefFormat(virBufferPtr buf,
> virBufferAddLit(buf, " </source>\n");
> }
> break;
> + case VIR_DOMAIN_DISK_TYPE_VOLUME:
> + /* Parsing guarantees the def->srcpool->volume cannot be NULL
> + * if def->srcpool->pool is not NULL.
> + */
> + if (def->srcpool->pool)
> + virBufferAsprintf(buf, " <source pool='%s' volume='%s'/>\n",
> + def->srcpool->pool, def->srcpool->volume);
> + break;
> default:
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("unexpected disk type %s"),
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index edddf25..988636e 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -447,6 +447,7 @@ enum virDomainDiskType {
> VIR_DOMAIN_DISK_TYPE_FILE,
> VIR_DOMAIN_DISK_TYPE_DIR,
> VIR_DOMAIN_DISK_TYPE_NETWORK,
> + VIR_DOMAIN_DISK_TYPE_VOLUME,
>
> VIR_DOMAIN_DISK_TYPE_LAST
> };
> @@ -606,6 +607,13 @@ struct _virDomainBlockIoTuneInfo {
> };
> typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr;
>
> +typedef struct _virDomainDiskSourcePoolDef virDomainDiskSourcePoolDef;
> +struct _virDomainDiskSourcePoolDef {
> + char *pool; /* pool name */
> + char *volume; /* volume name */
> +};
> +typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr;
> +
> /* Stores the virtual disk configuration */
> struct _virDomainDiskDef {
> int type;
> @@ -617,6 +625,7 @@ struct _virDomainDiskDef {
> int protocol;
> size_t nhosts;
> virDomainDiskHostDefPtr hosts;
> + virDomainDiskSourcePoolDefPtr srcpool;
> struct {
> char *username;
> int secretType; /* enum virDomainDiskSecretType */
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml
> new file mode 100644
> index 0000000..876eebe
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml
> @@ -0,0 +1,33 @@
> +<domain type='qemu'>
> + <name>QEMUGuest1</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219136</memory>
> + <currentMemory unit='KiB'>219136</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type arch='i686' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/qemu</emulator>
> + <disk type='volume' device='cdrom'>
> + <source pool='blk-pool0' volume='blk-pool0-vol0'/>
> + <target dev='hda' bus='ide'/>
> + <readonly/>
> + <address type='drive' controller='0' bus='0' target='0' unit='1'/>
> + </disk>
> + <disk type='file' device='disk'>
> + <source file='/tmp/idedisk.img'/>
> + <target dev='hdc' bus='ide'/>
> + <address type='drive' controller='0' bus='0' target='0' unit='2'/>
> + </disk>
> + <controller type='usb' index='0'/>
> + <controller type='ide' index='0'/>
> + <controller type='ide' index='1'/>
> + <memballoon model='virtio'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index ba9aa96..fa79960 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -252,6 +252,7 @@ mymain(void)
> DO_TEST("disk-scsi-lun-passthrough-sgio");
>
> DO_TEST("disk-scsi-disk-vpd");
> + DO_TEST("disk-source-pool");
>
> DO_TEST("virtio-rng-random");
> DO_TEST("virtio-rng-egd");
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list