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

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Thu, Mar 31, 2016 at 06:07:58AM -0600, Jan Beulich wrote:
> >>> On 31.03.16 at 13:43, <konrad@xxxxxxxxxx> wrote:
> > On Thu, Mar 31, 2016 at 12:30:09AM -0600, Jan Beulich wrote:
> >> >>> On 30.03.16 at 17:43, <JBeulich@xxxxxxxx> wrote:
> >> > Since they're all cosmetic, if you take care of all of them, feel free
> >> > to stick my ack on the result.
> >> 
> >> Actually - no, please don't. While the patch is fine content wise
> >> then from my perspective, I'm still lacking a convincing argument
> >> of why this new hypercall is needed in the first place. If others
> >> are convinced by the argumentation between (mostly, iirc) you
> >> and Andrew, I'm not going to stand in the way, but I'm also not
> >> going to approve of the code addition without being myself
> >> convinced.
> > 
> > Damm. I pushed the patch in yesterday in 'staging'!
> > 
> > We can always revert them..
> > 
> > "Others" being other maintainers I presume?
> 
> Any one of the REST maintainers, yes.

Changing the title to get their attention.
> 
> > The underlaying reason for me doing this is to expose the build-id.
> > 
> > It (build-id) originally was in sysctl, then folks wanted it in XENVER_.
> > Got it working in there as sub-ops, but Andrew last minute decided that

Here is the link to v3 which had it in XENVER_ subops:
http://lists.xen.org/archives/html/xen-devel/2016-02/msg04110.html

And this one v5 (in case folks had deleted this thread, v4 is almost
the same except it had VERSION instead of XEN_VERSION):
http://lists.xen.org/archives/html/xen-devel/2016-03/msg03302.html

> > it should not really be there but in a new hypercall that can do
> > three arguments (the length) and be able to return -EPERM. A sane
> > one, not the cobbled up XENVER one.
> 
> Well, -EPERM is now possible with the old one too. And nothing
> in that existing interface prevents a length to be passed in/out
> for new sub-ops. Nor do I really see anything truly insane with

We cannot expand the hypercall to have three arguments - it MUST
have two (as you had pointed out earlier). The length must be jammed
in the sub-ops:

/* Return value is the number of bytes written, or XEN_Exx on error.
 * Calling with empty parameter returns the size of build_id. */

#define XENVER_build_id 10
struct xen_build_id {
        uint32_t        len; /* IN: size of buf[]. */
#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
        unsigned char   buf[];
#elif defined(__GNUC__)
        unsigned char   buf[1]; /* OUT: Variable length buffer with build_id. */
#endif
};
typedef struct xen_build_id xen_build_id_t;

> that existing interface.
> 
> Jan
> 

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