[libvirt PATCHv3 0/4] Add support for QEMU's fmode and dmode

Brian Turek posted 4 patches 3 years, 6 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20201005184016.63355-1-brian.turek@gmail.com
There is a newer version of this series
docs/formatdomain.rst                         | 12 ++++
docs/schemas/domaincommon.rng                 | 16 +++++
src/conf/domain_conf.c                        | 27 ++++++++
src/conf/domain_conf.h                        |  2 +
src/qemu/qemu_capabilities.c                  |  2 +
src/qemu/qemu_capabilities.h                  |  1 +
src/qemu/qemu_command.c                       |  6 ++
src/qemu/qemu_validate.c                      | 18 ++++++
.../caps_2.10.0.aarch64.xml                   |  1 +
.../caps_2.10.0.ppc64.xml                     |  1 +
.../caps_2.10.0.s390x.xml                     |  1 +
.../caps_2.10.0.x86_64.xml                    |  1 +
.../caps_2.11.0.s390x.xml                     |  1 +
.../caps_2.11.0.x86_64.xml                    |  1 +
.../caps_2.12.0.aarch64.xml                   |  1 +
.../caps_2.12.0.ppc64.xml                     |  1 +
.../caps_2.12.0.s390x.xml                     |  1 +
.../caps_2.12.0.x86_64.xml                    |  1 +
.../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |  1 +
.../caps_3.0.0.riscv32.xml                    |  1 +
.../caps_3.0.0.riscv64.xml                    |  1 +
.../qemucapabilitiesdata/caps_3.0.0.s390x.xml |  1 +
.../caps_3.0.0.x86_64.xml                     |  1 +
.../qemucapabilitiesdata/caps_3.1.0.ppc64.xml |  1 +
.../caps_3.1.0.x86_64.xml                     |  1 +
.../caps_4.0.0.aarch64.xml                    |  1 +
.../qemucapabilitiesdata/caps_4.0.0.ppc64.xml |  1 +
.../caps_4.0.0.riscv32.xml                    |  1 +
.../caps_4.0.0.riscv64.xml                    |  1 +
.../qemucapabilitiesdata/caps_4.0.0.s390x.xml |  1 +
.../caps_4.0.0.x86_64.xml                     |  1 +
.../caps_4.1.0.x86_64.xml                     |  1 +
.../caps_4.2.0.aarch64.xml                    |  1 +
.../qemucapabilitiesdata/caps_4.2.0.ppc64.xml |  1 +
.../qemucapabilitiesdata/caps_4.2.0.s390x.xml |  1 +
.../caps_4.2.0.x86_64.xml                     |  1 +
.../caps_5.0.0.aarch64.xml                    |  1 +
.../qemucapabilitiesdata/caps_5.0.0.ppc64.xml |  1 +
.../caps_5.0.0.riscv64.xml                    |  1 +
.../caps_5.0.0.x86_64.xml                     |  1 +
.../caps_5.1.0.x86_64.xml                     |  1 +
.../caps_5.2.0.x86_64.xml                     |  1 +
.../virtio-9p-createmode.x86_64-latest.args   | 45 ++++++++++++++
.../qemuxml2argvdata/virtio-9p-createmode.xml | 58 ++++++++++++++++++
.../virtio-9p-createmode.x86_64-latest.xml    | 61 +++++++++++++++++++
tests/qemuxml2xmltest.c                       |  1 +
46 files changed, 283 insertions(+)
create mode 100644 tests/qemuxml2argvdata/virtio-9p-createmode.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/virtio-9p-createmode.xml
create mode 100644 tests/qemuxml2xmloutdata/virtio-9p-createmode.x86_64-latest.xml
[libvirt PATCHv3 0/4] Add support for QEMU's fmode and dmode
Posted by Brian Turek 3 years, 6 months ago
Apologies for the second submission here. I got a kickback on two of the
emails saying it was "rejected due to security policies."

This third version of the patches fixes a bug where QEMU interpreted the
command line value passed to it as base-10 rather than base-8.  This new
version ensures there is always a preceeding 0 in the QEMU args (using
%04o formatting) and explictly sets it in the generated XML.

Brian Turek (4):
  qemu: capabilities: add QEMU_CAPS_FSDEV_CREATEMODE
  qemu: add support for 'fmode' and 'dmode' options
  qemu: add schema 'fmode' and 'dmode' options
  qemu: add docs for 'fmode' and 'dmode' options

 docs/formatdomain.rst                         | 12 ++++
 docs/schemas/domaincommon.rng                 | 16 +++++
 src/conf/domain_conf.c                        | 27 ++++++++
 src/conf/domain_conf.h                        |  2 +
 src/qemu/qemu_capabilities.c                  |  2 +
 src/qemu/qemu_capabilities.h                  |  1 +
 src/qemu/qemu_command.c                       |  6 ++
 src/qemu/qemu_validate.c                      | 18 ++++++
 .../caps_2.10.0.aarch64.xml                   |  1 +
 .../caps_2.10.0.ppc64.xml                     |  1 +
 .../caps_2.10.0.s390x.xml                     |  1 +
 .../caps_2.10.0.x86_64.xml                    |  1 +
 .../caps_2.11.0.s390x.xml                     |  1 +
 .../caps_2.11.0.x86_64.xml                    |  1 +
 .../caps_2.12.0.aarch64.xml                   |  1 +
 .../caps_2.12.0.ppc64.xml                     |  1 +
 .../caps_2.12.0.s390x.xml                     |  1 +
 .../caps_2.12.0.x86_64.xml                    |  1 +
 .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |  1 +
 .../caps_3.0.0.riscv32.xml                    |  1 +
 .../caps_3.0.0.riscv64.xml                    |  1 +
 .../qemucapabilitiesdata/caps_3.0.0.s390x.xml |  1 +
 .../caps_3.0.0.x86_64.xml                     |  1 +
 .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml |  1 +
 .../caps_3.1.0.x86_64.xml                     |  1 +
 .../caps_4.0.0.aarch64.xml                    |  1 +
 .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml |  1 +
 .../caps_4.0.0.riscv32.xml                    |  1 +
 .../caps_4.0.0.riscv64.xml                    |  1 +
 .../qemucapabilitiesdata/caps_4.0.0.s390x.xml |  1 +
 .../caps_4.0.0.x86_64.xml                     |  1 +
 .../caps_4.1.0.x86_64.xml                     |  1 +
 .../caps_4.2.0.aarch64.xml                    |  1 +
 .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml |  1 +
 .../qemucapabilitiesdata/caps_4.2.0.s390x.xml |  1 +
 .../caps_4.2.0.x86_64.xml                     |  1 +
 .../caps_5.0.0.aarch64.xml                    |  1 +
 .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml |  1 +
 .../caps_5.0.0.riscv64.xml                    |  1 +
 .../caps_5.0.0.x86_64.xml                     |  1 +
 .../caps_5.1.0.x86_64.xml                     |  1 +
 .../caps_5.2.0.x86_64.xml                     |  1 +
 .../virtio-9p-createmode.x86_64-latest.args   | 45 ++++++++++++++
 .../qemuxml2argvdata/virtio-9p-createmode.xml | 58 ++++++++++++++++++
 .../virtio-9p-createmode.x86_64-latest.xml    | 61 +++++++++++++++++++
 tests/qemuxml2xmltest.c                       |  1 +
 46 files changed, 283 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/virtio-9p-createmode.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/virtio-9p-createmode.xml
 create mode 100644 tests/qemuxml2xmloutdata/virtio-9p-createmode.x86_64-latest.xml

-- 
2.25.1

Re: [libvirt PATCHv3 0/4] Add support for QEMU's fmode and dmode
Posted by Peter Krempa 3 years, 6 months ago
On Mon, Oct 05, 2020 at 19:40:12 +0100, Brian Turek wrote:
> Apologies for the second submission here. I got a kickback on two of the
> emails saying it was "rejected due to security policies."
> 
> This third version of the patches fixes a bug where QEMU interpreted the
> command line value passed to it as base-10 rather than base-8.  This new
> version ensures there is always a preceeding 0 in the QEMU args (using
> %04o formatting) and explictly sets it in the generated XML.

That sounds like a very bad design from qemu. Unfortunately this version
doesn't fix it completely either. The XML parser you've implemented
parses the passed number as octal but doesn't validate it's maximum value.

Since the qemu command line option is formatted as 4 octal digits, a
mode such as '1775' which is a valid mode for a directory will still be
formatted as something which looks like a decimal number:

-fsdev local,security_model=mapped,dmode=1775,id=fsdev-fs1,path=/export/fs1 \

Also the documentation doesn't mention whether sticky bit and such are
actually handled.

Re: [libvirt PATCHv3 0/4] Add support for QEMU's fmode and dmode
Posted by Brian Turek 3 years, 6 months ago
Peter Krempa wrote:
> Since the qemu command line option is formatted as 4 octal digits, a
> mode such as '1775' which is a valid mode for a directory will still be
> formatted as something which looks like a decimal number:
>
> -fsdev
local,security_model=mapped,dmode=1775,id=fsdev-fs1,path=/export/fs1 \
>
> Also the documentation doesn't mention whether sticky bit and such are
> actually handled.

This is totally fair.  QEMU has zero documentation on the intended limits on
these two options but the QEMU source masks them with 0777.  Given that we
only have the implementation to go off of rather than the intent, should we
assume that sticky bits will never be supported or that it's an
unintentional
shortcoming in the QEMU code?

I can either do similar masking or mask with 07777 and send along the
results
to QEMU as a 5-digit number (the first digit being a leading 0) .The
libvirt docs
could then say the behavior is ultimately determined by QEMU but, currently,
sticky bits are not supported?
Re: [libvirt PATCHv3 0/4] Add support for QEMU's fmode and dmode
Posted by Peter Krempa 3 years, 6 months ago
On Tue, Oct 06, 2020 at 19:36:03 +0100, Brian Turek wrote:
> Peter Krempa wrote:
> > Since the qemu command line option is formatted as 4 octal digits, a
> > mode such as '1775' which is a valid mode for a directory will still be
> > formatted as something which looks like a decimal number:
> >
> > -fsdev
> local,security_model=mapped,dmode=1775,id=fsdev-fs1,path=/export/fs1 \
> >
> > Also the documentation doesn't mention whether sticky bit and such are
> > actually handled.
> 
> This is totally fair.  QEMU has zero documentation on the intended limits on
> these two options but the QEMU source masks them with 0777.  Given that we
> only have the implementation to go off of rather than the intent, should we
> assume that sticky bits will never be supported or that it's an
> unintentional
> shortcoming in the QEMU code?
> 
> I can either do similar masking or mask with 07777 and send along the
> results
> to QEMU as a 5-digit number (the first digit being a leading 0) .The
> libvirt docs
> could then say the behavior is ultimately determined by QEMU but, currently,
> sticky bits are not supported?

If it's unclear what qemu does, we can always just limit the values to
0777 ourselves for now as that's known and is potentially more strict
than what qemu can do.

The validation code then needs to make sure that it's in the correct
range and thus the command line formatter will work correctly in the
state it's now.

So I'd go with a limit check in the validator and docs.

I also presume that mode '0000' is not useful in this case. The code as
it's now doesn't allow setting 0000 as the value is used as default when
user didn't provide any mode.