[libvirt PATCHv3 2/4] qemu: add support for 'fmode' and 'dmode' options

[Peter Krempa]

On Mon, Oct 05, 2020 at 19:40:14 +0100, Brian Turek wrote:
> Expose QEMU's 9pfs 'fmode' and 'dmode' options via attributes on the
> 'filesystem' node in the domain XML. These options control the creation
> mode of files and directories, respectively, when using
> accessmode=mapped.  QEMU defaults to creating files with mode 0600 and
> directories with mode 0700.
> 
> Signed-off-by: Brian Turek <brian.turek at gmail.com>
> ---
>  src/conf/domain_conf.c                        | 27 ++++++++
>  src/conf/domain_conf.h                        |  2 +
>  src/qemu/qemu_command.c                       |  6 ++
>  src/qemu/qemu_validate.c                      | 18 ++++++

This patch still mixes the XML bits with the qemu implementation.

>  .../virtio-9p-createmode.x86_64-latest.args   | 45 ++++++++++++++
>  .../qemuxml2argvdata/virtio-9p-createmode.xml | 58 ++++++++++++++++++
>  .../virtio-9p-createmode.x86_64-latest.xml    | 61 +++++++++++++++++++

Compilation of the tree doesn't pass 'virschematest' after this patch as
you are adding the RNG schema after adding the XML.


>  tests/qemuxml2xmltest.c                       |  1 +

You are missing a change to qemuxml2argvtest to actually invoke also the
testing of the command line formatter ...

>  8 files changed, 218 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/virtio-9p-createmode.x86_64-latest.args

... but you've included the output file.

>  create mode 100644 tests/qemuxml2argvdata/virtio-9p-createmode.xml
>  create mode 100644 tests/qemuxml2xmloutdata/virtio-9p-createmode.x86_64-latest.xml
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 175b632a38..e80b3b7ef6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c

[...]

> @@ -11524,6 +11526,24 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
>          def->accessmode = VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH;
>      }
>  
> +    fmode = virXMLPropString(node, "fmode");
> +    if (fmode) {
> +        if (virStrToLong_uip(fmode, NULL, 8, &def->fmode) < 0) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("invalid fmode: '%s'"), fmode);
> +            goto error;
> +        }
> +    }
> +
> +    dmode = virXMLPropString(node, "dmode");
> +    if (dmode) {
> +        if (virStrToLong_uip(dmode, NULL, 8, &def->dmode) < 0) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("invalid dmode: '%s'"), dmode);
> +            goto error;
> +        }

Both errors should be VIR_ERR_XML_ERROR.

> +    }
> +
>      model = virXMLPropString(node, "model");
>      if (model) {
>          if ((def->model = virDomainFSModelTypeFromString(model)) < 0 ||
> @@ -26211,6 +26231,13 @@ virDomainFSDefFormat(virBufferPtr buf,
>      }
>      if (def->multidevs)
>          virBufferAsprintf(buf, " multidevs='%s'", multidevs);
> +
> +    if (def->fmode)
> +        virBufferAsprintf(buf, " fmode='%04o'", def->fmode);
> +
> +    if (def->dmode)
> +        virBufferAsprintf(buf, " dmode='%04o'", def->dmode);
> +

See reply on cover letter for potential problems.

>      virBufferAddLit(buf, ">\n");
>  
>      virBufferAdjustIndent(buf, 2);

[...]

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 476cf6972e..b2da53c664 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2275,6 +2275,12 @@ qemuBuildFSStr(virDomainFSDefPtr fs)
>          } else if (fs->multidevs == VIR_DOMAIN_FS_MULTIDEVS_WARN) {
>              virBufferAddLit(&opt, ",multidevs=warn");
>          }
> +        if (fs->fmode) {
> +            virBufferAsprintf(&opt, ",fmode=%04o", fs->fmode);
> +        }
> +        if (fs->dmode) {
> +            virBufferAsprintf(&opt, ",dmode=%04o", fs->dmode);

See reply on cover letter for potential problems.

> +        }
>      } else if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_HANDLE) {
>          /* removed since qemu 4.0.0 see v3.1.0-29-g93aee84f57 */
>          virBufferAddLit(&opt, "handle");