On 01/16/2019 07:00 AM, Daniel P. Berrangé wrote: > 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@xxxxxxxxxx> >> --- >> 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> > It seems like this proposed XML was dropped for v4 but I just want to chime in: raw boolean <foo/> style options are a pain to deal with for apps. Anything like this should use the tristate <foo value='on|off'/> pattern, otherwise there's no way for XML to distinguish between 'user explicitly requested value=no' vs 'user didn't explicitly request anything' Thanks, Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
