[libvirt] [PATCH 2/8] Introduce new XMLs to specify disk source using libvirt storage
John Ferlan
jferlan at redhat.com
Fri Apr 5 22:24:54 UTC 2013
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
s/seclables/seclabels/
> 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
>
ACK, mechanically speaking what's here seems right - although I just get
the feeling something is missing. I just don't have enough time yet to
"just know" that the "<driver" and "<target" tags have a specific
meaning and whether something else might be missing.
The volume listed must be present in some pool and there's some sort of
linkage that I don't see/get yet with this patch to validate that.
Maybe this should have waited for the other patches you mentioned in the
response to Paolo.
My other hangup is my view of a volume is not a device like 'cdrom'
rather it's more like /dev/sda# (on hpux it could have been
/dev/rdisk/disk4). That is presented to the guest as a complete volume
or of course we also allowed partitions. The example above using
'fc18.img' speaks "file" to me.
John
> 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 @@
> <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");
>
More information about the libvir-list
mailing list