Re: [PATCH v4 00/23] migration: propagate vTPM errors using Error objects

[Fabiano Rosas <farosas@suse.de>]

Arun Menon <armenon@redhat.com> writes:

> Hello,
>
> Currently, when a migration of a VM with an encrypted vTPM
> fails on the destination host (e.g., due to a mismatch in secret values),
> the error message displayed on the source host is generic and unhelpful.
>
> For example, a typical error looks like this:
> "operation failed: job 'migration out' failed: Sibling indicated error 1.
> operation failed: job 'migration in' failed: load of migration failed:
> Input/output error"
>
> This message does not provide any specific indication of a vTPM failure.
> Such generic errors are logged using error_report(), which prints to
> the console/monitor but does not make the detailed error accessible via
> the QMP query-migrate command.
>
> This series addresses the issue, by ensuring that specific TPM error
> messages are propagated via the QEMU Error object.
> To make this possible,
> - A set of functions in the call stack is changed
>   to incorporate an Error object as an additional parameter.
> - Also, the TPM backend makes use of a new hook called post_load_errp()
>   that explicitly passes an Error object.
>
> It is organized as follows,
>  - Patches 1-21 focuses on pushing Error object into the functions
>    that are important in the call stack where TPM errors are observed.
>    We still need to make changes in rest of the functions in savevm.c
>    such that they also incorporate the errp object for propagating errors.
>  - Patch 22 introduces the new variants of the hooks in VMStateDescription
>    structure. These hooks should be used in future implementations.
>  - Patch 23 focuses on changing the TPM backend such that the errors are
>    set in the Error object.
>
> While this series focuses specifically on TPM error reporting during
> live migration, it lays the groundwork for broader improvements.
> A lot of methods in savevm.c that previously returned an integer now capture
> errors in the Error object, enabling other modules to adopt the
> post_load_errp hook in the future.
>
> One such change previously attempted:
> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg01727.html
>
> Resolves: https://issues.redhat.com/browse/RHEL-82826
>
> Signed-off-by: Arun Menon <armenon@redhat.com>
> ---
> Changes in v4:
> - Split the patches into smaller ones based on functions. Pass NULL in the
>   caller until errp is made available. Every function that has an
>   Error **errp object passed to it, ensures that it sets the errp object
>   in case of failure.
> - A few more functions within loadvm_process_command() now handle errors using
>   the errp object. I've converted these for consistency, taking Daniel's
>   patches (link above) as a reference.
> - Along with the post_load_errp() hook, other duplicate hooks are also introduced.
>   This will enable us to migrate to the newer versions eventually.
> - Fix some semantic errors, like using error_propagate_prepend() in places where
>   we need to preserve existing behaviour of accumulating the error in local_err
>   and then propagating it to errp. This can be refactored in a later commit.
> - Add more information in commit messages explaining the changes.
> - Link to v3: https://lore.kernel.org/qemu-devel/20250702-propagate_tpm_error-v3-0-986d94540528@redhat.com
>
> Changes in v3:
> - Split the 2nd patch into 2. Introducing post_load_with_error() hook
>   has been separated from using it in the backends TPM module. This is
>   so that it can be acknowledged.
> - Link to v2: https://lore.kernel.org/qemu-devel/20250627-propagate_tpm_error-v2-0-85990c89da29@redhat.com
>
> Changes in v2:
> - Combine the first two changes into one, focusing on passing the
>   Error object (errp) consistently through functions involved in
>   loading the VM's state. Other functions are not yet changed.
> - As suggested in the review comment, add null checks for errp
>   before adding error messages, preventing crashes.
>   We also now correctly set errors when post-copy migration fails.
> - In process_incoming_migration_co(), switch to error_prepend
>   instead of error_setg. This means we now null-check local_err in
>   the "fail" section before using it, preventing dereferencing issues.
> - Link to v1: https://lore.kernel.org/qemu-devel/20250624-propagate_tpm_error-v1-0-2171487a593d@redhat.com
>
> ---
> Arun Menon (23):
>       migration: push Error **errp into vmstate_subsection_load()
>       migration: push Error **errp into vmstate_load_state()
>       migration: push Error **errp into qemu_loadvm_state_header()
>       migration: push Error **errp into vmstate_load()
>       migration: push Error **errp into qemu_loadvm_section_start_full()
>       migration: push Error **errp into qemu_loadvm_section_part_end()
>       migration: push Error **errp into loadvm_process_command()
>       migration: push Error **errp into loadvm_handle_cmd_packaged()
>       migration: push Error **errp into ram_postcopy_incoming_init()
>       migration: push Error **errp into loadvm_postcopy_handle_advise()
>       migration: push Error **errp into loadvm_postcopy_handle_listen()
>       migration: push Error **errp into loadvm_postcopy_handle_run()
>       migration: push Error **errp into loadvm_postcopy_ram_handle_discard()
>       migration: make loadvm_postcopy_handle_resume() void
>       migration: push Error **errp into loadvm_handle_recv_bitmap()
>       migration: push Error **errp into loadvm_process_enable_colo()
>       migration: push Error **errp into loadvm_postcopy_handle_switchover_start()
>       migration: push Error **errp into qemu_loadvm_state_main()
>       migration: push Error **errp into qemu_loadvm_state()
>       migration: push Error **errp into qemu_load_device_state()
>       migration: Capture error in postcopy_ram_listen_thread()
>       migration: Add error-parameterized function variants in VMSD struct
>       backends/tpm: Propagate vTPM error on migration failure
>
>  backends/tpm/tpm_emulator.c |  39 +++---
>  hw/display/virtio-gpu.c     |   2 +-
>  hw/pci/pci.c                |   2 +-
>  hw/s390x/virtio-ccw.c       |   2 +-
>  hw/scsi/spapr_vscsi.c       |   2 +-
>  hw/vfio/pci.c               |   2 +-
>  hw/virtio/virtio-mmio.c     |   2 +-
>  hw/virtio/virtio-pci.c      |   2 +-
>  hw/virtio/virtio.c          |   4 +-
>  include/migration/colo.h    |   2 +-
>  include/migration/vmstate.h |  13 +-
>  migration/colo.c            |  10 +-
>  migration/cpr.c             |   4 +-
>  migration/migration.c       |  19 +--
>  migration/postcopy-ram.c    |   9 +-
>  migration/postcopy-ram.h    |   2 +-
>  migration/ram.c             |  14 +--
>  migration/ram.h             |   4 +-
>  migration/savevm.c          | 299 +++++++++++++++++++++++++-------------------
>  migration/savevm.h          |   7 +-
>  migration/vmstate-types.c   |  10 +-
>  migration/vmstate.c         |  83 ++++++++----
>  tests/unit/test-vmstate.c   |  18 +--
>  ui/vdagent.c                |   2 +-
>  24 files changed, 325 insertions(+), 228 deletions(-)
> ---
> base-commit: 9a4e273ddec3927920c5958d2226c6b38b543336
> change-id: 20250624-propagate_tpm_error-bf4ae6c23d30
>
> Best regards,

Hi Arun, make check is failing, please take a look:

QTEST_LOG=1 QTEST_QEMU_BINARY=./qemu-system-x86_64 \
./tests/qtest/migration-test \
--full -p /x86_64/migration/postcopy/recovery/double-failures/handshake
...
qemu-system-x86_64: ../util/error.c:65: error_setv: Assertion `*errp ==
NULL' failed.