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

[Yury Kotov]

25.03.2019, 14:58, "Juan Quintela" <address@hidden>:
> 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).
>

Sorry, what patches do you mean? It seems that I missed them...

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

Agreed. I also had such idea and I did a quick look on this, but din't found how
to do it without some refactoring.

> 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;

Regards,
Yury