[libvirt] [PATCH v2 0/2] qemu_capabilities; Drop virQEMUCapsSetVAList
Technically, this is v2 of: https://www.redhat.com/archives/libvir-list/2019-March/msg01778.html but is this one implements different approach. Plus it breaks dependency between two unrelated enums that I've noticed when debugging broken build on a 32bit arch. Michal Prívozník (2): qemu_capabilities; Drop virQEMUCapsSetVAList qemuxml2argvtest: Drop dependency between testInfoArgName and virQEMUCapsFlags enums src/qemu/qemu_capabilities.c | 15 +++---------- src/qemu/qemu_capabilities.h | 2 -- tests/qemuxml2argvtest.c | 41 +++++++++++++++++++++++------------- 3 files changed, 29 insertions(+), 29 deletions(-) -- 2.19.2
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 15 +++------------ src/qemu/qemu_capabilities.h | 2 -- tests/qemuxml2argvtest.c | 6 +++++- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c5954edaf0..56228e7a36 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1663,24 +1663,15 @@ virQEMUCapsSet(virQEMUCapsPtr qemuCaps, } -void -virQEMUCapsSetVAList(virQEMUCapsPtr qemuCaps, - va_list list) -{ - int flag; - - while ((flag = va_arg(list, int)) < QEMU_CAPS_LAST) - ignore_value(virBitmapSetBit(qemuCaps->flags, flag)); -} - - void virQEMUCapsSetList(virQEMUCapsPtr qemuCaps, ...) { va_list list; + int flag; va_start(list, qemuCaps); - virQEMUCapsSetVAList(qemuCaps, list); + while ((flag = va_arg(list, int)) < QEMU_CAPS_LAST) + virQEMUCapsSet(qemuCaps, flag); va_end(list); } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 7625d754a3..06c7606e2f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -518,8 +518,6 @@ virQEMUCapsPtr virQEMUCapsNew(void); void virQEMUCapsSet(virQEMUCapsPtr qemuCaps, virQEMUCapsFlags flag) ATTRIBUTE_NONNULL(1); -void virQEMUCapsSetVAList(virQEMUCapsPtr qemuCaps, - va_list list) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); void virQEMUCapsSetList(virQEMUCapsPtr qemuCaps, ...) ATTRIBUTE_NONNULL(1); void virQEMUCapsClear(virQEMUCapsPtr qemuCaps, diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 6e7d1b0b9a..4d6e4b0d39 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -642,6 +642,7 @@ testInfoSetArgs(struct testInfo *info, char *capsarch = NULL; char *capsver = NULL; VIR_AUTOFREE(char *) capsfile = NULL; + int flag; int ret = -1; va_start(argptr, capslatest); @@ -650,7 +651,10 @@ testInfoSetArgs(struct testInfo *info, case ARG_QEMU_CAPS: if (qemuCaps || !(qemuCaps = virQEMUCapsNew())) goto cleanup; - virQEMUCapsSetVAList(qemuCaps, argptr); + + while ((flag = va_arg(argptr, int)) < QEMU_CAPS_LAST) + virQEMUCapsSet(qemuCaps, flag); + break; case ARG_GIC: -- 2.19.2
On Wed, Mar 27, 2019 at 05:50:45PM +0100, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 15 +++------------ src/qemu/qemu_capabilities.h | 2 -- tests/qemuxml2argvtest.c | 6 +++++- 3 files changed, 8 insertions(+), 15 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Introduced in fdf6c89ee7b, this dependency looks weird. It was needed because of the way that while() loop was written - it fetches next argument in every iteration. Therefore, our only option was for ARG_END to have the same value as QEMU_CAPS_LAST. This also meant that QEMU_CAPS_* could have been only at the end of the __VA_ARGS__. This commit reworks the while() loop and removes the dependency. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuxml2argvtest.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4d6e4b0d39..0c0dcae197 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -616,19 +616,7 @@ typedef enum { ARG_PARSEFLAGS, ARG_CAPS_ARCH, ARG_CAPS_VER, - - /* ARG_END is our va_args sentinel. The value QEMU_CAPS_LATEST is - * necessary to handle the DO_TEST(..., NONE) case, which through macro - * magic will give the va_args list: - * - * ARG_QEMU_CAPS, NONE, QEMU_CAPS_LAST, ARG_END - * - * SetArgs consumes the first item, hands off control to virQEMUCapsX - * virQEMUCapsX sees NONE aka QEMU_CAPS_LAST, returns to SetArgs. - * SetArgs sees QEMU_CAPS_LAST aka ARG_END, and exits the parse loop. - * If ARG_END != QEMU_CAPS_LAST, this last step would generate an error. - */ - ARG_END = QEMU_CAPS_LAST, + ARG_END, } testInfoArgName; static int @@ -646,7 +634,8 @@ testInfoSetArgs(struct testInfo *info, int ret = -1; va_start(argptr, capslatest); - while ((argname = va_arg(argptr, testInfoArgName)) < ARG_END) { + argname = va_arg(argptr, testInfoArgName); + while (argname != ARG_END) { switch (argname) { case ARG_QEMU_CAPS: if (qemuCaps || !(qemuCaps = virQEMUCapsNew())) @@ -655,6 +644,22 @@ testInfoSetArgs(struct testInfo *info, while ((flag = va_arg(argptr, int)) < QEMU_CAPS_LAST) virQEMUCapsSet(qemuCaps, flag); + /* Some tests are run with NONE capabilities, which is just + * another name for QEMU_CAPS_LAST. If that is the case the + * arguments look like this : + * + * ARG_QEMU_CAPS, NONE, QEMU_CAPS_LAST, ARG_END + * + * Fetch one argument more and if it is QEMU_CAPS_LAST then + * break from the switch() to force getting next argument + * in the line. If it is not QEMU_CAPS_LAST then we've + * fetched real ARG_* and we must process it. + */ + if ((flag = va_arg(argptr, int)) != QEMU_CAPS_LAST) { + argname = flag; + continue; + } + break; case ARG_GIC: @@ -690,6 +695,8 @@ testInfoSetArgs(struct testInfo *info, fprintf(stderr, "Unexpected test info argument"); goto cleanup; } + + argname = va_arg(argptr, testInfoArgName); } if (!!capsarch ^ !!capsver) { -- 2.19.2
On Wed, Mar 27, 2019 at 05:50:46PM +0100, Michal Privoznik wrote:
Introduced in fdf6c89ee7b, this dependency looks weird. It was needed because of the way that while() loop was written - it fetches next argument in every iteration. Therefore, our only option was for ARG_END to have the same value as QEMU_CAPS_LAST. This also meant that QEMU_CAPS_* could have been only at the end of the __VA_ARGS__.
This commit reworks the while() loop and removes the dependency.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuxml2argvtest.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé -
Michal Privoznik