[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


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>



Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.