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

From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Wei Liu <wei.liu2@citrix.com>,
	ross.lagerwall@citrix.com,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	mpohlack@amazon.de, Julien Grall <julien.grall@arm.com>,
	Stefano Stabellini <stefano.stabellini@citrix.com>,
	sasha.levin@oracle.com, xen-devel@lists.xenproject.org,
	Daniel De Graaf <dgdegra@tycho.nsa.gov>,
	Keir Fraser <keir@xen.org>
Subject: REST MAINTAINERS feedback requested Was:Re: [PATCH v5 01/28] HYPERCALL_version_op. New hypercall mirroring XENVER_ but sane.
Date: Thu, 31 Mar 2016 09:28:41 -0400	[thread overview]
Message-ID: <20160331132841.GA20305@localhost.localdomain> (raw)
In-Reply-To: <56FD2F3E02000078000E1BC0@prv-mh.provo.novell.com>

On Thu, Mar 31, 2016 at 06:07:58AM -0600, Jan Beulich wrote:
> >>> On 31.03.16 at 13:43, <konrad@kernel.org> wrote:
> > On Thu, Mar 31, 2016 at 12:30:09AM -0600, Jan Beulich wrote:
> >> >>> On 30.03.16 at 17:43, <JBeulich@suse.com> 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@lists.xen.org
http://lists.xen.org/xen-devel