qemu-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-commits] [qemu/qemu] 772c62: migration: comment VMSTATE_UNUSED*()


From: Peter Maydell
Subject: [Qemu-commits] [qemu/qemu] 772c62: migration: comment VMSTATE_UNUSED*() properly
Date: Thu, 16 May 2019 04:54:23 -0700

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 772c6212d2bcdb6adcb2a0d906745b42903b2ecf
      
https://github.com/qemu/qemu/commit/772c6212d2bcdb6adcb2a0d906745b42903b2ecf
  Author: Peter Xu <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M include/migration/vmstate.h

  Log Message:
  -----------
  migration: comment VMSTATE_UNUSED*() properly

It is error prone to use VMSTATE_UNUSED*() sometimes especially when
the size of the migration stream of the field is not the same as the
size of the structure (boolean is one example).  Comment it well so
people will be aware of this when people want to use it.

Signed-off-by: Peter Xu <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>


  Commit: a94cd7b8abda18e1c8ef9a62bf0576d2d294f568
      
https://github.com/qemu/qemu/commit/a94cd7b8abda18e1c8ef9a62bf0576d2d294f568
  Author: Wei Yang <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M migration/savevm.c

  Log Message:
  -----------
  migration: not necessary to check ops again

During each iteration, se->ops is checked before each loop. So it is not
necessary to check it again and simplify the following check a little.

Signed-off-by: Wei Yang <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>


  Commit: 15d2d64cf58dcf666a07a695c0c4ec307da8f2ae
      
https://github.com/qemu/qemu/commit/15d2d64cf58dcf666a07a695c0c4ec307da8f2ae
  Author: Wei Yang <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M migration/migration.c
    M migration/migration.h

  Log Message:
  -----------
  migration: remove not used field xfer_limit

MigrationState->xfer_limit is only set to 0 in migrate_init().

Remove this unnecessary field.

Signed-off-by: Wei Yang <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>


  Commit: f2dd7eddf240fe77ab08ffb251f1abca79f9463a
      
https://github.com/qemu/qemu/commit/f2dd7eddf240fe77ab08ffb251f1abca79f9463a
  Author: Wei Yang <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M migration/vmstate.c

  Log Message:
  -----------
  vmstate: check subsection_found is enough

subsection_found is true implies vmdesc is not NULL.

This patch remove the additional check on vmdesc and rename
subsection_found to vmdesc_has_subsections to make it more self-explain.

Signed-off-by: Wei Yang <address@hidden>

Message-Id: <address@hidden>
Acked-by: Stefano Garzarella <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>


  Commit: aded9dfa74a83f8b235df1bd3a874e48e197c78e
      
https://github.com/qemu/qemu/commit/aded9dfa74a83f8b235df1bd3a874e48e197c78e
  Author: Cole Robinson <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M migration/savevm.c

  Log Message:
  -----------
  migration: savevm: fix error code with migration blockers

The only caller that checks the error code is looking for != 0,
so returning false is incorrect.

Fixes: 5aaac467938 "migration: savevm: consult migration blockers"

Signed-off-by: Cole Robinson <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Reviewed-by: Juan Quintela <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>


  Commit: c0913d1dfd0456a54f412a63c60659b72eb7093b
      
https://github.com/qemu/qemu/commit/c0913d1dfd0456a54f412a63c60659b72eb7093b
  Author: Zhang Chen <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M include/migration/colo.h
    M migration/colo-failover.c
    M migration/colo.c

  Log Message:
  -----------
  migration/colo.c: Remove redundant input parameter

The colo_do_failover no need the input parameter.

Signed-off-by: Zhang Chen <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>


  Commit: 9c16abcb9264e62e4c24875a2208fba90032b789
      
https://github.com/qemu/qemu/commit/9c16abcb9264e62e4c24875a2208fba90032b789
  Author: Zhang Chen <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M include/migration/colo.h

  Log Message:
  -----------
  migration/colo.h: Remove obsolete codes

Signed-off-by: Zhang Chen <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>


  Commit: 5aede7f4c71b06d5691d702f05f2c8d995e35f3c
      
https://github.com/qemu/qemu/commit/5aede7f4c71b06d5691d702f05f2c8d995e35f3c
  Author: Zhang Chen <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M qemu-options.hx

  Log Message:
  -----------
  qemu-option.hx: Update missed parameter for colo-compare

We missed the iothread related args in this file.
This patch is used to fix this issue.

Signed-off-by: Zhang Chen <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>


  Commit: bf21297923893250bdc594e22ce0176efa6ba1b8
      
https://github.com/qemu/qemu/commit/bf21297923893250bdc594e22ce0176efa6ba1b8
  Author: Wei Yang <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M migration/ram.c

  Log Message:
  -----------
  migration/ram.c: start of migration_bitmap_sync_range is always 0

We can eliminate to pass 0.

Signed-off-by: Wei Yang <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>


  Commit: 4633456ced3b0ecf63048e79c91cf9d73e7246b6
      
https://github.com/qemu/qemu/commit/4633456ced3b0ecf63048e79c91cf9d73e7246b6
  Author: Yi Wang <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M migration/ram.c

  Log Message:
  -----------
  migration: update comments of migration bitmap

Since the ram bitmap and the unsent bitmap are split by RAMBlock
in commit 6b6712e, it's better to update the comments about them.

Signed-off-by: Yi Wang <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>


  Commit: 5351e69af826665cef6858c960c3336a4228bb70
      
https://github.com/qemu/qemu/commit/5351e69af826665cef6858c960c3336a4228bb70
  Author: Wei Yang <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M migration/savevm.c

  Log Message:
  -----------
  migration/savevm: remove duplicate check of migration_is_blocked

Current call flow of save_snapshot is:

  save_snapshot
    migration_is_blocked
      qemu_savevm_state
        migration_is_blocked

Since qemu_savevm_state is only called in save_snapshot, this means
migration_is_blocked has been already checked.

Signed-off-by: Wei Yang <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>


  Commit: 9e14b849082755c80efe59d7a4e5a77b5ac24877
      
https://github.com/qemu/qemu/commit/9e14b849082755c80efe59d7a4e5a77b5ac24877
  Author: Wei Yang <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M migration/savevm.c

  Log Message:
  -----------
  migration/savevm: load_header before load_setup

In migration_thread() and qemu_savevm_state(), we savevm_state in
following sequence:

    qemu_savevm_state_header(f);
    qemu_savevm_state_setup(f);

Then it would be more proper to loadvm_state in the save sequence.

Signed-off-by: Wei Yang <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>


  Commit: 16015d32e4715d187d2d38561cdcd22f3a0294c0
      
https://github.com/qemu/qemu/commit/16015d32e4715d187d2d38561cdcd22f3a0294c0
  Author: Wei Yang <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M migration/savevm.c

  Log Message:
  -----------
  migration/savevm: wrap into qemu_loadvm_state_header()

On source side, we have qemu_savevm_state_header() to send related data,
while on the receiving side those steps are scattered in
qemu_loadvm_state().

This patch wrap those related steps into qemu_loadvm_state_header() to
make it friendly to read.

Signed-off-by: Wei Yang <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Daniel Henrique Barboza <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>


  Commit: fd392cfa8e6fb0dc34bd0327fc356dfbf6edf1fd
      
https://github.com/qemu/qemu/commit/fd392cfa8e6fb0dc34bd0327fc356dfbf6edf1fd
  Author: Yury Kotov <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M migration/migration.c

  Log Message:
  -----------
  migration: Fix use-after-free during process exit

It fixes heap-use-after-free which was found by clang's ASAN.

Control flow of this use-after-free:
main_thread:
    * Got SIGTERM and completes main loop
    * Calls migration_shutdown
      - migrate_fd_cancel (so, migration_thread begins to complete)
      - object_unref(OBJECT(current_migration));

migration_thread:
    * migration_iteration_finish -> schedule cleanup bh
    * object_unref(OBJECT(s)); (Now, current_migration is freed)
    * exits

main_thread:
    * Calls vm_shutdown -> drain bdrvs -> main loop
      -> cleanup_bh -> use after free

If you want to reproduce, these couple of sleeps will help:
vl.c:4613:
     migration_shutdown();
+    sleep(2);
migration.c:3269:
+    sleep(1);
     trace_migration_thread_after_loop();
     migration_iteration_finish(s);

Original output:
qemu-system-x86_64: terminating on signal 15 from pid 31980 (<unknown process>)
=================================================================
==31958==ERROR: AddressSanitizer: heap-use-after-free on address 0x61900001d210
  at pc 0x555558a535ca bp 0x7fffffffb190 sp 0x7fffffffb188
READ of size 8 at 0x61900001d210 thread T0 (qemu-vm-0)
    #0 0x555558a535c9 in migrate_fd_cleanup migration/migration.c:1502:23
    #1 0x5555594fde0a in aio_bh_call util/async.c:90:5
    #2 0x5555594fe522 in aio_bh_poll util/async.c:118:13
    #3 0x555559524783 in aio_poll util/aio-posix.c:725:17
    #4 0x555559504fb3 in aio_wait_bh_oneshot util/aio-wait.c:71:5
    #5 0x5555573bddf6 in virtio_blk_data_plane_stop
      hw/block/dataplane/virtio-blk.c:282:5
    #6 0x5555589d5c09 in virtio_bus_stop_ioeventfd hw/virtio/virtio-bus.c:246:9
    #7 0x5555589e9917 in virtio_pci_stop_ioeventfd hw/virtio/virtio-pci.c:287:5
    #8 0x5555589e22bf in virtio_pci_vmstate_change hw/virtio/virtio-pci.c:1072:9
    #9 0x555557628931 in virtio_vmstate_change hw/virtio/virtio.c:2257:9
    #10 0x555557c36713 in vm_state_notify vl.c:1605:9
    #11 0x55555716ef53 in do_vm_stop cpus.c:1074:9
    #12 0x55555716eeff in vm_shutdown cpus.c:1092:12
    #13 0x555557c4283e in main vl.c:4617:5
    #14 0x7fffdfdb482f in __libc_start_main
      (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #15 0x555556ecb118 in _start (x86_64-softmmu/qemu-system-x86_64+0x1977118)

0x61900001d210 is located 144 bytes inside of 952-byte region
  [0x61900001d180,0x61900001d538)
freed by thread T6 (live_migration) here:
    #0 0x555556f76782 in __interceptor_free
      
/tmp/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
    #1 0x555558d5fa94 in object_finalize qom/object.c:618:9
    #2 0x555558d57651 in object_unref qom/object.c:1068:9
    #3 0x555558a55588 in migration_thread migration/migration.c:3272:5
    #4 0x5555595393f2 in qemu_thread_start util/qemu-thread-posix.c:502:9
    #5 0x7fffe057f6b9 in start_thread 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

previously allocated by thread T0 (qemu-vm-0) here:
    #0 0x555556f76b03 in __interceptor_malloc
      
/tmp/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
    #1 0x7ffff6ee37b8 in g_malloc 
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4f7b8)
    #2 0x555558d58031 in object_new qom/object.c:640:12
    #3 0x555558a31f21 in migration_object_init migration/migration.c:139:25
    #4 0x555557c41398 in main vl.c:4320:5
    #5 0x7fffdfdb482f in __libc_start_main 
(/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

Thread T6 (live_migration) created by T0 (qemu-vm-0) here:
    #0 0x555556f5f0dd in pthread_create
      
/tmp/final/llvm.src/projects/compiler-rt/lib/asan/asan_interceptors.cc:210:3
    #1 0x555559538cf9 in qemu_thread_create util/qemu-thread-posix.c:539:11
    #2 0x555558a53304 in migrate_fd_connect migration/migration.c:3332:5
    #3 0x555558a72bd8 in migration_channel_connect migration/channel.c:92:5
    #4 0x555558a6ef87 in exec_start_outgoing_migration migration/exec.c:42:5
    #5 0x555558a4f3c2 in qmp_migrate migration/migration.c:1922:9
    #6 0x555558bb4f6a in qmp_marshal_migrate 
qapi/qapi-commands-migration.c:607:5
    #7 0x555559363738 in do_qmp_dispatch qapi/qmp-dispatch.c:131:5
    #8 0x555559362a15 in qmp_dispatch qapi/qmp-dispatch.c:174:11
    #9 0x5555571bac15 in monitor_qmp_dispatch monitor.c:4124:11
    #10 0x55555719a22d in monitor_qmp_bh_dispatcher monitor.c:4207:9
    #11 0x5555594fde0a in aio_bh_call util/async.c:90:5
    #12 0x5555594fe522 in aio_bh_poll util/async.c:118:13
    #13 0x5555595201e0 in aio_dispatch util/aio-posix.c:460:5
    #14 0x555559503553 in aio_ctx_dispatch util/async.c:261:5
    #15 0x7ffff6ede196 in g_main_context_dispatch
      (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4a196)

SUMMARY: AddressSanitizer: heap-use-after-free migration/migration.c:1502:23
  in migrate_fd_cleanup
Shadow bytes around the buggy address:
  0x0c327fffb9f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fffba00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fffba10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fffba20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fffba30: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c327fffba40: fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fffba50: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fffba60: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fffba70: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fffba80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fffba90: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable: 00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone: fa
  Freed heap region: fd
  Stack left redzone: f1
  Stack mid redzone: f2
  Stack right redzone: f3
  Stack after return: f5
  Stack use after scope: f8
  Global redzone: f9
  Global init order: f6
  Poisoned by user: f7
  Container overflow: fc
  Array cookie: ac
  Intra object redzone: bb
  ASan internal: fe
  Left alloca redzone: ca
  Right alloca redzone: cb
  Shadow gap: cc
==31958==ABORTING

Signed-off-by: Yury Kotov <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>
  Fixed up comment formatting


  Commit: a5f7b1a63ced35cc18fe8fa9e1679219fad9abde
      
https://github.com/qemu/qemu/commit/a5f7b1a63ced35cc18fe8fa9e1679219fad9abde
  Author: Wei Yang <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M migration/ram.c

  Log Message:
  -----------
  migration/ram.c: fix typos in comments

Signed-off-by: Wei Yang <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>


  Commit: 9d3250d5ba8c4c5389530b861686e22e77fddcc7
      
https://github.com/qemu/qemu/commit/9d3250d5ba8c4c5389530b861686e22e77fddcc7
  Author: Eduardo Habkost <address@hidden>
  Date:   2019-05-14 (Tue, 14 May 2019)

  Changed paths:
    M monitor.c

  Log Message:
  -----------
  monitor: Call mon_get_cpu() only once at hmp_gva2gpa()

hmp_gva2gpa() calls mon_get_cpu() twice, which is unnecessary.
Not an actual bug, but this is reported as a defect by Coverity
Scan (CID 1401346).

Signed-off-by: Eduardo Habkost <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Richard Henderson <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Signed-off-by: Dr. David Alan Gilbert <address@hidden>


  Commit: c1497fba36465d0259d4d04f2bf09ea59ed42680
      
https://github.com/qemu/qemu/commit/c1497fba36465d0259d4d04f2bf09ea59ed42680
  Author: Peter Maydell <address@hidden>
  Date:   2019-05-16 (Thu, 16 May 2019)

  Changed paths:
    M include/migration/colo.h
    M include/migration/vmstate.h
    M migration/colo-failover.c
    M migration/colo.c
    M migration/migration.c
    M migration/migration.h
    M migration/ram.c
    M migration/savevm.c
    M migration/vmstate.c
    M monitor.c
    M qemu-options.hx

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20190514b' 
into staging

Migration pull 2019-05-14

Small fixes/cleanups
One HMP/monitor fix

# gpg: Signature made Tue 14 May 2019 19:03:53 BST
# gpg:                using RSA key 45F5C71B4A0CB7FB977A9FA90516331EBC5BFDE7
# gpg: Good signature from "Dr. David Alan Gilbert (RH2) <address@hidden>" 
[full]
# Primary key fingerprint: 45F5 C71B 4A0C B7FB 977A  9FA9 0516 331E BC5B FDE7

* remotes/dgilbert/tags/pull-migration-20190514b:
  monitor: Call mon_get_cpu() only once at hmp_gva2gpa()
  migration/ram.c: fix typos in comments
  migration: Fix use-after-free during process exit
  migration/savevm: wrap into qemu_loadvm_state_header()
  migration/savevm: load_header before load_setup
  migration/savevm: remove duplicate check of migration_is_blocked
  migration: update comments of migration bitmap
  migration/ram.c: start of migration_bitmap_sync_range is always 0
  qemu-option.hx: Update missed parameter for colo-compare
  migration/colo.h: Remove obsolete codes
  migration/colo.c: Remove redundant input parameter
  migration: savevm: fix error code with migration blockers
  vmstate: check subsection_found is enough
  migration: remove not used field xfer_limit
  migration: not necessary to check ops again
  migration: comment VMSTATE_UNUSED*() properly

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/e329ad2ab72c...c1497fba3646



reply via email to

[Prev in Thread] Current Thread [Next in Thread]