qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH for-6.0 v1 0/4] migration: Fixes to the 'background-snapshot' code
@ 2021-04-01  9:22 Andrey Gruzdev
  2021-04-01  9:22 ` [PATCH for-6.0 v1 1/4] migration: Fix missing qemu_fflush() on buffer file in bg_migration_thread Andrey Gruzdev
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Andrey Gruzdev @ 2021-04-01  9:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: Den Lunev, Eric Blake, Paolo Bonzini, Juan Quintela,
	Dr . David Alan Gilbert, Markus Armbruster, Peter Xu,
	David Hildenbrand, Andrey Gruzdev

Changes v0->v1:
 * Fixes to coding style and commit messages
 * Renamed 'bs' to 'block' in migration/ram.c background snapshot code

This patch series contains:
 * Fix to the issue with occasionally truncated non-iterable device state
 * Solution to compatibility issues with virtio-balloon device
 * Fix to the issue when discarded or never populated pages miss UFFD
   write protection and get into migration stream in dirty state
 * Renaming of 'bs' to commonly used 'block' in migration/ram.c background
   snapshot code

Andrey Gruzdev (4):
  migration: Fix missing qemu_fflush() on buffer file in
    bg_migration_thread
  migration: Inhibit virtio-balloon for the duration of background
    snapshot
  migration: Pre-fault memory before starting background snasphot
  migration: Rename 'bs' to 'block' in background snapshot code

 hw/virtio/virtio-balloon.c |   8 ++-
 include/migration/misc.h   |   2 +
 migration/migration.c      |  22 ++++++-
 migration/ram.c            | 119 ++++++++++++++++++++++++++-----------
 migration/ram.h            |   1 +
 5 files changed, 115 insertions(+), 37 deletions(-)

-- 
2.27.0



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

* [PATCH for-6.0 v1 1/4] migration: Fix missing qemu_fflush() on buffer file in bg_migration_thread
  2021-04-01  9:22 [PATCH for-6.0 v1 0/4] migration: Fixes to the 'background-snapshot' code Andrey Gruzdev
@ 2021-04-01  9:22 ` Andrey Gruzdev
  2021-04-06 12:21   ` Dr. David Alan Gilbert
  2021-04-06 12:29   ` Dr. David Alan Gilbert
  2021-04-01  9:22 ` [PATCH for-6.0 v1 2/4] migration: Inhibit virtio-balloon for the duration of background snapshot Andrey Gruzdev
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 12+ messages in thread
From: Andrey Gruzdev @ 2021-04-01  9:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: Den Lunev, Eric Blake, Paolo Bonzini, Juan Quintela,
	Dr . David Alan Gilbert, Markus Armbruster, Peter Xu,
	David Hildenbrand, Andrey Gruzdev

Added missing qemu_fflush() on buffer file holding precopy device state.
Increased initial QIOChannelBuffer allocation to 512KB to avoid reallocs.
Typical configurations often require >200KB for device state and VMDESC.

Fixes: 8518278a6af589ccc401f06e35f171b1e6fae800 (migration: implementation
  of background snapshot thread)
Signed-off-by: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com>
---
 migration/migration.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index ca8b97baa5..00e13f9d58 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3812,7 +3812,7 @@ static void *bg_migration_thread(void *opaque)
      * with vCPUs running and, finally, write stashed non-RAM part of
      * the vmstate from the buffer to the migration stream.
      */
-    s->bioc = qio_channel_buffer_new(128 * 1024);
+    s->bioc = qio_channel_buffer_new(512 * 1024);
     qio_channel_set_name(QIO_CHANNEL(s->bioc), "vmstate-buffer");
     fb = qemu_fopen_channel_output(QIO_CHANNEL(s->bioc));
     object_unref(OBJECT(s->bioc));
@@ -3866,6 +3866,12 @@ static void *bg_migration_thread(void *opaque)
     if (qemu_savevm_state_complete_precopy_non_iterable(fb, false, false)) {
         goto fail;
     }
+    /*
+     * Since we are going to get non-iterable state data directly
+     * from s->bioc->data, explicit flush is needed here.
+     */
+    qemu_fflush(fb);
+
     /* Now initialize UFFD context and start tracking RAM writes */
     if (ram_write_tracking_start()) {
         goto fail;
-- 
2.27.0



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

* [PATCH for-6.0 v1 2/4] migration: Inhibit virtio-balloon for the duration of background snapshot
  2021-04-01  9:22 [PATCH for-6.0 v1 0/4] migration: Fixes to the 'background-snapshot' code Andrey Gruzdev
  2021-04-01  9:22 ` [PATCH for-6.0 v1 1/4] migration: Fix missing qemu_fflush() on buffer file in bg_migration_thread Andrey Gruzdev
@ 2021-04-01  9:22 ` Andrey Gruzdev
  2021-04-01  9:22 ` [PATCH for-6.0 v1 3/4] migration: Pre-fault memory before starting background snasphot Andrey Gruzdev
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Andrey Gruzdev @ 2021-04-01  9:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: Den Lunev, Eric Blake, Paolo Bonzini, Juan Quintela,
	Dr . David Alan Gilbert, Markus Armbruster, Peter Xu,
	David Hildenbrand, Andrey Gruzdev

The same thing as for incoming postcopy - we cannot deal with concurrent
RAM discards when using background snapshot feature in outgoing migration.

Fixes: 8518278a6af589ccc401f06e35f171b1e6fae800 (migration: implementation
  of background snapshot thread)
Signed-off-by: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com>
Reported-by: David Hildenbrand <david@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
 hw/virtio/virtio-balloon.c | 8 ++++++--
 include/migration/misc.h   | 2 ++
 migration/migration.c      | 8 ++++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index e770955176..d120bf8f43 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -66,8 +66,12 @@ static bool virtio_balloon_pbp_matches(PartiallyBalloonedPage *pbp,
 
 static bool virtio_balloon_inhibited(void)
 {
-    /* Postcopy cannot deal with concurrent discards, so it's special. */
-    return ram_block_discard_is_disabled() || migration_in_incoming_postcopy();
+    /*
+     * Postcopy cannot deal with concurrent discards,
+     * so it's special, as well as background snapshots.
+     */
+    return ram_block_discard_is_disabled() || migration_in_incoming_postcopy() ||
+            migration_in_bg_snapshot();
 }
 
 static void balloon_inflate_page(VirtIOBalloon *balloon,
diff --git a/include/migration/misc.h b/include/migration/misc.h
index bccc1b6b44..738675ef52 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -70,6 +70,8 @@ bool migration_in_postcopy_after_devices(MigrationState *);
 void migration_global_dump(Monitor *mon);
 /* True if incomming migration entered POSTCOPY_INCOMING_DISCARD */
 bool migration_in_incoming_postcopy(void);
+/* True if background snapshot is active */
+bool migration_in_bg_snapshot(void);
 
 /* migration/block-dirty-bitmap.c */
 void dirty_bitmap_mig_init(void);
diff --git a/migration/migration.c b/migration/migration.c
index 00e13f9d58..be4729e7c8 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1976,6 +1976,14 @@ bool migration_in_incoming_postcopy(void)
     return ps >= POSTCOPY_INCOMING_DISCARD && ps < POSTCOPY_INCOMING_END;
 }
 
+bool migration_in_bg_snapshot(void)
+{
+    MigrationState *s = migrate_get_current();
+
+    return migrate_background_snapshot() &&
+            migration_is_setup_or_active(s->state);
+}
+
 bool migration_is_idle(void)
 {
     MigrationState *s = current_migration;
-- 
2.27.0



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

* [PATCH for-6.0 v1 3/4] migration: Pre-fault memory before starting background snasphot
  2021-04-01  9:22 [PATCH for-6.0 v1 0/4] migration: Fixes to the 'background-snapshot' code Andrey Gruzdev
  2021-04-01  9:22 ` [PATCH for-6.0 v1 1/4] migration: Fix missing qemu_fflush() on buffer file in bg_migration_thread Andrey Gruzdev
  2021-04-01  9:22 ` [PATCH for-6.0 v1 2/4] migration: Inhibit virtio-balloon for the duration of background snapshot Andrey Gruzdev
@ 2021-04-01  9:22 ` Andrey Gruzdev
  2021-04-01  9:22 ` [PATCH for-6.0 v1 4/4] migration: Rename 'bs' to 'block' in background snapshot code Andrey Gruzdev
  2021-04-06 16:53 ` [PATCH for-6.0 v1 0/4] migration: Fixes to the 'background-snapshot' code Dr. David Alan Gilbert
  4 siblings, 0 replies; 12+ messages in thread
From: Andrey Gruzdev @ 2021-04-01  9:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: Den Lunev, Eric Blake, Paolo Bonzini, Juan Quintela,
	Dr . David Alan Gilbert, Markus Armbruster, Peter Xu,
	David Hildenbrand, Andrey Gruzdev

This commit solves the issue with userfault_fd WP feature that
background snapshot is based on. For any never poluated or discarded
memory page, the UFFDIO_WRITEPROTECT ioctl() would skip updating
PTE for that page, thereby loosing WP setting for it.

So we need to pre-fault pages for each RAM block to be protected
before making a userfault_fd wr-protect ioctl().

Fixes: 278e2f551a095b234de74dca9c214d5502a1f72c (migration: support
  UFFD write fault processing in ram_save_iterate())
Signed-off-by: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com>
Reported-by: David Hildenbrand <david@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
 migration/migration.c |  6 ++++++
 migration/ram.c       | 49 +++++++++++++++++++++++++++++++++++++++++++
 migration/ram.h       |  1 +
 3 files changed, 56 insertions(+)

diff --git a/migration/migration.c b/migration/migration.c
index be4729e7c8..71bce15a1b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3827,6 +3827,12 @@ static void *bg_migration_thread(void *opaque)
 
     update_iteration_initial_status(s);
 
+    /*
+     * Prepare for tracking memory writes with UFFD-WP - populate
+     * RAM pages before protecting.
+     */
+    ram_write_tracking_prepare();
+
     qemu_savevm_state_header(s->to_dst_file);
     qemu_savevm_state_setup(s->to_dst_file);
 
diff --git a/migration/ram.c b/migration/ram.c
index 40e78952ad..7e2bc0fdd3 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1560,6 +1560,55 @@ out:
     return ret;
 }
 
+/*
+ * ram_block_populate_pages: populate memory in the RAM block by reading
+ *   an integer from the beginning of each page.
+ *
+ * Since it's solely used for userfault_fd WP feature, here we just
+ *   hardcode page size to qemu_real_host_page_size.
+ *
+ * @bs: RAM block to populate
+ */
+static void ram_block_populate_pages(RAMBlock *bs)
+{
+    char *ptr = (char *) bs->host;
+
+    for (ram_addr_t offset = 0; offset < bs->used_length;
+            offset += qemu_real_host_page_size) {
+        char tmp = *(ptr + offset);
+
+        /* Don't optimize the read out */
+        asm volatile("" : "+r" (tmp));
+    }
+}
+
+/*
+ * ram_write_tracking_prepare: prepare for UFFD-WP memory tracking
+ */
+void ram_write_tracking_prepare(void)
+{
+    RAMBlock *bs;
+
+    RCU_READ_LOCK_GUARD();
+
+    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
+        /* Nothing to do with read-only and MMIO-writable regions */
+        if (bs->mr->readonly || bs->mr->rom_device) {
+            continue;
+        }
+
+        /*
+         * Populate pages of the RAM block before enabling userfault_fd
+         * write protection.
+         *
+         * This stage is required since ioctl(UFFDIO_WRITEPROTECT) with
+         * UFFDIO_WRITEPROTECT_MODE_WP mode setting would silently skip
+         * pages with pte_none() entries in page table.
+         */
+        ram_block_populate_pages(bs);
+    }
+}
+
 /*
  * ram_write_tracking_start: start UFFD-WP memory tracking
  *
diff --git a/migration/ram.h b/migration/ram.h
index 6378bb3ebc..4833e9fd5b 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -82,6 +82,7 @@ void colo_incoming_start_dirty_log(void);
 /* Background snapshot */
 bool ram_write_tracking_available(void);
 bool ram_write_tracking_compatible(void);
+void ram_write_tracking_prepare(void);
 int ram_write_tracking_start(void);
 void ram_write_tracking_stop(void);
 
-- 
2.27.0



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

* [PATCH for-6.0 v1 4/4] migration: Rename 'bs' to 'block' in background snapshot code
  2021-04-01  9:22 [PATCH for-6.0 v1 0/4] migration: Fixes to the 'background-snapshot' code Andrey Gruzdev
                   ` (2 preceding siblings ...)
  2021-04-01  9:22 ` [PATCH for-6.0 v1 3/4] migration: Pre-fault memory before starting background snasphot Andrey Gruzdev
@ 2021-04-01  9:22 ` Andrey Gruzdev
  2021-04-06 12:30   ` Dr. David Alan Gilbert
  2021-04-06 16:52   ` Dr. David Alan Gilbert
  2021-04-06 16:53 ` [PATCH for-6.0 v1 0/4] migration: Fixes to the 'background-snapshot' code Dr. David Alan Gilbert
  4 siblings, 2 replies; 12+ messages in thread
From: Andrey Gruzdev @ 2021-04-01  9:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: Den Lunev, Eric Blake, Paolo Bonzini, Juan Quintela,
	Dr . David Alan Gilbert, Markus Armbruster, Peter Xu,
	David Hildenbrand, Andrey Gruzdev

Rename 'bs' to commonly used 'block' in migration/ram.c background
snapshot code.

Signed-off-by: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com>
Reported-by: David Hildenbrand <david@redhat.com>
---
 migration/ram.c | 86 +++++++++++++++++++++++++------------------------
 1 file changed, 44 insertions(+), 42 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 7e2bc0fdd3..4682f3625c 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1455,7 +1455,7 @@ static RAMBlock *poll_fault_page(RAMState *rs, ram_addr_t *offset)
 {
     struct uffd_msg uffd_msg;
     void *page_address;
-    RAMBlock *bs;
+    RAMBlock *block;
     int res;
 
     if (!migrate_background_snapshot()) {
@@ -1468,9 +1468,9 @@ static RAMBlock *poll_fault_page(RAMState *rs, ram_addr_t *offset)
     }
 
     page_address = (void *)(uintptr_t) uffd_msg.arg.pagefault.address;
-    bs = qemu_ram_block_from_host(page_address, false, offset);
-    assert(bs && (bs->flags & RAM_UF_WRITEPROTECT) != 0);
-    return bs;
+    block = qemu_ram_block_from_host(page_address, false, offset);
+    assert(block && (block->flags & RAM_UF_WRITEPROTECT) != 0);
+    return block;
 }
 
 /**
@@ -1526,7 +1526,7 @@ bool ram_write_tracking_compatible(void)
 {
     const uint64_t uffd_ioctls_mask = BIT(_UFFDIO_WRITEPROTECT);
     int uffd_fd;
-    RAMBlock *bs;
+    RAMBlock *block;
     bool ret = false;
 
     /* Open UFFD file descriptor */
@@ -1537,15 +1537,15 @@ bool ram_write_tracking_compatible(void)
 
     RCU_READ_LOCK_GUARD();
 
-    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
+    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
         uint64_t uffd_ioctls;
 
         /* Nothing to do with read-only and MMIO-writable regions */
-        if (bs->mr->readonly || bs->mr->rom_device) {
+        if (block->mr->readonly || block->mr->rom_device) {
             continue;
         }
         /* Try to register block memory via UFFD-IO to track writes */
-        if (uffd_register_memory(uffd_fd, bs->host, bs->max_length,
+        if (uffd_register_memory(uffd_fd, block->host, block->max_length,
                 UFFDIO_REGISTER_MODE_WP, &uffd_ioctls)) {
             goto out;
         }
@@ -1567,13 +1567,13 @@ out:
  * Since it's solely used for userfault_fd WP feature, here we just
  *   hardcode page size to qemu_real_host_page_size.
  *
- * @bs: RAM block to populate
+ * @block: RAM block to populate
  */
-static void ram_block_populate_pages(RAMBlock *bs)
+static void ram_block_populate_pages(RAMBlock *block)
 {
-    char *ptr = (char *) bs->host;
+    char *ptr = (char *) block->host;
 
-    for (ram_addr_t offset = 0; offset < bs->used_length;
+    for (ram_addr_t offset = 0; offset < block->used_length;
             offset += qemu_real_host_page_size) {
         char tmp = *(ptr + offset);
 
@@ -1587,13 +1587,13 @@ static void ram_block_populate_pages(RAMBlock *bs)
  */
 void ram_write_tracking_prepare(void)
 {
-    RAMBlock *bs;
+    RAMBlock *block;
 
     RCU_READ_LOCK_GUARD();
 
-    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
+    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
         /* Nothing to do with read-only and MMIO-writable regions */
-        if (bs->mr->readonly || bs->mr->rom_device) {
+        if (block->mr->readonly || block->mr->rom_device) {
             continue;
         }
 
@@ -1605,7 +1605,7 @@ void ram_write_tracking_prepare(void)
          * UFFDIO_WRITEPROTECT_MODE_WP mode setting would silently skip
          * pages with pte_none() entries in page table.
          */
-        ram_block_populate_pages(bs);
+        ram_block_populate_pages(block);
     }
 }
 
@@ -1618,7 +1618,7 @@ int ram_write_tracking_start(void)
 {
     int uffd_fd;
     RAMState *rs = ram_state;
-    RAMBlock *bs;
+    RAMBlock *block;
 
     /* Open UFFD file descriptor */
     uffd_fd = uffd_create_fd(UFFD_FEATURE_PAGEFAULT_FLAG_WP, true);
@@ -1629,27 +1629,27 @@ int ram_write_tracking_start(void)
 
     RCU_READ_LOCK_GUARD();
 
-    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
+    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
         /* Nothing to do with read-only and MMIO-writable regions */
-        if (bs->mr->readonly || bs->mr->rom_device) {
+        if (block->mr->readonly || block->mr->rom_device) {
             continue;
         }
 
         /* Register block memory with UFFD to track writes */
-        if (uffd_register_memory(rs->uffdio_fd, bs->host,
-                bs->max_length, UFFDIO_REGISTER_MODE_WP, NULL)) {
+        if (uffd_register_memory(rs->uffdio_fd, block->host,
+                block->max_length, UFFDIO_REGISTER_MODE_WP, NULL)) {
             goto fail;
         }
         /* Apply UFFD write protection to the block memory range */
-        if (uffd_change_protection(rs->uffdio_fd, bs->host,
-                bs->max_length, true, false)) {
+        if (uffd_change_protection(rs->uffdio_fd, block->host,
+                block->max_length, true, false)) {
             goto fail;
         }
-        bs->flags |= RAM_UF_WRITEPROTECT;
-        memory_region_ref(bs->mr);
+        block->flags |= RAM_UF_WRITEPROTECT;
+        memory_region_ref(block->mr);
 
-        trace_ram_write_tracking_ramblock_start(bs->idstr, bs->page_size,
-                bs->host, bs->max_length);
+        trace_ram_write_tracking_ramblock_start(block->idstr, block->page_size,
+                block->host, block->max_length);
     }
 
     return 0;
@@ -1657,19 +1657,20 @@ int ram_write_tracking_start(void)
 fail:
     error_report("ram_write_tracking_start() failed: restoring initial memory state");
 
-    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
-        if ((bs->flags & RAM_UF_WRITEPROTECT) == 0) {
+    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
+        if ((block->flags & RAM_UF_WRITEPROTECT) == 0) {
             continue;
         }
         /*
          * In case some memory block failed to be write-protected
          * remove protection and unregister all succeeded RAM blocks
          */
-        uffd_change_protection(rs->uffdio_fd, bs->host, bs->max_length, false, false);
-        uffd_unregister_memory(rs->uffdio_fd, bs->host, bs->max_length);
+        uffd_change_protection(rs->uffdio_fd, block->host, block->max_length,
+                false, false);
+        uffd_unregister_memory(rs->uffdio_fd, block->host, block->max_length);
         /* Cleanup flags and remove reference */
-        bs->flags &= ~RAM_UF_WRITEPROTECT;
-        memory_region_unref(bs->mr);
+        block->flags &= ~RAM_UF_WRITEPROTECT;
+        memory_region_unref(block->mr);
     }
 
     uffd_close_fd(uffd_fd);
@@ -1683,24 +1684,25 @@ fail:
 void ram_write_tracking_stop(void)
 {
     RAMState *rs = ram_state;
-    RAMBlock *bs;
+    RAMBlock *block;
 
     RCU_READ_LOCK_GUARD();
 
-    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
-        if ((bs->flags & RAM_UF_WRITEPROTECT) == 0) {
+    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
+        if ((block->flags & RAM_UF_WRITEPROTECT) == 0) {
             continue;
         }
         /* Remove protection and unregister all affected RAM blocks */
-        uffd_change_protection(rs->uffdio_fd, bs->host, bs->max_length, false, false);
-        uffd_unregister_memory(rs->uffdio_fd, bs->host, bs->max_length);
+        uffd_change_protection(rs->uffdio_fd, block->host, block->max_length,
+                false, false);
+        uffd_unregister_memory(rs->uffdio_fd, block->host, block->max_length);
 
-        trace_ram_write_tracking_ramblock_stop(bs->idstr, bs->page_size,
-                bs->host, bs->max_length);
+        trace_ram_write_tracking_ramblock_stop(block->idstr, block->page_size,
+                block->host, block->max_length);
 
         /* Cleanup flags and remove reference */
-        bs->flags &= ~RAM_UF_WRITEPROTECT;
-        memory_region_unref(bs->mr);
+        block->flags &= ~RAM_UF_WRITEPROTECT;
+        memory_region_unref(block->mr);
     }
 
     /* Finally close UFFD file descriptor */
-- 
2.27.0



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

* Re: [PATCH for-6.0 v1 1/4] migration: Fix missing qemu_fflush() on buffer file in bg_migration_thread
  2021-04-01  9:22 ` [PATCH for-6.0 v1 1/4] migration: Fix missing qemu_fflush() on buffer file in bg_migration_thread Andrey Gruzdev
@ 2021-04-06 12:21   ` Dr. David Alan Gilbert
  2021-04-06 12:29   ` Dr. David Alan Gilbert
  1 sibling, 0 replies; 12+ messages in thread
From: Dr. David Alan Gilbert @ 2021-04-06 12:21 UTC (permalink / raw)
  To: Andrey Gruzdev
  Cc: Juan Quintela, David Hildenbrand, qemu-devel, Peter Xu,
	Markus Armbruster, Paolo Bonzini, Den Lunev

* Andrey Gruzdev (andrey.gruzdev@virtuozzo.com) wrote:
> Added missing qemu_fflush() on buffer file holding precopy device state.
> Increased initial QIOChannelBuffer allocation to 512KB to avoid reallocs.
> Typical configurations often require >200KB for device state and VMDESC.
> 
> Fixes: 8518278a6af589ccc401f06e35f171b1e6fae800 (migration: implementation
>   of background snapshot thread)
> Signed-off-by: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  migration/migration.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index ca8b97baa5..00e13f9d58 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3812,7 +3812,7 @@ static void *bg_migration_thread(void *opaque)
>       * with vCPUs running and, finally, write stashed non-RAM part of
>       * the vmstate from the buffer to the migration stream.
>       */
> -    s->bioc = qio_channel_buffer_new(128 * 1024);
> +    s->bioc = qio_channel_buffer_new(512 * 1024);
>      qio_channel_set_name(QIO_CHANNEL(s->bioc), "vmstate-buffer");
>      fb = qemu_fopen_channel_output(QIO_CHANNEL(s->bioc));
>      object_unref(OBJECT(s->bioc));
> @@ -3866,6 +3866,12 @@ static void *bg_migration_thread(void *opaque)
>      if (qemu_savevm_state_complete_precopy_non_iterable(fb, false, false)) {
>          goto fail;
>      }
> +    /*
> +     * Since we are going to get non-iterable state data directly
> +     * from s->bioc->data, explicit flush is needed here.
> +     */
> +    qemu_fflush(fb);
> +
>      /* Now initialize UFFD context and start tracking RAM writes */
>      if (ram_write_tracking_start()) {
>          goto fail;
> -- 
> 2.27.0
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



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

* Re: [PATCH for-6.0 v1 1/4] migration: Fix missing qemu_fflush() on buffer file in bg_migration_thread
  2021-04-01  9:22 ` [PATCH for-6.0 v1 1/4] migration: Fix missing qemu_fflush() on buffer file in bg_migration_thread Andrey Gruzdev
  2021-04-06 12:21   ` Dr. David Alan Gilbert
@ 2021-04-06 12:29   ` Dr. David Alan Gilbert
  2021-04-06 13:20     ` Andrey Gruzdev
  1 sibling, 1 reply; 12+ messages in thread
From: Dr. David Alan Gilbert @ 2021-04-06 12:29 UTC (permalink / raw)
  To: Andrey Gruzdev
  Cc: Juan Quintela, David Hildenbrand, qemu-devel, Peter Xu,
	Markus Armbruster, Paolo Bonzini, Den Lunev

* Andrey Gruzdev (andrey.gruzdev@virtuozzo.com) wrote:
> Added missing qemu_fflush() on buffer file holding precopy device state.
> Increased initial QIOChannelBuffer allocation to 512KB to avoid reallocs.
> Typical configurations often require >200KB for device state and VMDESC.
> 
> Fixes: 8518278a6af589ccc401f06e35f171b1e6fae800 (migration: implementation
>   of background snapshot thread)
> Signed-off-by: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

OK, but in future, please keep separate things separate - the buffer
size increase in particular.

> ---
>  migration/migration.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index ca8b97baa5..00e13f9d58 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3812,7 +3812,7 @@ static void *bg_migration_thread(void *opaque)
>       * with vCPUs running and, finally, write stashed non-RAM part of
>       * the vmstate from the buffer to the migration stream.
>       */
> -    s->bioc = qio_channel_buffer_new(128 * 1024);
> +    s->bioc = qio_channel_buffer_new(512 * 1024);
>      qio_channel_set_name(QIO_CHANNEL(s->bioc), "vmstate-buffer");
>      fb = qemu_fopen_channel_output(QIO_CHANNEL(s->bioc));
>      object_unref(OBJECT(s->bioc));
> @@ -3866,6 +3866,12 @@ static void *bg_migration_thread(void *opaque)
>      if (qemu_savevm_state_complete_precopy_non_iterable(fb, false, false)) {
>          goto fail;
>      }
> +    /*
> +     * Since we are going to get non-iterable state data directly
> +     * from s->bioc->data, explicit flush is needed here.
> +     */
> +    qemu_fflush(fb);
> +
>      /* Now initialize UFFD context and start tracking RAM writes */
>      if (ram_write_tracking_start()) {
>          goto fail;
> -- 
> 2.27.0
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



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

* Re: [PATCH for-6.0 v1 4/4] migration: Rename 'bs' to 'block' in background snapshot code
  2021-04-01  9:22 ` [PATCH for-6.0 v1 4/4] migration: Rename 'bs' to 'block' in background snapshot code Andrey Gruzdev
@ 2021-04-06 12:30   ` Dr. David Alan Gilbert
  2021-04-06 16:52   ` Dr. David Alan Gilbert
  1 sibling, 0 replies; 12+ messages in thread
From: Dr. David Alan Gilbert @ 2021-04-06 12:30 UTC (permalink / raw)
  To: Andrey Gruzdev
  Cc: Juan Quintela, David Hildenbrand, qemu-devel, Peter Xu,
	Markus Armbruster, Paolo Bonzini, Den Lunev

* Andrey Gruzdev (andrey.gruzdev@virtuozzo.com) wrote:
> Rename 'bs' to commonly used 'block' in migration/ram.c background
> snapshot code.
> 
> Signed-off-by: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com>
> Reported-by: David Hildenbrand <david@redhat.com>

Thanks,


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  migration/ram.c | 86 +++++++++++++++++++++++++------------------------
>  1 file changed, 44 insertions(+), 42 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 7e2bc0fdd3..4682f3625c 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1455,7 +1455,7 @@ static RAMBlock *poll_fault_page(RAMState *rs, ram_addr_t *offset)
>  {
>      struct uffd_msg uffd_msg;
>      void *page_address;
> -    RAMBlock *bs;
> +    RAMBlock *block;
>      int res;
>  
>      if (!migrate_background_snapshot()) {
> @@ -1468,9 +1468,9 @@ static RAMBlock *poll_fault_page(RAMState *rs, ram_addr_t *offset)
>      }
>  
>      page_address = (void *)(uintptr_t) uffd_msg.arg.pagefault.address;
> -    bs = qemu_ram_block_from_host(page_address, false, offset);
> -    assert(bs && (bs->flags & RAM_UF_WRITEPROTECT) != 0);
> -    return bs;
> +    block = qemu_ram_block_from_host(page_address, false, offset);
> +    assert(block && (block->flags & RAM_UF_WRITEPROTECT) != 0);
> +    return block;
>  }
>  
>  /**
> @@ -1526,7 +1526,7 @@ bool ram_write_tracking_compatible(void)
>  {
>      const uint64_t uffd_ioctls_mask = BIT(_UFFDIO_WRITEPROTECT);
>      int uffd_fd;
> -    RAMBlock *bs;
> +    RAMBlock *block;
>      bool ret = false;
>  
>      /* Open UFFD file descriptor */
> @@ -1537,15 +1537,15 @@ bool ram_write_tracking_compatible(void)
>  
>      RCU_READ_LOCK_GUARD();
>  
> -    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
> +    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
>          uint64_t uffd_ioctls;
>  
>          /* Nothing to do with read-only and MMIO-writable regions */
> -        if (bs->mr->readonly || bs->mr->rom_device) {
> +        if (block->mr->readonly || block->mr->rom_device) {
>              continue;
>          }
>          /* Try to register block memory via UFFD-IO to track writes */
> -        if (uffd_register_memory(uffd_fd, bs->host, bs->max_length,
> +        if (uffd_register_memory(uffd_fd, block->host, block->max_length,
>                  UFFDIO_REGISTER_MODE_WP, &uffd_ioctls)) {
>              goto out;
>          }
> @@ -1567,13 +1567,13 @@ out:
>   * Since it's solely used for userfault_fd WP feature, here we just
>   *   hardcode page size to qemu_real_host_page_size.
>   *
> - * @bs: RAM block to populate
> + * @block: RAM block to populate
>   */
> -static void ram_block_populate_pages(RAMBlock *bs)
> +static void ram_block_populate_pages(RAMBlock *block)
>  {
> -    char *ptr = (char *) bs->host;
> +    char *ptr = (char *) block->host;
>  
> -    for (ram_addr_t offset = 0; offset < bs->used_length;
> +    for (ram_addr_t offset = 0; offset < block->used_length;
>              offset += qemu_real_host_page_size) {
>          char tmp = *(ptr + offset);
>  
> @@ -1587,13 +1587,13 @@ static void ram_block_populate_pages(RAMBlock *bs)
>   */
>  void ram_write_tracking_prepare(void)
>  {
> -    RAMBlock *bs;
> +    RAMBlock *block;
>  
>      RCU_READ_LOCK_GUARD();
>  
> -    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
> +    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
>          /* Nothing to do with read-only and MMIO-writable regions */
> -        if (bs->mr->readonly || bs->mr->rom_device) {
> +        if (block->mr->readonly || block->mr->rom_device) {
>              continue;
>          }
>  
> @@ -1605,7 +1605,7 @@ void ram_write_tracking_prepare(void)
>           * UFFDIO_WRITEPROTECT_MODE_WP mode setting would silently skip
>           * pages with pte_none() entries in page table.
>           */
> -        ram_block_populate_pages(bs);
> +        ram_block_populate_pages(block);
>      }
>  }
>  
> @@ -1618,7 +1618,7 @@ int ram_write_tracking_start(void)
>  {
>      int uffd_fd;
>      RAMState *rs = ram_state;
> -    RAMBlock *bs;
> +    RAMBlock *block;
>  
>      /* Open UFFD file descriptor */
>      uffd_fd = uffd_create_fd(UFFD_FEATURE_PAGEFAULT_FLAG_WP, true);
> @@ -1629,27 +1629,27 @@ int ram_write_tracking_start(void)
>  
>      RCU_READ_LOCK_GUARD();
>  
> -    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
> +    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
>          /* Nothing to do with read-only and MMIO-writable regions */
> -        if (bs->mr->readonly || bs->mr->rom_device) {
> +        if (block->mr->readonly || block->mr->rom_device) {
>              continue;
>          }
>  
>          /* Register block memory with UFFD to track writes */
> -        if (uffd_register_memory(rs->uffdio_fd, bs->host,
> -                bs->max_length, UFFDIO_REGISTER_MODE_WP, NULL)) {
> +        if (uffd_register_memory(rs->uffdio_fd, block->host,
> +                block->max_length, UFFDIO_REGISTER_MODE_WP, NULL)) {
>              goto fail;
>          }
>          /* Apply UFFD write protection to the block memory range */
> -        if (uffd_change_protection(rs->uffdio_fd, bs->host,
> -                bs->max_length, true, false)) {
> +        if (uffd_change_protection(rs->uffdio_fd, block->host,
> +                block->max_length, true, false)) {
>              goto fail;
>          }
> -        bs->flags |= RAM_UF_WRITEPROTECT;
> -        memory_region_ref(bs->mr);
> +        block->flags |= RAM_UF_WRITEPROTECT;
> +        memory_region_ref(block->mr);
>  
> -        trace_ram_write_tracking_ramblock_start(bs->idstr, bs->page_size,
> -                bs->host, bs->max_length);
> +        trace_ram_write_tracking_ramblock_start(block->idstr, block->page_size,
> +                block->host, block->max_length);
>      }
>  
>      return 0;
> @@ -1657,19 +1657,20 @@ int ram_write_tracking_start(void)
>  fail:
>      error_report("ram_write_tracking_start() failed: restoring initial memory state");
>  
> -    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
> -        if ((bs->flags & RAM_UF_WRITEPROTECT) == 0) {
> +    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
> +        if ((block->flags & RAM_UF_WRITEPROTECT) == 0) {
>              continue;
>          }
>          /*
>           * In case some memory block failed to be write-protected
>           * remove protection and unregister all succeeded RAM blocks
>           */
> -        uffd_change_protection(rs->uffdio_fd, bs->host, bs->max_length, false, false);
> -        uffd_unregister_memory(rs->uffdio_fd, bs->host, bs->max_length);
> +        uffd_change_protection(rs->uffdio_fd, block->host, block->max_length,
> +                false, false);
> +        uffd_unregister_memory(rs->uffdio_fd, block->host, block->max_length);
>          /* Cleanup flags and remove reference */
> -        bs->flags &= ~RAM_UF_WRITEPROTECT;
> -        memory_region_unref(bs->mr);
> +        block->flags &= ~RAM_UF_WRITEPROTECT;
> +        memory_region_unref(block->mr);
>      }
>  
>      uffd_close_fd(uffd_fd);
> @@ -1683,24 +1684,25 @@ fail:
>  void ram_write_tracking_stop(void)
>  {
>      RAMState *rs = ram_state;
> -    RAMBlock *bs;
> +    RAMBlock *block;
>  
>      RCU_READ_LOCK_GUARD();
>  
> -    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
> -        if ((bs->flags & RAM_UF_WRITEPROTECT) == 0) {
> +    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
> +        if ((block->flags & RAM_UF_WRITEPROTECT) == 0) {
>              continue;
>          }
>          /* Remove protection and unregister all affected RAM blocks */
> -        uffd_change_protection(rs->uffdio_fd, bs->host, bs->max_length, false, false);
> -        uffd_unregister_memory(rs->uffdio_fd, bs->host, bs->max_length);
> +        uffd_change_protection(rs->uffdio_fd, block->host, block->max_length,
> +                false, false);
> +        uffd_unregister_memory(rs->uffdio_fd, block->host, block->max_length);
>  
> -        trace_ram_write_tracking_ramblock_stop(bs->idstr, bs->page_size,
> -                bs->host, bs->max_length);
> +        trace_ram_write_tracking_ramblock_stop(block->idstr, block->page_size,
> +                block->host, block->max_length);
>  
>          /* Cleanup flags and remove reference */
> -        bs->flags &= ~RAM_UF_WRITEPROTECT;
> -        memory_region_unref(bs->mr);
> +        block->flags &= ~RAM_UF_WRITEPROTECT;
> +        memory_region_unref(block->mr);
>      }
>  
>      /* Finally close UFFD file descriptor */
> -- 
> 2.27.0
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



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

* Re: [PATCH for-6.0 v1 1/4] migration: Fix missing qemu_fflush() on buffer file in bg_migration_thread
  2021-04-06 12:29   ` Dr. David Alan Gilbert
@ 2021-04-06 13:20     ` Andrey Gruzdev
  0 siblings, 0 replies; 12+ messages in thread
From: Andrey Gruzdev @ 2021-04-06 13:20 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: qemu-devel, Den Lunev, Eric Blake, Paolo Bonzini, Juan Quintela,
	Markus Armbruster, Peter Xu, David Hildenbrand

[-- Attachment #1: Type: text/plain, Size: 2115 bytes --]

On 06.04.2021 15:29, Dr. David Alan Gilbert wrote:
> * Andrey Gruzdev (andrey.gruzdev@virtuozzo.com) wrote:
>> Added missing qemu_fflush() on buffer file holding precopy device state.
>> Increased initial QIOChannelBuffer allocation to 512KB to avoid reallocs.
>> Typical configurations often require >200KB for device state and VMDESC.
>>
>> Fixes: 8518278a6af589ccc401f06e35f171b1e6fae800 (migration: implementation
>>    of background snapshot thread)
>> Signed-off-by: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> OK, but in future, please keep separate things separate - the buffer
> size increase in particular.

Got it, thanks.

>> ---
>>   migration/migration.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index ca8b97baa5..00e13f9d58 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -3812,7 +3812,7 @@ static void *bg_migration_thread(void *opaque)
>>        * with vCPUs running and, finally, write stashed non-RAM part of
>>        * the vmstate from the buffer to the migration stream.
>>        */
>> -    s->bioc = qio_channel_buffer_new(128 * 1024);
>> +    s->bioc = qio_channel_buffer_new(512 * 1024);
>>       qio_channel_set_name(QIO_CHANNEL(s->bioc), "vmstate-buffer");
>>       fb = qemu_fopen_channel_output(QIO_CHANNEL(s->bioc));
>>       object_unref(OBJECT(s->bioc));
>> @@ -3866,6 +3866,12 @@ static void *bg_migration_thread(void *opaque)
>>       if (qemu_savevm_state_complete_precopy_non_iterable(fb, false, false)) {
>>           goto fail;
>>       }
>> +    /*
>> +     * Since we are going to get non-iterable state data directly
>> +     * from s->bioc->data, explicit flush is needed here.
>> +     */
>> +    qemu_fflush(fb);
>> +
>>       /* Now initialize UFFD context and start tracking RAM writes */
>>       if (ram_write_tracking_start()) {
>>           goto fail;
>> -- 
>> 2.27.0
>>
>>


-- 
Andrey Gruzdev, Principal Engineer
Virtuozzo GmbH  +7-903-247-6397
                 virtuzzo.com


[-- Attachment #2: Type: text/html, Size: 3013 bytes --]

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

* Re: [PATCH for-6.0 v1 4/4] migration: Rename 'bs' to 'block' in background snapshot code
  2021-04-01  9:22 ` [PATCH for-6.0 v1 4/4] migration: Rename 'bs' to 'block' in background snapshot code Andrey Gruzdev
  2021-04-06 12:30   ` Dr. David Alan Gilbert
@ 2021-04-06 16:52   ` Dr. David Alan Gilbert
  1 sibling, 0 replies; 12+ messages in thread
From: Dr. David Alan Gilbert @ 2021-04-06 16:52 UTC (permalink / raw)
  To: Andrey Gruzdev
  Cc: Juan Quintela, David Hildenbrand, qemu-devel, Peter Xu,
	Markus Armbruster, Paolo Bonzini, Den Lunev

* Andrey Gruzdev (andrey.gruzdev@virtuozzo.com) wrote:
> Rename 'bs' to commonly used 'block' in migration/ram.c background
> snapshot code.
> 
> Signed-off-by: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com>
> Reported-by: David Hildenbrand <david@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  migration/ram.c | 86 +++++++++++++++++++++++++------------------------
>  1 file changed, 44 insertions(+), 42 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 7e2bc0fdd3..4682f3625c 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1455,7 +1455,7 @@ static RAMBlock *poll_fault_page(RAMState *rs, ram_addr_t *offset)
>  {
>      struct uffd_msg uffd_msg;
>      void *page_address;
> -    RAMBlock *bs;
> +    RAMBlock *block;
>      int res;
>  
>      if (!migrate_background_snapshot()) {
> @@ -1468,9 +1468,9 @@ static RAMBlock *poll_fault_page(RAMState *rs, ram_addr_t *offset)
>      }
>  
>      page_address = (void *)(uintptr_t) uffd_msg.arg.pagefault.address;
> -    bs = qemu_ram_block_from_host(page_address, false, offset);
> -    assert(bs && (bs->flags & RAM_UF_WRITEPROTECT) != 0);
> -    return bs;
> +    block = qemu_ram_block_from_host(page_address, false, offset);
> +    assert(block && (block->flags & RAM_UF_WRITEPROTECT) != 0);
> +    return block;
>  }
>  
>  /**
> @@ -1526,7 +1526,7 @@ bool ram_write_tracking_compatible(void)
>  {
>      const uint64_t uffd_ioctls_mask = BIT(_UFFDIO_WRITEPROTECT);
>      int uffd_fd;
> -    RAMBlock *bs;
> +    RAMBlock *block;
>      bool ret = false;
>  
>      /* Open UFFD file descriptor */
> @@ -1537,15 +1537,15 @@ bool ram_write_tracking_compatible(void)
>  
>      RCU_READ_LOCK_GUARD();
>  
> -    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
> +    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
>          uint64_t uffd_ioctls;
>  
>          /* Nothing to do with read-only and MMIO-writable regions */
> -        if (bs->mr->readonly || bs->mr->rom_device) {
> +        if (block->mr->readonly || block->mr->rom_device) {
>              continue;
>          }
>          /* Try to register block memory via UFFD-IO to track writes */
> -        if (uffd_register_memory(uffd_fd, bs->host, bs->max_length,
> +        if (uffd_register_memory(uffd_fd, block->host, block->max_length,
>                  UFFDIO_REGISTER_MODE_WP, &uffd_ioctls)) {
>              goto out;
>          }
> @@ -1567,13 +1567,13 @@ out:
>   * Since it's solely used for userfault_fd WP feature, here we just
>   *   hardcode page size to qemu_real_host_page_size.
>   *
> - * @bs: RAM block to populate
> + * @block: RAM block to populate
>   */
> -static void ram_block_populate_pages(RAMBlock *bs)
> +static void ram_block_populate_pages(RAMBlock *block)
>  {
> -    char *ptr = (char *) bs->host;
> +    char *ptr = (char *) block->host;
>  
> -    for (ram_addr_t offset = 0; offset < bs->used_length;
> +    for (ram_addr_t offset = 0; offset < block->used_length;
>              offset += qemu_real_host_page_size) {
>          char tmp = *(ptr + offset);
>  
> @@ -1587,13 +1587,13 @@ static void ram_block_populate_pages(RAMBlock *bs)
>   */
>  void ram_write_tracking_prepare(void)
>  {
> -    RAMBlock *bs;
> +    RAMBlock *block;
>  
>      RCU_READ_LOCK_GUARD();
>  
> -    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
> +    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
>          /* Nothing to do with read-only and MMIO-writable regions */
> -        if (bs->mr->readonly || bs->mr->rom_device) {
> +        if (block->mr->readonly || block->mr->rom_device) {
>              continue;
>          }
>  
> @@ -1605,7 +1605,7 @@ void ram_write_tracking_prepare(void)
>           * UFFDIO_WRITEPROTECT_MODE_WP mode setting would silently skip
>           * pages with pte_none() entries in page table.
>           */
> -        ram_block_populate_pages(bs);
> +        ram_block_populate_pages(block);
>      }
>  }
>  
> @@ -1618,7 +1618,7 @@ int ram_write_tracking_start(void)
>  {
>      int uffd_fd;
>      RAMState *rs = ram_state;
> -    RAMBlock *bs;
> +    RAMBlock *block;
>  
>      /* Open UFFD file descriptor */
>      uffd_fd = uffd_create_fd(UFFD_FEATURE_PAGEFAULT_FLAG_WP, true);
> @@ -1629,27 +1629,27 @@ int ram_write_tracking_start(void)
>  
>      RCU_READ_LOCK_GUARD();
>  
> -    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
> +    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
>          /* Nothing to do with read-only and MMIO-writable regions */
> -        if (bs->mr->readonly || bs->mr->rom_device) {
> +        if (block->mr->readonly || block->mr->rom_device) {
>              continue;
>          }
>  
>          /* Register block memory with UFFD to track writes */
> -        if (uffd_register_memory(rs->uffdio_fd, bs->host,
> -                bs->max_length, UFFDIO_REGISTER_MODE_WP, NULL)) {
> +        if (uffd_register_memory(rs->uffdio_fd, block->host,
> +                block->max_length, UFFDIO_REGISTER_MODE_WP, NULL)) {
>              goto fail;
>          }
>          /* Apply UFFD write protection to the block memory range */
> -        if (uffd_change_protection(rs->uffdio_fd, bs->host,
> -                bs->max_length, true, false)) {
> +        if (uffd_change_protection(rs->uffdio_fd, block->host,
> +                block->max_length, true, false)) {
>              goto fail;
>          }
> -        bs->flags |= RAM_UF_WRITEPROTECT;
> -        memory_region_ref(bs->mr);
> +        block->flags |= RAM_UF_WRITEPROTECT;
> +        memory_region_ref(block->mr);
>  
> -        trace_ram_write_tracking_ramblock_start(bs->idstr, bs->page_size,
> -                bs->host, bs->max_length);
> +        trace_ram_write_tracking_ramblock_start(block->idstr, block->page_size,
> +                block->host, block->max_length);
>      }
>  
>      return 0;
> @@ -1657,19 +1657,20 @@ int ram_write_tracking_start(void)
>  fail:
>      error_report("ram_write_tracking_start() failed: restoring initial memory state");
>  
> -    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
> -        if ((bs->flags & RAM_UF_WRITEPROTECT) == 0) {
> +    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
> +        if ((block->flags & RAM_UF_WRITEPROTECT) == 0) {
>              continue;
>          }
>          /*
>           * In case some memory block failed to be write-protected
>           * remove protection and unregister all succeeded RAM blocks
>           */
> -        uffd_change_protection(rs->uffdio_fd, bs->host, bs->max_length, false, false);
> -        uffd_unregister_memory(rs->uffdio_fd, bs->host, bs->max_length);
> +        uffd_change_protection(rs->uffdio_fd, block->host, block->max_length,
> +                false, false);
> +        uffd_unregister_memory(rs->uffdio_fd, block->host, block->max_length);
>          /* Cleanup flags and remove reference */
> -        bs->flags &= ~RAM_UF_WRITEPROTECT;
> -        memory_region_unref(bs->mr);
> +        block->flags &= ~RAM_UF_WRITEPROTECT;
> +        memory_region_unref(block->mr);
>      }
>  
>      uffd_close_fd(uffd_fd);
> @@ -1683,24 +1684,25 @@ fail:
>  void ram_write_tracking_stop(void)
>  {
>      RAMState *rs = ram_state;
> -    RAMBlock *bs;
> +    RAMBlock *block;
>  
>      RCU_READ_LOCK_GUARD();
>  
> -    RAMBLOCK_FOREACH_NOT_IGNORED(bs) {
> -        if ((bs->flags & RAM_UF_WRITEPROTECT) == 0) {
> +    RAMBLOCK_FOREACH_NOT_IGNORED(block) {
> +        if ((block->flags & RAM_UF_WRITEPROTECT) == 0) {
>              continue;
>          }
>          /* Remove protection and unregister all affected RAM blocks */
> -        uffd_change_protection(rs->uffdio_fd, bs->host, bs->max_length, false, false);
> -        uffd_unregister_memory(rs->uffdio_fd, bs->host, bs->max_length);
> +        uffd_change_protection(rs->uffdio_fd, block->host, block->max_length,
> +                false, false);
> +        uffd_unregister_memory(rs->uffdio_fd, block->host, block->max_length);
>  
> -        trace_ram_write_tracking_ramblock_stop(bs->idstr, bs->page_size,
> -                bs->host, bs->max_length);
> +        trace_ram_write_tracking_ramblock_stop(block->idstr, block->page_size,
> +                block->host, block->max_length);
>  
>          /* Cleanup flags and remove reference */
> -        bs->flags &= ~RAM_UF_WRITEPROTECT;
> -        memory_region_unref(bs->mr);
> +        block->flags &= ~RAM_UF_WRITEPROTECT;
> +        memory_region_unref(block->mr);
>      }
>  
>      /* Finally close UFFD file descriptor */
> -- 
> 2.27.0
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



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

* Re: [PATCH for-6.0 v1 0/4] migration: Fixes to the 'background-snapshot' code
  2021-04-01  9:22 [PATCH for-6.0 v1 0/4] migration: Fixes to the 'background-snapshot' code Andrey Gruzdev
                   ` (3 preceding siblings ...)
  2021-04-01  9:22 ` [PATCH for-6.0 v1 4/4] migration: Rename 'bs' to 'block' in background snapshot code Andrey Gruzdev
@ 2021-04-06 16:53 ` Dr. David Alan Gilbert
  2021-04-06 17:35   ` Andrey Gruzdev
  4 siblings, 1 reply; 12+ messages in thread
From: Dr. David Alan Gilbert @ 2021-04-06 16:53 UTC (permalink / raw)
  To: Andrey Gruzdev
  Cc: Juan Quintela, David Hildenbrand, qemu-devel, Peter Xu,
	Markus Armbruster, Paolo Bonzini, Den Lunev

* Andrey Gruzdev (andrey.gruzdev@virtuozzo.com) wrote:
> Changes v0->v1:
>  * Fixes to coding style and commit messages
>  * Renamed 'bs' to 'block' in migration/ram.c background snapshot code
> 
> This patch series contains:
>  * Fix to the issue with occasionally truncated non-iterable device state
>  * Solution to compatibility issues with virtio-balloon device
>  * Fix to the issue when discarded or never populated pages miss UFFD
>    write protection and get into migration stream in dirty state
>  * Renaming of 'bs' to commonly used 'block' in migration/ram.c background
>    snapshot code
> 
> Andrey Gruzdev (4):
>   migration: Fix missing qemu_fflush() on buffer file in
>     bg_migration_thread
>   migration: Inhibit virtio-balloon for the duration of background
>     snapshot
>   migration: Pre-fault memory before starting background snasphot
>   migration: Rename 'bs' to 'block' in background snapshot code
> 
>  hw/virtio/virtio-balloon.c |   8 ++-
>  include/migration/misc.h   |   2 +
>  migration/migration.c      |  22 ++++++-
>  migration/ram.c            | 119 ++++++++++++++++++++++++++-----------
>  migration/ram.h            |   1 +
>  5 files changed, 115 insertions(+), 37 deletions(-)

Queued

> -- 
> 2.27.0
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



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

* Re: [PATCH for-6.0 v1 0/4] migration: Fixes to the 'background-snapshot' code
  2021-04-06 16:53 ` [PATCH for-6.0 v1 0/4] migration: Fixes to the 'background-snapshot' code Dr. David Alan Gilbert
@ 2021-04-06 17:35   ` Andrey Gruzdev
  0 siblings, 0 replies; 12+ messages in thread
From: Andrey Gruzdev @ 2021-04-06 17:35 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: qemu-devel, Den Lunev, Eric Blake, Paolo Bonzini, Juan Quintela,
	Markus Armbruster, Peter Xu, David Hildenbrand

[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]

On 06.04.2021 19:53, Dr. David Alan Gilbert wrote:
> * Andrey Gruzdev (andrey.gruzdev@virtuozzo.com) wrote:
>> Changes v0->v1:
>>   * Fixes to coding style and commit messages
>>   * Renamed 'bs' to 'block' in migration/ram.c background snapshot code
>>
>> This patch series contains:
>>   * Fix to the issue with occasionally truncated non-iterable device state
>>   * Solution to compatibility issues with virtio-balloon device
>>   * Fix to the issue when discarded or never populated pages miss UFFD
>>     write protection and get into migration stream in dirty state
>>   * Renaming of 'bs' to commonly used 'block' in migration/ram.c background
>>     snapshot code
>>
>> Andrey Gruzdev (4):
>>    migration: Fix missing qemu_fflush() on buffer file in
>>      bg_migration_thread
>>    migration: Inhibit virtio-balloon for the duration of background
>>      snapshot
>>    migration: Pre-fault memory before starting background snasphot
>>    migration: Rename 'bs' to 'block' in background snapshot code
>>
>>   hw/virtio/virtio-balloon.c |   8 ++-
>>   include/migration/misc.h   |   2 +
>>   migration/migration.c      |  22 ++++++-
>>   migration/ram.c            | 119 ++++++++++++++++++++++++++-----------
>>   migration/ram.h            |   1 +
>>   5 files changed, 115 insertions(+), 37 deletions(-)
> Queued

Thanks!

>> -- 
>> 2.27.0
>>
>>


-- 
Andrey Gruzdev, Principal Engineer
Virtuozzo GmbH  +7-903-247-6397
                 virtuzzo.com


[-- Attachment #2: Type: text/html, Size: 2202 bytes --]

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

end of thread, other threads:[~2021-04-06 17:37 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-01  9:22 [PATCH for-6.0 v1 0/4] migration: Fixes to the 'background-snapshot' code Andrey Gruzdev
2021-04-01  9:22 ` [PATCH for-6.0 v1 1/4] migration: Fix missing qemu_fflush() on buffer file in bg_migration_thread Andrey Gruzdev
2021-04-06 12:21   ` Dr. David Alan Gilbert
2021-04-06 12:29   ` Dr. David Alan Gilbert
2021-04-06 13:20     ` Andrey Gruzdev
2021-04-01  9:22 ` [PATCH for-6.0 v1 2/4] migration: Inhibit virtio-balloon for the duration of background snapshot Andrey Gruzdev
2021-04-01  9:22 ` [PATCH for-6.0 v1 3/4] migration: Pre-fault memory before starting background snasphot Andrey Gruzdev
2021-04-01  9:22 ` [PATCH for-6.0 v1 4/4] migration: Rename 'bs' to 'block' in background snapshot code Andrey Gruzdev
2021-04-06 12:30   ` Dr. David Alan Gilbert
2021-04-06 16:52   ` Dr. David Alan Gilbert
2021-04-06 16:53 ` [PATCH for-6.0 v1 0/4] migration: Fixes to the 'background-snapshot' code Dr. David Alan Gilbert
2021-04-06 17:35   ` Andrey Gruzdev

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).