Re: [libvirt] [PATCH v3 01/11] conf: Add some defined NFS Storage Pool mount options
On Tue, Jan 15, 2019 at 08:09:12PM -0500, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1584663
Add the ability to parse/manage some defined NFS Storage Pool mount
options. Keep the set of defined options limited to noexec, nosuid,
nodev, and ro. Subsequent patches will add the ability to provide the
NFS nfsvers value and eventally any option via storage pool namespaces.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
docs/formatstorage.html.in | 21 +++++++++
docs/schemas/storagepool.rng | 20 ++++++++
src/conf/storage_conf.c | 47 +++++++++++++++++++
src/conf/storage_conf.h | 13 +++++
.../pool-netfs-mountopts.xml | 24 ++++++++++
.../pool-netfs-mountopts.xml | 24 ++++++++++
tests/storagepoolxml2xmltest.c | 1 +
7 files changed, 150 insertions(+)
create mode 100644 tests/storagepoolxml2xmlin/pool-netfs-mountopts.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-netfs-mountopts.xml
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index b6bf3edbd2..97c90f0797 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -121,6 +121,19 @@
</source>
...</pre>
+ <pre>
+...
+ <source>
+ <host name='localhost'/>
+ <dir path='/var/lib/libvirt/images'/>
+ <format type='nfs'/>
+ <mount_opts>
+ <option name='nodev'/>
+ <option name='nosuid'/>
+ </mount_opts>
+ </source>
+...</pre>
+
<dl>
<dt><code>device</code></dt>
<dd>Provides the source for pools backed by physical devices
@@ -386,6 +399,14 @@
LVM metadata type. All drivers are required to have a default
value for this, so it is optional. <span class="since">Since
0.4.1</span></dd>
+ <dt><code>mount_opts</code></dt>
+ <dd>Provide an optional set of mount options for the
<code>netfs</code>
+ pool startup or creation to be provided via the "-o" option.
+ The desired options are specified by using the subelement
+ <code>option</code> with the attribute <code>name</code>
to
+ provide the option. Supported option names are "noexec",
"nosuid",
+ "nodev", and "ro".
+ <span class="since">Since 5.1.0</span></dd>
Hmm, this has gone back to the "Mount options" concept from v1
which I don't think is appropriate. In v2 feedback I had suggested
that we should have some more explicit XML description for the subset
of options we wish to support, with each option potentially using a
different syntax, no generic mount options syntax.
For 'ro' we should use our normal <readonly/> empty element syntax.
For nosetuid, nodev, noexec, I'm still inclined to say we should
enable them by default. Despite it being a semantic change, I think
it is the right thing for a volume that is being used for storing
VM disk images.
If we did want it in the XML though, I think it should be via a
<features> element, not <mount_opts>, as the inverse - iow opt-in
to the insecure behaviour
<features>
<allow-suid/>
<allow-devnode/>
<allow-exec/>
</features>
This provides a generic framework that will work for other storage
pool backend features where there's no mount concept involved eg
LVM, iSCSI, etc
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|