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

[Dushyant Behl <myselfdushyantbehl@gmail.com>]

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

Please don't apply the patch yet. I have found a very small yet big
mistake with the patch.
I'll send the correct patch along with these things updated correctly.

Thanks and Regards,
Dushyant Behl