Re: REST MAINTAINERS feedback requested Was:Re: [PATCH v5 01/28] HYPERCALL_version_op. New hypercall mirroring XENVER_ but sane.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Apr 12, 2016 at 10:58:09AM +0100, George Dunlap wrote:
> On Mon, Apr 11, 2016 at 6:46 PM, Jan Beulich <JBeulich@suse.com> wrote:
> [snip]
> >> ISTM that the lower duplication (which in principle is an advantage
> >> which will be time limited if we are ever able to completely remove
> >> teh old hypercall) comes with the cost of (in the long term) increased
> >> mess in this particular subop.
> >
> > We cannot, ever, remove a not toolstack limited hypercall completely
> > (or if, then only by Kconfig option so that people knowing none of the
> > guests they care about use such deprecated ones).
> 
> We need to keep the hypercall around and functioning for ABI
> compatibility, but we could at least remove it from the public headers
> and point people to using the new one instead.  (Discussion about the
> merit of this idea below.)
> 
> [snip]
> >> ISTM that the lower duplication (which in principle is an advantage
> >> which will be time limited if we are ever able to completely remove
> >> teh old hypercall) comes with the cost of (in the long term) increased
> >> mess in this particular subop.
> >>
> >> Is that right ?  Obviously this cost is not very significant.  But
> >> maybe the duplication isn't that significant either.  I was kind of
> >> expecting to find stronger arguments on both sides in this
> >> discussion...
> >
> > If, btw, the cost of having to read the length argument from guest
> > memory was deemed undesirable, we'd certainly have the option
> > of specifying it to be passed through, say, the high half of the
> > current "cmd" parameter.
> 
> OK, so here are the options I see:
> 
> 1. Use the existing hypercall with the existing call semantics for
> build-id -- i.e., require the caller to have a large but fixed-length
> buffer (maybe 1024 or 2048).
> 
> 2. Use the existing hypercall but wedge in different calling semantics
> for the build-id subop.  We could have just that subop pay attention
> to an extra argument as a length, for example, and return an error if
> the length is too short.  Or we could make essentially a flag that
> asks, "How much space if I were to execute this subop for real?"

You can't wedge in the extra argument. Tried that an Jan pointed out
that we clobber it (specifically we clobber any non-used arguments).
> 
> 3. We could use a new hypercall only for new functionality, with the
> proposed new semantics.  This would at minimum be build-id, but
> probably also extraversion, compileinfo, changeset, maybe
> capabilities_info.
> 
> 4. Have the new hypercall replace the old hypercall.  The new
> hypercall will duplicate all the functionality of the old hypercall.
> Deprecate the hypercall for a release or two, then remove it from the
> public headers (although keep the code, because we need to maintain
> backwards compatibility).

5). Stick the build-id in the xsplice sysctl. Or just in the sysctl.

> 
> Honestly I still don't quite understand what the problem is with #1 --
> if build-id is mainly meant to be a UUID or a hash of the build to
> make sure that you're applying the right hotfix (please correct me if
> I'm wrong here -- I haven't had time to actually follow the patch
> series), 256 bytes should be enough for a properly hashed build, and
> 2048 should be more than enough.  Requiring the caller to have a
> 2048-byte buffer before calling doesn't really seem like that much of
> a hardship to me.  Basically:
> 
> a. It's nicer to have arbitrary-length strings (2-4), but reasonably
> large fixed-length ones aren't awful (1)
> 
> b. It's nicer for hypercall consumers to have a single hypercall with
> consistent semantics (1,4), but having two hypercalls (3) or a single
> one with inconsistent semantics (2) aren't that bad either.
> 
> c. It's nicer for hypervisor maintainers to have a single place to
> support any given bit of functionality (1-3), but having a slightly
> duplicated functionality that only has to be supported in an ABI
> backwards-compatible manner isn't that bad either (4).
> 
> This does seem to me an awful lot like a bike shed. :-)  All of the

This is really past bike shedding - all the bikes shed have been
already built (for all those options).

> options (1-4) seem perfectly fine to me.  FWIW my preferred color
> would probably be 1 because it's the easiest and least inconsistent
> with the current state of things. My least favorite would be 3,
> because although each individual piece of information is only in one
> place, the path to get there is duplicated; both the kernel developer
> and the hypervisor developer are forced to continue to deal with both.
> 


The state is that 1-3 have been Nacked by Andrew, 4 has been
Nacked by Jan. And 5) (the original way) was way way back Nacked
as well.

I believe this conversation is to break a tie-breaker between maintainers.
(See http://www.xenproject.org/governance.html, Refereeing).

That is any of the REST maintainers or Project Leads will override the
Nack/Acks. Granted, Jan, me, and Andrew are excluded as we are most
certainly not neutral. That means Ian, Tim, and Keir.

And then we all can go to the pub.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel