It's long enough to have a independant definition. --- docs/schemas/domaincommon.rng | 93 +++++++++++++++++++++-------------------- 1 files changed, 48 insertions(+), 45 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 049f232..87653ce 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1075,51 +1075,7 @@ <interleave> <optional> <element name="source"> - <attribute name="protocol"> - <choice> - <value>nbd</value> - <value>rbd</value> - <value>sheepdog</value> - <value>gluster</value> - </choice> - </attribute> - <optional> - <attribute name="name"/> - </optional> - <zeroOrMore> - <element name="host"> - <choice> - <group> - <optional> - <attribute name="transport"> - <choice> - <value>tcp</value> - <value>rdma</value> - </choice> - </attribute> - </optional> - <attribute name="name"> - <choice> - <ref name="dnsName"/> - <ref name="ipAddr"/> - </choice> - </attribute> - <attribute name="port"> - <ref name="unsignedInt"/> - </attribute> - </group> - <group> - <attribute name="transport"> - <value>unix</value> - </attribute> - <attribute name="socket"> - <ref name="absFilePath"/> - </attribute> - </group> - </choice> - </element> - </zeroOrMore> - <empty/> + <ref name='diskSourceNetwork'/> </element> </optional> <ref name="diskspec"/> @@ -1129,6 +1085,53 @@ </choice> </element> </define> + <define name="diskSourceNetwork"> + <attribute name="protocol"> + <choice> + <value>nbd</value> + <value>rbd</value> + <value>sheepdog</value> + <value>gluster</value> + </choice> + </attribute> + <optional> + <attribute name="name"/> + </optional> + <zeroOrMore> + <element name="host"> + <choice> + <group> + <optional> + <attribute name="transport"> + <choice> + <value>tcp</value> + <value>rdma</value> + </choice> + </attribute> + </optional> + <attribute name="name"> + <choice> + <ref name="dnsName"/> + <ref name="ipAddr"/> + </choice> + </attribute> + <attribute name="port"> + <ref name="unsignedInt"/> + </attribute> + </group> + <group> + <attribute name="transport"> + <value>unix</value> + </attribute> + <attribute name="socket"> + <ref name="absFilePath"/> + </attribute> + </group> + </choice> + </element> + </zeroOrMore> + <empty/> + </define> <define name="diskTarget"> <data type="string"> <param name="pattern">(ioemu:)?(fd|hd|sd|vd|xvd|ubd)[a-zA-Z0-9_]+</param> -- 1.7.7.6

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. * New helper virDomainDiskSourceDefFormat to format the disk source. tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml: tests/qemuxml2xmltest.c: * New test --- The data struct for disk source should be tidied up. It's a mess now, but it will be later patches. --- docs/formatdomain.html.in | 17 +- docs/schemas/domaincommon.rng | 18 ++ src/conf/domain_conf.c | 259 +++++++++++++------- src/conf/domain_conf.h | 9 + .../qemuxml2argv-disk-source-pool.xml | 33 +++ tests/qemuxml2xmltest.c | 1 + 6 files changed, 247 insertions(+), 90 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7ad8aea..ac5657a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1367,6 +1367,11 @@ &lt;blockio logical_block_size='512' physical_block_size='4096'/&gt; &lt;target dev='hda' bus='ide'/&gt; &lt;/disk&gt; + &lt;disk type='volume' device='disk'&gt; + &lt;driver name='qemu' type='raw'/&gt; + &lt;source pool='blk-pool0' volume='blk-pool0-vol0'/&gt; + &lt;target dev='hda' bus='ide'/&gt; + &lt;/disk&gt; &lt;/devices&gt; ...</pre> @@ -1374,7 +1379,7 @@ <dt><code>disk</code></dt> <dd>The <code>disk</code> element is the main container for describing disks. The <code>type</code> attribute is either "file", - "block", "dir", or "network" + "block", "dir", "network", or "volume". and refers to the underlying source for the disk. The optional <code>device</code> attribute indicates how the disk is to be exposed to the guest OS. Possible values for this attribute are @@ -1444,9 +1449,15 @@ volume/image will be used. 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. + 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</span><br/> + 0.7.5; <code>type='network'</code> since 0.8.7; + <code>type='volume'</code> since 1.0.3</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 87653ce..88612ae 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1081,6 +1081,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 8dbfb96..5e65406 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -182,7 +182,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", @@ -985,6 +986,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; @@ -994,6 +1007,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); @@ -3573,6 +3587,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) || (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 @@ -3666,7 +3720,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; @@ -3760,6 +3814,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"), @@ -4050,7 +4108,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 && !hosts && !def->srcpool && def->device != VIR_DOMAIN_DISK_DEVICE_CDROM && def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { virReportError(VIR_ERR_NO_SOURCE, @@ -4072,8 +4130,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; } @@ -12168,6 +12237,102 @@ static void virDomainDiskBlockIoDefFormat(virBufferPtr buf, } } +static int +virDomainDiskSourceDefFormat(virBufferPtr buf, + virDomainDiskDefPtr def) +{ + int n; + const char *startupPolicy = virDomainStartupPolicyTypeToString(def->startupPolicy); + + if (def->src || def->nhosts > 0 || def->srcpool || + def->startupPolicy) { + switch (def->type) { + case VIR_DOMAIN_DISK_TYPE_FILE: + if (def->src) + virBufferEscapeString(buf, " <source file='%s'", def->src); + if (def->startupPolicy) + virBufferEscapeString(buf, " startupPolicy='%s'", + startupPolicy); + if (def->nseclabels) { + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 8); + for (n = 0; n < def->nseclabels; n++) + virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]); + virBufferAdjustIndent(buf, -8); + virBufferAddLit(buf, " </source>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } + break; + case VIR_DOMAIN_DISK_TYPE_BLOCK: + virBufferEscapeString(buf, " <source dev='%s'", + def->src); + if (def->nseclabels) { + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 8); + for (n = 0; n < def->nseclabels; n++) + virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]); + virBufferAdjustIndent(buf, -8); + virBufferAddLit(buf, " </source>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } + break; + case VIR_DOMAIN_DISK_TYPE_DIR: + virBufferEscapeString(buf, " <source dir='%s'/>\n", + def->src); + break; + case VIR_DOMAIN_DISK_TYPE_NETWORK: + virBufferAsprintf(buf, " <source protocol='%s'", + virDomainDiskProtocolTypeToString(def->protocol)); + if (def->src) { + virBufferEscapeString(buf, " name='%s'", def->src); + } + if (def->nhosts == 0) { + virBufferAddLit(buf, "/>\n"); + } else { + int i; + + virBufferAddLit(buf, ">\n"); + for (i = 0; i < def->nhosts; i++) { + virBufferAddLit(buf, " <host"); + if (def->hosts[i].name) { + virBufferEscapeString(buf, " name='%s'", def->hosts[i].name); + } + if (def->hosts[i].port) { + virBufferEscapeString(buf, " port='%s'", + def->hosts[i].port); + } + if (def->hosts[i].transport) { + virBufferAsprintf(buf, " transport='%s'", + virDomainDiskProtocolTransportTypeToString(def->hosts[i].transport)); + } + if (def->hosts[i].socket) { + virBufferEscapeString(buf, " socket='%s'", def->hosts[i].socket); + } + virBufferAddLit(buf, "/>\n"); + } + 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"), + virDomainDiskTypeToString(def->type)); + return -1; + } + } + + return 0; +} static int virDomainDiskDefFormat(virBufferPtr buf, @@ -12184,10 +12349,8 @@ virDomainDiskDefFormat(virBufferPtr buf, const char *ioeventfd = virDomainIoEventFdTypeToString(def->ioeventfd); const char *event_idx = virDomainVirtioEventIdxTypeToString(def->event_idx); const char *copy_on_read = virDomainVirtioEventIdxTypeToString(def->copy_on_read); - const char *startupPolicy = virDomainStartupPolicyTypeToString(def->startupPolicy); const char *sgio = virDomainDiskSGIOTypeToString(def->sgio); - int n; char uuidstr[VIR_UUID_STRING_BUFLEN]; if (!type) { @@ -12283,86 +12446,8 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAddLit(buf, " </auth>\n"); } - if (def->src || def->nhosts > 0 || - def->startupPolicy) { - switch (def->type) { - case VIR_DOMAIN_DISK_TYPE_FILE: - virBufferAddLit(buf," <source"); - if (def->src) - virBufferEscapeString(buf, " file='%s'", def->src); - if (def->startupPolicy) - virBufferEscapeString(buf, " startupPolicy='%s'", - startupPolicy); - if (def->nseclabels) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 8); - for (n = 0; n < def->nseclabels; n++) - virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]); - virBufferAdjustIndent(buf, -8); - virBufferAddLit(buf, " </source>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } - break; - case VIR_DOMAIN_DISK_TYPE_BLOCK: - virBufferEscapeString(buf, " <source dev='%s'", - def->src); - if (def->nseclabels) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 8); - for (n = 0; n < def->nseclabels; n++) - virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]); - virBufferAdjustIndent(buf, -8); - virBufferAddLit(buf, " </source>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } - break; - case VIR_DOMAIN_DISK_TYPE_DIR: - virBufferEscapeString(buf, " <source dir='%s'/>\n", - def->src); - break; - case VIR_DOMAIN_DISK_TYPE_NETWORK: - virBufferAsprintf(buf, " <source protocol='%s'", - virDomainDiskProtocolTypeToString(def->protocol)); - if (def->src) { - virBufferEscapeString(buf, " name='%s'", def->src); - } - if (def->nhosts == 0) { - virBufferAddLit(buf, "/>\n"); - } else { - int i; - - virBufferAddLit(buf, ">\n"); - for (i = 0; i < def->nhosts; i++) { - virBufferAddLit(buf, " <host"); - if (def->hosts[i].name) { - virBufferEscapeString(buf, " name='%s'", def->hosts[i].name); - } - if (def->hosts[i].port) { - virBufferEscapeString(buf, " port='%s'", - def->hosts[i].port); - } - if (def->hosts[i].transport) { - virBufferAsprintf(buf, " transport='%s'", - virDomainDiskProtocolTransportTypeToString(def->hosts[i].transport)); - } - if (def->hosts[i].socket) { - virBufferEscapeString(buf, " socket='%s'", def->hosts[i].socket); - } - virBufferAddLit(buf, "/>\n"); - } - virBufferAddLit(buf, " </source>\n"); - } - break; - default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected disk type %s"), - virDomainDiskTypeToString(def->type)); - return -1; - } - } - + if (virDomainDiskSourceDefFormat(buf, def) < 0) + return -1; virDomainDiskGeometryDefFormat(buf, def); virDomainDiskBlockIoDefFormat(buf, def); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9a9e0b1..9919db3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -427,6 +427,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 }; @@ -585,6 +586,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; @@ -596,6 +604,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 160e958..7e174dd 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -241,6 +241,7 @@ mymain(void) DO_TEST("disk-scsi-lun-passthrough-sgio"); DO_TEST("disk-scsi-disk-vpd"); + DO_TEST("disk-source-pool"); /* These tests generate different XML */ DO_TEST_DIFFERENT("balloon-device-auto"); -- 1.7.7.6

On 2013年02月01日 01:17, John Ferlan wrote: Oh, thanks, it's left unchanged when I moved "if (!pool && !volume)" above. You are correct, it's old bug, which is fixed in later patch of this series.

This creates src/conf/domain_storage.h and src/conf/storage_conf.c, which defines a driver contains internal APIs to glue the domain and storage. Currently there is only one API, translateDiskSourcePool, which is to translate the specified pool/volume into the the real underlying disk source. Later patch will implement API. --- src/Makefile.am | 3 ++- src/conf/domain_storage.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_storage.h | 37 +++++++++++++++++++++++++++++++++++++ src/libvirt_private.syms | 4 ++++ 4 files changed, 87 insertions(+), 1 deletions(-) create mode 100644 src/conf/domain_storage.c create mode 100644 src/conf/domain_storage.h diff --git a/src/Makefile.am b/src/Makefile.am index 070a089..d664c48 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -203,7 +203,8 @@ DOMAIN_CONF_SOURCES = \ conf/domain_conf.c conf/domain_conf.h \ conf/domain_audit.c conf/domain_audit.h \ conf/domain_nwfilter.c conf/domain_nwfilter.h \ - conf/snapshot_conf.c conf/snapshot_conf.h + conf/snapshot_conf.c conf/snapshot_conf.h \ + conf/domain_storage.c conf/domain_storage.h DOMAIN_EVENT_SOURCES = \ conf/domain_event.c conf/domain_event.h diff --git a/src/conf/domain_storage.c b/src/conf/domain_storage.c new file mode 100644 index 0000000..299e9fd --- /dev/null +++ b/src/conf/domain_storage.c @@ -0,0 +1,44 @@ +/* + * domain_storage.c: Internal APIs to glue domain and storage + * + * Copyright (C) 2013 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Osier Yang <jyang(a)redhat.com&gt; + */ + +#include <config.h> + +#include "internal.h" + +#include "datatypes.h" +#include "domain_conf.h" +#include "domain_storage.h" + +static virDomainStorageDriverPtr domainStorageDriver; + +void +virRegisterDomainStorageDriver(virDomainStorageDriverPtr driver) { + domainStorageDriver = driver; +} + +int +virDomainStorageTranslateDiskSourcePool(virConnectPtr conn, + virDomainDefPtr def) { + if (domainStorageDriver != NULL) + return domainStorageDriver->translateDiskSourcePool(conn, def); + return 0; +} diff --git a/src/conf/domain_storage.h b/src/conf/domain_storage.h new file mode 100644 index 0000000..143b476 --- /dev/null +++ b/src/conf/domain_storage.h @@ -0,0 +1,37 @@ +/* + * domain_storage.h: Internal APIs to glue domain and storage + * + * Copyright (C) 2013 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Osier Yang <jyang(a)redhat.com&gt; + */ +#ifndef DOMAIN_STORAGE_H +# define DOMAIN_STORAGE_H + +typedef int (*virDomainStorageDiskSourcePoolTranslate)(virConnectPtr conn, + virDomainDefPtr def); +struct _virDomainStorageDriver { + virDomainStorageDiskSourcePoolTranslate translateDiskSourcePool; +}; +typedef struct _virDomainStorageDriver virDomainStorageDriver; +typedef virDomainStorageDriver *virDomainStorageDriverPtr; + +void virRegisterDomainStorageDriver(virDomainStorageDriverPtr driver); +int virDomainStorageTranslateDiskSourcePool(virConnectPtr conn, + virDomainDefPtr def); + +#endif /* DOMAIN_STORAGE_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c589236..a44f3ff 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -617,6 +617,10 @@ virDomainConfNWFilterTeardown; virDomainConfVMNWFilterTeardown; +# domain_storage.h +virDomainStorageTranslateDiskSourcePool; +virRegisterDomainStorageDriver; + # ebtables.h ebtablesAddForwardAllowIn; ebtablesAddForwardPolicyReject; -- 1.7.7.6

On Thu, Jan 31, 2013 at 02:11:28AM +0800, Osier Yang wrote: IMHO this abstraction is rather pointless. Just call virStoragePoolLookupbyUUID() at the time you need to generate the CLI args, just as we do with the existing <interface type='network'/> Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

It iterates over all the domain disks, and translate the source of all the disks of 'volume' type from storage pool/volume to the real underlying source. Disks of type 'file', 'block', and 'dir' are supported now. Network type is not supported yet, it will be another patch. src/storage/storage_driver.c: * New helper storagePoolObjFindByDiskSource to find the specified pool by name. * Implement translateDiskSourcePool as storageTranslateDomainDiskSourcePool --- src/storage/storage_driver.c | 90 ++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 90 insertions(+), 0 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index e98c18c..9f60f2c 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -47,6 +47,8 @@ #include "virlog.h" #include "virfile.h" #include "fdstream.h" +#include "domain_conf.h" +#include "domain_storage.h" #include "configmake.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -2367,6 +2369,88 @@ storageListAllPools(virConnectPtr conn, return ret; } +static virStoragePoolObjPtr +storagePoolObjFindByDiskSource(virConnectPtr conn, + const char *name) +{ + virStorageDriverStatePtr driver = conn->storagePrivateData; + virStoragePoolObjPtr pool = NULL; + + storageDriverLock(driver); + pool = virStoragePoolObjFindByName(&driver->pools, name); + storageDriverUnlock(driver); + + if (!pool) { + virReportError(VIR_ERR_NO_STORAGE_POOL, + _("no storage pool with matching name '%s'"), + name); + goto error; + } + + if (!virStoragePoolObjIsActive(pool)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("The specified pool '%s' is not active"), + name); + goto error; + } + + return pool; +error: + if (pool) + virStoragePoolObjUnlock(pool); + return NULL; +} + +static int +storageTranslateDomainDiskSourcePool(virConnectPtr conn, + virDomainDefPtr def) +{ + virStoragePoolObjPtr pool = NULL; + virStorageVolDefPtr vol = NULL; + int i; + int ret = -1; + + for (i = 0; i < def->ndisks; i++) { + virDomainDiskDefPtr disk = def->disks[i]; + + if (disk->type != VIR_DOMAIN_DISK_TYPE_VOLUME) + continue; + + if (!(pool = storagePoolObjFindByDiskSource(conn, disk->srcpool->pool))) + goto cleanup; + + if (!(vol = virStorageVolDefFindByName(pool, disk->srcpool->volume))) { + virReportError(VIR_ERR_NO_STORAGE_VOL, + _("no storage vol of specified pool with " + "matching name '%s'"), + disk->srcpool->volume); + goto cleanup; + } + + switch (vol->type) { + case VIR_STORAGE_VOL_FILE: + case VIR_STORAGE_VOL_BLOCK: + case VIR_STORAGE_VOL_DIR: + disk->src = strdup(vol->target.path); + break; + case VIR_STORAGE_VOL_NETWORK: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Using network volume as disk source is not supported")); + goto cleanup; + } + + virStoragePoolObjUnlock(pool); + pool = NULL; + } + + ret = 0; + +cleanup: + if (pool) + virStoragePoolObjUnlock(pool); + return ret; +} + static virStorageDriver storageDriver = { .name = "storage", .open = storageOpen, /* 0.4.0 */ @@ -2423,8 +2507,14 @@ static virStateDriver stateDriver = { .reload = storageDriverReload, }; +static virDomainStorageDriver domainStorageDriver = { + .translateDiskSourcePool = storageTranslateDomainDiskSourcePool, +}; + + int storageRegister(void) { virRegisterStorageDriver(&storageDriver); virRegisterStateDriver(&stateDriver); + virRegisterDomainStorageDriver(&domainStorageDriver); return 0; } -- 1.7.7.6

On 2013年02月01日 01:42, John Ferlan wrote: Agreed. Hum, this inspires me notice vol->target.path can be NULL (it's not mandatory in vol XML). And it should error out as long as the the disk is not CD-ROM or Floppy. But I don't think it's neccesary to check the return value of strdup. Will post v2.

On 02/01/2013 12:24 PM, Osier Yang wrote: Just checking - if !vol->target.path, then disk->src will be empty, is that expected at this point? You've added the first check since the initial pass at this and I want to make sure you've covered your cases. Without comments in the code I'm not sure what is expected. ACK (as long as you feel you've covered your cases for the disk storage pool) John

On Thu, Jan 31, 2013 at 02:11:29AM +0800, Osier Yang wrote: Again, I think this abstraction is all pretty pointless Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 01/30/2013 01:11 PM, Osier Yang wrote: ACK

On 01/30/2013 01:11 PM, Osier Yang wrote: ACK

On 01/30/2013 01:11 PM, Osier Yang wrote: ACK

On 01/30/2013 01:11 PM, Osier Yang wrote: s/seclable/seclabel ACK w/ the minor edit

On 01/30/2013 01:11 PM, Osier Yang wrote: What if hosts[n].name == NULL? Need a VIR_FREE(hosts); What happens if !disk->src here?