[libvirt] [PATCH v2 0/6] Allow adding mountOpts to the storage pool mount command

Wed Jan 9 18:48:59 UTC 2019

On Wed, Jan 09, 2019 at 01:26:19PM -0500, John Ferlan wrote:
> 
> 
> On 1/9/19 12:40 PM, Daniel P. Berrangé wrote:
> > On Wed, Jan 09, 2019 at 12:31:02PM -0500, John Ferlan wrote:
> >>
> >>
> >> On 1/9/19 12:09 PM, Daniel P. Berrangé wrote:
> >>> On Tue, Jan 08, 2019 at 12:52:20PM -0500, John Ferlan wrote:
> >>>> v1: https://www.redhat.com/archives/libvir-list/2018-December/msg00558.html
> >>>>
> >>>> Kept the subject the same, but the concept has been adjusted to follow
> >>>> issues pointed out by jtomko vis-a-vis allowing arbitrary options via XML.
> >>>> This series adds both the NFS and the RBD adjustments that were essentially
> >>>> in the referenced series from review.
> >>>
> >>> Can you give any info about what motivated this addition.
> >>
> >> There's a bz:
> >>
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1584663
> >>
> >> which along the way has had private comments - I probably should add the
> >> bz to at least one of the patches, but I think because of the (probably
> >> unnecessary) private comments in the bz, I was hesitant to do so.
> >>
> >> I tried a different mechanism and Jano pointed out:
> >>
> >> https://www.redhat.com/archives/libvir-list/2018-December/msg00667.html
> >>
> >> that previous attempts were rejected due to the arbitrary text.
> > 
> > Ok, from the rather limited information in the BZ above, it is clear
> > that this feature is required to be production supported, so that
> > rules out use of private XML namespaces as a viable solution.
> > 
> > I agree with Jano's rejected of arbitrary mount option passthrough in
> > v1 too though.
> 
> I understand the rejection and I also found namespaces to not be very
> user or developer friendly. Even less easy to understand were those
> "other' namespaces using "ns0" and/or "ns1". Proverbial rock and hard
> place scenario.
> 
> It's no small wonder why the patches that were rejected in June 2014 had
> no attempted followup that I found - it just didn't seem worth the
> effort and using their own private code to suit their own needs was far
> more simple!
> 
> > 
> > I think the right answer involves multiple approaches in the XML.
> > 
> > For the NFS pool, we should have an explicit way to request the NFS
> > protocol version in the XML.
> 
> You mean mount option "nfsvers=n"?

Yes,  which would be set using something like  

   <source>
     ....
     <protocol ver="4"/>
   </source>

> > For the general nosuid, nodev flags, I think we can do something like we
> > have for the <features/> element in the domain XML.
> > 
> 
> So that means to me that every option currently possible in mount would
> need to be listed.  Doesn't that feel excessive?  Not that mount changes
> that often, but I would think it's awfully painful to take that route.
> Especially for some of the "more complicated" mount options.

The mount command options are a tiny, trivial set compared to QEMU
command line options. The goal here is not to have simple libvirt
code, rather is to have portable & simple application code. Libvirt
exists to provide an abstraction layer over OS specific commands
line "mount" which can have different syntax on each OS. eg FreeBSD
mount command options may differ from Linux mount command options.

I'm not suggesting implementing support for every single mount
option though. Only those that we actually have a compelling
reason to support, just as we've not implemented every single
QEMU arg.

> > Or on second thoughts, the only reason for the storage pools mounts is to
> > provide VM disk images, so we should just unconditionally always set nosuid
> > & nodev when running mount. No storage volume we report should be setuid or
> > be a device node.  If the NFS mount has a directory tree for container
> > filesystems, then the previously discussed  virFileSystem APIs should be
> > actually implemented.
> 
> That'd be the really simple simple fix at least for nosuid and nodev. I
> didn't approach it from the viewpoint of providing VM disk images as it
> seemed to be more "general" provide the ability to pick-n-choose which
> mount option to use. Side bar, I also have this weird recollection about
> usage of rsize=n and/or wsize=n in some previous NFS issue I've chased
> (I think it was at my previous employer).

That was needed in NFSv2, but in modern NFSv3/4 IIUC the server decides
those values.

> There's also mention of noexec in one of the private comments, so while
> nosuid and nodev would be easier, it still doesn't cover everything
> noted in the bz.

noexec would be reasonable to turn on by default too IMHO.

> > We could none the less also still have a private XML namespace for
> > doing arbitrary mount argument passthrough, but that shouldn't be
> > the solution for this particular BZ. It is just a way to get a quick
> > workaround for an option while we decide how to model the desired
> > new mount option explicitly in the XML, as we do for QEMU options.
> > 
> 
> Does it become more work than it's worth though ;-)  How large of a wall
> needs to be built to stop the flow of code ;-)

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