Re: [Xen-devel] [RFC Patch v2 03/17] don't zero out ioreq page

[Wen Congyang <wency@xxxxxxxxxxxxxx>]

At 08/08/2014 05:08 PM, Andrew Cooper Write:
> On 08/08/14 09:19, Wen Congyang wrote:
>> ioreq page may contain some pending I/O requests, and we need to
>> handle the pending I/O req after migration.
>>
>> TODO:
>> 1. update qemu to handle the pending I/O req
> 
> Your very reasoning for introducing this patch proves you have a race
> condition between sending this page and sending the qemu state where an
> ioreq might get dropped or processed twice.
> 
> As I said before, you are now doing things with the live migration code
> which it was never intended/designed for.  I have learn't the hard way
> that there are many subtle corner cases (most of which lead to VM
> corruption), and you need to be able to prove that your new changes are
> safe.

In theories, live migration code also can trigger this problem. But the
probability is very low.

ioreq.state is change like this:
STATE_IOREQ_NONE ===> STATE_IOREQ_READY ===> STATE_IOREQ_INPROCESS ===> 
STATE_IORESP_READY ===> STATE_IOREQ_NONE....

We suspend the vm in the following steps:
1. stop vcpu
2. stop qemu

So after vm is suspended, ioreq.state can be STATE_IOREQ_NONE or 
STATE_IORESP_READY.
In most cases, ioreq.state is STATE_IOREQ_NONE. But if it is STATE_IORESP_READY,
we have no chance to handle the response if we clear the page.

Before resuming the vm after migration, the hypervisor only check ioreq.state.
If it is STATE_IOREQ_NONE, do nothing. If it is STATE_IORSP_READY, the 
hypervisor
will handle the I/O response. The source vm's qemu puts the I/O result to ioreq.
The ioreq page is in vm's memory, and will be sent to target host. So after 
migration,
ioreq contains correct I/O response. So hypervisor can handle it correctly.

Thanks
Wen Congyang

> 
> ~Andrew
> 
>>
>> Signed-off-by: Wen Congyang <wency@xxxxxxxxxxxxxx>
>> Cc: Paul Durrant <paul.durrant@xxxxxxxxxx>
>> ---
>>  tools/libxc/xc_domain_restore.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/tools/libxc/xc_domain_restore.c 
>> b/tools/libxc/xc_domain_restore.c
>> index 42abb22..2d6139c 100644
>> --- a/tools/libxc/xc_domain_restore.c
>> +++ b/tools/libxc/xc_domain_restore.c
>> @@ -2301,9 +2301,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, 
>> uint32_t dom,
>>      }
>>  
>>      /* These comms pages need to be zeroed at the start of day */
>> -    if ( xc_clear_domain_page(xch, dom, tailbuf.u.hvm.magicpfns[0]) ||
>> -         xc_clear_domain_page(xch, dom, tailbuf.u.hvm.magicpfns[1]) ||
>> -         xc_clear_domain_page(xch, dom, tailbuf.u.hvm.magicpfns[2]) )
>> +    if ( xc_clear_domain_page(xch, dom, tailbuf.u.hvm.magicpfns[2]) )
>>      {
>>          PERROR("error zeroing magic pages");
>>          goto out;
> 
> .
> 


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