[Qemu-commits] [qemu/qemu] bd4480: migration: Revert mapped-ram multifd support to fd...

[Peter Maydell]

  Branch: refs/heads/staging
  Home:   https://github.com/qemu/qemu
  Commit: bd4480b0d02ee760b51aa82cc0915174753716ee
      
https://github.com/qemu/qemu/commit/bd4480b0d02ee760b51aa82cc0915174753716ee
  Author: Fabiano Rosas <farosas@suse.de>
  Date:   2024-03-22 (Fri, 22 Mar 2024)

  Changed paths:
    M migration/fd.c
    M migration/fd.h
    M migration/file.c
    M migration/migration.c
    M migration/multifd.c
    M tests/qtest/migration-test.c

  Log Message:
  -----------
  migration: Revert mapped-ram multifd support to fd: URI

This reverts commit decdc76772c453ff1444612e910caa0d45cd8eac in full
and also the relevant migration-tests from
7a09f092834641b7a793d50a3a261073bbb404a6.

After the addition of the new QAPI-based migration address API in 8.2
we've been converting an "fd:" URI into a SocketAddress, missing the
fact that the "fd:" syntax could also be used for a plain file instead
of a socket. This is a problem because the SocketAddress is part of
the API, so we're effectively asking users to create a "socket"
channel to pass in a plain file.

The easiest way to fix this situation is to deprecate the usage of
both SocketAddress and "fd:" when used with a plain file for
migration. Since this has been possible since 8.2, we can wait until
9.1 to deprecate it.

For 9.0, however, we should avoid adding further support to migration
to a plain file using the old "fd:" syntax or the new SocketAddress
API, and instead require the usage of either the old-style "file:" URI
or the FileMigrationArgs::filename field of the new API with the
"/dev/fdset/NN" syntax, both of which are already supported.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240319210941.1907-1-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>


  Commit: 910c1647364f0c8c1f680f260a2b7d52587bc38e
      
https://github.com/qemu/qemu/commit/910c1647364f0c8c1f680f260a2b7d52587bc38e
  Author: Peter Xu <peterx@redhat.com>
  Date:   2024-03-22 (Fri, 22 Mar 2024)

  Changed paths:
    M migration/migration.c

  Log Message:
  -----------
  migration/postcopy: Fix high frequency sync

With current code base I can observe extremely high sync count during
precopy, as long as one enables postcopy-ram=on before switchover to
postcopy.

To provide some context of when QEMU decides to do a full sync: it checks
must_precopy (which implies "data must be sent during precopy phase"), and
as long as it is lower than the threshold size we calculated (out of
bandwidth and expected downtime) QEMU will kick off the slow/exact sync.

However, when postcopy is enabled (even if still during precopy phase), RAM
only reports all pages as can_postcopy, and report must_precopy==0.  Then
"must_precopy <= threshold_size" mostly always triggers and enforces a slow
sync for every call to migration_iteration_run() when postcopy is enabled
even if not used.  That is insane.

It turns out it was a regress bug introduced in the previous refactoring in
8.0 as reported by Nina [1]:

  (a) c8df4a7aef ("migration: Split save_live_pending() into state_pending_*")

Then a workaround patch is applied at the end of release (8.0-rc4) to fix it:

  (b) 28ef5339c3 ("migration: fix ram_state_pending_exact()")

However that "workaround" was overlooked when during the cleanup in this
9.0 release in this commit..

  (c) b0504edd40 ("migration: Drop unnecessary check in ram's pending_exact()")

Then the issue was re-exposed as reported by Nina [1].

The problem with (b) is that it only fixed the case for RAM, rather than
all the rest of iterators.  Here a slow sync should only be required if all
dirty data (precopy+postcopy) is less than the threshold_size that QEMU
calculated.  It is even debatable whether a sync is needed when switched to
postcopy.  Currently ram_state_pending_exact() will be mostly noop if
switched to postcopy, and that logic seems to apply too for all the rest of
iterators, as sync dirty bitmap during a postcopy doesn't make much sense.
However let's leave such change for later, as we're in rc phase.

So rather than reusing commit (b), this patch provides the complete fix for
all iterators.  When at it, cleanup a little bit on the lines around.

[1] https://gitlab.com/qemu-project/qemu/-/issues/1565

Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
Fixes: b0504edd40 ("migration: Drop unnecessary check in ram's pending_exact()")
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240320214453.584374-1-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>


  Commit: 8fa1a21c6edc2bf7de85984944848ab9ac49e937
      
https://github.com/qemu/qemu/commit/8fa1a21c6edc2bf7de85984944848ab9ac49e937
  Author: Fabiano Rosas <farosas@suse.de>
  Date:   2024-03-22 (Fri, 22 Mar 2024)

  Changed paths:
    M migration/multifd.c

  Log Message:
  -----------
  migration/multifd: Fix clearing of mapped-ram zero pages

When the zero page detection is done in the multifd threads, we need
to iterate the second part of the pages->offset array and clear the
file bitmap for each zero page. The piece of code we merged to do that
is wrong.

The reason this has passed all the tests is because the bitmap is
initialized with zeroes already, so clearing the bits only really has
an effect during live migration and when a data page goes from having
data to no data.

Fixes: 303e6f54f9 ("migration/multifd: Implement zero page transmission on the 
multifd thread.")
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240321201242.6009-1-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>


  Commit: 80e9791a93b856ae959cf0efa04cee53390ed000
      
https://github.com/qemu/qemu/commit/80e9791a93b856ae959cf0efa04cee53390ed000
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2024-03-25 (Mon, 25 Mar 2024)

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

  Log Message:
  -----------
  tests/qtest/npcm7xx_emc_test: Don't leak cmd_line

In test_rx() and test_tx() we allocate a GString *cmd_line
but never free it. This is pretty harmless in a test case, but
Coverity spotted it.

Resolves: Coverity CID 1507122
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 20240312183810.557768-2-peter.maydell@linaro.org


  Commit: e921e00d4ba6840063d69cb637331d0dc4905e4b
      
https://github.com/qemu/qemu/commit/e921e00d4ba6840063d69cb637331d0dc4905e4b
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2024-03-25 (Mon, 25 Mar 2024)

  Changed paths:
    M tests/unit/socket-helpers.c

  Log Message:
  -----------
  tests/unit/socket-helpers: Don't close(-1)

In socket_check_afunix_support() we call socket(PF_UNIX, SOCK_STREAM, 0)
to see if it works, but we call close() on the result whether it
worked or not. Only close the fd if the socket() call succeeded.
Spotted by Coverity.

Resolves: Coverity CID 1497481

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 20240312183810.557768-3-peter.maydell@linaro.org


  Commit: bed150be5b94ee499384fa6d052c0cb398a20d95
      
https://github.com/qemu/qemu/commit/bed150be5b94ee499384fa6d052c0cb398a20d95
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2024-03-25 (Mon, 25 Mar 2024)

  Changed paths:
    M net/af-xdp.c

  Log Message:
  -----------
  net/af-xdp.c: Don't leak sock_fds array in net_init_af_xdp()

In net_init_af_xdp() we parse the arguments and allocate
a buffer of ints into sock_fds. However, although we
free this in the error exit path, we don't ever free it
in the successful return path. Coverity spots this leak.

Switch to g_autofree so we don't need to manually free the
array.

Resolves: Coverity CID 1534906
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 20240312183810.557768-4-peter.maydell@linaro.org


  Commit: c67f7580697198800c57ced59f1dfbce1aaeb4ae
      
https://github.com/qemu/qemu/commit/c67f7580697198800c57ced59f1dfbce1aaeb4ae
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2024-03-25 (Mon, 25 Mar 2024)

  Changed paths:
    M hw/misc/pca9554.c

  Log Message:
  -----------
  hw/misc/pca9554: Correct error check bounds in get/set pin functions

In pca9554_get_pin() and pca9554_set_pin(), we try to detect an
incorrect pin value, but we get the condition wrong, using ">"
when ">=" was intended.

This has no actual effect, because in pca9554_initfn() we
use the correct test when creating the properties and so
we'll never be called with an out of range value. However,
Coverity complains about the mismatch between the check and
the later use of the pin value in a shift operation.

Use the correct condition.

Resolves: Coverity CID 1534917
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 20240312183810.557768-5-peter.maydell@linaro.org


  Commit: c16f4fb2dfa940ae65f40085c602d4763d094deb
      
https://github.com/qemu/qemu/commit/c16f4fb2dfa940ae65f40085c602d4763d094deb
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2024-03-25 (Mon, 25 Mar 2024)

  Changed paths:
    M hw/nvram/mac_nvram.c

  Log Message:
  -----------
  hw/nvram/mac_nvram: Report failure to write data

There's no way for the macio_nvram device to report failure to write
data, but we can at least report it to the user with error_report()
as we do in other devices like xlnx-efuse.

Spotted by Coverity.

Resolves: Coverity CID 1507628
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20240312183810.557768-6-peter.maydell@linaro.org


  Commit: c44a352a77225aeb8b7db9fe9af3361c08b5b1f9
      
https://github.com/qemu/qemu/commit/c44a352a77225aeb8b7db9fe9af3361c08b5b1f9
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2024-03-25 (Mon, 25 Mar 2024)

  Changed paths:
    M tests/unit/test-throttle.c

  Log Message:
  -----------
  tests/unit/test-throttle: Avoid unintended integer division

In test_compute_wait() we do
 double units = bkt.max / 10;
which does an integer division and then assigns it to a double variable,
and similarly later on in the expression for an assertion.

Use 10.0 so that we do a floating point division and calculate the
exact value, rather than doing an integer division.

Spotted by Coverity.

Resolves: Coverity CID 1432564
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20240312183810.557768-7-peter.maydell@linaro.org


  Commit: 55c79639d553c1b7a82b4cde781ad5f316f45b0e
      
https://github.com/qemu/qemu/commit/55c79639d553c1b7a82b4cde781ad5f316f45b0e
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2024-03-25 (Mon, 25 Mar 2024)

  Changed paths:
    M tests/qtest/libqtest.c

  Log Message:
  -----------
  tests/qtest/libqtest.c: Check for g_setenv() failure

Coverity points out that g_setenv() can fail and we don't
check for this in qtest_inproc_init(). In practice this will
only fail if a memory allocation failed in setenv() or if
the caller passed an invalid architecture name (e.g. one
with an '=' in it), so rather than requiring the callsite
to check for failure, make g_setenv() failure fatal here,
similarly to what we did in commit aca68d95c515.

Resolves: Coverity CID 1497485
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-id: 20240312183810.557768-8-peter.maydell@linaro.org


  Commit: bed4cb5e73c3175f9db41b4363d758acd269f07b
      
https://github.com/qemu/qemu/commit/bed4cb5e73c3175f9db41b4363d758acd269f07b
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2024-03-25 (Mon, 25 Mar 2024)

  Changed paths:
    M migration/fd.c
    M migration/fd.h
    M migration/file.c
    M migration/migration.c
    M migration/multifd.c
    M tests/qtest/migration-test.c

  Log Message:
  -----------
  Merge tag 'migration-20240322-pull-request' of https://gitlab.com/peterx/qemu 
into staging

Migration pull for 9.0-rc1

- Fabiano's patch to revert fd: support on mapped-ram
- Peter's fix on postcopy regression on unnecessary dirty syncs
- Fabiano's fix on mapped-ram rare corrupt on zero page handling

# -----BEGIN PGP SIGNATURE-----
#
# iIgEABYKADAWIQS5GE3CDMRX2s990ak7X8zN86vXBgUCZf2uIxIccGV0ZXJ4QHJl
# ZGhhdC5jb20ACgkQO1/MzfOr1waqTgD/RjaWrcUYlHcfFcWlEQGrYqikCtZYI+oW
# YYdbLcCBOlQBAL/ecCbsFyaWyPnB1Eg3YFcj5g8AgogDHdg37HSxydgL
# =aWGi
# -----END PGP SIGNATURE-----
# gpg: Signature made Fri 22 Mar 2024 16:13:23 GMT
# gpg:                using EDDSA key B9184DC20CC457DACF7DD1A93B5FCCCDF3ABD706
# gpg:                issuer "peterx@redhat.com"
# gpg: Good signature from "Peter Xu <xzpeter@gmail.com>" [marginal]
# gpg:                 aka "Peter Xu <peterx@redhat.com>" [marginal]
# gpg: WARNING: This key is not certified with sufficiently trusted signatures!
# gpg:          It is not certain that the signature belongs to the owner.
# Primary key fingerprint: B918 4DC2 0CC4 57DA CF7D  D1A9 3B5F CCCD F3AB D706

* tag 'migration-20240322-pull-request' of https://gitlab.com/peterx/qemu:
  migration/multifd: Fix clearing of mapped-ram zero pages
  migration/postcopy: Fix high frequency sync
  migration: Revert mapped-ram multifd support to fd: URI

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


  Commit: 7f5d6315c131d877adce8851b773a65f21a22d6e
      
https://github.com/qemu/qemu/commit/7f5d6315c131d877adce8851b773a65f21a22d6e
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2024-03-25 (Mon, 25 Mar 2024)

  Changed paths:
    M hw/misc/pca9554.c
    M hw/nvram/mac_nvram.c
    M net/af-xdp.c
    M tests/qtest/libqtest.c
    M tests/qtest/npcm7xx_emc-test.c
    M tests/unit/socket-helpers.c
    M tests/unit/test-throttle.c

  Log Message:
  -----------
  Merge tag 'pull-target-arm-20240325' of 
https://git.linaro.org/people/pmaydell/qemu-arm into staging

target-arm queue:
 * Fixes for seven minor coverity issues

# -----BEGIN PGP SIGNATURE-----
#
# iQJNBAABCAA3FiEE4aXFk81BneKOgxXPPCUl7RQ2DN4FAmYBb3oZHHBldGVyLm1h
# eWRlbGxAbGluYXJvLm9yZwAKCRA8JSXtFDYM3uoTD/99R+ocvelJIiHZ2YCj7MXA
# 8Xe87lWZ1pFDFBn8smj/Ggtsq6LMfQlkrd/RZoMcG8DXqmSRCf0Hf2wQOUEdVLOh
# 50W+0A+IkFLfPmq69rkwJBw5hQJFoDQpfkJgEeDforcjv0rTd48JPU2wXn3S6UpW
# Iml+cNbb/fFrkGURV836vc4U2G9C4S+aIlNgZDC09t38GlCLEHr52fqYaHpvgLbN
# bBYdTDoSNUN7xg/UnK/6YAgLDwecbFIL/ojV/Tf1caliLklsi1SZGxV3ZxHQyZuR
# Zs0Hl8feHSFqLU6O3f8lGczwVMMrMAvdqYeo3VJmIhAOIYJYxDpZEH29pSufB1my
# Jba3jn6aQE0fkI0IGcIHiuuY1Wy+rz+T2UlArOzKwh5+EMMIq8gFIgcmzQiz6dOK
# m415dbGPeCLZ7cykwDNskLPny3Mpe0KNkcA0wxb6z4LMtAZAyszEZvPjqUyF9Gfo
# 4EusSumLGJ0wK+qhR1IyT79wMUUypmfr/V52xwHa3HjVo15APzvG+hQMoP0oh3Hr
# 1gQ0w2qN8KotsIAtgmtR6kKDgv2SRRc9UqDm4erqE9m9WpcRHfO0O1bPpiJHec6A
# Bfg2oIghKKP/7ToTG24yQUxnPtvnqk0RUYYGuDjdhOOyn2QmGERwFrigOHmwOR82
# 2DnplgWlYI6pnJHmDXHm5A==
# =wrzj
# -----END PGP SIGNATURE-----
# gpg: Signature made Mon 25 Mar 2024 12:35:06 GMT
# gpg:                using RSA key E1A5C593CD419DE28E8315CF3C2525ED14360CDE
# gpg:                issuer "peter.maydell@linaro.org"
# gpg: Good signature from "Peter Maydell <peter.maydell@linaro.org>" [ultimate]
# gpg:                 aka "Peter Maydell <pmaydell@gmail.com>" [ultimate]
# gpg:                 aka "Peter Maydell <pmaydell@chiark.greenend.org.uk>" 
[ultimate]
# gpg:                 aka "Peter Maydell <peter@archaic.org.uk>" [ultimate]
# Primary key fingerprint: E1A5 C593 CD41 9DE2 8E83  15CF 3C25 25ED 1436 0CDE

* tag 'pull-target-arm-20240325' of 
https://git.linaro.org/people/pmaydell/qemu-arm:
  tests/qtest/libqtest.c: Check for g_setenv() failure
  tests/unit/test-throttle: Avoid unintended integer division
  hw/nvram/mac_nvram: Report failure to write data
  hw/misc/pca9554: Correct error check bounds in get/set pin functions
  net/af-xdp.c: Don't leak sock_fds array in net_init_af_xdp()
  tests/unit/socket-helpers: Don't close(-1)
  tests/qtest/npcm7xx_emc_test: Don't leak cmd_line

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


Compare: https://github.com/qemu/qemu/compare/853546f81284...7f5d6315c131

To unsubscribe from these emails, change your notification settings at 
https://github.com/qemu/qemu/settings/notifications