[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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

For the general nosuid, nodev flags, I think we can do something like we
have for the <features/> element in the domain XML.

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.

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.

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

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

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]