[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH 1/2] spapr: Fix stale HTAB during live migration (
From: |
Samuel Mendoza-Jonas |
Subject: |
Re: [Qemu-ppc] [PATCH 1/2] spapr: Fix stale HTAB during live migration (KVM) |
Date: |
Thu, 06 Nov 2014 08:56:03 +1100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 |
On 05/11/14 18:57, Alexander Graf wrote:
>
>
> On 05.11.14 07:17, Samuel Mendoza-Jonas wrote:
>> If a guest reboots during a running migration, changes to the
>> hash page table are not necessarily updated on the destination.
>> Opening a new file descriptor to the HTAB forces the migration
>> handler to resend the entire table.
>>
>> Signed-off-by: Samuel Mendoza-Jonas <address@hidden>
>> ---
>> hw/ppc/spapr.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>> include/hw/ppc/spapr.h | 2 ++
>> 2 files changed, 49 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 0a2bfe6..1610c28 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -833,6 +833,13 @@ static void spapr_reset_htab(sPAPREnvironment *spapr)
>> /* Kernel handles htab, we don't need to allocate one */
>> spapr->htab_shift = shift;
>> kvmppc_kern_htab = true;
>> +
>> + /* Tell readers to update their file descriptor */
>> + pthread_mutex_lock(&spapr->htab_mutex);
>
> I don't think you can directly use pthread functions in hw/. These files
> could be compiled on Windows which doesn't have pthread. Instead, please
> use the QEMU wrappers from util/qemu-thread-posix.c.
Thanks for catching this!
>
> Or maybe try and find out whether you actually do need the lock. Reboots
> can only happen when triggered via a HCALL which takes the BQL. I don't
> quite know how much the migration code became threaded, but I'd assume
> that at least device migration would happen under the BQL or after
> stopping the VM, but in a consistent place.
>
> So as long as we're guaranteed that the htab_fd_stale variable is set at
> the final "send all device contents" phase, we should automatically
> catch any reset that happened in between - even without a lock, no?
Good point, we only really *need* the flag to be set at least before the
call to save_live_complete(). I'll double check but yes, that should work.
>
>
> Alex
>
--
-----------
LTC Ozlabs
IBM