Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv) [and 1 more messages]

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, re 
qemu depriv)"):
> Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, 
> > So the actual hypercall call sites are all in-tree, in libxc.  If the
> > format of the struct used for one of these guest handle slots changes,
> > the same struct definition from the Xen headers is used both in the
> > hypervisor and in libxc, and any incompatibility will be detected.
> 
> Wait, no. The guest handle slots in Jenny's proposal are basically
> typeless:
...
> Each sub-op implies a certain type for each handle slot it actually
> uses.

Yes.  But in practice each such slot will in practice be accessed by
using copy_dm_buffer_from_guest (or whatever) to copy it to a struct.
It is intended that that same struct type will be used by libxc to
populate the buffer.

Now, it is true that the compiler cannot check that the _same type_ is
used in both places.  So, as I say:

   It's true that changes to the semantics of these slots (for example, a
   change of a slot from "array of struct X" to "one struct Y") will not
   be detected by the compiler.

But changes in the contents of the specific struct /will/ be spotted.

For example, consider the struct device_model_op, which in Jennifer's
document is in slot 0.  If someone edits struct device_model_op to
change its contents, both libxc and Xen see the same modified struct.

The problem can only occur if someone accesses a slot with entirely
the wrong struct type, or by the wrong slot number.  In such a
situation at least in new code, the thing probably wouldn't work at
all.

And even if we had some kind of typesafe system, the typesafety still
wouldn't spot uncontrolled ABI changes which would silently break old
callers.  So the need for careful review of DMOP changes is still
present.

So overall I think this typesafety problem is fairly limited.

> And then I disagree with the general view taken here: The issue
> I see is not with caller of the libxc interfaces (it was always clear
> that it would be possible to have type safety at that layer), but
> between libxc as the consumer and Xen as the producer of the
> interface.

I think you have misunderstood me.  I was discussing precisely
typesafety of that interface.


> > But *all* ABI changes to the DMOP interface need to be accompanied by
> > changes to libxc to add compatibility code.  I think it will be fairly
> > straightforward to check, during review, that each DMOP change comes
> > with a corresponding libxc change.
> 
> Well, yes, we can only hope for that. The main risk is that someone
> doesn't update the other side at all, when a change to one side gets
> done which compiles fine at both ends, and perhaps doesn't even
> alter the public header.

As I say I think if people are making uncontrolled changes to this
area, we have ABI stability problems not covered by the guest handle
typesafety system.  If we wanted to sort that would probably need to
make a proper DSL for the ABI.


> > But if you do not agree, then how about hiding the slots with a macro
> > setup ?  Something like:
> >[...]
> 
> Well, that's exactly the kind of workaround I'd like to avoid.

Well, I think it's a lot of effort for not much gain.  But it's
certainly doable and if you think it worthwhile I'm prepared to try
it.


Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, re 
qemu depriv)"):
> On 21.09.16 at 13:28, <george.dunlap@xxxxxxxxxx> wrote:
> > If someone feels strongly enough to Nack the other version, please say
> > so now; otherwise, I think Ian (since it seems like he'll be the one
> > implementing it) should make his best judgement based on the arguments
> > available and begin implementation.
> 
> No, I don't think I'd nack David's / Jenny's version (after all the type
> issue can be taken care off by sufficient discipline of both submitters
> and reviewers),

Thanks.

Personally I think David/Jennifer's version is better than the
previous proposal based on implicit or separate address limits.


>  but I do think that it failed to point out this downside,
> which imo moves the balance between the two proposals.

I'm afraid that I want to complain about this part of your approach to
the thread, which I think is unduly harsh.

It is of course fair to point out a potential downside of the proposal,
which wasn't clearly discussed in the discussion document.  And it is
sensible for us all to consider that potential downside along with the
others - as indeed I do above.

But I don't think it is really fair to criticise *the discussion
document* (which is what Jennifer's email was) on the grounds that it
ommitted to discuss a potential downside which its authors felt was
minor[1].  The purpose of a discussion document is to put forward a
proposal and/or summarise previous discussion.  It is not required to
be either neutral or, indeed, comprehensive - and even so, I found
this one quite comprehensive.

Personally I think Jennifer's email was an excellent contribution [2]
and I would be inclined to use it as an example if anyone were to ask
in future what a constructive design discussion document should look
like.

If there was a plan to *commit* the discussion document to xen.git or
formally publish it somewhere then you might say "please also discuss
this question".  Then it would be worth the discouragement implied bty
criticism, to improve the document.

But since the pros/cons section of this document has now served its
purpose there is only harm in criticising it in this way.  (And as I
say I think the criticism is unfounded.)

In summary: writing up a good summary of the discussion is a very
virtuous thing to do, which we in the Xen community should do more of.

The goal of encouraging people to do that is undermined by complaining
about the contents of such a writeup, rather than by constructively
addressing the actual substantive issues at stake.

Thanks,
Ian.


[1] Full disclosure: Jennifer sent me a preview of the email
beforehand, and I didn't think the typesafety issue was significant
enough to suggest she should mention it.

[2] As indeed I said to Jennifer in private email before she posted
it.

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