Re: [PATCH V4] migration: simplify blockers
On 7/7/23 15:20, Steve Sistare wrote: Modify migrate_add_blocker and migrate_del_blocker to take an Error ** reason. This allows migration to own the Error object, so that if an error occurs, migration code can free the Error and clear the client handle, simplifying client code. This is also a pre-requisite for future patches that will add a mode argument to migration requests to support live update, and will maintain a list of blockers for each mode. A blocker may apply to a single mode or to multiple modes, and passing Error** will allow one Error object to be registered for multiple modes. No functional change. Tested-by: Michael Galaxy Reviewed-by: Michael Galaxy Signed-off-by: Steve Sistare --- backends/tpm/tpm_emulator.c | 10 ++ block/parallels.c| 6 ++ block/qcow.c | 6 ++ block/vdi.c | 6 ++ block/vhdx.c | 6 ++ block/vmdk.c | 6 ++ block/vpc.c | 6 ++ block/vvfat.c| 6 ++ dump/dump.c | 4 ++-- hw/9pfs/9p.c | 10 ++ hw/display/virtio-gpu-base.c | 8 ++-- hw/intc/arm_gic_kvm.c| 3 +-- hw/intc/arm_gicv3_its_kvm.c | 3 +-- hw/intc/arm_gicv3_kvm.c | 3 +-- hw/misc/ivshmem.c| 8 ++-- hw/ppc/pef.c | 2 +- hw/ppc/spapr.c | 2 +- hw/ppc/spapr_events.c| 2 +- hw/ppc/spapr_rtas.c | 2 +- hw/remote/proxy.c| 7 ++- hw/s390x/s390-virtio-ccw.c | 9 +++-- hw/scsi/vhost-scsi.c | 8 +++- hw/vfio/common.c | 26 -- hw/vfio/migration.c | 16 ++-- hw/virtio/vhost.c| 8 ++-- include/migration/blocker.h | 24 +--- migration/migration.c| 22 ++ stubs/migr-blocker.c | 4 ++-- target/i386/kvm/kvm.c| 8 target/i386/nvmm/nvmm-all.c | 3 +-- target/i386/sev.c| 2 +- target/i386/whpx/whpx-all.c | 3 +-- ui/vdagent.c | 5 ++--- 33 files changed, 89 insertions(+), 155 deletions(-) diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c index 402a2d6..bf1a90f 100644 --- a/backends/tpm/tpm_emulator.c +++ b/backends/tpm/tpm_emulator.c @@ -534,11 +534,8 @@ static int tpm_emulator_block_migration(TPMEmulator *tpm_emu) error_setg(&tpm_emu->migration_blocker, "Migration disabled: TPM emulator does not support " "migration"); -if (migrate_add_blocker(tpm_emu->migration_blocker, &err) < 0) { +if (migrate_add_blocker(&tpm_emu->migration_blocker, &err) < 0) { error_report_err(err); -error_free(tpm_emu->migration_blocker); -tpm_emu->migration_blocker = NULL; - return -1; } } @@ -1016,10 +1013,7 @@ static void tpm_emulator_inst_finalize(Object *obj) qapi_free_TPMEmulatorOptions(tpm_emu->options); -if (tpm_emu->migration_blocker) { -migrate_del_blocker(tpm_emu->migration_blocker); -error_free(tpm_emu->migration_blocker); -} +migrate_del_blocker(&tpm_emu->migration_blocker); tpm_sized_buffer_reset(&state_blobs->volatil); tpm_sized_buffer_reset(&state_blobs->permanent); diff --git a/block/parallels.c b/block/parallels.c index 18e34ae..c2d92c4 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -960,9 +960,8 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, error_setg(&s->migration_blocker, "The Parallels format used by node '%s' " "does not support live migration", bdrv_get_device_or_node_name(bs)); -ret = migrate_add_blocker(s->migration_blocker, errp); +ret = migrate_add_blocker(&s->migration_blocker, errp); if (ret < 0) { -error_free(s->migration_blocker); goto fail; } qemu_co_mutex_init(&s->lock); @@ -994,8 +993,7 @@ static void parallels_close(BlockDriverState *bs) g_free(s->bat_dirty_bmap); qemu_vfree(s->header); -migrate_del_blocker(s->migration_blocker); -error_free(s->migration_blocker); +migrate_del_blocker(&s->migration_blocker); } static BlockDriver bdrv_parallels = { diff --git a/block/qcow.c b/block/qcow.c index 577bd70..feedad5 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -304,9 +304,8 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, error_setg(&s->migration_blocker, "The qcow format used by node '%s' " "does not support live migration", bdrv_get_device_or_node_name(bs)); -ret = migrate_add_blocker(s->migration_blocker, errp); +ret = migrate_add_blocker(&s->migration_blocker, errp); if (ret < 0) { -error_free(s->migration_blocker);
Re: [PATCH V4] migration: simplify blockers
On 7/7/2023 5:02 PM, Peter Xu wrote: > On Fri, Jul 07, 2023 at 01:20:48PM -0700, Steve Sistare wrote: >> Modify migrate_add_blocker and migrate_del_blocker to take an Error ** >> reason. This allows migration to own the Error object, so that if >> an error occurs, migration code can free the Error and clear the client >> handle, simplifying client code. >> >> This is also a pre-requisite for future patches that will add a mode >> argument to migration requests to support live update, and will maintain >> a list of blockers for each mode. A blocker may apply to a single mode >> or to multiple modes, and passing Error** will allow one Error object to >> be registered for multiple modes. >> >> No functional change. >> >> Signed-off-by: Steve Sistare > > What's the change comparing to v3? > > If no major change, feel free to keep my R-b. No change, just a rebase. - Steve
Re: [PATCH V4] migration: simplify blockers
On Fri, Jul 07, 2023 at 01:20:48PM -0700, Steve Sistare wrote: > Modify migrate_add_blocker and migrate_del_blocker to take an Error ** > reason. This allows migration to own the Error object, so that if > an error occurs, migration code can free the Error and clear the client > handle, simplifying client code. > > This is also a pre-requisite for future patches that will add a mode > argument to migration requests to support live update, and will maintain > a list of blockers for each mode. A blocker may apply to a single mode > or to multiple modes, and passing Error** will allow one Error object to > be registered for multiple modes. > > No functional change. > > Signed-off-by: Steve Sistare What's the change comparing to v3? If no major change, feel free to keep my R-b. Thanks, -- Peter Xu
[PATCH V4] migration: simplify blockers
Modify migrate_add_blocker and migrate_del_blocker to take an Error ** reason. This allows migration to own the Error object, so that if an error occurs, migration code can free the Error and clear the client handle, simplifying client code. This is also a pre-requisite for future patches that will add a mode argument to migration requests to support live update, and will maintain a list of blockers for each mode. A blocker may apply to a single mode or to multiple modes, and passing Error** will allow one Error object to be registered for multiple modes. No functional change. Signed-off-by: Steve Sistare --- backends/tpm/tpm_emulator.c | 10 ++ block/parallels.c| 6 ++ block/qcow.c | 6 ++ block/vdi.c | 6 ++ block/vhdx.c | 6 ++ block/vmdk.c | 6 ++ block/vpc.c | 6 ++ block/vvfat.c| 6 ++ dump/dump.c | 4 ++-- hw/9pfs/9p.c | 10 ++ hw/display/virtio-gpu-base.c | 8 ++-- hw/intc/arm_gic_kvm.c| 3 +-- hw/intc/arm_gicv3_its_kvm.c | 3 +-- hw/intc/arm_gicv3_kvm.c | 3 +-- hw/misc/ivshmem.c| 8 ++-- hw/ppc/pef.c | 2 +- hw/ppc/spapr.c | 2 +- hw/ppc/spapr_events.c| 2 +- hw/ppc/spapr_rtas.c | 2 +- hw/remote/proxy.c| 7 ++- hw/s390x/s390-virtio-ccw.c | 9 +++-- hw/scsi/vhost-scsi.c | 8 +++- hw/vfio/common.c | 26 -- hw/vfio/migration.c | 16 ++-- hw/virtio/vhost.c| 8 ++-- include/migration/blocker.h | 24 +--- migration/migration.c| 22 ++ stubs/migr-blocker.c | 4 ++-- target/i386/kvm/kvm.c| 8 target/i386/nvmm/nvmm-all.c | 3 +-- target/i386/sev.c| 2 +- target/i386/whpx/whpx-all.c | 3 +-- ui/vdagent.c | 5 ++--- 33 files changed, 89 insertions(+), 155 deletions(-) diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c index 402a2d6..bf1a90f 100644 --- a/backends/tpm/tpm_emulator.c +++ b/backends/tpm/tpm_emulator.c @@ -534,11 +534,8 @@ static int tpm_emulator_block_migration(TPMEmulator *tpm_emu) error_setg(&tpm_emu->migration_blocker, "Migration disabled: TPM emulator does not support " "migration"); -if (migrate_add_blocker(tpm_emu->migration_blocker, &err) < 0) { +if (migrate_add_blocker(&tpm_emu->migration_blocker, &err) < 0) { error_report_err(err); -error_free(tpm_emu->migration_blocker); -tpm_emu->migration_blocker = NULL; - return -1; } } @@ -1016,10 +1013,7 @@ static void tpm_emulator_inst_finalize(Object *obj) qapi_free_TPMEmulatorOptions(tpm_emu->options); -if (tpm_emu->migration_blocker) { -migrate_del_blocker(tpm_emu->migration_blocker); -error_free(tpm_emu->migration_blocker); -} +migrate_del_blocker(&tpm_emu->migration_blocker); tpm_sized_buffer_reset(&state_blobs->volatil); tpm_sized_buffer_reset(&state_blobs->permanent); diff --git a/block/parallels.c b/block/parallels.c index 18e34ae..c2d92c4 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -960,9 +960,8 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, error_setg(&s->migration_blocker, "The Parallels format used by node '%s' " "does not support live migration", bdrv_get_device_or_node_name(bs)); -ret = migrate_add_blocker(s->migration_blocker, errp); +ret = migrate_add_blocker(&s->migration_blocker, errp); if (ret < 0) { -error_free(s->migration_blocker); goto fail; } qemu_co_mutex_init(&s->lock); @@ -994,8 +993,7 @@ static void parallels_close(BlockDriverState *bs) g_free(s->bat_dirty_bmap); qemu_vfree(s->header); -migrate_del_blocker(s->migration_blocker); -error_free(s->migration_blocker); +migrate_del_blocker(&s->migration_blocker); } static BlockDriver bdrv_parallels = { diff --git a/block/qcow.c b/block/qcow.c index 577bd70..feedad5 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -304,9 +304,8 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, error_setg(&s->migration_blocker, "The qcow format used by node '%s' " "does not support live migration", bdrv_get_device_or_node_name(bs)); -ret = migrate_add_blocker(s->migration_blocker, errp); +ret = migrate_add_blocker(&s->migration_blocker, errp); if (ret < 0) { -error_free(s->migration_blocker); goto fail; } @@ -796,8 +795,7 @@ static void qcow_close(BlockDriverState *bs) g_free(s->cluster_cache); g_free(s->cluster_data); -migrate_del_