From 791c90cb8d0596939e275b52656c383e9c9b63c2 Mon Sep 17 00:00:00 2001 From: "Jason J. Herne" Date: Tue, 12 Apr 2016 10:13:27 -0400 Subject: [PATCH] Libvirt: virTypedParamsValidate: Fix detection of multiple parameters virTypedParamsValidate currently uses an index based check to find duplicate parameters. This check does not work. Consider the following simple example: We have only 2 keys A (multiples allowed) B (multiples NOT allowed) We are given the following list of parameters to check: A A B If you work through the validation loop you will see that our last iteration through the loop has i=2 and j=1. In this case, i > j and keys[j].value.i will indicate that multiples are not allowed. Both conditionals are satisfied so an incorrect error will be given: "parameter '%s' occurs multiple times" This patch replaces the index based check with code that remembers the name of the last parameter seen and only triggers the error case if the current parameter name equals the last one. This works because the list is sorted and duplicate parameters will be grouped together. In reality, we hit this bug while using selective block migration to migrate a guest with 5 disks. 5 was apparently just the right number to push i > j and hit this bug. virsh migrate --live guestname --copy-storage-all --migrate-disks vdb,vdc,vdd,vde,vdf qemu+ssh://dsthost/system Signed-off-by: Jason J. Herne Reviewed-by: Eric Farman (cherry picked from commit 0e570a6acc13ebbca913e69026f1468daf28240f) --- src/util/virtypedparam.c | 6 ++++-- tests/virtypedparamtest.c | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 138fc64381..23109e1c2e 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -66,7 +66,7 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) va_list ap; int ret = -1; size_t i, j; - const char *name; + const char *name, *last_name = NULL; int type; size_t nkeys = 0, nkeysalloc = 0; virTypedParameterPtr sorted = NULL, keys = NULL; @@ -106,7 +106,8 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) if (STRNEQ(sorted[i].field, keys[j].field)) { j++; } else { - if (i > j && !(keys[j].value.i & VIR_TYPED_PARAM_MULTIPLE)) { + if (STREQ_NULLABLE(last_name, sorted[i].field) && + !(keys[j].value.i & VIR_TYPED_PARAM_MULTIPLE)) { virReportError(VIR_ERR_INVALID_ARG, _("parameter '%s' occurs multiple times"), sorted[i].field); @@ -125,6 +126,7 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) virTypedParameterTypeToString(keys[j].type)); goto cleanup; } + last_name = sorted[i].field; i++; } } diff --git a/tests/virtypedparamtest.c b/tests/virtypedparamtest.c index f4a57925e5..d125491596 100644 --- a/tests/virtypedparamtest.c +++ b/tests/virtypedparamtest.c @@ -56,6 +56,7 @@ testTypedParamsValidate(const void *opaque) "foobar", VIR_TYPED_PARAM_STRING | test->foobar_flags, "foo", VIR_TYPED_PARAM_INT, "bar", VIR_TYPED_PARAM_UINT, + "zzz", VIR_TYPED_PARAM_UINT, NULL); if (test->expected_errcode) { @@ -258,6 +259,19 @@ testTypedParamsValidator(void) ) .expected_errcode = 0, .expected_errmessage = NULL, }, + { + .name = "BUG, non-duplicate marked as duplicate", + .foobar_flags = VIR_TYPED_PARAM_MULTIPLE, + PARAMS( + { .field = "foobar", .type = VIR_TYPED_PARAM_STRING }, + { .field = "foobar", .type = VIR_TYPED_PARAM_STRING }, + { .field = "foobar", .type = VIR_TYPED_PARAM_STRING }, + { .field = "foobar", .type = VIR_TYPED_PARAM_STRING }, + { .field = "foobar", .type = VIR_TYPED_PARAM_STRING }, + { .field = "zzz", .type = VIR_TYPED_PARAM_UINT }, + ) + .expected_errcode = 0, .expected_errmessage = NULL, + }, { .name = NULL } -- GitLab