* [PATCH 1/3] migration: Use unsigned instead of int for bit set of MigMode
2025-10-27 6:45 [PATCH 0/3] migration: Cleanup around MigMode sets Markus Armbruster
@ 2025-10-27 6:45 ` Markus Armbruster
2025-10-27 6:45 ` [PATCH 2/3] migration: Use bitset of MigMode instead of variable arguments Markus Armbruster
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2025-10-27 6:45 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, farosas, berrange, guobin
Signed operands in bitwise operations are unwise. I believe they're
safe here, but avoiding them is easy, so let's do that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
migration/migration.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index a63b46bbef..9f0a91b1d9 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1672,9 +1672,9 @@ void migration_cancel(void)
}
}
-static int get_modes(MigMode mode, va_list ap);
+static unsigned get_modes(MigMode mode, va_list ap);
-static void add_notifiers(NotifierWithReturn *notify, int modes)
+static void add_notifiers(NotifierWithReturn *notify, unsigned modes)
{
for (MigMode mode = 0; mode < MIG_MODE__MAX; mode++) {
if (modes & BIT(mode)) {
@@ -1687,7 +1687,7 @@ static void add_notifiers(NotifierWithReturn *notify, int modes)
void migration_add_notifier_modes(NotifierWithReturn *notify,
MigrationNotifyFunc func, MigMode mode, ...)
{
- int modes;
+ unsigned modes;
va_list ap;
va_start(ap, mode);
@@ -1876,7 +1876,7 @@ static bool is_busy(Error **reasonp, Error **errp)
return false;
}
-static bool is_only_migratable(Error **reasonp, Error **errp, int modes)
+static bool is_only_migratable(Error **reasonp, Error **errp, unsigned modes)
{
ERRP_GUARD();
@@ -1890,9 +1890,9 @@ static bool is_only_migratable(Error **reasonp, Error **errp, int modes)
return false;
}
-static int get_modes(MigMode mode, va_list ap)
+static unsigned get_modes(MigMode mode, va_list ap)
{
- int modes = 0;
+ unsigned modes = 0;
while (mode != -1 && mode != MIG_MODE_ALL) {
assert(mode >= MIG_MODE_NORMAL && mode < MIG_MODE__MAX);
@@ -1905,7 +1905,7 @@ static int get_modes(MigMode mode, va_list ap)
return modes;
}
-static int add_blockers(Error **reasonp, Error **errp, int modes)
+static int add_blockers(Error **reasonp, Error **errp, unsigned modes)
{
for (MigMode mode = 0; mode < MIG_MODE__MAX; mode++) {
if (modes & BIT(mode)) {
@@ -1928,7 +1928,7 @@ int migrate_add_blocker_normal(Error **reasonp, Error **errp)
int migrate_add_blocker_modes(Error **reasonp, Error **errp, MigMode mode, ...)
{
- int modes;
+ unsigned modes;
va_list ap;
va_start(ap, mode);
@@ -1945,7 +1945,7 @@ int migrate_add_blocker_modes(Error **reasonp, Error **errp, MigMode mode, ...)
int migrate_add_blocker_internal(Error **reasonp, Error **errp)
{
- int modes = BIT(MIG_MODE__MAX) - 1;
+ unsigned modes = BIT(MIG_MODE__MAX) - 1;
if (is_busy(reasonp, errp)) {
return -EBUSY;
--
2.49.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] migration: Use bitset of MigMode instead of variable arguments
2025-10-27 6:45 [PATCH 0/3] migration: Cleanup around MigMode sets Markus Armbruster
2025-10-27 6:45 ` [PATCH 1/3] migration: Use unsigned instead of int for bit set of MigMode Markus Armbruster
@ 2025-10-27 6:45 ` Markus Armbruster
2025-10-27 6:45 ` [PATCH 3/3] migration: Put Error **errp parameter last Markus Armbruster
2025-10-27 13:24 ` [PATCH 0/3] migration: Cleanup around MigMode sets Fabiano Rosas
3 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2025-10-27 6:45 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, farosas, berrange, guobin
migrate_add_blocker_modes() and migration_add_notifier_modes use
variable arguments for a set of migration modes. The variable
arguments get collected into a bitset for processsing. Take a bitset
argument instead, it's simpler.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/migration/blocker.h | 9 +++-----
include/migration/misc.h | 10 ++++-----
hw/vfio/container-legacy.c | 6 +++---
hw/vfio/cpr-iommufd.c | 6 +++---
hw/vfio/cpr-legacy.c | 8 +++----
hw/vfio/cpr.c | 5 ++---
hw/vfio/device.c | 4 ++--
migration/migration.c | 43 ++++++-------------------------------
stubs/migr-blocker.c | 2 +-
system/physmem.c | 8 +++----
10 files changed, 33 insertions(+), 68 deletions(-)
diff --git a/include/migration/blocker.h b/include/migration/blocker.h
index a687ac0efe..80b75ad5cb 100644
--- a/include/migration/blocker.h
+++ b/include/migration/blocker.h
@@ -16,8 +16,6 @@
#include "qapi/qapi-types-migration.h"
-#define MIG_MODE_ALL MIG_MODE__MAX
-
/**
* @migrate_add_blocker - prevent all modes of migration from proceeding
*
@@ -82,16 +80,15 @@ int migrate_add_blocker_normal(Error **reasonp, Error **errp);
*
* @reasonp - address of an error to be returned whenever migration is attempted
*
- * @errp - [out] The reason (if any) we cannot block migration right now.
+ * @modes - the migration modes to be blocked, a bit set of MigMode
*
- * @mode - one or more migration modes to be blocked. The list is terminated
- * by -1 or MIG_MODE_ALL. For the latter, all modes are blocked.
+ * @errp - [out] The reason (if any) we cannot block migration right now.
*
* @returns - 0 on success, -EBUSY/-EACCES on failure, with errp set.
*
* *@reasonp is freed and set to NULL if failure is returned.
* On success, the caller must not free *@reasonp before the blocker is removed.
*/
-int migrate_add_blocker_modes(Error **reasonp, Error **errp, MigMode mode, ...);
+int migrate_add_blocker_modes(Error **reasonp, unsigned modes, Error **errp);
#endif
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 592b93021e..e26d418a6e 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -90,18 +90,18 @@ void migration_add_notifier(NotifierWithReturn *notify,
MigrationNotifyFunc func);
/*
- * Same as migration_add_notifier, but applies to be specified @mode.
+ * Same as migration_add_notifier, but applies to the specified @mode
+ * instead of MIG_MODE_NORMAL.
*/
void migration_add_notifier_mode(NotifierWithReturn *notify,
MigrationNotifyFunc func, MigMode mode);
/*
- * Same as migration_add_notifier, but applies to all @mode in the argument
- * list. The list is terminated by -1 or MIG_MODE_ALL. For the latter,
- * the notifier is added for all modes.
+ * Same as migration_add_notifier, but applies to the specified @modes
+ * (a bitset of MigMode).
*/
void migration_add_notifier_modes(NotifierWithReturn *notify,
- MigrationNotifyFunc func, MigMode mode, ...);
+ MigrationNotifyFunc func, unsigned modes);
/*
* Remove a notifier from all modes.
diff --git a/hw/vfio/container-legacy.c b/hw/vfio/container-legacy.c
index 8e9639603e..32c260b345 100644
--- a/hw/vfio/container-legacy.c
+++ b/hw/vfio/container-legacy.c
@@ -977,9 +977,9 @@ static bool vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev,
if (vbasedev->mdev) {
error_setg(&vbasedev->cpr.mdev_blocker,
"CPR does not support vfio mdev %s", vbasedev->name);
- if (migrate_add_blocker_modes(&vbasedev->cpr.mdev_blocker, errp,
- MIG_MODE_CPR_TRANSFER, MIG_MODE_CPR_EXEC,
- -1) < 0) {
+ if (migrate_add_blocker_modes(&vbasedev->cpr.mdev_blocker,
+ BIT(MIG_MODE_CPR_TRANSFER) | BIT(MIG_MODE_CPR_EXEC),
+ errp) < 0) {
goto hiod_unref_exit;
}
}
diff --git a/hw/vfio/cpr-iommufd.c b/hw/vfio/cpr-iommufd.c
index 8a4d65de5e..c244db88fb 100644
--- a/hw/vfio/cpr-iommufd.c
+++ b/hw/vfio/cpr-iommufd.c
@@ -158,9 +158,9 @@ bool vfio_iommufd_cpr_register_iommufd(IOMMUFDBackend *be, Error **errp)
Error **cpr_blocker = &be->cpr_blocker;
if (!vfio_cpr_supported(be, cpr_blocker)) {
- return migrate_add_blocker_modes(cpr_blocker, errp,
- MIG_MODE_CPR_TRANSFER,
- MIG_MODE_CPR_EXEC, -1) == 0;
+ return migrate_add_blocker_modes(cpr_blocker,
+ BIT(MIG_MODE_CPR_TRANSFER) | BIT(MIG_MODE_CPR_TRANSFER),
+ errp);
}
vmstate_register(NULL, -1, &iommufd_cpr_vmstate, be);
diff --git a/hw/vfio/cpr-legacy.c b/hw/vfio/cpr-legacy.c
index 7184c93991..86c943158e 100644
--- a/hw/vfio/cpr-legacy.c
+++ b/hw/vfio/cpr-legacy.c
@@ -176,9 +176,9 @@ bool vfio_legacy_cpr_register_container(VFIOLegacyContainer *container,
MIG_MODE_CPR_REBOOT);
if (!vfio_cpr_supported(container, cpr_blocker)) {
- return migrate_add_blocker_modes(cpr_blocker, errp,
- MIG_MODE_CPR_TRANSFER,
- MIG_MODE_CPR_EXEC, -1) == 0;
+ return migrate_add_blocker_modes(cpr_blocker,
+ BIT(MIG_MODE_CPR_TRANSFER) | BIT(MIG_MODE_CPR_EXEC),
+ errp) == 0;
}
vfio_cpr_add_kvm_notifier();
@@ -187,7 +187,7 @@ bool vfio_legacy_cpr_register_container(VFIOLegacyContainer *container,
migration_add_notifier_modes(&container->cpr.transfer_notifier,
vfio_cpr_fail_notifier,
- MIG_MODE_CPR_TRANSFER, MIG_MODE_CPR_EXEC, -1);
+ BIT(MIG_MODE_CPR_TRANSFER) | BIT(MIG_MODE_CPR_EXEC));
return true;
}
diff --git a/hw/vfio/cpr.c b/hw/vfio/cpr.c
index db462aabcb..998230d271 100644
--- a/hw/vfio/cpr.c
+++ b/hw/vfio/cpr.c
@@ -197,8 +197,7 @@ void vfio_cpr_add_kvm_notifier(void)
if (!kvm_close_notifier.notify) {
migration_add_notifier_modes(&kvm_close_notifier,
vfio_cpr_kvm_close_notifier,
- MIG_MODE_CPR_TRANSFER, MIG_MODE_CPR_EXEC,
- -1);
+ BIT(MIG_MODE_CPR_TRANSFER) | BIT(MIG_MODE_CPR_EXEC));
}
}
@@ -285,7 +284,7 @@ void vfio_cpr_pci_register_device(VFIOPCIDevice *vdev)
{
migration_add_notifier_modes(&vdev->cpr.transfer_notifier,
vfio_cpr_pci_notifier,
- MIG_MODE_CPR_TRANSFER, MIG_MODE_CPR_EXEC, -1);
+ BIT(MIG_MODE_CPR_TRANSFER) | BIT(MIG_MODE_CPR_EXEC));
}
void vfio_cpr_pci_unregister_device(VFIOPCIDevice *vdev)
diff --git a/hw/vfio/device.c b/hw/vfio/device.c
index 8b63e765ac..76869828fc 100644
--- a/hw/vfio/device.c
+++ b/hw/vfio/device.c
@@ -345,8 +345,8 @@ bool vfio_device_get_name(VFIODevice *vbasedev, Error **errp)
"vfio device with fd=%d needs an id property",
vbasedev->fd);
return migrate_add_blocker_modes(&vbasedev->cpr.id_blocker,
- errp, MIG_MODE_CPR_TRANSFER,
- -1) == 0;
+ BIT(MIG_MODE_CPR_TRANSFER),
+ errp) == 0;
}
}
}
diff --git a/migration/migration.c b/migration/migration.c
index 9f0a91b1d9..5f1c83aafd 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1672,8 +1672,6 @@ void migration_cancel(void)
}
}
-static unsigned get_modes(MigMode mode, va_list ap);
-
static void add_notifiers(NotifierWithReturn *notify, unsigned modes)
{
for (MigMode mode = 0; mode < MIG_MODE__MAX; mode++) {
@@ -1685,15 +1683,8 @@ static void add_notifiers(NotifierWithReturn *notify, unsigned modes)
}
void migration_add_notifier_modes(NotifierWithReturn *notify,
- MigrationNotifyFunc func, MigMode mode, ...)
+ MigrationNotifyFunc func, unsigned modes)
{
- unsigned modes;
- va_list ap;
-
- va_start(ap, mode);
- modes = get_modes(mode, ap);
- va_end(ap);
-
notify->notify = (NotifierWithReturnFunc)func;
add_notifiers(notify, modes);
}
@@ -1701,13 +1692,13 @@ void migration_add_notifier_modes(NotifierWithReturn *notify,
void migration_add_notifier_mode(NotifierWithReturn *notify,
MigrationNotifyFunc func, MigMode mode)
{
- migration_add_notifier_modes(notify, func, mode, -1);
+ migration_add_notifier_modes(notify, func, BIT(mode));
}
void migration_add_notifier(NotifierWithReturn *notify,
MigrationNotifyFunc func)
{
- migration_add_notifier_modes(notify, func, MIG_MODE_NORMAL, -1);
+ migration_add_notifier_mode(notify, func, MIG_MODE_NORMAL);
}
void migration_remove_notifier(NotifierWithReturn *notify)
@@ -1890,21 +1881,6 @@ static bool is_only_migratable(Error **reasonp, Error **errp, unsigned modes)
return false;
}
-static unsigned get_modes(MigMode mode, va_list ap)
-{
- unsigned modes = 0;
-
- while (mode != -1 && mode != MIG_MODE_ALL) {
- assert(mode >= MIG_MODE_NORMAL && mode < MIG_MODE__MAX);
- modes |= BIT(mode);
- mode = va_arg(ap, MigMode);
- }
- if (mode == MIG_MODE_ALL) {
- modes = BIT(MIG_MODE__MAX) - 1;
- }
- return modes;
-}
-
static int add_blockers(Error **reasonp, Error **errp, unsigned modes)
{
for (MigMode mode = 0; mode < MIG_MODE__MAX; mode++) {
@@ -1918,23 +1894,16 @@ static int add_blockers(Error **reasonp, Error **errp, unsigned modes)
int migrate_add_blocker(Error **reasonp, Error **errp)
{
- return migrate_add_blocker_modes(reasonp, errp, MIG_MODE_ALL);
+ return migrate_add_blocker_modes(reasonp, -1u, errp);
}
int migrate_add_blocker_normal(Error **reasonp, Error **errp)
{
- return migrate_add_blocker_modes(reasonp, errp, MIG_MODE_NORMAL, -1);
+ return migrate_add_blocker_modes(reasonp, BIT(MIG_MODE_NORMAL), errp);
}
-int migrate_add_blocker_modes(Error **reasonp, Error **errp, MigMode mode, ...)
+int migrate_add_blocker_modes(Error **reasonp, unsigned modes, Error **errp)
{
- unsigned modes;
- va_list ap;
-
- va_start(ap, mode);
- modes = get_modes(mode, ap);
- va_end(ap);
-
if (is_only_migratable(reasonp, errp, modes)) {
return -EACCES;
} else if (is_busy(reasonp, errp)) {
diff --git a/stubs/migr-blocker.c b/stubs/migr-blocker.c
index 11cbff268f..e54c7160d3 100644
--- a/stubs/migr-blocker.c
+++ b/stubs/migr-blocker.c
@@ -11,7 +11,7 @@ int migrate_add_blocker_normal(Error **reasonp, Error **errp)
return 0;
}
-int migrate_add_blocker_modes(Error **reasonp, Error **errp, MigMode mode, ...)
+int migrate_add_blocker_modes(Error **reasonp, unsigned modes, Error **errp)
{
return 0;
}
diff --git a/system/physmem.c b/system/physmem.c
index a340ca3e61..a7e2a5d07f 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -2255,8 +2255,8 @@ static void ram_block_add(RAMBlock *new_block, Error **errp)
"Memory region %s uses guest_memfd, "
"which is not supported with CPR.",
memory_region_name(new_block->mr));
- migrate_add_blocker_modes(&new_block->cpr_blocker, errp,
- MIG_MODE_CPR_TRANSFER, -1);
+ migrate_add_blocker_modes(&new_block->cpr_blocker,
+ BIT(MIG_MODE_CPR_TRANSFER), errp);
}
}
@@ -4462,8 +4462,8 @@ void ram_block_add_cpr_blocker(RAMBlock *rb, Error **errp)
"Memory region %s is not compatible with CPR. share=on is "
"required for memory-backend objects, and aux-ram-share=on is "
"required.", memory_region_name(rb->mr));
- migrate_add_blocker_modes(&rb->cpr_blocker, errp, MIG_MODE_CPR_TRANSFER,
- -1);
+ migrate_add_blocker_modes(&rb->cpr_blocker, BIT(MIG_MODE_CPR_TRANSFER),
+ errp);
}
void ram_block_del_cpr_blocker(RAMBlock *rb)
--
2.49.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] migration: Put Error **errp parameter last
2025-10-27 6:45 [PATCH 0/3] migration: Cleanup around MigMode sets Markus Armbruster
2025-10-27 6:45 ` [PATCH 1/3] migration: Use unsigned instead of int for bit set of MigMode Markus Armbruster
2025-10-27 6:45 ` [PATCH 2/3] migration: Use bitset of MigMode instead of variable arguments Markus Armbruster
@ 2025-10-27 6:45 ` Markus Armbruster
2025-10-27 13:24 ` [PATCH 0/3] migration: Cleanup around MigMode sets Fabiano Rosas
3 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2025-10-27 6:45 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, farosas, berrange, guobin
qapi/error.h's big comment:
* - Functions that use Error to report errors have an Error **errp
* parameter. It should be the last parameter, except for functions
* taking variable arguments.
is_only_migratable() and add_blockers() have it in the middle. Clean
them up.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
migration/migration.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 5f1c83aafd..8a06b54167 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1867,7 +1867,7 @@ static bool is_busy(Error **reasonp, Error **errp)
return false;
}
-static bool is_only_migratable(Error **reasonp, Error **errp, unsigned modes)
+static bool is_only_migratable(Error **reasonp, unsigned modes, Error **errp)
{
ERRP_GUARD();
@@ -1881,7 +1881,7 @@ static bool is_only_migratable(Error **reasonp, Error **errp, unsigned modes)
return false;
}
-static int add_blockers(Error **reasonp, Error **errp, unsigned modes)
+static int add_blockers(Error **reasonp, unsigned modes, Error **errp)
{
for (MigMode mode = 0; mode < MIG_MODE__MAX; mode++) {
if (modes & BIT(mode)) {
@@ -1904,12 +1904,12 @@ int migrate_add_blocker_normal(Error **reasonp, Error **errp)
int migrate_add_blocker_modes(Error **reasonp, unsigned modes, Error **errp)
{
- if (is_only_migratable(reasonp, errp, modes)) {
+ if (is_only_migratable(reasonp, modes, errp)) {
return -EACCES;
} else if (is_busy(reasonp, errp)) {
return -EBUSY;
}
- return add_blockers(reasonp, errp, modes);
+ return add_blockers(reasonp, modes, errp);
}
int migrate_add_blocker_internal(Error **reasonp, Error **errp)
@@ -1919,7 +1919,7 @@ int migrate_add_blocker_internal(Error **reasonp, Error **errp)
if (is_busy(reasonp, errp)) {
return -EBUSY;
}
- return add_blockers(reasonp, errp, modes);
+ return add_blockers(reasonp, modes, errp);
}
void migrate_del_blocker(Error **reasonp)
--
2.49.0
^ permalink raw reply related [flat|nested] 6+ messages in thread