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

[George Dunlap <dunlapg@xxxxxxxxx>]

On Mon, Apr 11, 2016 at 6:46 PM, Jan Beulich <JBeulich@xxxxxxxx> 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?"

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

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

 -George

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