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

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"?

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

> 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).

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.

> 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 ;-)