Re: [Qemu-devel] [PATCH] hostmem: Disable add/del memory during migration

[Juan Quintela]

Yury Kotov <address@hidden> wrote:
> I found a bug in QEMU 2.12 with adding memory-backend while live migration
> thread is running.
>
> But it seems that this bug was implicitly fixed in this commit (QEMU 3.0):
>   b895de50: migration: discard non-migratable RAMBlocks
>
> I think it's better to disallow add/del memory backends during migration to
> to prevent other possible problems. Anyway, user can't use this memory because
> of disabled hotplug/hotunplug devs.

Hi

My understanding is that we already disable memory hotplug/unplug during
migration. At least, the idea of those patches was to disable all
hotplug/unplug during migration.  The only reason that I can think for
using this patch is if anyone is planning about support some
hotplug/upplug during migration (to my knowledge, nobody is working on
that).

So, I think it is better to just disallow on high level all hoplug
operations, instead of in all backends.

But will wait to see what everybody else think about it.

BTW,   if we plan to ship this for an old qemu, I will try to disable
hotplug for all devices, not only memory, no?

Later, Juan.

> The idea of this commit is the same as that:
>   b06424de: migration: Disable hotplug/unplug memory during migration
>
> Backtrace of this bug in QEMU 2.12:
> 0  find_next_bit (address@hidden, address@hidden, address@hidden) at 
> util/bitops.c:46
> 1  migration_bitmap_find_dirty (rs=0x7f58f80008c0, start=0, 
> rb=0x5557e66e3200) at migration/ram.c:816
> 2  find_dirty_block (again=<synthetic pointer>, pss=<synthetic pointer>, 
> rs=0x7f58f80008c0) at migration/ram.c:1243
> 3  ram_find_and_save_block (address@hidden, address@hidden) at 
> migration/ram.c:1592
> 4  ram_find_and_save_block (last_stage=false, rs=0x7f58f80008c0) at 
> migration/ram.c:2335
> 5  ram_save_iterate (f=0x5557e69f1000, opaque=<optimized out>) at 
> migration/ram.c:2338
> 6  qemu_savevm_state_iterate (f=0x5557e69f1000, postcopy=false) at 
> migration/savevm.c:1191
> 7  migration_iteration_run (s=0x5557e666b030) at migration/migration.c:2301
> 8  migration_thread (opaque=0x5557e666b030) at migration/migration.c:2409
> 9  start_thread (arg=0x7f59055d5700) at pthread_create.c:333
> 10 clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
>
> Signed-off-by: Yury Kotov <address@hidden>
> ---
>  backends/hostmem.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index f61093654e..5c71bd3f6b 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -18,6 +18,7 @@
>  #include "qapi/visitor.h"
>  #include "qemu/config-file.h"
>  #include "qom/object_interfaces.h"
> +#include "migration/misc.h"
>  
>  #ifdef CONFIG_NUMA
>  #include <numaif.h>
> @@ -271,6 +272,11 @@ host_memory_backend_memory_complete(UserCreatable *uc, 
> Error **errp)
>      void *ptr;
>      uint64_t sz;
>  
> +    if (!migration_is_idle()) {
> +        error_setg(errp, "Adding memory-backend isn't allowed while 
> migrating");
> +        goto out;
> +    }
> +
>      if (bc->alloc) {
>          bc->alloc(backend, &local_err);
>          if (local_err) {
> @@ -344,7 +350,8 @@ out:
>  static bool
>  host_memory_backend_can_be_deleted(UserCreatable *uc)
>  {
> -    if (host_memory_backend_is_mapped(MEMORY_BACKEND(uc))) {
> +    if (host_memory_backend_is_mapped(MEMORY_BACKEND(uc)) ||
> +        !migration_is_idle()) {
>          return false;
>      } else {
>          return true;