|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 3/4] libxc: stop migration in case of p2m list structural changes
On 11/12/15 16:20, Andrew Cooper wrote:
> On 11/12/15 11:31, Juergen Gross wrote:
>> With support of the virtual mapped linear p2m list for migration it is
>> now possible to detect structural changes of the p2m list which before
>> would either lead to a crashing or otherwise wrong behaving domU.
>>
>> A guest supporting the linear p2m list will increment the
>> p2m_generation counter located in the shared info page before and after
>> each modification of a mapping related to the p2m list. A change of
>> that counter can be detected by the tools and reacted upon.
>>
>> As such a change should occur only very rarely once the domU is up the
>> most simple reaction is to cancel migration in such an event.
>>
>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>> ---
>> tools/libxc/xc_sr_common.h | 11 ++++++++++
>> tools/libxc/xc_sr_save.c | 4 ++++
>> tools/libxc/xc_sr_save_x86_hvm.c | 7 +++++++
>> tools/libxc/xc_sr_save_x86_pv.c | 44
>> ++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 66 insertions(+)
>>
>> diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
>> index 9aecde2..bfb9602 100644
>> --- a/tools/libxc/xc_sr_common.h
>> +++ b/tools/libxc/xc_sr_common.h
>> @@ -83,6 +83,14 @@ struct xc_sr_save_ops
>> int (*end_of_checkpoint)(struct xc_sr_context *ctx);
>>
>> /**
>> + * Check whether new iteration can be started. This is called before
>> each
>> + * iteration to check whether all criteria for the migration are still
>> + * met. If that's not the case either migration is cancelled via a bad
>> rc
>> + * or the situation is handled, e.g. by sending appropriate records.
>> + */
>> + int (*check_iteration)(struct xc_sr_context *ctx);
>> +
>
> This is slightly ambiguous, especially with the not-so-different
> differences between live migration and remus checkpoints.
>
> I would be tempted to name it check_vm_state() and document simply that
> it is called periodically, to allow for fixup (or abort) for guest state
> which may have changed while the VM was running.
Yes, this is better.
> On the remus side, it needs to be called between start_of_checkpoint()
> and send_memory_***() in save(), as the guest gets to run between the
> checkpoints.
Okay.
>
>> + /**
>> * Clean up the local environment. Will be called exactly once, either
>> * after a successful save, or upon encountering an error.
>> */
>> @@ -280,6 +288,9 @@ struct xc_sr_context
>> /* Read-only mapping of guests shared info page */
>> shared_info_any_t *shinfo;
>>
>> + /* p2m generation count for verifying validity of local p2m. */
>> + uint64_t p2m_generation;
>> +
>> union
>> {
>> struct
>> diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
>> index cefcef5..c235706 100644
>> --- a/tools/libxc/xc_sr_save.c
>> +++ b/tools/libxc/xc_sr_save.c
>> @@ -370,6 +370,10 @@ static int send_dirty_pages(struct xc_sr_context *ctx,
>> DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
>> &ctx->save.dirty_bitmap_hbuf);
>>
>> + rc = ctx->save.ops.check_iteration(ctx);
>> + if ( rc )
>> + return rc;
>> +
>
> As there is now a call at each start of checkpoint, this call
> essentially becomes back-to-back. I would suggest having it after the
> batch, rather than ahead.
Okay.
>
>> for ( p = 0, written = 0; p < ctx->save.p2m_size; ++p )
>> {
>> if ( !test_bit(p, dirty_bitmap) )
>> diff --git a/tools/libxc/xc_sr_save_x86_hvm.c
>> b/tools/libxc/xc_sr_save_x86_hvm.c
>> index f3d6cee..aa24f90 100644
>> --- a/tools/libxc/xc_sr_save_x86_hvm.c
>> +++ b/tools/libxc/xc_sr_save_x86_hvm.c
>> @@ -175,6 +175,12 @@ static int x86_hvm_start_of_checkpoint(struct
>> xc_sr_context *ctx)
>> return 0;
>> }
>>
>> +static int x86_hvm_check_iteration(struct xc_sr_context *ctx)
>> +{
>> + /* no-op */
>> + return 0;
>> +}
>> +
>> static int x86_hvm_end_of_checkpoint(struct xc_sr_context *ctx)
>> {
>> int rc;
>> @@ -221,6 +227,7 @@ struct xc_sr_save_ops save_ops_x86_hvm =
>> .start_of_stream = x86_hvm_start_of_stream,
>> .start_of_checkpoint = x86_hvm_start_of_checkpoint,
>> .end_of_checkpoint = x86_hvm_end_of_checkpoint,
>> + .check_iteration = x86_hvm_check_iteration,
>> .cleanup = x86_hvm_cleanup,
>> };
>>
>> diff --git a/tools/libxc/xc_sr_save_x86_pv.c
>> b/tools/libxc/xc_sr_save_x86_pv.c
>> index 0237378..3a58d0d 100644
>> --- a/tools/libxc/xc_sr_save_x86_pv.c
>> +++ b/tools/libxc/xc_sr_save_x86_pv.c
>> @@ -268,6 +268,39 @@ err:
>> }
>>
>> /*
>> + * Get p2m_generation count.
>> + * Returns an error if the generation count has changed since the last call.
>> + */
>> +static int get_p2m_generation(struct xc_sr_context *ctx)
>> +{
>> + uint64_t p2m_generation;
>> + int rc;
>> +
>> + p2m_generation = GET_FIELD(ctx->x86_pv.shinfo, arch.p2m_generation,
>> + ctx->x86_pv.width);
>> +
>> + rc = (p2m_generation == ctx->x86_pv.p2m_generation) ? 0 : -1;
>> + ctx->x86_pv.p2m_generation = p2m_generation;
>> +
>> + return rc;
>> +}
>> +
>> +static int x86_pv_check_iteration_p2m_list(struct xc_sr_context *ctx)
>> +{
>> + xc_interface *xch = ctx->xch;
>> + int rc;
>> +
>> + if ( !ctx->save.live )
>> + return 0;
>> +
>> + rc = get_p2m_generation(ctx);
>> + if ( rc )
>> + ERROR("p2m generation count changed. Migration aborted.");
>> +
>> + return rc;
>> +}
>> +
>> +/*
>> * Map the guest p2m frames specified via a cr3 value, a virtual address,
>> and
>> * the maximum pfn.
>> */
>> @@ -281,6 +314,9 @@ static int map_p2m_list(struct xc_sr_context *ctx,
>> uint64_t p2m_cr3)
>> unsigned fpp, n_pages, level, shift, idx_start, idx_end, idx, saved_idx;
>> int rc = -1;
>>
>> + /* Before each iteration check for local p2m list still valid. */
>> + ctx->save.ops.check_iteration = x86_pv_check_iteration_p2m_list;
>> +
>
> This is admittedly the first, but definitely not the only eventual thing
> needed for check iteration. To avoid clobbering one check with another
> in the future, it would be cleaner to have a single
> x86_pv_check_iteration() which performs the get_p2m_generation() check
> iff linear p2m is in use.
Agreed.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |