Re: [Xen-devel] [PATCH v2 2/3] x86/ioreq server: Rename p2m_mmio_write_dm to p2m_ioreq_server

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> George Dunlap <george.dunlap@xxxxxxxxxx> 04/20/16 6:30 PM >>>
>On Wed, Apr 20, 2016 at 4:02 PM, George Dunlap <george.dunlap@xxxxxxxxxx> 
>wrote:
>> On 19/04/16 12:02, Yu, Zhang wrote:
>>> So I suppose the only place we need change for this patch is
>>> for hvmmem_type_t, which should be defined like this?
>>>
>>> typedef enum {
>>>     HVMMEM_ram_rw,             /* Normal read/write guest RAM */
>>>     HVMMEM_ram_ro,             /* Read-only; writes are discarded */
>>>     HVMMEM_mmio_dm,            /* Reads and write go to the device model */
>>> #if __XEN_INTERFACE_VERSION__ >= 0x00040700
>>>     HVMMEM_ioreq_server
>>> #else
>>>     HVMMEM_mmio_write_dm
>>> #endif
>>> } hvmmem_type_t;
>>>
>>> Besides, does 4.7 still accept freeze exception? It would be great
>>> if we can get an approval for this.
>>
>> Wait, do we *actually* need this?  Is anyone actually using this?
>>
>> I'd say remove it, and if anyone complains, *then* do the #ifdef'ery as
>> a bug-fix.  I'm pretty sure that's Linux's policy -- You Must Keep
>> Userspace Working, but you can break it to see if anyone complains first.

We don't normally do it like that - we aim at keeping things compatible
right away. I don't know of a case where we would have knowingly broken
compatibility for users of the public headers (leaving aside tool stack only
stuff of course).

>Going further than this:
>
>The proposed patch series not only changes the name, it changes the
>functionality.  We do not want code to *compile* against 4.7 and then
>not *work* against 4.7; and the worst of all is to compile and sort of
>work but do it incorrectly.

I had the impression that the renaming patch was what it is - a renaming
patch, without altering behavior.

>Does the ioreq server have a way of asking Xen what version of the ABI
>it's providing?  I'm assuming the answer is "no"; in which case code
>that is compiled against the 4.6 interface but run on a 4.8 interface
>that looks like this will fail in a somewhat unpredictable way.

The only thing it can do is ask for the Xen version. The ABI version is not
being returned by anything (but perhaps should be).

>Given that:
>
>1. When we do check the ioreq server functionality in, what's the
>correct way to deal with code that wants to use the old interface, and
>what do we do with code compiled against the old interface but running
>on the new one?

For the full series I'm not sure I can really tell.But as said, for the rename
patch alone I thought it is just a rename. And that's what we want to get
in (see Paul's earlier reply - he wants to see the old name gone, so it won't
be used any further).

>2. What's the best thing to do for this release?

If the entire series (no matter whether to go in now or later) is changing
behavior, then the only choice is to consider the currently used enum
value burnt, and use a fresh one for the new semantics.

>If it's the case that the only code that uses this is in XenServer,
>then I'd say the answer to #1 can be simply, "Don't compile" and
>"Don't do that" respectively; and the answer to #2 can be either
>"Leave it be" or "Remove the enum from the public interface".
>
>If there are other projects that have started to use this interface,
>then we need a better answer to #1 than "Compile but fail in
>unpredicatble ways".

How would we know whether there are other users?

Jan


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