Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] spapr: Fix stale HTAB during live migration (KVM)

[Alexander Graf]

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.

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?


Alex