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@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