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

[John Ferlan]

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

John

>>
>>>
>>> Anything that is implemented via a separate XML namespace is
>>> generally considered
>>>
>>>    "unsupported, you're on your own when it breaks"
>>>
>>> so if this feature is neeeded for any management application like
>>> oVirt, OpenStack, etc, then using a custom namespace is not going
>>> to be a suitable approach.
>>
>> And I don't think it would be palatable to add N 'text' options to the
>> XML that map to the same N 'text' options in the mount command. So this
>> was the next best option as far as I saw it. That'd be one of those
>> never ending tasks to add the favorite option.
> 
> Regards,
> Daniel
>