Re: [Qemu-devel] [PATCH v10 10/12] migration: add postcopy migration of dirty bitmaps

2018-03-13 Thread Dr. David Alan Gilbert
* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:
> 12.03.2018 19:09, Dr. David Alan Gilbert wrote:
> > * Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:
> > > Postcopy migration of dirty bitmaps. Only named dirty bitmaps are 
> > > migrated.
> > > 
> > > +
> > > +init_dirty_bitmap_incoming_migration();
> > > +
> > You might want to consider if that's better in vl.c near where
> > ram_mig_init() is, OR whether there should be a call in
> > migratation_incoming_state_destroy to clean it up.
> > (Although I doubt the cases where the destroy happens are interesting
> > for postcopy bitmaps).
> 
> If you don't mind, let's leave it as is for now

Yep, that's OK.

Dave

> > 
> > >   once = true;
> > >   }
> > >   return &mis_current;
> > > @@ -297,6 +300,8 @@ static void process_incoming_migration_bh(void 
> > > *opaque)
> > >  state, we need to obey autostart. Any other state is set with
> > >  runstate_set. */
> > > +dirty_bitmap_mig_before_vm_start();
> > > +
> > >   if (!global_state_received() ||
> > >   global_state_get_runstate() == RUN_STATE_RUNNING) {
> > >   if (autostart) {
> > > diff --git a/migration/savevm.c b/migration/savevm.c
> > > index e5d557458e..93b339646b 100644
> > > --- a/migration/savevm.c
> > > +++ b/migration/savevm.c
> > > @@ -1673,6 +1673,8 @@ static void loadvm_postcopy_handle_run_bh(void 
> > > *opaque)
> > >   trace_loadvm_postcopy_handle_run_vmstart();
> > > +dirty_bitmap_mig_before_vm_start();
> > > +
> > >   if (autostart) {
> > >   /* Hold onto your hats, starting the CPU */
> > >   vm_start();
> > > diff --git a/vl.c b/vl.c
> > > index e517a8d995..0ef3f2b5a2 100644
> > > --- a/vl.c
> > > +++ b/vl.c
> > > @@ -4514,6 +4514,7 @@ int main(int argc, char **argv, char **envp)
> > >   blk_mig_init();
> > >   ram_mig_init();
> > > +dirty_bitmap_mig_init();
> > >   /* If the currently selected machine wishes to override the 
> > > units-per-bus
> > >* property of its default HBA interface type, do so now. */
> > > diff --git a/migration/Makefile.objs b/migration/Makefile.objs
> > > index 99e038024d..c83ec47ba8 100644
> > > --- a/migration/Makefile.objs
> > > +++ b/migration/Makefile.objs
> > > @@ -6,6 +6,7 @@ common-obj-y += qemu-file.o global_state.o
> > >   common-obj-y += qemu-file-channel.o
> > >   common-obj-y += xbzrle.o postcopy-ram.o
> > >   common-obj-y += qjson.o
> > > +common-obj-y += block-dirty-bitmap.o
> > >   common-obj-$(CONFIG_RDMA) += rdma.o
> > > diff --git a/migration/trace-events b/migration/trace-events
> > > index a04fffb877..e9eb8078d4 100644
> > > --- a/migration/trace-events
> > > +++ b/migration/trace-events
> > > @@ -227,3 +227,17 @@ colo_vm_state_change(const char *old, const char 
> > > *new) "Change '%s' => '%s'"
> > >   colo_send_message(const char *msg) "Send '%s' message"
> > >   colo_receive_message(const char *msg) "Receive '%s' message"
> > >   colo_failover_set_state(const char *new_state) "new state %s"
> > > +
> > > +# migration/block-dirty-bitmap.c
> > > +send_bitmap_header_enter(void) ""
> > > +send_bitmap_bits(uint32_t flags, uint64_t start_sector, uint32_t 
> > > nr_sectors, uint64_t data_size) "\n   flags:0x%x\n   
> > > start_sector: %" PRIu64 "\n   nr_sectors:   %" PRIu32 "\n   data_size:
> > > %" PRIu64 "\n"
> > Tracing doesn't have \n's in
> 
> will fix.
> 
> > 
> > > +dirty_bitmap_save_iterate(int in_postcopy) "in postcopy: %d"
> > > +dirty_bitmap_save_complete_enter(void) ""
> > > +dirty_bitmap_save_complete_finish(void) ""
> > > +dirty_bitmap_save_pending(uint64_t pending, uint64_t max_size) "pending 
> > > %" PRIu64 " max: %" PRIu64
> > > +dirty_bitmap_load_complete(void) ""
> > > +dirty_bitmap_load_bits_enter(uint64_t first_sector, uint32_t nr_sectors) 
> > > "chunk: %" PRIu64 " %" PRIu32
> > > +dirty_bitmap_load_bits_zeroes(void) ""
> > > +dirty_bitmap_load_header(uint32_t flags) "flags 0x%x"
> > > +dirty_bitmap_load_enter(void) ""
> > > +dirty_bitmap_load_success(void) ""
> > So other than minor bits, this one looks OK from a migration side; I
> > can't say I've followed the block side of the patch though.
> > 
> > Dave
> > 
> > > -- 
> > > 2.11.1
> > > 
> > --
> > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> 
> 
> -- 
> Best regards,
> Vladimir
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH v10 10/12] migration: add postcopy migration of dirty bitmaps

2018-03-13 Thread Vladimir Sementsov-Ogievskiy

12.03.2018 19:09, Dr. David Alan Gilbert wrote:

* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:

Postcopy migration of dirty bitmaps. Only named dirty bitmaps are migrated.

If destination qemu is already containing a dirty bitmap with the same name
as a migrated bitmap (for the same node), then, if their granularities are
the same the migration will be done, otherwise the error will be generated.

If destination qemu doesn't contain such bitmap it will be created.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  include/migration/misc.h   |   3 +
  migration/migration.h  |   3 +
  migration/block-dirty-bitmap.c | 737 +
  migration/migration.c  |   5 +
  migration/savevm.c |   2 +
  vl.c   |   1 +
  migration/Makefile.objs|   1 +
  migration/trace-events |  14 +
  8 files changed, 766 insertions(+)
  create mode 100644 migration/block-dirty-bitmap.c

diff --git a/include/migration/misc.h b/include/migration/misc.h
index 77fd4f587c..4ebf24c6c2 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -56,4 +56,7 @@ bool migration_has_failed(MigrationState *);
  bool migration_in_postcopy_after_devices(MigrationState *);
  void migration_global_dump(Monitor *mon);
  
+/* migration/block-dirty-bitmap.c */

+void dirty_bitmap_mig_init(void);
+
  #endif
diff --git a/migration/migration.h b/migration/migration.h
index 861cdfaa96..79f72b7e50 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -233,4 +233,7 @@ void migrate_send_rp_pong(MigrationIncomingState *mis,
  void migrate_send_rp_req_pages(MigrationIncomingState *mis, const char* 
rbname,
ram_addr_t start, size_t len);
  
+void dirty_bitmap_mig_before_vm_start(void);

+void init_dirty_bitmap_incoming_migration(void);
+
  #endif
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
new file mode 100644
index 00..5b41f7140d
--- /dev/null
+++ b/migration/block-dirty-bitmap.c
@@ -0,0 +1,737 @@
+/*
+ * Block dirty bitmap postcopy migration
+ *
+ * Copyright IBM, Corp. 2009
+ * Copyright (c) 2016-2017 Virtuozzo International GmbH. All rights reserved.
+ *
+ * Authors:
+ *  Liran Schour   
+ *  Vladimir Sementsov-Ogievskiy 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ * This file is derived from migration/block.c, so it's author and IBM 
copyright
+ * are here, although content is quite different.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ *
+ ****
+ *
+ * Here postcopy migration of dirty bitmaps is realized. Only QMP-addressable
+ * bitmaps are migrated.
+ *
+ * Bitmap migration implies creating bitmap with the same name and granularity
+ * in destination QEMU. If the bitmap with the same name (for the same node)
+ * already exists on destination an error will be generated.
+ *
+ * format of migration:
+ *
+ * # Header (shared for different chunk types)
+ * 1, 2 or 4 bytes: flags (see qemu_{put,put}_flags)
+ * [ 1 byte: node name size ] \  flags & DEVICE_NAME
+ * [ n bytes: node name ] /
+ * [ 1 byte: bitmap name size ] \  flags & BITMAP_NAME
+ * [ n bytes: bitmap name ] /
+ *
+ * # Start of bitmap migration (flags & START)
+ * header
+ * be64: granularity
+ * 1 byte: bitmap flags (corresponds to BdrvDirtyBitmap)
+ *   bit 0-  bitmap is enabled
+ *   bit 1-  bitmap is persistent
+ *   bit 2-  bitmap is autoloading
+ *   bits 3-7 - reserved, must be zero
+ *
+ * # Complete of bitmap migration (flags & COMPLETE)
+ * header
+ *
+ * # Data chunk of bitmap migration
+ * header
+ * be64: start sector
+ * be32: number of sectors
+ * [ be64: buffer size  ] \ ! (flags & ZEROES)
+ * [ n bytes: buffer] /
+ *
+ * The last chunk in stream should contain flags & EOS. The chunk may skip
+ * device and/or bitmap names, assuming them to be the same with the previous
+ * chunk.
+ */
+
+#include "qemu/osdep.h"
+#include "block/block.h"
+#include "block/block_int.h"
+#include "sysemu/block-backend.h"
+#include "qemu/main-loop.h"
+#include "qemu/error-report.h"
+#include "migration/misc.h"
+#include "migration/migration.h"
+#include "migration/qemu-file.h"
+#include "migration/vmstate.h"
+#include "migration/register.h"
+#include "qemu/hbitmap.h"
+#include "sysemu/sysemu.h"
+#include "qemu/cutils.h"
+#include "qapi/error.h"
+#include "trace.h"
+
+#define CHUNK_SIZE (1 << 10)
+
+/* Flags occupy one, two or four bytes (Big Endian). The size is determined as
+ * follows:
+ * in first (most significant) byte bit 8 is clear  -->  one byte
+ * in first byte bit 8 is set-->  two or four bytes, depending on second
+ *byte:
+ *| in second byte bit 8 is clear  -->  two bytes
+ *| in second 

Re: [Qemu-devel] [PATCH v10 10/12] migration: add postcopy migration of dirty bitmaps

2018-03-12 Thread Dr. David Alan Gilbert
* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:
> Postcopy migration of dirty bitmaps. Only named dirty bitmaps are migrated.
> 
> If destination qemu is already containing a dirty bitmap with the same name
> as a migrated bitmap (for the same node), then, if their granularities are
> the same the migration will be done, otherwise the error will be generated.
> 
> If destination qemu doesn't contain such bitmap it will be created.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  include/migration/misc.h   |   3 +
>  migration/migration.h  |   3 +
>  migration/block-dirty-bitmap.c | 737 
> +
>  migration/migration.c  |   5 +
>  migration/savevm.c |   2 +
>  vl.c   |   1 +
>  migration/Makefile.objs|   1 +
>  migration/trace-events |  14 +
>  8 files changed, 766 insertions(+)
>  create mode 100644 migration/block-dirty-bitmap.c
> 
> diff --git a/include/migration/misc.h b/include/migration/misc.h
> index 77fd4f587c..4ebf24c6c2 100644
> --- a/include/migration/misc.h
> +++ b/include/migration/misc.h
> @@ -56,4 +56,7 @@ bool migration_has_failed(MigrationState *);
>  bool migration_in_postcopy_after_devices(MigrationState *);
>  void migration_global_dump(Monitor *mon);
>  
> +/* migration/block-dirty-bitmap.c */
> +void dirty_bitmap_mig_init(void);
> +
>  #endif
> diff --git a/migration/migration.h b/migration/migration.h
> index 861cdfaa96..79f72b7e50 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -233,4 +233,7 @@ void migrate_send_rp_pong(MigrationIncomingState *mis,
>  void migrate_send_rp_req_pages(MigrationIncomingState *mis, const char* 
> rbname,
>ram_addr_t start, size_t len);
>  
> +void dirty_bitmap_mig_before_vm_start(void);
> +void init_dirty_bitmap_incoming_migration(void);
> +
>  #endif
> diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
> new file mode 100644
> index 00..5b41f7140d
> --- /dev/null
> +++ b/migration/block-dirty-bitmap.c
> @@ -0,0 +1,737 @@
> +/*
> + * Block dirty bitmap postcopy migration
> + *
> + * Copyright IBM, Corp. 2009
> + * Copyright (c) 2016-2017 Virtuozzo International GmbH. All rights reserved.
> + *
> + * Authors:
> + *  Liran Schour   
> + *  Vladimir Sementsov-Ogievskiy 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + * This file is derived from migration/block.c, so it's author and IBM 
> copyright
> + * are here, although content is quite different.
> + *
> + * Contributions after 2012-01-13 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
> + *
> + ****
> + *
> + * Here postcopy migration of dirty bitmaps is realized. Only QMP-addressable
> + * bitmaps are migrated.
> + *
> + * Bitmap migration implies creating bitmap with the same name and 
> granularity
> + * in destination QEMU. If the bitmap with the same name (for the same node)
> + * already exists on destination an error will be generated.
> + *
> + * format of migration:
> + *
> + * # Header (shared for different chunk types)
> + * 1, 2 or 4 bytes: flags (see qemu_{put,put}_flags)
> + * [ 1 byte: node name size ] \  flags & DEVICE_NAME
> + * [ n bytes: node name ] /
> + * [ 1 byte: bitmap name size ] \  flags & BITMAP_NAME
> + * [ n bytes: bitmap name ] /
> + *
> + * # Start of bitmap migration (flags & START)
> + * header
> + * be64: granularity
> + * 1 byte: bitmap flags (corresponds to BdrvDirtyBitmap)
> + *   bit 0-  bitmap is enabled
> + *   bit 1-  bitmap is persistent
> + *   bit 2-  bitmap is autoloading
> + *   bits 3-7 - reserved, must be zero
> + *
> + * # Complete of bitmap migration (flags & COMPLETE)
> + * header
> + *
> + * # Data chunk of bitmap migration
> + * header
> + * be64: start sector
> + * be32: number of sectors
> + * [ be64: buffer size  ] \ ! (flags & ZEROES)
> + * [ n bytes: buffer] /
> + *
> + * The last chunk in stream should contain flags & EOS. The chunk may skip
> + * device and/or bitmap names, assuming them to be the same with the previous
> + * chunk.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "block/block.h"
> +#include "block/block_int.h"
> +#include "sysemu/block-backend.h"
> +#include "qemu/main-loop.h"
> +#include "qemu/error-report.h"
> +#include "migration/misc.h"
> +#include "migration/migration.h"
> +#include "migration/qemu-file.h"
> +#include "migration/vmstate.h"
> +#include "migration/register.h"
> +#include "qemu/hbitmap.h"
> +#include "sysemu/sysemu.h"
> +#include "qemu/cutils.h"
> +#include "qapi/error.h"
> +#include "trace.h"
> +
> +#define CHUNK_SIZE (1 << 10)
> +
> +/* Flags occupy one, two or four bytes (Big Endian). The size is determined 
> as
> + * follows:
> + * in first (most significant) byte bit 8 is clear  -->