Re: [Xen-devel] [PATCH v2 1/2] altp2m: Merge p2m_set_altp2m_mem_access and p2m_set_mem_access

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Fri, 2016-01-29 at 09:47 -0700, Jan Beulich wrote:
> > > > On 29.01.16 at 17:32, <tlengyel@xxxxxxxxxxx> wrote:
> > On Fri, Jan 29, 2016 at 9:19 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> > > > > > On 29.01.16 at 17:12, <tlengyel@xxxxxxxxxxx> wrote:
> > > > On Fri, Jan 29, 2016 at 4:03 AM, Jan Beulich <JBeulich@xxxxxxxx>
> > > > wrote:
> > > > > > > > On 28.01.16 at 21:58, <tlengyel@xxxxxxxxxxx> wrote:
> > > > > > --- a/xen/include/public/memory.h
> > > > > > +++ b/xen/include/public/memory.h
> > > > > > @@ -423,11 +423,14 @@ struct xen_mem_access_op {
> > > > > > ÂÂÂÂÂ/* xenmem_access_t */
> > > > > > ÂÂÂÂÂuint8_t access;
> > > > > > ÂÂÂÂÂdomid_t domid;
> > > > > > +ÂÂÂÂuint16_t altp2m_idx;
> > > > > > +ÂÂÂÂuint16_t _pad;
> > > > > > ÂÂÂÂÂ/*
> > > > > > ÂÂÂÂÂÂ* Number of pages for set op
> > > > > > ÂÂÂÂÂÂ* Ignored on setting default access and other ops
> > > > > > ÂÂÂÂÂÂ*/
> > > > > > ÂÂÂÂÂuint32_t nr;
> > > > > > +ÂÂÂÂuint32_t _pad2;
> > > > > 
> > > > > Repeating what I had said on v1: So this is a tools only
> > > > > interface,
> > > > > yes. But it's not versioned (other than e.g. domctl and sysctl),
> > > > > so
> > > > > altering the interface structure is at least fragile.
> > > > 
> > > > Not sure what I can do to address this.
> > > 
> > > Deprecate the old interface and introduce a new one. But other
> > > maintainers' opinions would be welcome.
> > 
> > That seems like a very heavy handed solution to me.
> 
> I understand that - hence the request for others' opinions.

It's unfortunate that we've found ourselves here, but I think rather than
deprecating the current and adding a new op alongside we should just accept
the one-time fragility this time around, add the version field as part of
this set of changes and try and remember to include a version number for
next time we add a tools only interface. I don't think xenaccess is yet
widely used outside of Tamas and the Bitdfender folks, who I would assume
can cope with such a change.

I could accept changing the op number would make sense, but I don't think
we should deprecate the old one (which implies continuing to support it in
parallel), if we go this route we should just retire the old number to
straight away to return -ENOSYS (or maybe -EACCESS, which is what a version
mismatch would have resulted in).

Ian.


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