[Qemu-commits] [qemu/qemu] 5e32ff: tests/qtest/migration-test.c: use 127.0.0.1 instea...

[Peter Maydell]

  Branch: refs/heads/staging
  Home:   https://github.com/qemu/qemu
  Commit: 5e32ffd346429b2e92545c425de96d92e88a7498
      
https://github.com/qemu/qemu/commit/5e32ffd346429b2e92545c425de96d92e88a7498
  Author: Dr. David Alan Gilbert <dgilbert@redhat.com>
  Date:   2021-07-26 (Mon, 26 Jul 2021)

  Changed paths:
    M tests/qtest/migration-test.c

  Log Message:
  -----------
  tests/qtest/migration-test.c: use 127.0.0.1 instead of 0

OpenBSD doesn't like :0 as an address, switch to using 127.0.0.1
in baddest; it's really testing the :0 port number that isn't allowed
on anything.

(The test doesn't currently run anyway because of the userfault
problem that Peter noticed, but this gets us closer to being able to
reenable it)

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20210719185217.122105-1-dgilbert@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


  Commit: 53021ea1659b8a9074c6f5eb6c65a4e5dddddaec
      
https://github.com/qemu/qemu/commit/53021ea1659b8a9074c6f5eb6c65a4e5dddddaec
  Author: Peter Xu <peterx@redhat.com>
  Date:   2021-07-26 (Mon, 26 Jul 2021)

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

  Log Message:
  -----------
  migration: Fix missing join() of rp_thread

It's possible that the migration thread skip the join() of the rp_thread in
below race and crash on src right at finishing migration:

       migration_thread                     rp_thread
       ----------------                     ---------
    migration_completion()
                                        (before rp_thread quits)
                                        from_dst_file=NULL
                                        [thread got scheduled out]
      s->rp_state.from_dst_file==NULL
        (skip join() of rp_thread)
    migrate_fd_cleanup()
      qemu_fclose(s->to_dst_file)
      yank_unregister_instance()
        assert(yank_find_entry())  <------- crash

It could mostly happen with postcopy, but that shouldn't be required, e.g., I
think it could also trigger with MIGRATION_CAPABILITY_RETURN_PATH set.

It's suspected that above race could be the root cause of a recent (but rare)
migration-test break reported by either Dave or PMM:

https://lore.kernel.org/qemu-devel/YPamXAHwan%2FPPXLf@work-vm/

The issue is: from_dst_file is reset in the rp_thread, so if the thread reset
it to NULL fast enough then the migration thread will assume there's no
rp_thread at all.

This could potentially cause more severe issue (e.g. crash) after the yank code.

Fix it by using a boolean to keep "whether we've created rp_thread".

Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20210722175841.938739-2-peterx@redhat.com>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


  Commit: 43044ac0ee5758d92b639843c045123c2de578d1
      
https://github.com/qemu/qemu/commit/43044ac0ee5758d92b639843c045123c2de578d1
  Author: Peter Xu <peterx@redhat.com>
  Date:   2021-07-26 (Mon, 26 Jul 2021)

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

  Log Message:
  -----------
  migration: Make from_dst_file accesses thread-safe

Accessing from_dst_file is potentially racy in current code base like below:

  if (s->from_dst_file)
    do_something(s->from_dst_file);

Because from_dst_file can be reset right after the check in another
thread (rp_thread).  One example is migrate_fd_cancel().

Use the same qemu_file_lock to protect it too, just like to_dst_file.

When it's safe to access without lock, comment it.

There's one special reference in migration_thread() that can be replaced by
the newly introduced rp_thread_created flag.

Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Message-Id: <20210722175841.938739-3-peterx@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
  with Peter's fixup


  Commit: 18711405b506e7ca3822ed19830f1c562e0247f9
      
https://github.com/qemu/qemu/commit/18711405b506e7ca3822ed19830f1c562e0247f9
  Author: Peter Xu <peterx@redhat.com>
  Date:   2021-07-26 (Mon, 26 Jul 2021)

  Changed paths:
    M migration/channel.c
    M migration/multifd.c
    M migration/qemu-file-channel.c
    M migration/yank_functions.c
    M migration/yank_functions.h

  Log Message:
  -----------
  migration: Introduce migration_ioc_[un]register_yank()

There're plenty of places in migration/* that checks against either socket or
tls typed ioc for yank operations.  Provide two helpers to hide all these
information.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20210722175841.938739-4-peterx@redhat.com>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


  Commit: c6ad5be7ae5b8c6bbbd0c592bbf18d4a78540516
      
https://github.com/qemu/qemu/commit/c6ad5be7ae5b8c6bbbd0c592bbf18d4a78540516
  Author: Peter Xu <peterx@redhat.com>
  Date:   2021-07-26 (Mon, 26 Jul 2021)

  Changed paths:
    M migration/qemu-file-channel.c
    M migration/qemu-file.c
    M migration/qemu-file.h
    M migration/ram.c
    M migration/savevm.c

  Log Message:
  -----------
  migration: Teach QEMUFile to be QIOChannel-aware

migration uses QIOChannel typed qemufiles.  In follow up patches, we'll need
the capability to identify this fact, so that we can get the backing QIOChannel
from a QEMUFile.

We can also define types for QEMUFile but so far since we only need to be able
to identify QIOChannel, introduce a boolean which is simpler.

Introduce another helper qemu_file_get_ioc() to return the ioc backend of a
qemufile if has_ioc is set.

No functional change.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20210722175841.938739-5-peterx@redhat.com>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


  Commit: 39675ffffb3394d44b880d083a214c5e44786170
      
https://github.com/qemu/qemu/commit/39675ffffb3394d44b880d083a214c5e44786170
  Author: Peter Xu <peterx@redhat.com>
  Date:   2021-07-26 (Mon, 26 Jul 2021)

  Changed paths:
    M migration/migration.c
    M migration/qemu-file-channel.c
    M migration/savevm.c
    M migration/yank_functions.c
    M migration/yank_functions.h

  Log Message:
  -----------
  migration: Move the yank unregister of channel_close out

It's efficient, but hackish to call yank unregister calls in channel_close(),
especially it'll be hard to debug when qemu crashed with some yank function
leaked.

Remove that hack, but instead explicitly unregister yank functions at the
places where needed, they are:

  (on src)
  - migrate_fd_cleanup
  - postcopy_pause

  (on dst)
  - migration_incoming_state_destroy
  - postcopy_pause_incoming

Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20210722175841.938739-6-peterx@redhat.com>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


  Commit: 3143577d6a3f363514219c03d936e653ede44f32
      
https://github.com/qemu/qemu/commit/3143577d6a3f363514219c03d936e653ede44f32
  Author: Wei Wang <wei.w.wang@intel.com>
  Date:   2021-07-26 (Mon, 26 Jul 2021)

  Changed paths:
    M migration/ram.c

  Log Message:
  -----------
  migration: clear the memory region dirty bitmap when skipping free pages

When skipping free pages to send, their corresponding dirty bits in the
memory region dirty bitmap need to be cleared. Otherwise the skipped
pages will be sent in the next round after the migration thread syncs
dirty bits from the memory region dirty bitmap.

Cc: David Hildenbrand <david@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Reported-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Message-Id: <20210722083055.23352-1-wei.w.wang@intel.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


  Commit: ca4b5ef371d6602b73bc5eec08e3199b05caf146
      
https://github.com/qemu/qemu/commit/ca4b5ef371d6602b73bc5eec08e3199b05caf146
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2021-07-27 (Tue, 27 Jul 2021)

  Changed paths:
    M migration/channel.c
    M migration/migration.c
    M migration/migration.h
    M migration/multifd.c
    M migration/qemu-file-channel.c
    M migration/qemu-file.c
    M migration/qemu-file.h
    M migration/ram.c
    M migration/savevm.c
    M migration/yank_functions.c
    M migration/yank_functions.h
    M tests/qtest/migration-test.c

  Log Message:
  -----------
  Merge remote-tracking branch 
'remotes/dgilbert-gitlab/tags/pull-migration-20210726a' into staging

Migration fixes 2021-07-26

Peter's fix for a bunch of races
 -> Seem to fix the occasional crash seen by Peter

Wei's fix for migration with free page hinting
 -> Bug has been around for a while, but makes a huge difference

My fix for OpenBSD test corner case

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

# gpg: Signature made Mon 26 Jul 2021 13:42:16 BST
# gpg:                using RSA key 45F5C71B4A0CB7FB977A9FA90516331EBC5BFDE7
# gpg: Good signature from "Dr. David Alan Gilbert (RH2) <dgilbert@redhat.com>" 
[full]
# Primary key fingerprint: 45F5 C71B 4A0C B7FB 977A  9FA9 0516 331E BC5B FDE7

* remotes/dgilbert-gitlab/tags/pull-migration-20210726a:
  migration: clear the memory region dirty bitmap when skipping free pages
  migration: Move the yank unregister of channel_close out
  migration: Teach QEMUFile to be QIOChannel-aware
  migration: Introduce migration_ioc_[un]register_yank()
  migration: Make from_dst_file accesses thread-safe
  migration: Fix missing join() of rp_thread
  tests/qtest/migration-test.c: use 127.0.0.1 instead of 0

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Compare: https://github.com/qemu/qemu/compare/c08ccd1b53f4...ca4b5ef371d6