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

["Yu, Zhang" <yu.c.zhang@linux.intel.com>]

maintainers,

  Thanks for the discussion and sorry for my delayed reply...

On 4/21/2016 12:52 AM, Jan Beulich wrote:
>>>> George Dunlap <george.dunlap@citrix.com> 04/20/16 6:30 PM >>>
>> On Wed, Apr 20, 2016 at 4:02 PM, George Dunlap <george.dunlap@citrix.com> 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.
>

The 3rd patch in this V2 series do change the semantics of this type.

But IIUC, we have reached an agreement in the discussion of patch 3
that for now, we only support the write emulation, and it shall be no
p2m_ioreq_server entries before an ioreq server claimed its ownership.
So in my V3 series(nearly finished, not sent out yet), this type shall
have the same semantics with the old one.

Besides, I'm sorry, and I do not quite understand what "consider the
currently used enum value burnt" means. Any example? :)

>> 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?
>

B.R.
Yu

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