[Qemu-devel] [PULL 0/8] migration queue

[Qemu-devel] [PULL 0/8] migration queue

[Dr. David Alan Gilbert (git)]

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

The following changes since commit b07d1c2f5607489d4d4a6a65ce36a3e896ac065e:

  Revert "kvm: use DIV_ROUND_UP" (2017-09-05 18:55:40 +0100)

are available in the git repository at:

  git://github.com/dagrh/qemu.git tags/pull-migration-20170906a

for you to fetch changes up to a31fedeed764ce0b0d6097d4334c5770e74641a0:

  migration: dump str in migrate_set_state trace (2017-09-06 16:36:38 +0100)

----------------------------------------------------------------
migration pull 2017-09-06

----------------------------------------------------------------
Dr. David Alan Gilbert (4):
      migration: Report when bdrv_inactivate_all fails
      runstate/migrate: Two more transitions
      migration: Reset rather than destroy main_thread_load_event
      snapshot/tests: Try loadvm twice

Markus Armbruster (3):
      xbzrle: Drop unused cache_resize()
      host-utils: Proactively fix pow2floor(), switch to unsigned
      host-utils: Simplify pow2ceil()

Peter Xu (1):
      migration: dump str in migrate_set_state trace

 include/qemu/host-utils.h  | 36 +++++++++++++++++------------
 migration/migration.c      |  5 +++--
 migration/page_cache.c     | 56 ----------------------------------------------
 migration/page_cache.h     | 11 ---------
 migration/savevm.c         |  2 ++
 migration/trace-events     |  2 +-
 tests/qemu-iotests/068     |  2 +-
 tests/qemu-iotests/068.out |  4 ++++
 vl.c                       |  2 ++
 9 files changed, 35 insertions(+), 85 deletions(-)

[Qemu-devel] [PULL 1/8] migration: Report when bdrv_inactivate_all fails

[Dr. David Alan Gilbert (git)]

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

If the bdrv_inactivate_all fails near the end of the migration,
the migration will fail and often the only diagnostics in the log
are an I/O error which you can't distinguish from an error on
the socket connection.

Add an error so we know when it's actually a block problem.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20170822170212.27347-1-dgilbert@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/savevm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/migration/savevm.c b/migration/savevm.c
index fdd15fa0a7..7a55023d1a 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1157,6 +1157,8 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only,
          * bdrv_invalidate_cache_all() on the other end won't fail. */
         ret = bdrv_inactivate_all();
         if (ret) {
+            error_report("%s: bdrv_inactivate_all() failed (%d)",
+                         __func__, ret);
             qemu_file_set_error(f, ret);
             return ret;
         }
-- 
2.13.5

[Qemu-devel] [PULL 2/8] xbzrle: Drop unused cache_resize()

[Dr. David Alan Gilbert (git)]

From: Markus Armbruster <armbru@redhat.com>

Unused since commit fd8cec XBZRLE: Fix qemu crash when resize the
xbzrle cache.

Cc: Juan Quintela <quintela@redhat.com>
Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1501148776-16890-2-git-send-email-armbru@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/page_cache.c | 56 --------------------------------------------------
 migration/page_cache.h | 11 ----------
 2 files changed, 67 deletions(-)

diff --git a/migration/page_cache.c b/migration/page_cache.c
index 5f8578736e..ba984c4858 100644
--- a/migration/page_cache.c
+++ b/migration/page_cache.c
@@ -178,59 +178,3 @@ int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata,
 
     return 0;
 }
-
-int64_t cache_resize(PageCache *cache, int64_t new_num_pages)
-{
-    PageCache *new_cache;
-    int64_t i;
-
-    CacheItem *old_it, *new_it;
-
-    g_assert(cache);
-
-    /* cache was not inited */
-    if (cache->page_cache == NULL) {
-        return -1;
-    }
-
-    /* same size */
-    if (pow2floor(new_num_pages) == cache->max_num_items) {
-        return cache->max_num_items;
-    }
-
-    new_cache = cache_init(new_num_pages, cache->page_size);
-    if (!(new_cache)) {
-        DPRINTF("Error creating new cache\n");
-        return -1;
-    }
-
-    /* move all data from old cache */
-    for (i = 0; i < cache->max_num_items; i++) {
-        old_it = &cache->page_cache[i];
-        if (old_it->it_addr != -1) {
-            /* check for collision, if there is, keep MRU page */
-            new_it = cache_get_by_addr(new_cache, old_it->it_addr);
-            if (new_it->it_data && new_it->it_age >= old_it->it_age) {
-                /* keep the MRU page */
-                g_free(old_it->it_data);
-            } else {
-                if (!new_it->it_data) {
-                    new_cache->num_items++;
-                }
-                g_free(new_it->it_data);
-                new_it->it_data = old_it->it_data;
-                new_it->it_age = old_it->it_age;
-                new_it->it_addr = old_it->it_addr;
-            }
-        }
-    }
-
-    g_free(cache->page_cache);
-    cache->page_cache = new_cache->page_cache;
-    cache->max_num_items = new_cache->max_num_items;
-    cache->num_items = new_cache->num_items;
-
-    g_free(new_cache);
-
-    return cache->max_num_items;
-}
diff --git a/migration/page_cache.h b/migration/page_cache.h
index 10ed53274c..4fadd0c501 100644
--- a/migration/page_cache.h
+++ b/migration/page_cache.h
@@ -72,15 +72,4 @@ uint8_t *get_cached_data(const PageCache *cache, uint64_t addr);
 int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata,
                  uint64_t current_age);
 
-/**
- * cache_resize: resize the page cache. In case of size reduction the extra
- * pages will be freed
- *
- * Returns -1 on error new cache size on success
- *
- * @cache pointer to the PageCache struct
- * @num_pages: new page cache size (in pages)
- */
-int64_t cache_resize(PageCache *cache, int64_t num_pages);
-
 #endif
-- 
2.13.5

[Qemu-devel] [PULL 3/8] host-utils: Proactively fix pow2floor(), switch to unsigned

[Dr. David Alan Gilbert (git)]

From: Markus Armbruster <armbru@redhat.com>

The function's stated contract is simple enough: "round down to the
nearest power of 2".  Suggests the domain is the representable numbers
>= 1, because that's the smallest power of two.

The implementation doesn't check for domain errors, but returns
garbage instead:

* For negative arguments, pow2floor() returns -2^63, which is not even
  a power of two, let alone the nearest one.

  What sort of works is passing *unsigned* arguments >= 2^63.  The
  implicit conversion to signed is implementation defined, but
  commonly yields the (negative) two's complement.  pow2floor() then
  returns -2^63.  Callers that convert that back to unsigned get the
  correct value 2^63.

* For a zero argument, pow2floor() shifts right by 64.  Undefined
  behavior.  Common actual behavior is to shift by 0, yielding -2^63.

Fix by switching from int64_t to uint64_t and amending the contract to
map zero to zero.

Callers are fine with that:

* memory_access_size()

  This function makes no sense unless the argument is positive and the
  return value fits into int.

* raw_refresh_limits()

  Passes an int between 1 and BDRV_REQUEST_MAX_BYTES.

* iscsi_refresh_limits()

  Passes an integer between 0 and INT_MAX, converts the result to
  uint32_t.  Passing zero would be undefined behavior, but commonly
  yield zero.  The patch gives us the zero without the undefined
  behavior.

* cache_init()

  Passes a positive int64_t argument.

* xbzrle_cache_resize()

  Passes a positive int64_t argument (>= TARGET_PAGE_SIZE, actually).

* spapr_node0_size()

  Passes a positive uint64_t argument, and converts the result to
  hwaddr, i.e. uint64_t.

* spapr_populate_memory()

  Passes a positive hwaddr argument, and converts the result to
  hwaddr.

Cc: Juan Quintela <quintela@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1501148776-16890-3-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/qemu/host-utils.h | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h
index 95cf4f4163..6c6005f5cf 100644
--- a/include/qemu/host-utils.h
+++ b/include/qemu/host-utils.h
@@ -369,13 +369,16 @@ static inline bool is_power_of_2(uint64_t value)
     return !(value & (value - 1));
 }
 
-/* round down to the nearest power of 2*/
-static inline int64_t pow2floor(int64_t value)
+/**
+ * Return @value rounded down to the nearest power of two or zero.
+ */
+static inline uint64_t pow2floor(uint64_t value)
 {
-    if (!is_power_of_2(value)) {
-        value = 0x8000000000000000ULL >> clz64(value);
+    if (!value) {
+        /* Avoid undefined shift by 64 */
+        return 0;
     }
-    return value;
+    return 0x8000000000000000ull >> clz64(value);
 }
 
 /* round up to the nearest power of 2 (0 if overflow) */
-- 
2.13.5

[Qemu-devel] [PULL 4/8] host-utils: Simplify pow2ceil()

[Dr. David Alan Gilbert (git)]

From: Markus Armbruster <armbru@redhat.com>

Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1501148776-16890-4-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/qemu/host-utils.h | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h
index 6c6005f5cf..5ac621cf1f 100644
--- a/include/qemu/host-utils.h
+++ b/include/qemu/host-utils.h
@@ -381,18 +381,23 @@ static inline uint64_t pow2floor(uint64_t value)
     return 0x8000000000000000ull >> clz64(value);
 }
 
-/* round up to the nearest power of 2 (0 if overflow) */
+/*
+ * Return @value rounded up to the nearest power of two modulo 2^64.
+ * This is *zero* for @value > 2^63, so be careful.
+ */
 static inline uint64_t pow2ceil(uint64_t value)
 {
-    uint8_t nlz = clz64(value);
-
-    if (is_power_of_2(value)) {
-        return value;
-    }
-    if (!nlz) {
-        return 0;
+    int n = clz64(value - 1);
+
+    if (!n) {
+        /*
+         * @value - 1 has no leading zeroes, thus @value - 1 >= 2^63
+         * Therefore, either @value == 0 or @value > 2^63.
+         * If it's 0, return 1, else return 0.
+         */
+        return !value;
     }
-    return 1ULL << (64 - nlz);
+    return 0x8000000000000000ull >> (n - 1);
 }
 
 /**
-- 
2.13.5

[Qemu-devel] [PULL 5/8] runstate/migrate: Two more transitions

[Dr. David Alan Gilbert (git)]

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

There's a race if someone does a 'stop' near the end of migrate;
the migration process goes through two runstates:
    'finish migrate'
    'postmigrate'

If the user issues a 'stop' between the two we end up with invalid
state transitions.
Add the transitions as valid.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20170804175011.21944-1-dgilbert@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 vl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/vl.c b/vl.c
index e75757f977..fb1f05b937 100644
--- a/vl.c
+++ b/vl.c
@@ -621,6 +621,7 @@ static const RunStateTransition runstate_transitions_def[] = {
 
     { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
     { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
+    { RUN_STATE_PAUSED, RUN_STATE_POSTMIGRATE },
     { RUN_STATE_PAUSED, RUN_STATE_PRELAUNCH },
     { RUN_STATE_PAUSED, RUN_STATE_COLO},
 
@@ -633,6 +634,7 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
 
     { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
+    { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PAUSED },
     { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
     { RUN_STATE_FINISH_MIGRATE, RUN_STATE_PRELAUNCH },
     { RUN_STATE_FINISH_MIGRATE, RUN_STATE_COLO},
-- 
2.13.5

[Qemu-devel] [PULL 6/8] migration: Reset rather than destroy main_thread_load_event

[Dr. David Alan Gilbert (git)]

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

migration_incoming_state_destroy doesn't really destroy, it cleans up.
After a loadvm it's called, but the loadvm command can be run twice,
and so destroying an init-once mutex breaks on the second loadvm.

Reported-by: Stafford Horne <shorne@gmail.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20170825141940.20740-2-dgilbert@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Tested-by: Stafford Horne <shorne@gmail.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 1a2b3ebd1a..9838ccc885 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -166,7 +166,7 @@ void migration_incoming_state_destroy(void)
         mis->from_src_file = NULL;
     }
 
-    qemu_event_destroy(&mis->main_thread_load_event);
+    qemu_event_reset(&mis->main_thread_load_event);
 }
 
 static void migrate_generate_event(int new_state)
-- 
2.13.5

[Qemu-devel] [PULL 7/8] snapshot/tests: Try loadvm twice

[Dr. David Alan Gilbert (git)]

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

It's legal to loadvm twice, modify the existing save/loadvm test
to do it twice.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20170825141940.20740-3-dgilbert@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tests/qemu-iotests/068     | 2 +-
 tests/qemu-iotests/068.out | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
index cfa0f2aed5..e7fca6a494 100755
--- a/tests/qemu-iotests/068
+++ b/tests/qemu-iotests/068
@@ -78,7 +78,7 @@ for extra_args in \
     # Give qemu some time to boot before saving the VM state
     { sleep 1; printf "savevm 0\nquit\n"; } | _qemu $extra_args
     # Now try to continue from that VM state (this should just work)
-    echo quit | _qemu $extra_args -loadvm 0
+    { sleep 1; printf "loadvm 0\nloadvm 0\nquit\n"; } | _qemu $extra_args -S
 done
 
 # success, all done
diff --git a/tests/qemu-iotests/068.out b/tests/qemu-iotests/068.out
index aa063cf711..f07a938a38 100644
--- a/tests/qemu-iotests/068.out
+++ b/tests/qemu-iotests/068.out
@@ -7,6 +7,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm 0
 (qemu) quit
 QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) loadvm 0
+(qemu) loadvm 0
 (qemu) quit
 
 === Saving and reloading a VM state to/from a qcow2 image (-object iothread,id=iothread0 -set device.hba0.iothread=iothread0) ===
@@ -16,5 +18,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) savevm 0
 (qemu) quit
 QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) loadvm 0
+(qemu) loadvm 0
 (qemu) quit
 *** done
-- 
2.13.5

[Qemu-devel] [PULL 8/8] migration: dump str in migrate_set_state trace

[Dr. David Alan Gilbert (git)]

From: Peter Xu <peterx@redhat.com>

Strings are more readable for debugging.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <1504081950-2528-5-git-send-email-peterx@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
  Fixed up merge with 977c73
---
 migration/migration.c  | 3 ++-
 migration/trace-events | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 9838ccc885..959e8ec88e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -913,8 +913,9 @@ void qmp_migrate_start_postcopy(Error **errp)
 
 void migrate_set_state(int *state, int old_state, int new_state)
 {
+    assert(new_state < MIGRATION_STATUS__MAX);
     if (atomic_cmpxchg(state, old_state, new_state) == old_state) {
-        trace_migrate_set_state(new_state);
+        trace_migrate_set_state(MigrationStatus_str(new_state));
         migrate_generate_event(new_state);
     }
 }
diff --git a/migration/trace-events b/migration/trace-events
index 7a3b5144ff..d2910a6e7b 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -80,7 +80,7 @@ ram_save_queue_pages(const char *rbname, size_t start, size_t len) "%s: start: 0
 # migration/migration.c
 await_return_path_close_on_source_close(void) ""
 await_return_path_close_on_source_joining(void) ""
-migrate_set_state(int new_state) "new state %d"
+migrate_set_state(const char *new_state) "new state %s"
 migrate_fd_cleanup(void) ""
 migrate_fd_error(const char *error_desc) "error=%s"
 migrate_fd_cancel(void) ""
-- 
2.13.5

Re: [Qemu-devel] [PULL 0/8] migration queue

[Peter Maydell]

On 6 September 2017 at 19:41, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The following changes since commit b07d1c2f5607489d4d4a6a65ce36a3e896ac065e:
>
>   Revert "kvm: use DIV_ROUND_UP" (2017-09-05 18:55:40 +0100)
>
> are available in the git repository at:
>
>   git://github.com/dagrh/qemu.git tags/pull-migration-20170906a
>
> for you to fetch changes up to a31fedeed764ce0b0d6097d4334c5770e74641a0:
>
>   migration: dump str in migrate_set_state trace (2017-09-06 16:36:38 +0100)
>
> ----------------------------------------------------------------
> migration pull 2017-09-06
>

Applied, thanks.

-- PMM

[Qemu-devel] [PULL 0/8] migration queue

[Juan Quintela]

Hi

This includes:
- Peter changes to make VMSTATE_STRUCT_POINTER more consistent
- Fix migration with hpratio (ppc on ppc64 basically)
- Orit/Arei cleanups/fixes to xbzrle

Thanks, please apply.


The following changes since commit 8cfc114a2f293c40077d1bdb7500b29db359ca22:

  linux-user: Fix trampoline code for CRIS (2014-02-03 14:04:00 +0000)

are available in the git repository at:

  git://github.com/juanquintela/qemu.git tags/migration/20140204

for you to fetch changes up to 2c429c2e4540ff74f2396b41e34371bd97c9337f:

  exec: fix ram_list dirty map optimization (2014-02-04 15:52:31 +0100)

----------------------------------------------------------------
migration/next for 20140204

----------------------------------------------------------------
Alexey Kardashevskiy (1):
      exec: fix ram_list dirty map optimization

Gonglei (Arei) (1):
      migration:fix free XBZRLE decoded_buf wrong

Orit Wasserman (5):
      Set xbzrle buffers to NULL after freeing them to avoid double free errors
      Add check for cache size smaller than page size
      XBZRLE cache size should not be larger than guest memory size
      Don't abort on out of memory when creating page cache
      Don't abort on memory allocation error

Peter Maydell (1):
      vmstate: Make VMSTATE_STRUCT_POINTER take type, not ptr-to-type

 arch_init.c                    | 47 +++++++++++++++++++++++++++++++-----------
 hw/arm/pxa2xx.c                |  2 +-
 include/exec/ram_addr.h        |  3 ++-
 include/hw/ptimer.h            | 10 ++-------
 include/migration/migration.h  |  1 +
 include/migration/page_cache.h |  4 +++-
 include/migration/vmstate.h    |  8 +++----
 migration.c                    | 18 +++++++++++++++-
 page_cache.c                   | 34 ++++++++++++++++++++++--------
 9 files changed, 90 insertions(+), 37 deletions(-)

Re: [Qemu-devel] [PULL 0/8] migration queue

[Juan Quintela]

Juan Quintela <quintela@redhat.com> wrote:
> Hi
>
> This includes:
> - Peter changes to make VMSTATE_STRUCT_POINTER more consistent
> - Fix migration with hpratio (ppc on ppc64 basically)
> - Orit/Arei cleanups/fixes to xbzrle
>
> Thanks, please apply.

NACK myself.  It has the wrong patch series from Orit.

Sorry, and resending a new one.

Later, Juan.