Re: [PATCH V4] migration: simplify blockers

2023-07-13 Thread Michael Galaxy



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

2023-07-07 Thread Steven Sistare
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

2023-07-07 Thread Peter Xu
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

2023-07-07 Thread Steve Sistare
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_