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. Thus, all uninitialized .optinnal fields of the static array qemuMigrationParamsFlagMap[] will be implicitly initialized with value 0 (QEMU_MIGRATION_CAP_XBZRLE). To my opinion, introducing a separate enum for optional capabilities, would be a better solution. Found by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Dmitry Frolov <frolov@swemel.ru> --- src/qemu/qemu_migration_params.c | 16 +++++++++++----- src/qemu/qemu_migration_params.h | 12 ++++++++++-- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -XXX,XX +XXX,XX @@ VIR_ENUM_IMPL(qemuMigrationCapability, "dirty-bitmaps", "return-path", "zero-copy-send", +); + +VIR_ENUM_IMPL(qemuMigrationOptCap, + QEMU_MIGRATION_OPTCAP_LAST, + "none", "postcopy-preempt", "switchover-ack", ); @@ -XXX,XX +XXX,XX @@ struct _qemuMigrationParamsFlagMapItem { /* An optional capability to set in addition to @cap in case it is * supported. Depending on @part either one or both sides of migration * has to support the optional capability to be enabled. */ - qemuMigrationCapability optional; + qemuMigrationOptCap optional; /* Bit-wise OR of qemuMigrationParty. Determines whether the capability has * to be enabled on the source, on the destination, or on both sides of * migration. */ @@ -XXX,XX +XXX,XX @@ static const qemuMigrationParamsFlagMapItem qemuMigrationParamsFlagMap[] = { {.match = QEMU_MIGRATION_FLAG_REQUIRED, .flag = VIR_MIGRATE_POSTCOPY, .cap = QEMU_MIGRATION_CAP_POSTCOPY, - .optional = QEMU_MIGRATION_CAP_POSTCOPY_PREEMPT, + .optional = QEMU_MIGRATION_OPTCAP_POSTCOPY_PREEMPT, .party = QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION}, {.match = QEMU_MIGRATION_FLAG_REQUIRED, @@ -XXX,XX +XXX,XX @@ static const qemuMigrationParamsFlagMapItem qemuMigrationParamsFlagMap[] = { {.match = QEMU_MIGRATION_FLAG_FORBIDDEN, .flag = VIR_MIGRATE_TUNNELLED, .cap = QEMU_MIGRATION_CAP_RETURN_PATH, - .optional = QEMU_MIGRATION_CAP_SWITCHOVER_ACK, + .optional = QEMU_MIGRATION_OPTCAP_SWITCHOVER_ACK, .party = QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION}, {.match = QEMU_MIGRATION_FLAG_REQUIRED, @@ -XXX,XX +XXX,XX @@ qemuMigrationParamsFromFlags(virTypedParameterPtr params, qemuMigrationCapabilityTypeToString(item->cap)); ignore_value(virBitmapSetBit(migParams->caps, item->cap)); - if (item->optional) { - qemuMigrationCapability opt = item->optional; + if (item->optional > QEMU_MIGRATION_OPTCAP_NONE && + item->optional < QEMU_MIGRATION_OPTCAP_LAST) { + qemuMigrationOptCap opt = item->optional; ignore_value(virBitmapSetBit(migParams->optional, opt)); if (item->party != party) ignore_value(virBitmapSetBit(migParams->remoteOptional, opt)); diff --git a/src/qemu/qemu_migration_params.h b/src/qemu/qemu_migration_params.h index XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_migration_params.h +++ b/src/qemu/qemu_migration_params.h @@ -XXX,XX +XXX,XX @@ typedef enum { QEMU_MIGRATION_CAP_BLOCK_DIRTY_BITMAPS, QEMU_MIGRATION_CAP_RETURN_PATH, QEMU_MIGRATION_CAP_ZERO_COPY_SEND, - QEMU_MIGRATION_CAP_POSTCOPY_PREEMPT, - QEMU_MIGRATION_CAP_SWITCHOVER_ACK, QEMU_MIGRATION_CAP_LAST } qemuMigrationCapability; VIR_ENUM_DECL(qemuMigrationCapability); +typedef enum { + QEMU_MIGRATION_OPTCAP_NONE, + QEMU_MIGRATION_OPTCAP_POSTCOPY_PREEMPT, + QEMU_MIGRATION_OPTCAP_SWITCHOVER_ACK, + + QEMU_MIGRATION_OPTCAP_LAST +} qemuMigrationOptCap; +VIR_ENUM_DECL(qemuMigrationOptCap); + + typedef enum { QEMU_MIGRATION_PARAM_COMPRESS_LEVEL, QEMU_MIGRATION_PARAM_COMPRESS_THREADS, -- 2.34.1 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@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 XXXXXXX..XXXXXXX 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -XXX,XX +XXX,XX @@ 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; +} qemuMigrationParams * qemuMigrationParamsFromFlags(virTypedParameterPtr params, @@ -XXX,XX +XXX,XX @@ qemuMigrationParamsFromFlags(virTypedParameterPtr params, qemuMigrationCapabilityTypeToString(item->cap)); ignore_value(virBitmapSetBit(migParams->caps, item->cap)); - if (item->optional) { + if (qemuMigrationCapabilityIsOptional(item->optional)) { qemuMigrationCapability opt = item->optional; ignore_value(virBitmapSetBit(migParams->optional, opt)); if (item->party != party) -- 2.34.1