Re: [Xen-devel] [RFC PATCH V2 1/8] xen/mem_event: Cleanup of mem_event structures

[Tim Deegan <tim@xxxxxxx>]

Hi,

At 16:17 +0100 on 18 Jan (1421594274), Tamas K Lengyel wrote:
> From: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> 
> The public mem_event structures used to communicate with helper applications 
> via
> shared rings have been used in different settings. However, the variable names
> within this structure have not reflected this fact, resulting in the reuse of
> variables to mean different things under different scenarios.
> 
> This patch remedies the issue by clearly defining the structure members based 
> on
> the actual context within which the structure is used.
> 
> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>

This looks like a nice improvement.  If everyone who commented about
making ABI changes is happy with it, I am too.

It would be nice if you could add some comments to the new struct
definitions saying what the various fields mean (e.g. when the event
triggers and what the fields will contain).

One nit in the patch itself:

> @@ -592,13 +592,12 @@ int main(int argc, char *argv[])
>                  }
>  
>  
> -                rsp.gfn = req.gfn;
> -                rsp.p2mt = req.p2mt;
> +                rsp.mem_access_event.gfn = req.mem_access_event.gfn;

You're dropping a p2mt update here.  Is that an oversight?
With the obvious equivalent update inserted,

Acked-by: Tim Deegan <tim@xxxxxxx>

Cheers,

Tim.

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