qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] Fixups for non-migratable RAMBlocks
@ 2018-06-05 16:25 Dr. David Alan Gilbert (git)
  2018-06-05 16:25 ` [Qemu-devel] [PATCH 1/2] migration: Fixes " Dr. David Alan Gilbert (git)
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2018-06-05 16:25 UTC (permalink / raw)
  To: qemu-devel, peterx, clg

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Hi,
  Cédric's b895de50 got merged at about the same
time as some of Peter's changes and there's a rdma
case as well; so tidy up those cases that were looping
over all (rather than just migratable) RAMBlocks
and poison it so we don't end up doing the same thing
again.

Dave

Dr. David Alan Gilbert (2):
  migration: Fixes for non-migratable RAMBlocks
  migration: Poison ramblock loops in migration

 include/exec/ramlist.h | 4 +++-
 migration/migration.h  | 3 +++
 migration/ram.c        | 8 +++++---
 migration/rdma.c       | 2 +-
 4 files changed, 12 insertions(+), 5 deletions(-)

-- 
2.17.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 1/2] migration: Fixes for non-migratable RAMBlocks
  2018-06-05 16:25 [Qemu-devel] [PATCH 0/2] Fixups for non-migratable RAMBlocks Dr. David Alan Gilbert (git)
@ 2018-06-05 16:25 ` Dr. David Alan Gilbert (git)
  2018-06-06  3:38   ` Peter Xu
  2018-06-06 18:04   ` Cédric Le Goater
  2018-06-05 16:25 ` [Qemu-devel] [PATCH 2/2] migration: Poison ramblock loops in migration Dr. David Alan Gilbert (git)
  2018-06-15 11:21 ` [Qemu-devel] [PATCH 0/2] Fixups for non-migratable RAMBlocks Dr. David Alan Gilbert
  2 siblings, 2 replies; 10+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2018-06-05 16:25 UTC (permalink / raw)
  To: qemu-devel, peterx, clg

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

There are still a few cases where migration code is using the macros
and functions that do all RAMBlocks rather than just the migratable
blocks; fix those up.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/ram.c  | 4 ++--
 migration/rdma.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index a500015a2f..a7807cea84 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2516,7 +2516,7 @@ static void ram_state_resume_prepare(RAMState *rs, QEMUFile *out)
      * about dirty page logging as well.
      */
 
-    RAMBLOCK_FOREACH(block) {
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
         pages += bitmap_count_one(block->bmap,
                                   block->used_length >> TARGET_PAGE_BITS);
     }
@@ -3431,7 +3431,7 @@ static int ram_dirty_bitmap_sync_all(MigrationState *s, RAMState *rs)
 
     trace_ram_dirty_bitmap_sync_start();
 
-    RAMBLOCK_FOREACH(block) {
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
         qemu_savevm_send_recv_bitmap(file, block->idstr);
         trace_ram_dirty_bitmap_request(block->idstr);
         ramblock_count++;
diff --git a/migration/rdma.c b/migration/rdma.c
index 05aee3d591..8bd7159059 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -635,7 +635,7 @@ static int qemu_rdma_init_ram_blocks(RDMAContext *rdma)
 
     assert(rdma->blockmap == NULL);
     memset(local, 0, sizeof *local);
-    qemu_ram_foreach_block(qemu_rdma_init_one_block, rdma);
+    qemu_ram_foreach_migratable_block(qemu_rdma_init_one_block, rdma);
     trace_qemu_rdma_init_ram_blocks(local->nb_blocks);
     rdma->dest_blocks = g_new0(RDMADestBlock,
                                rdma->local_ram_blocks.nb_blocks);
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH 2/2] migration: Poison ramblock loops in migration
  2018-06-05 16:25 [Qemu-devel] [PATCH 0/2] Fixups for non-migratable RAMBlocks Dr. David Alan Gilbert (git)
  2018-06-05 16:25 ` [Qemu-devel] [PATCH 1/2] migration: Fixes " Dr. David Alan Gilbert (git)
@ 2018-06-05 16:25 ` Dr. David Alan Gilbert (git)
       [not found]   ` <20180606033600.GA2606@xz-mi>
  2018-06-06 18:05   ` Cédric Le Goater
  2018-06-15 11:21 ` [Qemu-devel] [PATCH 0/2] Fixups for non-migratable RAMBlocks Dr. David Alan Gilbert
  2 siblings, 2 replies; 10+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2018-06-05 16:25 UTC (permalink / raw)
  To: qemu-devel, peterx, clg

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The migration code should be using the
  RAMBLOCK_FOREACH_MIGRATABLE and qemu_ram_foreach_block_migratable
not the all-block versions;  poison them so that we can't accidentally
use them.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/exec/ramlist.h | 4 +++-
 migration/migration.h  | 3 +++
 migration/ram.c        | 4 +++-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
index 2e2ac6cb99..bc4faa1b00 100644
--- a/include/exec/ramlist.h
+++ b/include/exec/ramlist.h
@@ -56,8 +56,10 @@ typedef struct RAMList {
 extern RAMList ram_list;
 
 /* Should be holding either ram_list.mutex, or the RCU lock. */
-#define  RAMBLOCK_FOREACH(block)  \
+#define  INTERNAL_RAMBLOCK_FOREACH(block)  \
     QLIST_FOREACH_RCU(block, &ram_list.blocks, next)
+/* Never use the INTERNAL_ version except for defining other macros */
+#define RAMBLOCK_FOREACH(block) INTERNAL_RAMBLOCK_FOREACH(block)
 
 void qemu_mutex_lock_ramlist(void);
 void qemu_mutex_unlock_ramlist(void);
diff --git a/migration/migration.h b/migration/migration.h
index 5af57d616c..31d3ed12dc 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -284,4 +284,7 @@ void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value);
 void dirty_bitmap_mig_before_vm_start(void);
 void init_dirty_bitmap_incoming_migration(void);
 
+#define qemu_ram_foreach_block \
+  #warning "Use qemu_ram_foreach_block_migratable in migration code"
+
 #endif
diff --git a/migration/ram.c b/migration/ram.c
index a7807cea84..e0d19305ee 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -159,9 +159,11 @@ out:
 
 /* Should be holding either ram_list.mutex, or the RCU lock. */
 #define RAMBLOCK_FOREACH_MIGRATABLE(block)             \
-    RAMBLOCK_FOREACH(block)                            \
+    INTERNAL_RAMBLOCK_FOREACH(block)                   \
         if (!qemu_ram_is_migratable(block)) {} else
 
+#undef RAMBLOCK_FOREACH
+
 static void ramblock_recv_map_init(void)
 {
     RAMBlock *rb;
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] migration: Poison ramblock loops in migration
       [not found]   ` <20180606033600.GA2606@xz-mi>
@ 2018-06-06  3:37     ` Peter Xu
  2018-06-06  8:35       ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Xu @ 2018-06-06  3:37 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, peterx, clg

On Wed, Jun 06, 2018 at 11:36:00AM +0800, Peter Xu wrote:
> On Tue, Jun 05, 2018 at 05:25:45PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > The migration code should be using the
> >   RAMBLOCK_FOREACH_MIGRATABLE and qemu_ram_foreach_block_migratable
> > not the all-block versions;  poison them so that we can't accidentally
> > use them.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  include/exec/ramlist.h | 4 +++-
> >  migration/migration.h  | 3 +++
> >  migration/ram.c        | 4 +++-
> >  3 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
> > index 2e2ac6cb99..bc4faa1b00 100644
> > --- a/include/exec/ramlist.h
> > +++ b/include/exec/ramlist.h
> > @@ -56,8 +56,10 @@ typedef struct RAMList {
> >  extern RAMList ram_list;
> >  
> >  /* Should be holding either ram_list.mutex, or the RCU lock. */
> > -#define  RAMBLOCK_FOREACH(block)  \
> > +#define  INTERNAL_RAMBLOCK_FOREACH(block)  \
> >      QLIST_FOREACH_RCU(block, &ram_list.blocks, next)
> > +/* Never use the INTERNAL_ version except for defining other macros */
> > +#define RAMBLOCK_FOREACH(block) INTERNAL_RAMBLOCK_FOREACH(block)
> >  
> >  void qemu_mutex_lock_ramlist(void);
> >  void qemu_mutex_unlock_ramlist(void);
> > diff --git a/migration/migration.h b/migration/migration.h
> > index 5af57d616c..31d3ed12dc 100644
> > --- a/migration/migration.h
> > +++ b/migration/migration.h
> > @@ -284,4 +284,7 @@ void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value);
> >  void dirty_bitmap_mig_before_vm_start(void);
> >  void init_dirty_bitmap_incoming_migration(void);
> >  
> > +#define qemu_ram_foreach_block \
> > +  #warning "Use qemu_ram_foreach_block_migratable in migration code"
> > +
> >  #endif
> > diff --git a/migration/ram.c b/migration/ram.c
> > index a7807cea84..e0d19305ee 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -159,9 +159,11 @@ out:
> >  
> >  /* Should be holding either ram_list.mutex, or the RCU lock. */
> >  #define RAMBLOCK_FOREACH_MIGRATABLE(block)             \
> > -    RAMBLOCK_FOREACH(block)                            \
> > +    INTERNAL_RAMBLOCK_FOREACH(block)                   \
> >          if (!qemu_ram_is_migratable(block)) {} else
> >  
> > +#undef RAMBLOCK_FOREACH
> 
> This will only cover the ram.c file.  How about we move
> RAMBLOCK_FOREACH_MIGRATABLE() directly into migration.h then undef
> RAMBLOCK_FOREACH there?  Then since migration.h should be used in all
> the migration internal source files so the poisoned bit can be used in
> a broader aspect?

(I forgot to reply-all.... forwarding to the list)

> 
> Thanks,
> 
> > +
> >  static void ramblock_recv_map_init(void)
> >  {
> >      RAMBlock *rb;
> > -- 
> > 2.17.0
> > 
> 
> -- 
> Peter Xu

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] migration: Fixes for non-migratable RAMBlocks
  2018-06-05 16:25 ` [Qemu-devel] [PATCH 1/2] migration: Fixes " Dr. David Alan Gilbert (git)
@ 2018-06-06  3:38   ` Peter Xu
  2018-06-06 18:04   ` Cédric Le Goater
  1 sibling, 0 replies; 10+ messages in thread
From: Peter Xu @ 2018-06-06  3:38 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, clg

On Tue, Jun 05, 2018 at 05:25:44PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> There are still a few cases where migration code is using the macros
> and functions that do all RAMBlocks rather than just the migratable
> blocks; fix those up.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Indeed...

Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks,

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] migration: Poison ramblock loops in migration
  2018-06-06  3:37     ` Peter Xu
@ 2018-06-06  8:35       ` Dr. David Alan Gilbert
  2018-06-06  9:09         ` Peter Xu
  0 siblings, 1 reply; 10+ messages in thread
From: Dr. David Alan Gilbert @ 2018-06-06  8:35 UTC (permalink / raw)
  To: Peter Xu; +Cc: qemu-devel, clg

* Peter Xu (peterx@redhat.com) wrote:
> On Wed, Jun 06, 2018 at 11:36:00AM +0800, Peter Xu wrote:
> > On Tue, Jun 05, 2018 at 05:25:45PM +0100, Dr. David Alan Gilbert (git) wrote:
> > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > 
> > > The migration code should be using the
> > >   RAMBLOCK_FOREACH_MIGRATABLE and qemu_ram_foreach_block_migratable
> > > not the all-block versions;  poison them so that we can't accidentally
> > > use them.
> > > 
> > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > ---
> > >  include/exec/ramlist.h | 4 +++-
> > >  migration/migration.h  | 3 +++
> > >  migration/ram.c        | 4 +++-
> > >  3 files changed, 9 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
> > > index 2e2ac6cb99..bc4faa1b00 100644
> > > --- a/include/exec/ramlist.h
> > > +++ b/include/exec/ramlist.h
> > > @@ -56,8 +56,10 @@ typedef struct RAMList {
> > >  extern RAMList ram_list;
> > >  
> > >  /* Should be holding either ram_list.mutex, or the RCU lock. */
> > > -#define  RAMBLOCK_FOREACH(block)  \
> > > +#define  INTERNAL_RAMBLOCK_FOREACH(block)  \
> > >      QLIST_FOREACH_RCU(block, &ram_list.blocks, next)
> > > +/* Never use the INTERNAL_ version except for defining other macros */
> > > +#define RAMBLOCK_FOREACH(block) INTERNAL_RAMBLOCK_FOREACH(block)
> > >  
> > >  void qemu_mutex_lock_ramlist(void);
> > >  void qemu_mutex_unlock_ramlist(void);
> > > diff --git a/migration/migration.h b/migration/migration.h
> > > index 5af57d616c..31d3ed12dc 100644
> > > --- a/migration/migration.h
> > > +++ b/migration/migration.h
> > > @@ -284,4 +284,7 @@ void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value);
> > >  void dirty_bitmap_mig_before_vm_start(void);
> > >  void init_dirty_bitmap_incoming_migration(void);
> > >  
> > > +#define qemu_ram_foreach_block \
> > > +  #warning "Use qemu_ram_foreach_block_migratable in migration code"
> > > +
> > >  #endif
> > > diff --git a/migration/ram.c b/migration/ram.c
> > > index a7807cea84..e0d19305ee 100644
> > > --- a/migration/ram.c
> > > +++ b/migration/ram.c
> > > @@ -159,9 +159,11 @@ out:
> > >  
> > >  /* Should be holding either ram_list.mutex, or the RCU lock. */
> > >  #define RAMBLOCK_FOREACH_MIGRATABLE(block)             \
> > > -    RAMBLOCK_FOREACH(block)                            \
> > > +    INTERNAL_RAMBLOCK_FOREACH(block)                   \
> > >          if (!qemu_ram_is_migratable(block)) {} else
> > >  
> > > +#undef RAMBLOCK_FOREACH
> > 
> > This will only cover the ram.c file.  How about we move
> > RAMBLOCK_FOREACH_MIGRATABLE() directly into migration.h then undef
> > RAMBLOCK_FOREACH there?  Then since migration.h should be used in all
> > the migration internal source files so the poisoned bit can be used in
> > a broader aspect?
> 
> (I forgot to reply-all.... forwarding to the list)

Yes, we could; although it's only the ram.c that's compiled
target specific and can include the header that allows use of those macros anyway.

Dave

> > 
> > Thanks,
> > 
> > > +
> > >  static void ramblock_recv_map_init(void)
> > >  {
> > >      RAMBlock *rb;
> > > -- 
> > > 2.17.0
> > > 
> > 
> > -- 
> > Peter Xu
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] migration: Poison ramblock loops in migration
  2018-06-06  8:35       ` Dr. David Alan Gilbert
@ 2018-06-06  9:09         ` Peter Xu
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Xu @ 2018-06-06  9:09 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: qemu-devel, clg

On Wed, Jun 06, 2018 at 09:35:24AM +0100, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
> > On Wed, Jun 06, 2018 at 11:36:00AM +0800, Peter Xu wrote:
> > > On Tue, Jun 05, 2018 at 05:25:45PM +0100, Dr. David Alan Gilbert (git) wrote:
> > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > > 
> > > > The migration code should be using the
> > > >   RAMBLOCK_FOREACH_MIGRATABLE and qemu_ram_foreach_block_migratable
> > > > not the all-block versions;  poison them so that we can't accidentally
> > > > use them.
> > > > 
> > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > > ---
> > > >  include/exec/ramlist.h | 4 +++-
> > > >  migration/migration.h  | 3 +++
> > > >  migration/ram.c        | 4 +++-
> > > >  3 files changed, 9 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
> > > > index 2e2ac6cb99..bc4faa1b00 100644
> > > > --- a/include/exec/ramlist.h
> > > > +++ b/include/exec/ramlist.h
> > > > @@ -56,8 +56,10 @@ typedef struct RAMList {
> > > >  extern RAMList ram_list;
> > > >  
> > > >  /* Should be holding either ram_list.mutex, or the RCU lock. */
> > > > -#define  RAMBLOCK_FOREACH(block)  \
> > > > +#define  INTERNAL_RAMBLOCK_FOREACH(block)  \
> > > >      QLIST_FOREACH_RCU(block, &ram_list.blocks, next)
> > > > +/* Never use the INTERNAL_ version except for defining other macros */
> > > > +#define RAMBLOCK_FOREACH(block) INTERNAL_RAMBLOCK_FOREACH(block)
> > > >  
> > > >  void qemu_mutex_lock_ramlist(void);
> > > >  void qemu_mutex_unlock_ramlist(void);
> > > > diff --git a/migration/migration.h b/migration/migration.h
> > > > index 5af57d616c..31d3ed12dc 100644
> > > > --- a/migration/migration.h
> > > > +++ b/migration/migration.h
> > > > @@ -284,4 +284,7 @@ void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value);
> > > >  void dirty_bitmap_mig_before_vm_start(void);
> > > >  void init_dirty_bitmap_incoming_migration(void);
> > > >  
> > > > +#define qemu_ram_foreach_block \
> > > > +  #warning "Use qemu_ram_foreach_block_migratable in migration code"
> > > > +
> > > >  #endif
> > > > diff --git a/migration/ram.c b/migration/ram.c
> > > > index a7807cea84..e0d19305ee 100644
> > > > --- a/migration/ram.c
> > > > +++ b/migration/ram.c
> > > > @@ -159,9 +159,11 @@ out:
> > > >  
> > > >  /* Should be holding either ram_list.mutex, or the RCU lock. */
> > > >  #define RAMBLOCK_FOREACH_MIGRATABLE(block)             \
> > > > -    RAMBLOCK_FOREACH(block)                            \
> > > > +    INTERNAL_RAMBLOCK_FOREACH(block)                   \
> > > >          if (!qemu_ram_is_migratable(block)) {} else
> > > >  
> > > > +#undef RAMBLOCK_FOREACH
> > > 
> > > This will only cover the ram.c file.  How about we move
> > > RAMBLOCK_FOREACH_MIGRATABLE() directly into migration.h then undef
> > > RAMBLOCK_FOREACH there?  Then since migration.h should be used in all
> > > the migration internal source files so the poisoned bit can be used in
> > > a broader aspect?
> > 
> > (I forgot to reply-all.... forwarding to the list)
> 
> Yes, we could; although it's only the ram.c that's compiled
> target specific and can include the header that allows use of those macros anyway.

Yes, ram.c seems to be the only user.  Then I'm fine with the patch:

Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks,

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] migration: Fixes for non-migratable RAMBlocks
  2018-06-05 16:25 ` [Qemu-devel] [PATCH 1/2] migration: Fixes " Dr. David Alan Gilbert (git)
  2018-06-06  3:38   ` Peter Xu
@ 2018-06-06 18:04   ` Cédric Le Goater
  1 sibling, 0 replies; 10+ messages in thread
From: Cédric Le Goater @ 2018-06-06 18:04 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git), qemu-devel, peterx

On 06/05/2018 06:25 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> There are still a few cases where migration code is using the macros
> and functions that do all RAMBlocks rather than just the migratable
> blocks; fix those up.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org> 

Thanks,

C.

> ---
>  migration/ram.c  | 4 ++--
>  migration/rdma.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index a500015a2f..a7807cea84 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2516,7 +2516,7 @@ static void ram_state_resume_prepare(RAMState *rs, QEMUFile *out)
>       * about dirty page logging as well.
>       */
>  
> -    RAMBLOCK_FOREACH(block) {
> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
>          pages += bitmap_count_one(block->bmap,
>                                    block->used_length >> TARGET_PAGE_BITS);
>      }
> @@ -3431,7 +3431,7 @@ static int ram_dirty_bitmap_sync_all(MigrationState *s, RAMState *rs)
>  
>      trace_ram_dirty_bitmap_sync_start();
>  
> -    RAMBLOCK_FOREACH(block) {
> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
>          qemu_savevm_send_recv_bitmap(file, block->idstr);
>          trace_ram_dirty_bitmap_request(block->idstr);
>          ramblock_count++;
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 05aee3d591..8bd7159059 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -635,7 +635,7 @@ static int qemu_rdma_init_ram_blocks(RDMAContext *rdma)
>  
>      assert(rdma->blockmap == NULL);
>      memset(local, 0, sizeof *local);
> -    qemu_ram_foreach_block(qemu_rdma_init_one_block, rdma);
> +    qemu_ram_foreach_migratable_block(qemu_rdma_init_one_block, rdma);
>      trace_qemu_rdma_init_ram_blocks(local->nb_blocks);
>      rdma->dest_blocks = g_new0(RDMADestBlock,
>                                 rdma->local_ram_blocks.nb_blocks);
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] migration: Poison ramblock loops in migration
  2018-06-05 16:25 ` [Qemu-devel] [PATCH 2/2] migration: Poison ramblock loops in migration Dr. David Alan Gilbert (git)
       [not found]   ` <20180606033600.GA2606@xz-mi>
@ 2018-06-06 18:05   ` Cédric Le Goater
  1 sibling, 0 replies; 10+ messages in thread
From: Cédric Le Goater @ 2018-06-06 18:05 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git), qemu-devel, peterx

On 06/05/2018 06:25 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> The migration code should be using the
>   RAMBLOCK_FOREACH_MIGRATABLE and qemu_ram_foreach_block_migratable
> not the all-block versions;  poison them so that we can't accidentally
> use them.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org> 

Thanks,

C.


> ---
>  include/exec/ramlist.h | 4 +++-
>  migration/migration.h  | 3 +++
>  migration/ram.c        | 4 +++-
>  3 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
> index 2e2ac6cb99..bc4faa1b00 100644
> --- a/include/exec/ramlist.h
> +++ b/include/exec/ramlist.h
> @@ -56,8 +56,10 @@ typedef struct RAMList {
>  extern RAMList ram_list;
>  
>  /* Should be holding either ram_list.mutex, or the RCU lock. */
> -#define  RAMBLOCK_FOREACH(block)  \
> +#define  INTERNAL_RAMBLOCK_FOREACH(block)  \
>      QLIST_FOREACH_RCU(block, &ram_list.blocks, next)
> +/* Never use the INTERNAL_ version except for defining other macros */
> +#define RAMBLOCK_FOREACH(block) INTERNAL_RAMBLOCK_FOREACH(block)
>  
>  void qemu_mutex_lock_ramlist(void);
>  void qemu_mutex_unlock_ramlist(void);
> diff --git a/migration/migration.h b/migration/migration.h
> index 5af57d616c..31d3ed12dc 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -284,4 +284,7 @@ void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value);
>  void dirty_bitmap_mig_before_vm_start(void);
>  void init_dirty_bitmap_incoming_migration(void);
>  
> +#define qemu_ram_foreach_block \
> +  #warning "Use qemu_ram_foreach_block_migratable in migration code"
> +
>  #endif
> diff --git a/migration/ram.c b/migration/ram.c
> index a7807cea84..e0d19305ee 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -159,9 +159,11 @@ out:
>  
>  /* Should be holding either ram_list.mutex, or the RCU lock. */
>  #define RAMBLOCK_FOREACH_MIGRATABLE(block)             \
> -    RAMBLOCK_FOREACH(block)                            \
> +    INTERNAL_RAMBLOCK_FOREACH(block)                   \
>          if (!qemu_ram_is_migratable(block)) {} else
>  
> +#undef RAMBLOCK_FOREACH
> +
>  static void ramblock_recv_map_init(void)
>  {
>      RAMBlock *rb;
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] Fixups for non-migratable RAMBlocks
  2018-06-05 16:25 [Qemu-devel] [PATCH 0/2] Fixups for non-migratable RAMBlocks Dr. David Alan Gilbert (git)
  2018-06-05 16:25 ` [Qemu-devel] [PATCH 1/2] migration: Fixes " Dr. David Alan Gilbert (git)
  2018-06-05 16:25 ` [Qemu-devel] [PATCH 2/2] migration: Poison ramblock loops in migration Dr. David Alan Gilbert (git)
@ 2018-06-15 11:21 ` Dr. David Alan Gilbert
  2 siblings, 0 replies; 10+ messages in thread
From: Dr. David Alan Gilbert @ 2018-06-15 11:21 UTC (permalink / raw)
  To: qemu-devel, peterx, clg; +Cc: quintela

* Dr. David Alan Gilbert (git) (dgilbert@redhat.com) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Hi,
>   Cédric's b895de50 got merged at about the same
> time as some of Peter's changes and there's a rdma
> case as well; so tidy up those cases that were looping
> over all (rather than just migratable) RAMBlocks
> and poison it so we don't end up doing the same thing
> again.

Queued.

> Dave
> 
> Dr. David Alan Gilbert (2):
>   migration: Fixes for non-migratable RAMBlocks
>   migration: Poison ramblock loops in migration
> 
>  include/exec/ramlist.h | 4 +++-
>  migration/migration.h  | 3 +++
>  migration/ram.c        | 8 +++++---
>  migration/rdma.c       | 2 +-
>  4 files changed, 12 insertions(+), 5 deletions(-)
> 
> -- 
> 2.17.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-06-15 11:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-05 16:25 [Qemu-devel] [PATCH 0/2] Fixups for non-migratable RAMBlocks Dr. David Alan Gilbert (git)
2018-06-05 16:25 ` [Qemu-devel] [PATCH 1/2] migration: Fixes " Dr. David Alan Gilbert (git)
2018-06-06  3:38   ` Peter Xu
2018-06-06 18:04   ` Cédric Le Goater
2018-06-05 16:25 ` [Qemu-devel] [PATCH 2/2] migration: Poison ramblock loops in migration Dr. David Alan Gilbert (git)
     [not found]   ` <20180606033600.GA2606@xz-mi>
2018-06-06  3:37     ` Peter Xu
2018-06-06  8:35       ` Dr. David Alan Gilbert
2018-06-06  9:09         ` Peter Xu
2018-06-06 18:05   ` Cédric Le Goater
2018-06-15 11:21 ` [Qemu-devel] [PATCH 0/2] Fixups for non-migratable RAMBlocks Dr. David Alan Gilbert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).