Re: [Xen-devel] [PATCH v2 1/3] [GSOC14] refactored mempaging code from xenpaging to libxc.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2014-06-16 at 23:50 +0530, Dushyant Behl wrote:
> This patch is part of the work done under the gsoc project -
> Lazy Restore Using Memory Paging.

For future patches please can you omit this, it's not really relevant to
the commit history. If you really want to include it please put it below
the --- marker. Or just mention it in the 0/N mail.

> 
> This patch moves the code to initialize and teardown mempaging from xenpaging
> to libxc. The code refractored from xenpaging is the code which sets up 
> paging,

"refactored"


> initializes a shared ring and event channel to communicate with xen. This
> communication is done between the hypervisor and the dom0 tool which performs
> the activity of pager. Another routine is also added by this patch which will
> teardown the ring and mem paging setup. The xenpaging code is changed to use
> the newly created routines and is tested to properly build and work with this 
> code.
> 
> The refractoring is done so that any tool which will act as pager in

"refactoring"

> lazy restore or use memory paging can use a same routines to initialize 
> mempaging.
> This refactoring will also allow any future (in-tree) tools to use mempaging.
> 
> The refractored code in xc_mem_paging_ring_setup is to be compiled into

and again.

I'll fix these up on commit, which I'll do as soon as someone confirms
that...

> +    /* Now that the ring is set, remove it from the guest's physmap */
> +    if ( xc_domain_decrease_reservation_exact(xch, domain_id, 1, 0, 
> &ring_pfn) )

... the lack of a domain pause and/or clearing the ring content in this
function does not represent an issue similar to
http://xenbits.xen.org/xsa/advisory-99.html.

> +    /* Unbind the event channel. */
> +    rc = xc_evtchn_unbind(xce_handle, *port);
> +    if ( rc != 0 )
> +    {
> +        PERROR("Error unbinding event port");
> +    }

We don't in general require {}'s around single line statements.

> +/*
> + * The name is kept BUF_RING_SIZE, because the name RING_SIZE 
> + * collides with the xen shared ring definitions in io/ring.h
> + */

This sort of comment generally belongs in the commit log.

Neither of those last two are a blocker for applying though.

Ian.


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