Re: [PATCH v2] qemu: fix qemuMigrationCapability enum
On Wed, May 07, 2025 at 10:07:41 +0300, Dmitry Frolov via Devel wrote:
Enum variable of type qemuMigrationCapability is checked for zero in
src/qemu/qemu_migration_params.c:729.
"if (item->optional) { ..."
Actualy, QEMU_MIGRATION_CAP_XBZRLE enum constant has value 0.
So, at least, the condition is incorrect.
v1: introducing a separate enum for optional capabilities
v2: another approach: fix only the incorrect condition
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Dmitry Frolov <frolov(a)swemel.ru>
---
src/qemu/qemu_migration_params.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index c10660d6f2..98d314cf2d 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -700,6 +700,12 @@ qemuMigrationParamsSetBlockDirtyBitmapMapping(qemuMigrationParams
*migParams,
ignore_value(virBitmapClearBit(migParams->caps,
QEMU_MIGRATION_CAP_BLOCK_DIRTY_BITMAPS));
}
+static bool
+qemuMigrationCapabilityIsOptional(qemuMigrationCapability cap)
+{
+ return cap == QEMU_MIGRATION_CAP_POSTCOPY_PREEMPT ||
+ cap == QEMU_MIGRATION_CAP_SWITCHOVER_ACK;
+}
This makes the code much more fragile for exactly 0 benefit.
Arguably yes QEMU_MIGRATION_CAP_XBZRLE can't be used, but it isn't used
either.
I'm not really persuaded that it is a problem worth changing the code
for (e.g. just docum,ent at the definition of
qemuMigrationParamsFlagMap[] that QEMU_MIGRATION_CAP_XBZRLE must not be
used in the current state.
Alternative is to initialize all members of the array where the optional
field is not used to QEMU_MIGRATION_CAP_LAST. Yes it's more lines and
yes it will need to be documented as well.
Please do not make the code worse just to appease a static code
validator's false positive.
qemuMigrationParams *
qemuMigrationParamsFromFlags(virTypedParameterPtr params,
@@ -725,7 +731,7 @@ qemuMigrationParamsFromFlags(virTypedParameterPtr params,
qemuMigrationCapabilityTypeToString(item->cap));
ignore_value(virBitmapSetBit(migParams->caps, item->cap));
- if (item->optional) {
and check that item->optional != QEMU_MIGRATION_CAP_LAST
+ if
(qemuMigrationCapabilityIsOptional(item->optional)) {
qemuMigrationCapability opt = item->optional;
ignore_value(virBitmapSetBit(migParams->optional, opt));
if (item->party != party)
--
2.34.1