Re: [PATCH v3 libvirt 1/3] conf: add <model> child element to <filesystem>

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

On Tue, Aug 19, 2014 at 10:11:35PM +0200, Giuseppe Scrivano wrote:
> Signed-off-by: Giuseppe Scrivano <gscrivan@xxxxxxxxxx>
> ---
>  docs/formatdomain.html.in              |  6 ++++++
>  docs/schemas/domaincommon.rng          | 13 +++++++++++++
>  src/conf/domain_conf.c                 | 26 ++++++++++++++++++++++++++
>  src/conf/domain_conf.h                 | 11 +++++++++++
>  tests/domainconfdata/getfilesystem.xml |  5 +++++
>  tests/domainconftest.c                 |  1 +
>  6 files changed, 62 insertions(+)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index ed17389..d0c1ce2 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2300,6 +2300,8 @@
>        &lt;driver type='path' wrpolicy='immediate'/&gt;
>        &lt;source dir='/export/to/guest'/&gt;
>        &lt;target dir='/import/from/host'/&gt;
> +      &lt;model type='9p'/&gt;
> +      &lt;model type='mtp'/&gt;
>        &lt;readonly/&gt;
>      &lt;/filesystem&gt;
>      &lt;filesystem type='file' accessmode='passthrough'&gt;
> @@ -2337,6 +2339,10 @@
>          while the value <code>immediate</code> means that a host writeback
>          is immediately triggered for all pages touched during a guest file
>          write operation <span class="since">(since 0.9.10)</span>.
> +        A "filesystem" element has an optional
> +        attribute <code>model</code><span class="since"> (since
> +        1.2.8)</span>, which is one of "9p", "mtp" (used by QEMU/KVM),
> +        if this element is not specified the default is "9p".
>          </dd>
>          <dt><code>type='template'</code></dt>
>          <dd>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 033f2f6..684acec 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1937,6 +1937,19 @@
>            </element>
>          </optional>
>        </interleave>
> +      <interleave>
> +        <optional>
> +          <element name="model">
> +            <attribute name="type">
> +              <choice>
> +                <value>9p</value>
> +                <value>mtp</value>
> +              </choice>
> +            </attribute>
> +            <empty/>
> +          </element>
> +        </optional>
> +      </interleave>
>      </element>
>    </define>
>    <define name="fsDriver">
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 14338ba..98dbe14 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -344,6 +344,11 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
>                "ram",
>                "bind")
>  
> +VIR_ENUM_IMPL(virDomainFSModel, VIR_DOMAIN_FS_MODEL_LAST,
> +              "default",
> +              "9p",
> +              "mtp")
> +
>  VIR_ENUM_IMPL(virDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
>                "default",
>                "path",
> @@ -6459,6 +6464,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
>      virDomainFSDefPtr def;
>      xmlNodePtr cur, save_node = ctxt->node;
>      char *type = NULL;
> +    char *model = NULL;
>      char *fsdriver = NULL;
>      char *source = NULL;
>      char *target = NULL;
> @@ -6536,6 +6542,9 @@ virDomainFSDefParseXML(xmlNodePtr node,
>                      wrpolicy = virXMLPropString(cur, "wrpolicy");
>                  if (!format)
>                      format = virXMLPropString(cur, "format");
> +            } else if (!model &&
> +                       xmlStrEqual(cur->name, BAD_CAST "model")) {
> +                model = virXMLPropString(cur, "type");
>              }
>          }
>          cur = cur->next;
> @@ -6557,6 +6566,14 @@ virDomainFSDefParseXML(xmlNodePtr node,
>          }
>      }
>  
> +    if (model) {
> +        if ((def->model = virDomainFSModelTypeFromString(model)) <= 0) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("unknown model value '%s'"), model);
> +            goto error;
> +        }
> +    }
> +
>      if (wrpolicy) {
>          if ((def->wrpolicy = virDomainFSWrpolicyTypeFromString(wrpolicy)) <= 0) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -6616,6 +6633,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
>      VIR_FREE(usage);
>      VIR_FREE(units);
>      VIR_FREE(format);
> +    VIR_FREE(model);
>  
>      return def;
>  
> @@ -15786,6 +15804,14 @@ virDomainFSDefFormat(virBufferPtr buf,
>  
>      switch (def->type) {
>      case VIR_DOMAIN_FS_TYPE_MOUNT:
> +        virBufferEscapeString(buf, "<source dir='%s'/>\n",
> +                              def->src);
> +        if (def->model) {
> +            virBufferEscapeString(buf, "<model type='%s'/>\n",
> +                                  virDomainFSModelTypeToString(def->model));
> +        }

>From the XML pov, we shouldn't restrict use of <model> to only be
for FS_TYPE_MOUNT. That is a QEMU driver implementation restriction,
so just make the QEMU driver raise VIR_ERR_CONFIG_UNSUPPORTED for
the cases we don't want, when building the CLI args

> +        break;
> +
>      case VIR_DOMAIN_FS_TYPE_BIND:
>          virBufferEscapeString(buf, "<source dir='%s'/>\n",
>                                def->src);

Regards,
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 :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list