Re: [Xen-devel] [PATCH RFC 0/7] xen: Clean-up of mem_event subsystem

[Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>]

On Wed, Nov 12, 2014 at 4:48 PM, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:

On 12/11/14 15:31, Tamas K Lengyel wrote:
> This patch series aims to clean up the mem_event subsystem within Xen. The
> original use-case for this system was to allow external helper applications
> running in privileged domains to control various memory operations performed
> by Xen. Amongs these were paging, sharing and access control. The subsystem
> has since been extended to also deliver non-memory related events, namely
> various HVM debugging events (INT3, MTF, MOV-TO-CR). The structures and naming
> of related functions however has not caught up to these new use-cases, thus
> leaving many ambigouities in the code.
>
> In this series we convert the mem_event structures to a union of sub-structures
> which clearly define the scope of information that is transmitted via the event
> delivery mechanism. Afterwards, we clean up the naming of the structures and
> related functions to more clearly be in line with their actual operations.
>
> This PATCH RFC series is also available at:
> https://github.com/tklengyel/xen/tree/mem_event_cleanup
>

<snip>

>  xen/include/public/domctl.h         |  44 +--
>  xen/include/public/hvm/params.h     |   2 +-
>  xen/include/public/mem_event.h      | 134 -------
>  xen/include/public/memory.h         |   6 +-
>  xen/include/public/vm_event.h       | 179 +++++++++

While in principle I think this series is a very good thing, there is a
problem with editing the pubic header files.

The contents of mem_event.h is not currently hidden behind #ifdef
__XEN_TOOLS__

As a result, it is strictly speaking part of the VM-visible public
API/ABI and not permitted to change in a backwards incompatible manor.

Having said that, it is currently only usable by privileged domains, so
there is an argument to be made for declaring that it should have been
hidden behind __XEN_TOOLS__ in the first place, making it permittable to
change.

~Andrew

I agree, I think it's safe to say most users of mem_event.h already made use of it in conjuction with xenctrl.h which is already behind __XEN_TOOLS__. Going forward we should probably have this header behind __XEN_TOOLS__ as well just to be explicit.

Tamas

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