1. Patchew
  2. QEMU
  3. [Qemu-devel] [RFC 00/29] Migration: postcopy failure recovery
  4. View patch

[Qemu-devel] [RFC 27/29] migration: setup ramstate for resume

Peter Xu posted 29 patches 8 years, 6 months ago
There is a newer version of this series
[Qemu-devel] [RFC 27/29] migration: setup ramstate for resume
Posted by Peter Xu 8 years, 6 months ago 
After we updated the dirty bitmaps of ramblocks, we also need to update
the critical fields in RAMState to make sure it is ready for a resume.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/ram.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index c695b13..427bf6e 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1947,6 +1947,31 @@ static int ram_state_init(RAMState **rsp)
     return 0;
 }
 
+static void ram_state_resume_prepare(RAMState *rs)
+{
+    RAMBlock *block;
+    long pages = 0;
+
+    /*
+     * Postcopy is not using xbzrle/compression, so no need for that.
+     * Also, since source are already halted, we don't need to care
+     * about dirty page logging as well.
+     */
+
+    RAMBLOCK_FOREACH(block) {
+        pages += bitmap_count_one(block->bmap,
+                                  block->max_length >> TARGET_PAGE_BITS);
+    }
+
+    /* This may not be aligned with current bitmaps. Recalculate. */
+    rs->migration_dirty_pages = pages;
+
+    rs->last_seen_block = NULL;
+    rs->last_sent_block = NULL;
+    rs->last_page = 0;
+    rs->last_version = ram_list.version;
+}
+
 /*
  * Each of ram_save_setup, ram_save_iterate and ram_save_complete has
  * long-running RCU critical section.  When rcu-reclaims in the code
@@ -2842,8 +2867,16 @@ int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *block)
 static int ram_resume_prepare(MigrationState *s, void *opaque)
 {
     RAMState *rs = *(RAMState **)opaque;
+    int ret;
 
-    return ram_dirty_bitmap_sync_all(s, rs);
+    ret = ram_dirty_bitmap_sync_all(s, rs);
+    if (ret) {
+        return ret;
+    }
+
+    ram_state_resume_prepare(rs);
+
+    return 0;
 }
 
 static SaveVMHandlers savevm_ram_handlers = {
-- 
2.7.4
Re: [Qemu-devel] [RFC 27/29] migration: setup ramstate for resume
Posted by Dr. David Alan Gilbert 8 years, 6 months ago 
* Peter Xu (peterx@redhat.com) wrote:
> After we updated the dirty bitmaps of ramblocks, we also need to update
> the critical fields in RAMState to make sure it is ready for a resume.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/ram.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index c695b13..427bf6e 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1947,6 +1947,31 @@ static int ram_state_init(RAMState **rsp)
>      return 0;
>  }
>  
> +static void ram_state_resume_prepare(RAMState *rs)
> +{
> +    RAMBlock *block;
> +    long pages = 0;
> +
> +    /*
> +     * Postcopy is not using xbzrle/compression, so no need for that.
> +     * Also, since source are already halted, we don't need to care
> +     * about dirty page logging as well.
> +     */
> +
> +    RAMBLOCK_FOREACH(block) {
> +        pages += bitmap_count_one(block->bmap,
> +                                  block->max_length >> TARGET_PAGE_BITS);

Again I think that needs to be block->used_length (see
migration_bitmap_sync).

> +    }
> +
> +    /* This may not be aligned with current bitmaps. Recalculate. */
> +    rs->migration_dirty_pages = pages;
> +
> +    rs->last_seen_block = NULL;
> +    rs->last_sent_block = NULL;
> +    rs->last_page = 0;
> +    rs->last_version = ram_list.version;

A trace at this point with the pages count might be worthwhile.

> +}
> +
>  /*
>   * Each of ram_save_setup, ram_save_iterate and ram_save_complete has
>   * long-running RCU critical section.  When rcu-reclaims in the code
> @@ -2842,8 +2867,16 @@ int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *block)
>  static int ram_resume_prepare(MigrationState *s, void *opaque)
>  {
>      RAMState *rs = *(RAMState **)opaque;
> +    int ret;
>  
> -    return ram_dirty_bitmap_sync_all(s, rs);
> +    ret = ram_dirty_bitmap_sync_all(s, rs);

Interesting; I'd assumed you'd load directly into this
bitmap rather than loading into the bitmap on each block.
Do we ever get the case where a bitmap is set on the source
bitmap but not in the loaded bitmap?

Dave

> +    if (ret) {
> +        return ret;
> +    }
> +
> +    ram_state_resume_prepare(rs);
> +
> +    return 0;
>  }
>  
>  static SaveVMHandlers savevm_ram_handlers = {
> -- 
> 2.7.4
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Re: [Qemu-devel] [RFC 27/29] migration: setup ramstate for resume
Posted by Peter Xu 8 years, 6 months ago 
On Thu, Aug 03, 2017 at 01:37:04PM +0100, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
> > After we updated the dirty bitmaps of ramblocks, we also need to update
> > the critical fields in RAMState to make sure it is ready for a resume.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  migration/ram.c | 35 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> > 
> > diff --git a/migration/ram.c b/migration/ram.c
> > index c695b13..427bf6e 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -1947,6 +1947,31 @@ static int ram_state_init(RAMState **rsp)
> >      return 0;
> >  }
> >  
> > +static void ram_state_resume_prepare(RAMState *rs)
> > +{
> > +    RAMBlock *block;
> > +    long pages = 0;
> > +
> > +    /*
> > +     * Postcopy is not using xbzrle/compression, so no need for that.
> > +     * Also, since source are already halted, we don't need to care
> > +     * about dirty page logging as well.
> > +     */
> > +
> > +    RAMBLOCK_FOREACH(block) {
> > +        pages += bitmap_count_one(block->bmap,
> > +                                  block->max_length >> TARGET_PAGE_BITS);
> 
> Again I think that needs to be block->used_length (see
> migration_bitmap_sync).

Fixing.

> 
> > +    }
> > +
> > +    /* This may not be aligned with current bitmaps. Recalculate. */
> > +    rs->migration_dirty_pages = pages;
> > +
> > +    rs->last_seen_block = NULL;
> > +    rs->last_sent_block = NULL;
> > +    rs->last_page = 0;
> > +    rs->last_version = ram_list.version;
> 
> A trace at this point with the pages count might be worthwhile.

Added.

> 
> > +}
> > +
> >  /*
> >   * Each of ram_save_setup, ram_save_iterate and ram_save_complete has
> >   * long-running RCU critical section.  When rcu-reclaims in the code
> > @@ -2842,8 +2867,16 @@ int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *block)
> >  static int ram_resume_prepare(MigrationState *s, void *opaque)
> >  {
> >      RAMState *rs = *(RAMState **)opaque;
> > +    int ret;
> >  
> > -    return ram_dirty_bitmap_sync_all(s, rs);
> > +    ret = ram_dirty_bitmap_sync_all(s, rs);
> 
> Interesting; I'd assumed you'd load directly into this
> bitmap rather than loading into the bitmap on each block.
> Do we ever get the case where a bitmap is set on the source
> bitmap but not in the loaded bitmap?

(confirmed with Dave offlist that this is not a problem, blame myself
 on using a poor function name "ram_dirty_bitmap_sync_all" which is
 too close to existng "migration_bitmap_sync")

-- 
Peter Xu

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.