[libvirt] [PATCH] Introduce <driver> under <filesystem> to support open-by-handle.
Harsh Bora
harsh at linux.vnet.ibm.com
Mon Oct 10 11:46:32 UTC 2011
On 10/10/2011 03:47 PM, Daniel P. Berrange wrote:
> On Fri, Oct 07, 2011 at 04:32:42PM +0530, Harsh Prateek Bora wrote:
>> VirtFS allows the user to choose between local/handle for fs driver.
>> As of now, libvirt hardcode to use local driver only. This patch provides a
>> solution to allow user to choose between local/handle as fs driver.
>>
>> Sample:
>>
>> <filesystem type='mount'>
>> <driver type='handle'/>
>> <source dir='/folder/to/share'/>
>> <target dir='mount_tag'/>
>> </filesystem>
>>
>> Signed-off-by: Harsh Prateek Bora<harsh at linux.vnet.ibm.com>
>> ---
>> docs/schemas/domaincommon.rng | 9 +++++++++
>> src/conf/domain_conf.c | 23 +++++++++++++++++++++++
>> src/conf/domain_conf.h | 10 ++++++++++
>> src/qemu/qemu_command.c | 7 ++++++-
>> 4 files changed, 48 insertions(+), 1 deletions(-)
>>
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 492a41d..3a0cb86 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -1025,6 +1025,15 @@
>> </choice>
>> <optional>
>> <ref name="address"/>
>> +<element name="driver">
>> +<attribute name="type">
>> +<choice>
>> +<value>local</value>
>> +<value>handle</value>
>
>
> I must say I never found 'local' to be a very good name for this.
> Perhaps in libvirt we should just call it 'path', since that is
> how it works. We'd translate it back to 'local' when invoking
> QEMU of course.
makes sense.
>
>> +</choice>
>> +</attribute>
>> +<empty/>
>> +</element>
>> <attribute name="accessmode">
>> <choice>
>> <value>passthrough</value>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index a537251..04410f2 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -238,6 +238,10 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
>> "file",
>> "template")
>>
>> +VIR_ENUM_IMPL(virDomainFSDriverType, VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
>> + "local",
>> + "handle")
>> +
>> VIR_ENUM_IMPL(virDomainFSAccessMode, VIR_DOMAIN_FS_ACCESSMODE_LAST,
>> "passthrough",
>> "mapped",
>> @@ -2828,6 +2832,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
>> virDomainFSDefPtr def;
>> xmlNodePtr cur;
>> char *type = NULL;
>> + char *fsdriver = NULL;
>> char *source = NULL;
>> char *target = NULL;
>> char *accessmode = NULL;
>> @@ -2878,11 +2883,23 @@ virDomainFSDefParseXML(xmlNodePtr node,
>> target = virXMLPropString(cur, "dir");
>> } else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) {
>> def->readonly = 1;
>> + } else if ((fsdriver == NULL)&& (xmlStrEqual(cur->name, BAD_CAST "driver"))) {
>> + fsdriver = virXMLPropString(cur, "type");
>> }
>> }
>> cur = cur->next;
>> }
>>
>> + if (fsdriver) {
>> + if ((def->fsdriver = virDomainFSDriverTypeTypeFromString(fsdriver))< 0) {
>> + virDomainReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("unknown fs driver type '%s'"), fsdriver);
>> + goto error;
>> + }
>> + } else {
>> + def->fsdriver = VIR_DOMAIN_FS_DRIVER_TYPE_LOCAL;
>> + }
>> +
>> if (source == NULL) {
>> virDomainReportError(VIR_ERR_NO_SOURCE,
>> target ? "%s" : NULL, target);
>> @@ -2905,6 +2922,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
>>
>> cleanup:
>> VIR_FREE(type);
>> + VIR_FREE(fsdriver);
>> VIR_FREE(target);
>> VIR_FREE(source);
>> VIR_FREE(accessmode);
>> @@ -9351,6 +9369,7 @@ virDomainFSDefFormat(virBufferPtr buf,
>> {
>> const char *type = virDomainFSTypeToString(def->type);
>> const char *accessmode = virDomainFSAccessModeTypeToString(def->accessmode);
>> + const char *fsdriver = virDomainFSDriverTypeTypeToString(def->fsdriver);
>>
>> if (!type) {
>> virDomainReportError(VIR_ERR_INTERNAL_ERROR,
>> @@ -9369,6 +9388,10 @@ virDomainFSDefFormat(virBufferPtr buf,
>> "<filesystem type='%s' accessmode='%s'>\n",
>> type, accessmode);
>>
>> + if (def->fsdriver) {
>> + virBufferAsprintf(buf, "<driver type='%s'/>\n", fsdriver);
>> + }
>
> If we're going to have this optional, then we should add another entry
> in the enum of 'VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT' at value 0.
Okay, I see default being used at some places, still not sure why is it
required ? Just to go with convention so that reader knows its optional ?
>
>> +
>> if (def->src) {
>> switch (def->type) {
>> case VIR_DOMAIN_FS_TYPE_MOUNT:
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index e07fd2f..d42325e 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -368,6 +368,14 @@ enum virDomainFSType {
>> VIR_DOMAIN_FS_TYPE_LAST
>> };
>>
>> +/* Filesystem driver type */
>> +enum virDomainFSDriverType {
>> + VIR_DOMAIN_FS_DRIVER_TYPE_LOCAL,
>> + VIR_DOMAIN_FS_DRIVER_TYPE_HANDLE,
>> +
>> + VIR_DOMAIN_FS_DRIVER_TYPE_LAST
>> +};
>> +
>> /* Filesystem mount access mode */
>> enum virDomainFSAccessMode {
>> VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH,
>> @@ -381,6 +389,7 @@ typedef struct _virDomainFSDef virDomainFSDef;
>> typedef virDomainFSDef *virDomainFSDefPtr;
>> struct _virDomainFSDef {
>> int type;
>> + int fsdriver;
>> int accessmode;
>> char *src;
>> char *dst;
>> @@ -1856,6 +1865,7 @@ VIR_ENUM_DECL(virDomainController)
>> VIR_ENUM_DECL(virDomainControllerModelSCSI)
>> VIR_ENUM_DECL(virDomainControllerModelUSB)
>> VIR_ENUM_DECL(virDomainFS)
>> +VIR_ENUM_DECL(virDomainFSDriverType)
>> VIR_ENUM_DECL(virDomainFSAccessMode)
>> VIR_ENUM_DECL(virDomainNet)
>> VIR_ENUM_DECL(virDomainNetBackend)
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index cf99f89..a989aa7 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -1818,7 +1818,12 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs,
>> goto error;
>> }
>>
>> - virBufferAddLit(&opt, "local");
>> + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_LOCAL) {
>> + virBufferAddLit(&opt, "local");
>> + } else if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_HANDLE) {
>> + virBufferAddLit(&opt, "handle");
>> + }
>
> We can use an enum for this too. eg
>
> VIR_ENUM_DECL(qemuFSDriverName);
> VIR_ENUM_IMPL(qemuFSDriverName, VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
> "local", "handle");
>
> virBufferAddLit(&opt, qemuFSDriverNameTypeToString(fs->fsdriver));
>
I tried that, but it gives a compiler err because both are macros and
not functions. I shall post v2 with rest of the changes now.
- Harsh
>
> Regards,
> Daniel
More information about the libvir-list
mailing list