9:43 a.m.
Right now in qemuxml2argv we have a proliferation of DO_*TEST* macros.
They essentially fill in data in the testInfo struct and invoke the
test function.
There's several bits of data that we want to specify for a small
subset of tests: flags, parseFlags, migrateFrom/migrateFd, gic. The
base macros need to handle these in their argument lists, and we
provide many convenience macros that fill in default values for the
less common parameters. Any time we want to add a new bit of data
though, the base macros are extended, and every caller of those
needs to be adjusted, and we are faced with the question of whether
to extend the combinatorial explosion of convenience macros which
have only a subset of options exposed.
This series adds a testInfoSetArgs which uses va_args to make these
mandatory parameters optional. The general format is:
DO_TEST_FULL(...
ARG_FOO, foovalue,
ARG_BAR, barvalue, ...)
Specifically, one of the migrate tests went from:
DO_TEST_FULL("restore-v2", "exec:cat", 7, 0, 0, GIC_NONE, NONE);
to:
DO_TEST_FULL("restore-v2",
ARG_MIGRATE_FROM, "exec:cat",
ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS, NONE);
Which may not seem directly compelling, but adding new testInfo fields
will be much easier. This will also be a base for a later series to
share the VIR_TEST_CAPS infrastructure with qemuxml2xmltest
Cole Robinson (21):
qemu: add virQEMUCapsSetVList
tests: qemuxml2argv: add testInfoSetArgs
tests: qemuxml2argv: add va_arg enum handling
tests: qemuxml2argv: push ARG_QEMU_CAPS to callers
tests: qemuxml2argv: break apart testInitQEMUCaps
tests: qemuxml2argv: handle gic with vaargs
tests: qemuxml2argv: handle migrate* with varargs
tests: qemuxml2argv: handle flags with varargs
tests: qemuxml2argv: handle parseFlags with varargs
tests: qemuxml2argv: remove DO_TEST_PARSE_FLAGS_ERROR
tests: qemuxml2argv: remove unused DO_TEST_CAPS* macros
tests: qemuxml2argv: add a comment separating DO_TEST* macros
tests: qemuxml2argv: remove unused CAPS migrateFrom
tests: qemuxml2argv: use varargs for CAPS flags
tests: qemuxml2argv: remove full testInfo initialization
tests: qemuxml2argv: centralize CAPS suffix building
tests: qemuxml2argv: build capsfile in DO_TEST_CAPS_INTERNAL
tests: qemuxml2argv: add testInfoClear
tests: qemuxml2argv: move DO_TEST qemuCaps init
tests: qemuxml2argv: move DO_CAPS_TEST* qemuCaps init
tests: qemuxml2argv: add TEST_INTERNAL
src/qemu/qemu_capabilities.c | 14 +-
src/qemu/qemu_capabilities.h | 2 +
tests/qemuxml2argvtest.c | 273 +++++++++++++++++++++++------------
3 files changed, 193 insertions(+), 96 deletions(-)
--
2.20.1
9:43 a.m.
New subject: [libvirt] [PATCH 01/21] qemu: add virQEMUCapsSetVList
It will be used in future patches
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 14 +++++++++++---
src/qemu/qemu_capabilities.h | 2 ++
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4c8229fbda..a274ce7992 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1664,14 +1664,22 @@ virQEMUCapsSet(virQEMUCapsPtr qemuCaps,
void
-virQEMUCapsSetList(virQEMUCapsPtr qemuCaps, ...)
+virQEMUCapsSetVList(virQEMUCapsPtr qemuCaps, va_list list)
{
- va_list list;
int flag;
- va_start(list, qemuCaps);
while ((flag = va_arg(list, int)) < QEMU_CAPS_LAST)
ignore_value(virBitmapSetBit(qemuCaps->flags, flag));
+}
+
+
+void
+virQEMUCapsSetList(virQEMUCapsPtr qemuCaps, ...)
+{
+ va_list list;
+
+ va_start(list, qemuCaps);
+ virQEMUCapsSetVList(qemuCaps, list);
va_end(list);
}
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 06c7606e2f..3601207989 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -518,6 +518,8 @@ virQEMUCapsPtr virQEMUCapsNew(void);
void virQEMUCapsSet(virQEMUCapsPtr qemuCaps,
virQEMUCapsFlags flag) ATTRIBUTE_NONNULL(1);
+void virQEMUCapsSetVList(virQEMUCapsPtr qemuCaps,
+ va_list list) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
void virQEMUCapsSetList(virQEMUCapsPtr qemuCaps, ...) ATTRIBUTE_NONNULL(1);
void virQEMUCapsClear(virQEMUCapsPtr qemuCaps,
--
2.20.1
4:51 a.m.
New subject: [libvirt] [PATCH 01/21] qemu: add virQEMUCapsSetVList
On Thu, 2019-03-14 at 10:43 -0400, Cole Robinson wrote:
It will be used in future patches
You're actually changing the existing virQEMUCapsSetList() to use
it... I'd point that out in the commit message. You can still
mention the fact that you're going to add more users later on.
[...]
void
+virQEMUCapsSetVList(virQEMUCapsPtr qemuCaps, va_list list)
I'd call this virQEMUCapsSetVAList(), given the type of the second
argument; the existing virCommandNewVAList() already uses this naming
scheme.
The second argument should be on a separate line.
Everything else looks good, so
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
9:43 a.m.
New subject: [libvirt] [PATCH 02/21] tests: qemuxml2argv: add testInfoSetArgs
For now it just fills in the qemuCaps list. We will expand it
in future patches
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 67c5c74ec5..3b90cd1873 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -624,6 +624,18 @@ testCompareXMLToArgv(const void *data)
return ret;
}
+static int
+testInfoSetArgs(struct testInfo *info, ...)
+{
+ va_list argptr;
+ int ret = 0;
+
+ va_start(argptr, info);
+ virQEMUCapsSetVList(info->qemuCaps, argptr);
+ va_end(argptr);
+ return ret;
+}
+
# define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XXXXXX"
static int
@@ -809,7 +821,8 @@ mymain(void)
}; \
if (testInitQEMUCaps(&info, gic) < 0) \
return EXIT_FAILURE; \
- virQEMUCapsSetList(info.qemuCaps, __VA_ARGS__, QEMU_CAPS_LAST); \
+ if (testInfoSetArgs(&info, __VA_ARGS__, QEMU_CAPS_LAST) < 0) \
+ return EXIT_FAILURE; \
if (virTestRun("QEMU XML-2-ARGV " name, \
testCompareXMLToArgv, &info) < 0) \
ret = -1; \
--
2.20.1
4:58 a.m.
New subject: [libvirt] [PATCH 02/21] tests: qemuxml2argv: add testInfoSetArgs
On Thu, 2019-03-14 at 10:43 -0400, Cole Robinson wrote:
[...]
+static int
+testInfoSetArgs(struct testInfo *info, ...)
+{
+ va_list argptr;
+ int ret = 0;
+
+ va_start(argptr, info);
+ virQEMUCapsSetVList(info->qemuCaps, argptr);
+ va_end(argptr);
Please leave an extra empty line here.
+ return ret;
+}
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
9:43 a.m.
New subject: [libvirt] [PATCH 03/21] tests: qemuxml2argv: add va_arg enum handling
This establishes a pattern that will allow us to make test macros
more general purpose, by taking optional arguments. The general
format will be:
DO_TEST_FULL(...
ARG_FOO, <value1>,
ARG_BAR, <value2>)
ARG_X are just enum values that we look for in the va_args and know
how to interpret.
Implement this for the existing implicit qemuCaps va_args
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 3b90cd1873..0dba908c70 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -624,14 +624,35 @@ testCompareXMLToArgv(const void *data)
return ret;
}
+typedef enum {
+ ARG_QEMU_CAPS = 1,
+
+ ARG_END = QEMU_CAPS_LAST,
+} testInfoArgNames;
+
static int
testInfoSetArgs(struct testInfo *info, ...)
{
va_list argptr;
- int ret = 0;
+ testInfoArgNames argname;
+ int ret = -1;
va_start(argptr, info);
- virQEMUCapsSetVList(info->qemuCaps, argptr);
+ while ((argname = va_arg(argptr, int)) < ARG_END) {
+ switch (argname) {
+ case ARG_QEMU_CAPS:
+ virQEMUCapsSetVList(info->qemuCaps, argptr);
+ break;
+
+ case ARG_END:
+ default:
+ fprintf(stderr, "Unexpected test info argument");
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+ cleanup:
va_end(argptr);
return ret;
}
@@ -821,7 +842,8 @@ mymain(void)
}; \
if (testInitQEMUCaps(&info, gic) < 0) \
return EXIT_FAILURE; \
- if (testInfoSetArgs(&info, __VA_ARGS__, QEMU_CAPS_LAST) < 0) \
+ if (testInfoSetArgs(&info, ARG_QEMU_CAPS, \
+ __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \
return EXIT_FAILURE; \
if (virTestRun("QEMU XML-2-ARGV " name, \
testCompareXMLToArgv, &info) < 0) \
--
2.20.1
10:23 a.m.
New subject: [libvirt] [PATCH 03/21] tests: qemuxml2argv: add va_arg enum handling
On 3/14/19 9:43 AM, Cole Robinson wrote:
This establishes a pattern that will allow us to make test macros
more general purpose, by taking optional arguments. The general
format will be:
DO_TEST_FULL(...
ARG_FOO, <value1>,
ARG_BAR, <value2>)
ARG_X are just enum values that we look for in the va_args and know
how to interpret.
Implement this for the existing implicit qemuCaps va_args
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
+typedef enum {
+ ARG_QEMU_CAPS = 1,
+
+ ARG_END = QEMU_CAPS_LAST,
+} testInfoArgNames;
+
Reading this after my cover letter reply:
Oh, so you _do_ have a sentinel...
static int
testInfoSetArgs(struct testInfo *info, ...)
{
va_list argptr;
- int ret = 0;
+ testInfoArgNames argname;
+ int ret = -1;
va_start(argptr, info);
- virQEMUCapsSetVList(info->qemuCaps, argptr);
+ while ((argname = va_arg(argptr, int)) < ARG_END) {
+ switch (argname) {
+ case ARG_QEMU_CAPS:
+ virQEMUCapsSetVList(info->qemuCaps, argptr);
+ break;
+
+ case ARG_END:
+ default:
+ fprintf(stderr, "Unexpected test info argument");
...and you are handling it (except that you ALWAYS handle it by printing
an error, is that intentional?),...
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+ cleanup:
va_end(argptr);
return ret;
}
@@ -821,7 +842,8 @@ mymain(void)
}; \
if (testInitQEMUCaps(&info, gic) < 0) \
return EXIT_FAILURE; \
- if (testInfoSetArgs(&info, __VA_ARGS__, QEMU_CAPS_LAST) < 0) \
+ if (testInfoSetArgs(&info, ARG_QEMU_CAPS, \
+ __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \
...and you are merely ensuring that the macro supplies the sentinel
automatically, instead of the users having to be aware of it. Works
because all users are calling a macro rather than the direct function.
In fact, for this patch, you are supplying a double-sentinel, and I'm
suspecting (without reading ahead) that later patches improve things as
you add more ARG_ markers, and replacing QEMU_CAPS_LAST (which is now
identical to ARG_END, and confusingly given twice) with something more
obvious.
Okay, you can ignore my noise on the cover letter.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
2:42 p.m.
New subject: [libvirt] [PATCH 03/21] tests: qemuxml2argv: add va_arg enum handling
On 3/14/19 11:23 AM, Eric Blake wrote:
On 3/14/19 9:43 AM, Cole Robinson wrote:
> This establishes a pattern that will allow us to make test macros
> more general purpose, by taking optional arguments. The general
> format will be:
>
> DO_TEST_FULL(...
> ARG_FOO, <value1>,
> ARG_BAR, <value2>)
>
> ARG_X are just enum values that we look for in the va_args and know
> how to interpret.
>
> Implement this for the existing implicit qemuCaps va_args
>
> Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
> ---
> tests/qemuxml2argvtest.c | 28 +++++++++++++++++++++++++---
> 1 file changed, 25 insertions(+), 3 deletions(-)
> +typedef enum {
> + ARG_QEMU_CAPS = 1,
> +
> + ARG_END = QEMU_CAPS_LAST,
> +} testInfoArgNames;
> +
Reading this after my cover letter reply:
Oh, so you _do_ have a sentinel...
> static int
> testInfoSetArgs(struct testInfo *info, ...)
> {
> va_list argptr;
> - int ret = 0;
> + testInfoArgNames argname;
> + int ret = -1;
>
> va_start(argptr, info);
> - virQEMUCapsSetVList(info->qemuCaps, argptr);
> + while ((argname = va_arg(argptr, int)) < ARG_END) {
> + switch (argname) {
> + case ARG_QEMU_CAPS:
> + virQEMUCapsSetVList(info->qemuCaps, argptr);
> + break;
> +
> + case ARG_END:
> + default:
> + fprintf(stderr, "Unexpected test info argument");
...and you are handling it (except that you ALWAYS handle it by printing
an error, is that intentional?),...
See the while() condition: if we see ARG_END, we exit the loop, so we
shouldn't ever hit this condition and it's only in the switch to appease gcc
> + goto cleanup;
> + }
> + }
> +
> + ret = 0;
> + cleanup:
> va_end(argptr);
> return ret;
> }
> @@ -821,7 +842,8 @@ mymain(void)
> }; \
> if (testInitQEMUCaps(&info, gic) < 0) \
> return EXIT_FAILURE; \
> - if (testInfoSetArgs(&info, __VA_ARGS__, QEMU_CAPS_LAST) < 0) \
> + if (testInfoSetArgs(&info, ARG_QEMU_CAPS, \
> + __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \
...and you are merely ensuring that the macro supplies the sentinel
automatically, instead of the users having to be aware of it. Works
because all users are calling a macro rather than the direct function.
In fact, for this patch, you are supplying a double-sentinel, and I'm
suspecting (without reading ahead) that later patches improve things as
you add more ARG_ markers, and replacing QEMU_CAPS_LAST (which is now
identical to ARG_END, and confusingly given twice) with something more
obvious.
The double sentinel is actually a requirement of the current code,
because once we see ARG_QEMU_CAPS, we pass off the va_list to
virQEMUCapsSetVList which does its own arg processing and uses
QEMU_CAPS_LAST as a sentinel. We then kick back to this while() loop,
which sees ARG_END, and completes parsing.
The ARG_END = QEMU_CAPS_LAST is a bit weird, but it handles the
DO_TEST(..., NONE) case, which translates to
testInfoSetArgs(&info, ARG_QEMU_CAPS, NONE, QEMU_CAPS_LAST, ARG_END)
Sine NONE == QEMU_CAPS_LAST == ARG_END, we finish parsing on
QEMU_CAPS_LAST. If ARG_END != QEMU_CAPS_LAST, the loop would try to
interpret QEMU_CAPS_LAST as an ARG_X value, and fail
Thanks,
Cole
3:29 p.m.
New subject: [libvirt] [PATCH 03/21] tests: qemuxml2argv: add va_arg enum handling
On 3/14/19 2:42 PM, Cole Robinson wrote:
>
>> +typedef enum {
>> + ARG_QEMU_CAPS = 1,
>> +
>> + ARG_END = QEMU_CAPS_LAST,
>> +} testInfoArgNames;
>> +
Do you need some sort of compile-time check that QEMU_CAPS_LAST doesn't
overlap with any other ARG_*?
>> + while ((argname = va_arg(argptr, int)) < ARG_END) {
>> + switch (argname) {
>> + case ARG_QEMU_CAPS:
>> + virQEMUCapsSetVList(info->qemuCaps, argptr);
>> + break;
>> +
>> + case ARG_END:
>> + default:
>> + fprintf(stderr, "Unexpected test info argument");
>
> ...and you are handling it (except that you ALWAYS handle it by printing
> an error, is that intentional?),...
>
See the while() condition: if we see ARG_END, we exit the loop, so we
shouldn't ever hit this condition and it's only in the switch to appease
gcc
Indeed, now that you point it out, it makes sense.
> In fact, for this patch, you are supplying a double-sentinel, and
I'm
> suspecting (without reading ahead) that later patches improve things as
> you add more ARG_ markers, and replacing QEMU_CAPS_LAST (which is now
> identical to ARG_END, and confusingly given twice) with something more
> obvious.
>
The double sentinel is actually a requirement of the current code,
because once we see ARG_QEMU_CAPS, we pass off the va_list to
virQEMUCapsSetVList which does its own arg processing and uses
QEMU_CAPS_LAST as a sentinel. We then kick back to this while() loop,
which sees ARG_END, and completes parsing.
The ARG_END = QEMU_CAPS_LAST is a bit weird, but it handles the
DO_TEST(..., NONE) case, which translates to
testInfoSetArgs(&info, ARG_QEMU_CAPS, NONE, QEMU_CAPS_LAST, ARG_END)
Sine NONE == QEMU_CAPS_LAST == ARG_END, we finish parsing on
QEMU_CAPS_LAST. If ARG_END != QEMU_CAPS_LAST, the loop would try to
interpret QEMU_CAPS_LAST as an ARG_X value, and fail
Clever. May be worth a comment in the code and/or commit message, but
you've convinced me why it works.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
1:47 p.m.
New subject: [libvirt] [PATCH 03/21] tests: qemuxml2argv: add va_arg enum handling
On 3/14/19 4:29 PM, Eric Blake wrote:
On 3/14/19 2:42 PM, Cole Robinson wrote:
>>
>>> +typedef enum {
>>> + ARG_QEMU_CAPS = 1,
>>> +
>>> + ARG_END = QEMU_CAPS_LAST,
>>> +} testInfoArgNames;
>>> +
Do you need some sort of compile-time check that QEMU_CAPS_LAST doesn't
overlap with any other ARG_*?
Sure but it seems extremely unlikely that will ever happen, there's
presently 300+ QEMU_CAPS_X. But if wanna provide the magic incantation
I'll squash it in
>>> + while ((argname = va_arg(argptr, int)) < ARG_END) {
>>> + switch (argname) {
>>> + case ARG_QEMU_CAPS:
>>> + virQEMUCapsSetVList(info->qemuCaps, argptr);
>>> + break;
>>> +
>>> + case ARG_END:
>>> + default:
>>> + fprintf(stderr, "Unexpected test info argument");
>>
>> ...and you are handling it (except that you ALWAYS handle it by printing
>> an error, is that intentional?),...
>>
>
> See the while() condition: if we see ARG_END, we exit the loop, so we
> shouldn't ever hit this condition and it's only in the switch to appease
> gcc
Indeed, now that you point it out, it makes sense.
>> In fact, for this patch, you are supplying a double-sentinel, and I'm
>> suspecting (without reading ahead) that later patches improve things as
>> you add more ARG_ markers, and replacing QEMU_CAPS_LAST (which is now
>> identical to ARG_END, and confusingly given twice) with something more
>> obvious.
>>
>
> The double sentinel is actually a requirement of the current code,
> because once we see ARG_QEMU_CAPS, we pass off the va_list to
> virQEMUCapsSetVList which does its own arg processing and uses
> QEMU_CAPS_LAST as a sentinel. We then kick back to this while() loop,
> which sees ARG_END, and completes parsing.
>
> The ARG_END = QEMU_CAPS_LAST is a bit weird, but it handles the
> DO_TEST(..., NONE) case, which translates to
>
> testInfoSetArgs(&info, ARG_QEMU_CAPS, NONE, QEMU_CAPS_LAST, ARG_END)
>
> Sine NONE == QEMU_CAPS_LAST == ARG_END, we finish parsing on
> QEMU_CAPS_LAST. If ARG_END != QEMU_CAPS_LAST, the loop would try to
> interpret QEMU_CAPS_LAST as an ARG_X value, and fail
Clever. May be worth a comment in the code and/or commit message, but
you've convinced me why it works.
Good point, I'll squash this in
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0dba908c70..d56acc8de1 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -627,6 +627,17 @@ testCompareXMLToArgv(const void *data)
typedef enum {
ARG_QEMU_CAPS = 1,
+ /* 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,
} testInfoArgNames;
- Cole
7:46 a.m.
New subject: [libvirt] [PATCH 03/21] tests: qemuxml2argv: add va_arg enum handling
On Fri, 2019-03-15 at 14:47 -0400, Cole Robinson wrote:
On 3/14/19 4:29 PM, Eric Blake wrote:
> On 3/14/19 2:42 PM, Cole Robinson wrote:
> > The double sentinel is actually a requirement of the current code,
> > because once we see ARG_QEMU_CAPS, we pass off the va_list to
> > virQEMUCapsSetVList which does its own arg processing and uses
> > QEMU_CAPS_LAST as a sentinel. We then kick back to this while() loop,
> > which sees ARG_END, and completes parsing.
> >
> > The ARG_END = QEMU_CAPS_LAST is a bit weird, but it handles the
> > DO_TEST(..., NONE) case, which translates to
> >
> > testInfoSetArgs(&info, ARG_QEMU_CAPS, NONE, QEMU_CAPS_LAST, ARG_END)
> >
> > Sine NONE == QEMU_CAPS_LAST == ARG_END, we finish parsing on
> > QEMU_CAPS_LAST. If ARG_END != QEMU_CAPS_LAST, the loop would try to
> > interpret QEMU_CAPS_LAST as an ARG_X value, and fail
>
> Clever. May be worth a comment in the code and/or commit message, but
> you've convinced me why it works.
Good point, I'll squash this in
[...]
+ /* 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.
+ */
Definitely squash in the comment :)
My only concern is that ARG_QEMU_CAPS requires you to be very
careful with its usage: not only you have to make sure you have
QEMU_CAPS_LAST as sentinel, but also you can only reasonably use it
as the last argument because QEMU_CAPS_LAST sharing the same value
as ARG_END would cause a macro like
#define DO_TEST_GIC_BROKEN(name, gic, ...) \
TEST_INTERNAL(name, "", \
ARG_QEMU_CAPS, __VA_ARGS__, QEMU_CAPS_LAST, \
ARG_GIC, gic)
to ignore the ARG_GIC argument when invoked as
DO_TEST_GIC_BROKEN("broken", GIC_V2, NONE);
I guess it's fair to expect both the developer and the reviewer to
be more careful than usual when touching this kind of dark magic
though, so including the comment is probably enough.
--
Andrea Bolognani / Red Hat / Virtualization
6:36 a.m.
New subject: [libvirt] [PATCH 03/21] tests: qemuxml2argv: add va_arg enum handling
On Thu, 2019-03-14 at 10:43 -0400, Cole Robinson wrote:
[...]
+typedef enum {
+ ARG_QEMU_CAPS = 1,
Any specific reason to start from 1 rather than 0, or even leaving
out the start value entirely?
+
+ ARG_END = QEMU_CAPS_LAST,
+} testInfoArgNames;
The name of the enum should be singular, otherwise it will look
weird when you declare a variable of the type, like...
static int
testInfoSetArgs(struct testInfo *info, ...)
{
va_list argptr;
- int ret = 0;
+ testInfoArgNames argname;
... here.
[...]
+ while ((argname = va_arg(argptr, int)) < ARG_END) {
+ switch (argname) {
I think you should either call va_arg() with testInfoArgNames as the
second argument, or make the argname variable int and then have an
explicit cast in the switch().
[...]
+ case ARG_END:
+ default:
+ fprintf(stderr, "Unexpected test info argument");
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
One extra empty line here, please.
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
11:52 a.m.
New subject: [libvirt] [PATCH 03/21] tests: qemuxml2argv: add va_arg enum handling
On 3/19/19 7:36 AM, Andrea Bolognani wrote:
On Thu, 2019-03-14 at 10:43 -0400, Cole Robinson wrote:
[...]
> +typedef enum {
> + ARG_QEMU_CAPS = 1,
Any specific reason to start from 1 rather than 0, or even leaving
out the start value entirely?
Nope it's not necessary, I've dropped it. I think it was left over from
earlier patch state
> +
> + ARG_END = QEMU_CAPS_LAST,
> +} testInfoArgNames;
The name of the enum should be singular, otherwise it will look
weird when you declare a variable of the type, like...
> static int
> testInfoSetArgs(struct testInfo *info, ...)
> {
> va_list argptr;
> - int ret = 0;
> + testInfoArgNames argname;
... here.
[...]
> + while ((argname = va_arg(argptr, int)) < ARG_END) {
> + switch (argname) {
I think you should either call va_arg() with testInfoArgNames as the
second argument, or make the argname variable int and then have an
explicit cast in the switch().
I went with the former
Thanks,
Cole
9:43 a.m.
New subject: [libvirt] [PATCH 04/21] tests: qemuxml2argv: push ARG_QEMU_CAPS to callers
This is necessary before we can start adding more optional parameter
implementations to DO_TEST_FULL
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 38 +++++++++++++++++++++++++-------------
1 file changed, 25 insertions(+), 13 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0dba908c70..393d2399fa 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -842,8 +842,7 @@ mymain(void)
}; \
if (testInitQEMUCaps(&info, gic) < 0) \
return EXIT_FAILURE; \
- if (testInfoSetArgs(&info, ARG_QEMU_CAPS, \
- __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \
+ if (testInfoSetArgs(&info, __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \
return EXIT_FAILURE; \
if (virTestRun("QEMU XML-2-ARGV " name, \
testCompareXMLToArgv, &info) < 0) \
@@ -852,24 +851,29 @@ mymain(void)
} while (0)
# define DO_TEST(name, ...) \
- DO_TEST_FULL(name, NULL, -1, 0, 0, GIC_NONE, __VA_ARGS__)
+ DO_TEST_FULL(name, NULL, -1, 0, 0, GIC_NONE, \
+ ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_GIC(name, gic, ...) \
- DO_TEST_FULL(name, NULL, -1, 0, 0, gic, __VA_ARGS__)
+ DO_TEST_FULL(name, NULL, -1, 0, 0, gic, \
+ ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_FAILURE(name, ...) \
DO_TEST_FULL(name, NULL, -1, FLAG_EXPECT_FAILURE, \
- 0, GIC_NONE, __VA_ARGS__)
+ 0, GIC_NONE, \
+ ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_PARSE_ERROR(name, ...) \
DO_TEST_FULL(name, NULL, -1, \
FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE, \
- 0, GIC_NONE, __VA_ARGS__)
+ 0, GIC_NONE, \
+ ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_PARSE_FLAGS_ERROR(name, parseFlags, ...) \
DO_TEST_FULL(name, NULL, -1, \
FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE, \
- parseFlags, GIC_NONE, __VA_ARGS__)
+ parseFlags, GIC_NONE, \
+ ARG_QEMU_CAPS, __VA_ARGS__)
# define NONE QEMU_CAPS_LAST
@@ -1707,12 +1711,17 @@ mymain(void)
QEMU_CAPS_CCW_CSSID_UNRESTRICTED,
QEMU_CAPS_DEVICE_VFIO_CCW);
- DO_TEST_FULL("restore-v2", "exec:cat", 7, 0, 0, GIC_NONE, NONE);
- DO_TEST_FULL("restore-v2-fd", "stdio", 7, 0, 0, GIC_NONE, NONE);
- DO_TEST_FULL("restore-v2-fd", "fd:7", 7, 0, 0, GIC_NONE, NONE);
- DO_TEST_FULL("migrate", "tcp:10.0.0.1:5000", -1, 0, 0, GIC_NONE,
NONE);
+ DO_TEST_FULL("restore-v2", "exec:cat", 7, 0, 0, GIC_NONE,
+ ARG_QEMU_CAPS, NONE);
+ DO_TEST_FULL("restore-v2-fd", "stdio", 7, 0, 0, GIC_NONE,
+ ARG_QEMU_CAPS, NONE);
+ DO_TEST_FULL("restore-v2-fd", "fd:7", 7, 0, 0, GIC_NONE,
+ ARG_QEMU_CAPS, NONE);
+ DO_TEST_FULL("migrate", "tcp:10.0.0.1:5000", -1, 0, 0, GIC_NONE,
+ ARG_QEMU_CAPS, NONE);
DO_TEST_FULL("migrate-numa-unaligned", "stdio", 7, 0, 0,
GIC_NONE,
+ ARG_QEMU_CAPS,
QEMU_CAPS_NUMA,
QEMU_CAPS_OBJECT_MEMORY_RAM);
@@ -1756,10 +1765,12 @@ mymain(void)
DO_TEST("cpu-host-model-vendor", NONE);
DO_TEST_FULL("cpu-host-model-fallback", NULL, -1,
FLAG_SKIP_LEGACY_CPUS, 0,
- GIC_NONE, NONE);
+ GIC_NONE,
+ ARG_QEMU_CAPS, NONE);
DO_TEST_FULL("cpu-host-model-nofallback", NULL, -1,
FLAG_SKIP_LEGACY_CPUS | FLAG_EXPECT_FAILURE,
- 0, GIC_NONE, NONE);
+ 0, GIC_NONE,
+ ARG_QEMU_CAPS, NONE);
DO_TEST("cpu-host-passthrough", QEMU_CAPS_KVM);
DO_TEST_FAILURE("cpu-qemu-host-passthrough", QEMU_CAPS_KVM);
@@ -2849,6 +2860,7 @@ mymain(void)
QEMU_CAPS_PIIX3_USB_UHCI);
DO_TEST_FULL("ppc64-usb-controller-qemu-xhci", NULL, -1, 0,
VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, GIC_NONE,
+ ARG_QEMU_CAPS,
QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
QEMU_CAPS_NEC_USB_XHCI,
QEMU_CAPS_DEVICE_QEMU_XHCI);
--
2.20.1
7:54 a.m.
New subject: [libvirt] [PATCH 04/21] tests: qemuxml2argv: push ARG_QEMU_CAPS to callers
On Thu, 2019-03-14 at 10:43 -0400, Cole Robinson wrote:
This is necessary before we can start adding more optional parameter
implementations to DO_TEST_FULL
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 38 +++++++++++++++++++++++++-------------
1 file changed, 25 insertions(+), 13 deletions(-)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
9:43 a.m.
New subject: [libvirt] [PATCH 05/21] tests: qemuxml2argv: break apart testInitQEMUCaps
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 23 +++--------------------
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 393d2399fa..06a28178d0 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -378,25 +378,6 @@ testAddCPUModels(virQEMUCapsPtr caps, bool skipLegacy)
}
-static int
-testInitQEMUCaps(struct testInfo *info,
- int gic)
-{
- int ret = -1;
-
- if (!(info->qemuCaps = virQEMUCapsNew()))
- goto cleanup;
-
- if (testQemuCapsSetGIC(info->qemuCaps, gic) < 0)
- goto cleanup;
-
- ret = 0;
-
- cleanup:
- return ret;
-}
-
-
static int
testUpdateQEMUCaps(const struct testInfo *info,
virDomainObjPtr vm,
@@ -840,7 +821,9 @@ mymain(void)
static struct testInfo info = { \
name, NULL, NULL, migrateFrom, migrateFd, (flags), parseFlags, \
}; \
- if (testInitQEMUCaps(&info, gic) < 0) \
+ if (!(info.qemuCaps = virQEMUCapsNew())) \
+ return EXIT_FAILURE; \
+ if (testQemuCapsSetGIC(info.qemuCaps, gic) < 0) \
return EXIT_FAILURE; \
if (testInfoSetArgs(&info, __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \
return EXIT_FAILURE; \
--
2.20.1
7:57 a.m.
New subject: [libvirt] [PATCH 05/21] tests: qemuxml2argv: break apart testInitQEMUCaps
On Thu, 2019-03-14 at 10:43 -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 23 +++--------------------
1 file changed, 3 insertions(+), 20 deletions(-)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
9:43 a.m.
New subject: [libvirt] [PATCH 06/21] tests: qemuxml2argv: handle gic with vaargs
This allows us to drop stub gic values from DO_TEST_FULL calls
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 06a28178d0..60df2c3a00 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -607,6 +607,7 @@ testCompareXMLToArgv(const void *data)
typedef enum {
ARG_QEMU_CAPS = 1,
+ ARG_GIC,
ARG_END = QEMU_CAPS_LAST,
} testInfoArgNames;
@@ -625,6 +626,12 @@ testInfoSetArgs(struct testInfo *info, ...)
virQEMUCapsSetVList(info->qemuCaps, argptr);
break;
+ case ARG_GIC:
+ if (testQemuCapsSetGIC(info->qemuCaps,
+ va_arg(argptr, int)) < 0)
+ goto cleanup;
+ break;
+
case ARG_END:
default:
fprintf(stderr, "Unexpected test info argument");
@@ -816,15 +823,13 @@ mymain(void)
DO_TEST_CAPS_FULL(name, 0, 0, ver)
# define DO_TEST_FULL(name, migrateFrom, migrateFd, flags, \
- parseFlags, gic, ...) \
+ parseFlags, ...) \
do { \
static struct testInfo info = { \
name, NULL, NULL, migrateFrom, migrateFd, (flags), parseFlags, \
}; \
if (!(info.qemuCaps = virQEMUCapsNew())) \
return EXIT_FAILURE; \
- if (testQemuCapsSetGIC(info.qemuCaps, gic) < 0) \
- return EXIT_FAILURE; \
if (testInfoSetArgs(&info, __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \
return EXIT_FAILURE; \
if (virTestRun("QEMU XML-2-ARGV " name, \
@@ -834,28 +839,29 @@ mymain(void)
} while (0)
# define DO_TEST(name, ...) \
- DO_TEST_FULL(name, NULL, -1, 0, 0, GIC_NONE, \
+ DO_TEST_FULL(name, NULL, -1, 0, 0, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_GIC(name, gic, ...) \
- DO_TEST_FULL(name, NULL, -1, 0, 0, gic, \
+ DO_TEST_FULL(name, NULL, -1, 0, 0, \
+ ARG_GIC, gic, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_FAILURE(name, ...) \
DO_TEST_FULL(name, NULL, -1, FLAG_EXPECT_FAILURE, \
- 0, GIC_NONE, \
+ 0, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_PARSE_ERROR(name, ...) \
DO_TEST_FULL(name, NULL, -1, \
FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE, \
- 0, GIC_NONE, \
+ 0, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_PARSE_FLAGS_ERROR(name, parseFlags, ...) \
DO_TEST_FULL(name, NULL, -1, \
FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE, \
- parseFlags, GIC_NONE, \
+ parseFlags, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define NONE QEMU_CAPS_LAST
@@ -1694,16 +1700,16 @@ mymain(void)
QEMU_CAPS_CCW_CSSID_UNRESTRICTED,
QEMU_CAPS_DEVICE_VFIO_CCW);
- DO_TEST_FULL("restore-v2", "exec:cat", 7, 0, 0, GIC_NONE,
+ DO_TEST_FULL("restore-v2", "exec:cat", 7, 0, 0,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("restore-v2-fd", "stdio", 7, 0, 0, GIC_NONE,
+ DO_TEST_FULL("restore-v2-fd", "stdio", 7, 0, 0,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("restore-v2-fd", "fd:7", 7, 0, 0, GIC_NONE,
+ DO_TEST_FULL("restore-v2-fd", "fd:7", 7, 0, 0,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("migrate", "tcp:10.0.0.1:5000", -1, 0, 0, GIC_NONE,
+ DO_TEST_FULL("migrate", "tcp:10.0.0.1:5000", -1, 0, 0,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("migrate-numa-unaligned", "stdio", 7, 0, 0,
GIC_NONE,
+ DO_TEST_FULL("migrate-numa-unaligned", "stdio", 7, 0, 0,
ARG_QEMU_CAPS,
QEMU_CAPS_NUMA,
QEMU_CAPS_OBJECT_MEMORY_RAM);
@@ -1748,11 +1754,10 @@ mymain(void)
DO_TEST("cpu-host-model-vendor", NONE);
DO_TEST_FULL("cpu-host-model-fallback", NULL, -1,
FLAG_SKIP_LEGACY_CPUS, 0,
- GIC_NONE,
ARG_QEMU_CAPS, NONE);
DO_TEST_FULL("cpu-host-model-nofallback", NULL, -1,
FLAG_SKIP_LEGACY_CPUS | FLAG_EXPECT_FAILURE,
- 0, GIC_NONE,
+ 0,
ARG_QEMU_CAPS, NONE);
DO_TEST("cpu-host-passthrough", QEMU_CAPS_KVM);
DO_TEST_FAILURE("cpu-qemu-host-passthrough", QEMU_CAPS_KVM);
@@ -2842,7 +2847,7 @@ mymain(void)
QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
QEMU_CAPS_PIIX3_USB_UHCI);
DO_TEST_FULL("ppc64-usb-controller-qemu-xhci", NULL, -1, 0,
- VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, GIC_NONE,
+ VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
ARG_QEMU_CAPS,
QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
QEMU_CAPS_NEC_USB_XHCI,
--
2.20.1
8:05 a.m.
New subject: [libvirt] [PATCH 06/21] tests: qemuxml2argv: handle gic with vaargs
On Thu, 2019-03-14 at 10:43 -0400, Cole Robinson wrote:
Subject: [libvirt] [PATCH 06/21] tests: qemuxml2argv: handle gic with
vaargs
This allows us to drop stub gic values from DO_TEST_FULL calls
s/vaargs/varargs/
s/gic/GIC/
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
9:44 a.m.
New subject: [libvirt] [PATCH 07/21] tests: qemuxml2argv: handle migrate* with varargs
This allows us to drop migrateFrom and migrateFd from DO_TEST_FULL
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 46 ++++++++++++++++++++++++++--------------
1 file changed, 30 insertions(+), 16 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 60df2c3a00..c6bd240c54 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -608,6 +608,8 @@ testCompareXMLToArgv(const void *data)
typedef enum {
ARG_QEMU_CAPS = 1,
ARG_GIC,
+ ARG_MIGRATE_FROM,
+ ARG_MIGRATE_FD,
ARG_END = QEMU_CAPS_LAST,
} testInfoArgNames;
@@ -632,6 +634,14 @@ testInfoSetArgs(struct testInfo *info, ...)
goto cleanup;
break;
+ case ARG_MIGRATE_FROM:
+ info->migrateFrom = va_arg(argptr, char *);
+ break;
+
+ case ARG_MIGRATE_FD:
+ info->migrateFd = va_arg(argptr, int);
+ break;
+
case ARG_END:
default:
fprintf(stderr, "Unexpected test info argument");
@@ -822,11 +832,10 @@ mymain(void)
# define DO_TEST_CAPS(name, ver) \
DO_TEST_CAPS_FULL(name, 0, 0, ver)
-# define DO_TEST_FULL(name, migrateFrom, migrateFd, flags, \
- parseFlags, ...) \
+# define DO_TEST_FULL(name, flags, parseFlags, ...) \
do { \
static struct testInfo info = { \
- name, NULL, NULL, migrateFrom, migrateFd, (flags), parseFlags, \
+ name, NULL, NULL, NULL, -1, (flags), parseFlags, \
}; \
if (!(info.qemuCaps = virQEMUCapsNew())) \
return EXIT_FAILURE; \
@@ -839,27 +848,27 @@ mymain(void)
} while (0)
# define DO_TEST(name, ...) \
- DO_TEST_FULL(name, NULL, -1, 0, 0, \
+ DO_TEST_FULL(name, 0, 0, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_GIC(name, gic, ...) \
- DO_TEST_FULL(name, NULL, -1, 0, 0, \
+ DO_TEST_FULL(name, 0, 0, \
ARG_GIC, gic, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_FAILURE(name, ...) \
- DO_TEST_FULL(name, NULL, -1, FLAG_EXPECT_FAILURE, \
+ DO_TEST_FULL(name, FLAG_EXPECT_FAILURE, \
0, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_PARSE_ERROR(name, ...) \
- DO_TEST_FULL(name, NULL, -1, \
+ DO_TEST_FULL(name, \
FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE, \
0, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_PARSE_FLAGS_ERROR(name, parseFlags, ...) \
- DO_TEST_FULL(name, NULL, -1, \
+ DO_TEST_FULL(name, \
FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE, \
parseFlags, \
ARG_QEMU_CAPS, __VA_ARGS__)
@@ -1700,16 +1709,21 @@ mymain(void)
QEMU_CAPS_CCW_CSSID_UNRESTRICTED,
QEMU_CAPS_DEVICE_VFIO_CCW);
- DO_TEST_FULL("restore-v2", "exec:cat", 7, 0, 0,
+ DO_TEST_FULL("restore-v2", 0, 0,
+ ARG_MIGRATE_FROM, "exec:cat", ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("restore-v2-fd", "stdio", 7, 0, 0,
+ DO_TEST_FULL("restore-v2-fd", 0, 0,
+ ARG_MIGRATE_FROM, "stdio", ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("restore-v2-fd", "fd:7", 7, 0, 0,
+ DO_TEST_FULL("restore-v2-fd", 0, 0,
+ ARG_MIGRATE_FROM, "fd:7", ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("migrate", "tcp:10.0.0.1:5000", -1, 0, 0,
+ DO_TEST_FULL("migrate", 0, 0,
+ ARG_MIGRATE_FROM, "tcp:10.0.0.1:5000",
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("migrate-numa-unaligned", "stdio", 7, 0, 0,
+ DO_TEST_FULL("migrate-numa-unaligned", 0, 0,
+ ARG_MIGRATE_FROM, "stdio", ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS,
QEMU_CAPS_NUMA,
QEMU_CAPS_OBJECT_MEMORY_RAM);
@@ -1752,10 +1766,10 @@ mymain(void)
DO_TEST("cpu-numa-memshared", QEMU_CAPS_OBJECT_MEMORY_FILE);
DO_TEST("cpu-host-model", NONE);
DO_TEST("cpu-host-model-vendor", NONE);
- DO_TEST_FULL("cpu-host-model-fallback", NULL, -1,
+ DO_TEST_FULL("cpu-host-model-fallback",
FLAG_SKIP_LEGACY_CPUS, 0,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("cpu-host-model-nofallback", NULL, -1,
+ DO_TEST_FULL("cpu-host-model-nofallback",
FLAG_SKIP_LEGACY_CPUS | FLAG_EXPECT_FAILURE,
0,
ARG_QEMU_CAPS, NONE);
@@ -2846,7 +2860,7 @@ mymain(void)
DO_TEST("ppc64-usb-controller-legacy",
QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
QEMU_CAPS_PIIX3_USB_UHCI);
- DO_TEST_FULL("ppc64-usb-controller-qemu-xhci", NULL, -1, 0,
+ DO_TEST_FULL("ppc64-usb-controller-qemu-xhci", 0,
VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
ARG_QEMU_CAPS,
QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
--
2.20.1
8:08 a.m.
New subject: [libvirt] [PATCH 07/21] tests: qemuxml2argv: handle migrate* with varargs
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
[...]
- DO_TEST_FULL("restore-v2", "exec:cat", 7, 0,
0,
+ DO_TEST_FULL("restore-v2", 0, 0,
+ ARG_MIGRATE_FROM, "exec:cat", ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("restore-v2-fd", "stdio", 7, 0, 0,
+ DO_TEST_FULL("restore-v2-fd", 0, 0,
+ ARG_MIGRATE_FROM, "stdio", ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("restore-v2-fd", "fd:7", 7, 0, 0,
+ DO_TEST_FULL("restore-v2-fd", 0, 0,
+ ARG_MIGRATE_FROM, "fd:7", ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS, NONE);
I'd put ARG_MIGRATE_FROM and ARG_MIGRATE_FD on separate lines here.
Either way,
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
8:10 a.m.
New subject: [libvirt] [PATCH 07/21] tests: qemuxml2argv: handle migrate* with varargs
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
This allows us to drop migrateFrom and migrateFd from DO_TEST_FULL
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 46 ++++++++++++++++++++++++++--------------
1 file changed, 30 insertions(+), 16 deletions(-)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
8:11 a.m.
New subject: [libvirt] [PATCH 07/21] tests: qemuxml2argv: handle migrate* with varargs
On Tue, 2019-03-19 at 14:10 +0100, Andrea Bolognani wrote:
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
> This allows us to drop migrateFrom and migrateFd from DO_TEST_FULL
>
> Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
> ---
> tests/qemuxml2argvtest.c | 46 ++++++++++++++++++++++++++--------------
> 1 file changed, 30 insertions(+), 16 deletions(-)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
Disregard this message - I replied to the wrong patch by mistake.
--
Andrea Bolognani / Red Hat / Virtualization
9:44 a.m.
New subject: [libvirt] [PATCH 08/21] tests: qemuxml2argv: handle flags with varargs
This allows us to drop flags from DO_TEST_FULL
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c6bd240c54..bb3d53070f 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -610,6 +610,7 @@ typedef enum {
ARG_GIC,
ARG_MIGRATE_FROM,
ARG_MIGRATE_FD,
+ ARG_FLAGS,
ARG_END = QEMU_CAPS_LAST,
} testInfoArgNames;
@@ -642,6 +643,10 @@ testInfoSetArgs(struct testInfo *info, ...)
info->migrateFd = va_arg(argptr, int);
break;
+ case ARG_FLAGS:
+ info->flags = va_arg(argptr, int);
+ break;
+
case ARG_END:
default:
fprintf(stderr, "Unexpected test info argument");
@@ -832,10 +837,10 @@ mymain(void)
# define DO_TEST_CAPS(name, ver) \
DO_TEST_CAPS_FULL(name, 0, 0, ver)
-# define DO_TEST_FULL(name, flags, parseFlags, ...) \
+# define DO_TEST_FULL(name, parseFlags, ...) \
do { \
static struct testInfo info = { \
- name, NULL, NULL, NULL, -1, (flags), parseFlags, \
+ name, NULL, NULL, NULL, -1, 0, parseFlags, \
}; \
if (!(info.qemuCaps = virQEMUCapsNew())) \
return EXIT_FAILURE; \
@@ -848,29 +853,30 @@ mymain(void)
} while (0)
# define DO_TEST(name, ...) \
- DO_TEST_FULL(name, 0, 0, \
+ DO_TEST_FULL(name, 0, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_GIC(name, gic, ...) \
- DO_TEST_FULL(name, 0, 0, \
+ DO_TEST_FULL(name, 0, \
ARG_GIC, gic, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_FAILURE(name, ...) \
- DO_TEST_FULL(name, FLAG_EXPECT_FAILURE, \
+ DO_TEST_FULL(name, \
0, \
+ ARG_FLAGS, FLAG_EXPECT_FAILURE, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_PARSE_ERROR(name, ...) \
DO_TEST_FULL(name, \
- FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE, \
0, \
+ ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_PARSE_FLAGS_ERROR(name, parseFlags, ...) \
DO_TEST_FULL(name, \
- FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE, \
parseFlags, \
+ ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define NONE QEMU_CAPS_LAST
@@ -1709,20 +1715,20 @@ mymain(void)
QEMU_CAPS_CCW_CSSID_UNRESTRICTED,
QEMU_CAPS_DEVICE_VFIO_CCW);
- DO_TEST_FULL("restore-v2", 0, 0,
+ DO_TEST_FULL("restore-v2", 0,
ARG_MIGRATE_FROM, "exec:cat", ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("restore-v2-fd", 0, 0,
+ DO_TEST_FULL("restore-v2-fd", 0,
ARG_MIGRATE_FROM, "stdio", ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("restore-v2-fd", 0, 0,
+ DO_TEST_FULL("restore-v2-fd", 0,
ARG_MIGRATE_FROM, "fd:7", ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("migrate", 0, 0,
+ DO_TEST_FULL("migrate", 0,
ARG_MIGRATE_FROM, "tcp:10.0.0.1:5000",
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("migrate-numa-unaligned", 0, 0,
+ DO_TEST_FULL("migrate-numa-unaligned", 0,
ARG_MIGRATE_FROM, "stdio", ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS,
QEMU_CAPS_NUMA,
@@ -1767,11 +1773,12 @@ mymain(void)
DO_TEST("cpu-host-model", NONE);
DO_TEST("cpu-host-model-vendor", NONE);
DO_TEST_FULL("cpu-host-model-fallback",
- FLAG_SKIP_LEGACY_CPUS, 0,
+ 0,
+ ARG_FLAGS, FLAG_SKIP_LEGACY_CPUS,
ARG_QEMU_CAPS, NONE);
DO_TEST_FULL("cpu-host-model-nofallback",
- FLAG_SKIP_LEGACY_CPUS | FLAG_EXPECT_FAILURE,
0,
+ ARG_FLAGS, FLAG_SKIP_LEGACY_CPUS | FLAG_EXPECT_FAILURE,
ARG_QEMU_CAPS, NONE);
DO_TEST("cpu-host-passthrough", QEMU_CAPS_KVM);
DO_TEST_FAILURE("cpu-qemu-host-passthrough", QEMU_CAPS_KVM);
@@ -2860,7 +2867,7 @@ mymain(void)
DO_TEST("ppc64-usb-controller-legacy",
QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
QEMU_CAPS_PIIX3_USB_UHCI);
- DO_TEST_FULL("ppc64-usb-controller-qemu-xhci", 0,
+ DO_TEST_FULL("ppc64-usb-controller-qemu-xhci",
VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
ARG_QEMU_CAPS,
QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
--
2.20.1
8:12 a.m.
New subject: [libvirt] [PATCH 08/21] tests: qemuxml2argv: handle flags with varargs
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
This allows us to drop flags from DO_TEST_FULL
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
9:44 a.m.
New subject: [libvirt] [PATCH 09/21] tests: qemuxml2argv: handle parseFlags with varargs
This allows us to drop parseFlags from DO_TEST_FULL
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index bb3d53070f..070f7f0a05 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -611,6 +611,7 @@ typedef enum {
ARG_MIGRATE_FROM,
ARG_MIGRATE_FD,
ARG_FLAGS,
+ ARG_PARSEFLAGS,
ARG_END = QEMU_CAPS_LAST,
} testInfoArgNames;
@@ -647,6 +648,10 @@ testInfoSetArgs(struct testInfo *info, ...)
info->flags = va_arg(argptr, int);
break;
+ case ARG_PARSEFLAGS:
+ info->parseFlags = va_arg(argptr, int);
+ break;
+
case ARG_END:
default:
fprintf(stderr, "Unexpected test info argument");
@@ -837,10 +842,10 @@ mymain(void)
# define DO_TEST_CAPS(name, ver) \
DO_TEST_CAPS_FULL(name, 0, 0, ver)
-# define DO_TEST_FULL(name, parseFlags, ...) \
+# define DO_TEST_FULL(name, ...) \
do { \
static struct testInfo info = { \
- name, NULL, NULL, NULL, -1, 0, parseFlags, \
+ name, NULL, NULL, NULL, -1, 0, 0, \
}; \
if (!(info.qemuCaps = virQEMUCapsNew())) \
return EXIT_FAILURE; \
@@ -853,29 +858,27 @@ mymain(void)
} while (0)
# define DO_TEST(name, ...) \
- DO_TEST_FULL(name, 0, \
+ DO_TEST_FULL(name, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_GIC(name, gic, ...) \
- DO_TEST_FULL(name, 0, \
+ DO_TEST_FULL(name, \
ARG_GIC, gic, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_FAILURE(name, ...) \
DO_TEST_FULL(name, \
- 0, \
ARG_FLAGS, FLAG_EXPECT_FAILURE, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_PARSE_ERROR(name, ...) \
DO_TEST_FULL(name, \
- 0, \
ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE, \
ARG_QEMU_CAPS, __VA_ARGS__)
# define DO_TEST_PARSE_FLAGS_ERROR(name, parseFlags, ...) \
DO_TEST_FULL(name, \
- parseFlags, \
+ ARG_PARSEFLAGS, parseFlags, \
ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE, \
ARG_QEMU_CAPS, __VA_ARGS__)
@@ -1715,20 +1718,20 @@ mymain(void)
QEMU_CAPS_CCW_CSSID_UNRESTRICTED,
QEMU_CAPS_DEVICE_VFIO_CCW);
- DO_TEST_FULL("restore-v2", 0,
+ DO_TEST_FULL("restore-v2",
ARG_MIGRATE_FROM, "exec:cat", ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("restore-v2-fd", 0,
+ DO_TEST_FULL("restore-v2-fd",
ARG_MIGRATE_FROM, "stdio", ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("restore-v2-fd", 0,
+ DO_TEST_FULL("restore-v2-fd",
ARG_MIGRATE_FROM, "fd:7", ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("migrate", 0,
+ DO_TEST_FULL("migrate",
ARG_MIGRATE_FROM, "tcp:10.0.0.1:5000",
ARG_QEMU_CAPS, NONE);
- DO_TEST_FULL("migrate-numa-unaligned", 0,
+ DO_TEST_FULL("migrate-numa-unaligned",
ARG_MIGRATE_FROM, "stdio", ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS,
QEMU_CAPS_NUMA,
@@ -1773,11 +1776,9 @@ mymain(void)
DO_TEST("cpu-host-model", NONE);
DO_TEST("cpu-host-model-vendor", NONE);
DO_TEST_FULL("cpu-host-model-fallback",
- 0,
ARG_FLAGS, FLAG_SKIP_LEGACY_CPUS,
ARG_QEMU_CAPS, NONE);
DO_TEST_FULL("cpu-host-model-nofallback",
- 0,
ARG_FLAGS, FLAG_SKIP_LEGACY_CPUS | FLAG_EXPECT_FAILURE,
ARG_QEMU_CAPS, NONE);
DO_TEST("cpu-host-passthrough", QEMU_CAPS_KVM);
@@ -2868,7 +2869,7 @@ mymain(void)
QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
QEMU_CAPS_PIIX3_USB_UHCI);
DO_TEST_FULL("ppc64-usb-controller-qemu-xhci",
- VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
+ ARG_PARSEFLAGS, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
ARG_QEMU_CAPS,
QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
QEMU_CAPS_NEC_USB_XHCI,
--
2.20.1
8:14 a.m.
New subject: [libvirt] [PATCH 09/21] tests: qemuxml2argv: handle parseFlags with varargs
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
This allows us to drop parseFlags from DO_TEST_FULL
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
9:44 a.m.
New subject: [libvirt] [PATCH 10/21] tests: qemuxml2argv: remove DO_TEST_PARSE_FLAGS_ERROR
It only has one caller. Just use DO_TEST_FULL
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 070f7f0a05..1ce8bade48 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -876,12 +876,6 @@ mymain(void)
ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE, \
ARG_QEMU_CAPS, __VA_ARGS__)
-# define DO_TEST_PARSE_FLAGS_ERROR(name, parseFlags, ...) \
- DO_TEST_FULL(name, \
- ARG_PARSEFLAGS, parseFlags, \
- ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE, \
- ARG_QEMU_CAPS, __VA_ARGS__)
-
# define NONE QEMU_CAPS_LAST
/* Unset or set all envvars here that are copied in qemudBuildCommandLine
@@ -2887,9 +2881,10 @@ mymain(void)
/* VM XML has invalid arch/ostype/virttype combo, but the SKIP flag
* will avoid the error. Still, we expect qemu driver to complain about
* missing machine error, and not crash */
- DO_TEST_PARSE_FLAGS_ERROR("missing-machine",
- VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE,
- NONE);
+ DO_TEST_FULL("missing-machine",
+ ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE,
+ ARG_PARSEFLAGS, VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE,
+ ARG_QEMU_CAPS, NONE);
DO_TEST("name-escape",
QEMU_CAPS_NAME_DEBUG_THREADS,
--
2.20.1
8:24 a.m.
New subject: [libvirt] [PATCH 10/21] tests: qemuxml2argv: remove DO_TEST_PARSE_FLAGS_ERROR
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
[...]
+ DO_TEST_FULL("missing-machine",
+ ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE,
+ ARG_PARSEFLAGS, VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE,
+ ARG_QEMU_CAPS, NONE);
Technically speaking you should be able to leave ARG_QEMU_CAPS out
here, but it's okay if you prefer to leave it in.
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
11:52 a.m.
New subject: [libvirt] [PATCH 10/21] tests: qemuxml2argv: remove DO_TEST_PARSE_FLAGS_ERROR
On 3/19/19 9:24 AM, Andrea Bolognani wrote:
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
[...]
> + DO_TEST_FULL("missing-machine",
> + ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE,
> + ARG_PARSEFLAGS, VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE,
> + ARG_QEMU_CAPS, NONE);
Technically speaking you should be able to leave ARG_QEMU_CAPS out
here, but it's okay if you prefer to leave it in.
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
The other DO_TEST_FULL instances still have it explicitly listed, so I
left it in for this one too
Thanks,
Cole
9:44 a.m.
New subject: [libvirt] [PATCH 11/21] tests: qemuxml2argv: remove unused DO_TEST_CAPS* macros
They are potentially useful at the moment, but we will be making
things much more flexible
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 1ce8bade48..130ab29d6e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -832,16 +832,6 @@ mymain(void)
# define DO_TEST_CAPS_LATEST_PARSE_ERROR(name) \
DO_TEST_CAPS_ARCH_LATEST_FULL(name, "x86_64", FLAG_EXPECT_PARSE_ERROR, 0)
-/**
- * The following test macros should be used only in cases when the tests require
- * testing of some non-standard combination of capability flags
- */
-# define DO_TEST_CAPS_FULL(name, flags, parseFlags, ver) \
- DO_TEST_CAPS_ARCH(name, NULL, flags, parseFlags, GIC_NONE, "x86_64", ver)
-
-# define DO_TEST_CAPS(name, ver) \
- DO_TEST_CAPS_FULL(name, 0, 0, ver)
-
# define DO_TEST_FULL(name, ...) \
do { \
static struct testInfo info = { \
--
2.20.1
8:31 a.m.
New subject: [libvirt] [PATCH 11/21] tests: qemuxml2argv: remove unused DO_TEST_CAPS* macros
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
[...]
-# define DO_TEST_CAPS(name, ver) \
- DO_TEST_CAPS_FULL(name, 0, 0, ver)
I fail to see why this one existed in the first place - isn't it
basically the same as DO_TEST_CAPS_VER()?
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
9:21 a.m.
New subject: [libvirt] [PATCH 11/21] tests: qemuxml2argv: remove unused DO_TEST_CAPS* macros
On Tue, Mar 19, 2019 at 02:31:47PM +0100, Andrea Bolognani wrote:
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
[...]
> -# define DO_TEST_CAPS(name, ver) \
> - DO_TEST_CAPS_FULL(name, 0, 0, ver)
I fail to see why this one existed in the first place - isn't it
basically the same as DO_TEST_CAPS_VER()?
Probably a leftover from earlier versions.
It is unused since its introduction in:
commit 976fbb98c8dbccbba93fb5f5a8bb2f94df0be346
tests: qemuxml2argv: Add infrastructure for testing with real qemuCaps
Jano
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
9:44 a.m.
New subject: [libvirt] [PATCH 12/21] tests: qemuxml2argv: add a comment separating DO_TEST* macros
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 130ab29d6e..b555166e82 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -832,6 +832,9 @@ mymain(void)
# define DO_TEST_CAPS_LATEST_PARSE_ERROR(name) \
DO_TEST_CAPS_ARCH_LATEST_FULL(name, "x86_64", FLAG_EXPECT_PARSE_ERROR, 0)
+
+/* All the following macros require an explicit QEMU_CAPS_* list
+ * at the end of the argument list, or the NONE placeholder */
# define DO_TEST_FULL(name, ...) \
do { \
static struct testInfo info = { \
--
2.20.1
8:36 a.m.
New subject: [libvirt] [PATCH 12/21] tests: qemuxml2argv: add a comment separating DO_TEST* macros
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 130ab29d6e..b555166e82 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -832,6 +832,9 @@ mymain(void)
# define DO_TEST_CAPS_LATEST_PARSE_ERROR(name) \
DO_TEST_CAPS_ARCH_LATEST_FULL(name, "x86_64", FLAG_EXPECT_PARSE_ERROR, 0)
+
+/* All the following macros require an explicit QEMU_CAPS_* list
+ * at the end of the argument list, or the NONE placeholder */
# define DO_TEST_FULL(name, ...) \
do { \
static struct testInfo info = { \
The comment does actually not apply to DO_TEST_FULL(), which expects
a series of ARG_* and corresponding arguments rather than a list of
QEMU_CAPS_*, so you should move it one macro down - right before
DO_TEST() - instead.
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
9:44 a.m.
New subject: [libvirt] [PATCH 13/21] tests: qemuxml2argv: remove unused CAPS migrateFrom
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index b555166e82..7708e8db70 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -786,11 +786,11 @@ mymain(void)
* the test cases should be forked using DO_TEST_CAPS_VER with the appropriate
* version.
*/
-# define DO_TEST_CAPS_INTERNAL(name, suffix, migrateFrom, flags, parseFlags, \
+# define DO_TEST_CAPS_INTERNAL(name, suffix, flags, parseFlags, \
arch, capsfile, stripmachinealiases) \
do { \
static struct testInfo info = { \
- name, "." suffix, NULL, migrateFrom, migrateFrom ? 7 : -1,\
+ name, "." suffix, NULL, NULL, -1,\
(flags | FLAG_REAL_CAPS), parseFlags, \
}; \
if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \
@@ -807,7 +807,7 @@ mymain(void)
# define TEST_CAPS_PATH abs_srcdir "/qemucapabilitiesdata/caps_"
# define DO_TEST_CAPS_ARCH_VER_FULL(name, flags, parseFlags, arch, ver) \
- DO_TEST_CAPS_INTERNAL(name, arch "-" ver, NULL, flags, parseFlags, \
+ DO_TEST_CAPS_INTERNAL(name, arch "-" ver, flags, parseFlags, \
arch, TEST_CAPS_PATH ver "." arch ".xml",
false)
# define DO_TEST_CAPS_ARCH_VER(name, arch, ver) \
@@ -817,7 +817,7 @@ mymain(void)
DO_TEST_CAPS_ARCH_VER(name, "x86_64", ver)
# define DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, flags, parseFlags) \
- DO_TEST_CAPS_INTERNAL(name, arch "-latest", NULL, flags, parseFlags, arch,
\
+ DO_TEST_CAPS_INTERNAL(name, arch "-latest", flags, parseFlags, arch, \
virHashLookup(capslatest, arch), true)
# define DO_TEST_CAPS_ARCH_LATEST(name, arch) \
--
2.20.1
8:38 a.m.
New subject: [libvirt] [PATCH 13/21] tests: qemuxml2argv: remove unused CAPS migrateFrom
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
9:44 a.m.
New subject: [libvirt] [PATCH 14/21] tests: qemuxml2argv: use varargs for CAPS flags
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 34 ++++++++++++++++++++--------------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 7708e8db70..176e4eff3e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -786,18 +786,20 @@ mymain(void)
* the test cases should be forked using DO_TEST_CAPS_VER with the appropriate
* version.
*/
-# define DO_TEST_CAPS_INTERNAL(name, suffix, flags, parseFlags, \
- arch, capsfile, stripmachinealiases) \
+# define DO_TEST_CAPS_INTERNAL(name, suffix, \
+ arch, capsfile, stripmachinealiases, ...) \
do { \
static struct testInfo info = { \
- name, "." suffix, NULL, NULL, -1,\
- (flags | FLAG_REAL_CAPS), parseFlags, \
+ name, "." suffix, NULL, NULL, -1, 0, 0, \
}; \
if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \
capsfile))) \
return EXIT_FAILURE; \
if (stripmachinealiases) \
virQEMUCapsStripMachineAliases(info.qemuCaps); \
+ if (testInfoSetArgs(&info, __VA_ARGS__, ARG_END) < 0) \
+ return EXIT_FAILURE; \
+ info.flags |= FLAG_REAL_CAPS; \
if (virTestRun("QEMU XML-2-ARGV " name "." suffix, \
testCompareXMLToArgv, &info) < 0) \
ret = -1; \
@@ -806,31 +808,35 @@ mymain(void)
# define TEST_CAPS_PATH abs_srcdir "/qemucapabilitiesdata/caps_"
-# define DO_TEST_CAPS_ARCH_VER_FULL(name, flags, parseFlags, arch, ver) \
- DO_TEST_CAPS_INTERNAL(name, arch "-" ver, flags, parseFlags, \
- arch, TEST_CAPS_PATH ver "." arch ".xml",
false)
+# define DO_TEST_CAPS_ARCH_VER_FULL(name, arch, ver, ...) \
+ DO_TEST_CAPS_INTERNAL(name, arch "-" ver, \
+ arch, TEST_CAPS_PATH ver "." arch ".xml",
false, \
+ __VA_ARGS__)
# define DO_TEST_CAPS_ARCH_VER(name, arch, ver) \
- DO_TEST_CAPS_ARCH_VER_FULL(name, 0, 0, arch, ver)
+ DO_TEST_CAPS_ARCH_VER_FULL(name, arch, ver, ARG_END)
# define DO_TEST_CAPS_VER(name, ver) \
DO_TEST_CAPS_ARCH_VER(name, "x86_64", ver)
-# define DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, flags, parseFlags) \
- DO_TEST_CAPS_INTERNAL(name, arch "-latest", flags, parseFlags, arch, \
- virHashLookup(capslatest, arch), true)
+# define DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, ...) \
+ DO_TEST_CAPS_INTERNAL(name, arch "-latest", arch, \
+ virHashLookup(capslatest, arch), true, \
+ __VA_ARGS__)
# define DO_TEST_CAPS_ARCH_LATEST(name, arch) \
- DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, 0, 0)
+ DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, ARG_END)
# define DO_TEST_CAPS_LATEST(name) \
DO_TEST_CAPS_ARCH_LATEST(name, "x86_64")
# define DO_TEST_CAPS_LATEST_FAILURE(name) \
- DO_TEST_CAPS_ARCH_LATEST_FULL(name, "x86_64", FLAG_EXPECT_FAILURE, 0)
+ DO_TEST_CAPS_ARCH_LATEST_FULL(name, "x86_64", \
+ ARG_FLAGS, FLAG_EXPECT_FAILURE)
# define DO_TEST_CAPS_LATEST_PARSE_ERROR(name) \
- DO_TEST_CAPS_ARCH_LATEST_FULL(name, "x86_64", FLAG_EXPECT_PARSE_ERROR, 0)
+ DO_TEST_CAPS_ARCH_LATEST_FULL(name, "x86_64", \
+ ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR)
/* All the following macros require an explicit QEMU_CAPS_* list
--
2.20.1
8:55 a.m.
New subject: [libvirt] [PATCH 14/21] tests: qemuxml2argv: use varargs for CAPS flags
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
[...]
+# define DO_TEST_CAPS_INTERNAL(name, suffix, \
+ arch, capsfile, stripmachinealiases, ...) \
do { \
static struct testInfo info = { \
- name, "." suffix, NULL, NULL, -1,\
- (flags | FLAG_REAL_CAPS), parseFlags, \
+ name, "." suffix, NULL, NULL, -1, 0, 0, \
}; \
if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \
capsfile))) \
return EXIT_FAILURE; \
if (stripmachinealiases) \
virQEMUCapsStripMachineAliases(info.qemuCaps); \
+ if (testInfoSetArgs(&info, __VA_ARGS__, ARG_END) < 0) \
+ return EXIT_FAILURE; \
Since you already have an explicit ARG_END here, it's kinda ugly
that you need another one...
[...]
+# define DO_TEST_CAPS_ARCH_VER_FULL(name, arch, ver, ...) \
+ DO_TEST_CAPS_INTERNAL(name, arch "-" ver, \
+ arch, TEST_CAPS_PATH ver "." arch ".xml",
false, \
+ __VA_ARGS__)
# define DO_TEST_CAPS_ARCH_VER(name, arch, ver) \
- DO_TEST_CAPS_ARCH_VER_FULL(name, 0, 0, arch, ver)
+ DO_TEST_CAPS_ARCH_VER_FULL(name, arch, ver, ARG_END)
... both here...
[...]
+# define DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, ...) \
+ DO_TEST_CAPS_INTERNAL(name, arch "-latest", arch, \
+ virHashLookup(capslatest, arch), true, \
+ __VA_ARGS__)
# define DO_TEST_CAPS_ARCH_LATEST(name, arch) \
- DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, 0, 0)
+ DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, ARG_END)
... and here, but the underlying macro can't take zero variadic
arguments so there's no way around it I guess...
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
9:44 a.m.
New subject: [libvirt] [PATCH 15/21] tests: qemuxml2argv: remove full testInfo initialization
Only initialize the fields that are passed in
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 176e4eff3e..b1c18c6c09 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -786,11 +786,12 @@ mymain(void)
* the test cases should be forked using DO_TEST_CAPS_VER with the appropriate
* version.
*/
-# define DO_TEST_CAPS_INTERNAL(name, suffix, \
+# define DO_TEST_CAPS_INTERNAL(_name, _suffix, \
arch, capsfile, stripmachinealiases, ...) \
do { \
static struct testInfo info = { \
- name, "." suffix, NULL, NULL, -1, 0, 0, \
+ .name = _name, \
+ .suffix = "." _suffix, \
}; \
if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \
capsfile))) \
@@ -800,7 +801,7 @@ mymain(void)
if (testInfoSetArgs(&info, __VA_ARGS__, ARG_END) < 0) \
return EXIT_FAILURE; \
info.flags |= FLAG_REAL_CAPS; \
- if (virTestRun("QEMU XML-2-ARGV " name "." suffix, \
+ if (virTestRun("QEMU XML-2-ARGV " _name "." _suffix, \
testCompareXMLToArgv, &info) < 0) \
ret = -1; \
virObjectUnref(info.qemuCaps); \
@@ -841,16 +842,16 @@ mymain(void)
/* All the following macros require an explicit QEMU_CAPS_* list
* at the end of the argument list, or the NONE placeholder */
-# define DO_TEST_FULL(name, ...) \
+# define DO_TEST_FULL(_name, ...) \
do { \
static struct testInfo info = { \
- name, NULL, NULL, NULL, -1, 0, 0, \
+ .name = _name, \
}; \
if (!(info.qemuCaps = virQEMUCapsNew())) \
return EXIT_FAILURE; \
if (testInfoSetArgs(&info, __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \
return EXIT_FAILURE; \
- if (virTestRun("QEMU XML-2-ARGV " name, \
+ if (virTestRun("QEMU XML-2-ARGV " _name, \
testCompareXMLToArgv, &info) < 0) \
ret = -1; \
virObjectUnref(info.qemuCaps); \
--
2.20.1
9:12 a.m.
New subject: [libvirt] [PATCH 15/21] tests: qemuxml2argv: remove full testInfo initialization
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
[...]
+# define DO_TEST_CAPS_INTERNAL(_name, _suffix, \
arch, capsfile, stripmachinealiases, ...) \
do { \
static struct testInfo info = { \
- name, "." suffix, NULL, NULL, -1, 0, 0, \
+ .name = _name, \
+ .suffix = "." _suffix, \
}; \
It would probably have been a lot better to perform this kind of
conversion early in the series, and then gradually get rid of the
initialization for single members at the same time as support for
the corresponding ARG_* was implemented... But I'm not gonna ask
you to go back and do that :)
Note that we're also effectively changing the default value for
migrateFd from -1 to 0, but doing so doesn't cause any failures
since the value is only used when migrateFrom is "stdio", and in
all test cases where that is true we also already set migrateFd
explicitly.
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
11:53 a.m.
New subject: [libvirt] [PATCH 15/21] tests: qemuxml2argv: remove full testInfo initialization
On 3/19/19 10:12 AM, Andrea Bolognani wrote:
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
[...]
> +# define DO_TEST_CAPS_INTERNAL(_name, _suffix, \
> arch, capsfile, stripmachinealiases, ...) \
> do { \
> static struct testInfo info = { \
> - name, "." suffix, NULL, NULL, -1, 0, 0, \
> + .name = _name, \
> + .suffix = "." _suffix, \
> }; \
It would probably have been a lot better to perform this kind of
conversion early in the series, and then gradually get rid of the
initialization for single members at the same time as support for
the corresponding ARG_* was implemented... But I'm not gonna ask
you to go back and do that :)
Thank you :)
Note that we're also effectively changing the default value for
migrateFd from -1 to 0, but doing so doesn't cause any failures
since the value is only used when migrateFrom is "stdio", and in
all test cases where that is true we also already set migrateFd
explicitly.
Hmm I didn't notice that... but since like you say it doesn't cause test
failures, I'll leave it as is
Thanks for the reviews. I fixed all the other issues in patches 1-19 and
pushed. I'll send a v2 with patch #20+ fixed
Thanks,
Cole
9:44 a.m.
New subject: [libvirt] [PATCH 16/21] tests: qemuxml2argv: centralize CAPS suffix building
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index b1c18c6c09..4b34bbcb63 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -786,12 +786,12 @@ mymain(void)
* the test cases should be forked using DO_TEST_CAPS_VER with the appropriate
* version.
*/
-# define DO_TEST_CAPS_INTERNAL(_name, _suffix, \
- arch, capsfile, stripmachinealiases, ...) \
+# define DO_TEST_CAPS_INTERNAL(_name, arch, ver, \
+ capsfile, stripmachinealiases, ...) \
do { \
static struct testInfo info = { \
.name = _name, \
- .suffix = "." _suffix, \
+ .suffix = "." arch "-" ver, \
}; \
if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \
capsfile))) \
@@ -801,7 +801,7 @@ mymain(void)
if (testInfoSetArgs(&info, __VA_ARGS__, ARG_END) < 0) \
return EXIT_FAILURE; \
info.flags |= FLAG_REAL_CAPS; \
- if (virTestRun("QEMU XML-2-ARGV " _name "." _suffix, \
+ if (virTestRun("QEMU XML-2-ARGV " _name "." arch
"-" ver, \
testCompareXMLToArgv, &info) < 0) \
ret = -1; \
virObjectUnref(info.qemuCaps); \
@@ -810,8 +810,8 @@ mymain(void)
# define TEST_CAPS_PATH abs_srcdir "/qemucapabilitiesdata/caps_"
# define DO_TEST_CAPS_ARCH_VER_FULL(name, arch, ver, ...) \
- DO_TEST_CAPS_INTERNAL(name, arch "-" ver, \
- arch, TEST_CAPS_PATH ver "." arch ".xml",
false, \
+ DO_TEST_CAPS_INTERNAL(name, arch, ver, \
+ TEST_CAPS_PATH ver "." arch ".xml", false,
\
__VA_ARGS__)
# define DO_TEST_CAPS_ARCH_VER(name, arch, ver) \
@@ -821,7 +821,7 @@ mymain(void)
DO_TEST_CAPS_ARCH_VER(name, "x86_64", ver)
# define DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, ...) \
- DO_TEST_CAPS_INTERNAL(name, arch "-latest", arch, \
+ DO_TEST_CAPS_INTERNAL(name, arch, "latest", \
virHashLookup(capslatest, arch), true, \
__VA_ARGS__)
--
2.20.1
9:17 a.m.
New subject: [libvirt] [PATCH 16/21] tests: qemuxml2argv: centralize CAPS suffix building
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
9:44 a.m.
New subject: [libvirt] [PATCH 17/21] tests: qemuxml2argv: build capsfile in DO_TEST_CAPS_INTERNAL
Rather than make callers do it. The operative info is just arch
and ver which we are passing in already.
Fold in stripmachinealiases too since it is just dependent on
ver value
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 4b34bbcb63..496e33f4e5 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -786,13 +786,20 @@ mymain(void)
* the test cases should be forked using DO_TEST_CAPS_VER with the appropriate
* version.
*/
-# define DO_TEST_CAPS_INTERNAL(_name, arch, ver, \
- capsfile, stripmachinealiases, ...) \
+# define TEST_CAPS_PATH abs_srcdir "/qemucapabilitiesdata/caps_"
+
+# define DO_TEST_CAPS_INTERNAL(_name, arch, ver, ...) \
do { \
static struct testInfo info = { \
.name = _name, \
.suffix = "." arch "-" ver, \
}; \
+ static const char *capsfile = TEST_CAPS_PATH ver "." arch
".xml"; \
+ static bool stripmachinealiases; \
+ if (STREQ(ver, "latest")) { \
+ capsfile = virHashLookup(capslatest, arch); \
+ stripmachinealiases = true; \
+ } \
if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \
capsfile))) \
return EXIT_FAILURE; \
@@ -807,23 +814,15 @@ mymain(void)
virObjectUnref(info.qemuCaps); \
} while (0)
-# define TEST_CAPS_PATH abs_srcdir "/qemucapabilitiesdata/caps_"
-
-# define DO_TEST_CAPS_ARCH_VER_FULL(name, arch, ver, ...) \
- DO_TEST_CAPS_INTERNAL(name, arch, ver, \
- TEST_CAPS_PATH ver "." arch ".xml", false,
\
- __VA_ARGS__)
# define DO_TEST_CAPS_ARCH_VER(name, arch, ver) \
- DO_TEST_CAPS_ARCH_VER_FULL(name, arch, ver, ARG_END)
+ DO_TEST_CAPS_INTERNAL(name, arch, ver, ARG_END)
# define DO_TEST_CAPS_VER(name, ver) \
DO_TEST_CAPS_ARCH_VER(name, "x86_64", ver)
# define DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, ...) \
- DO_TEST_CAPS_INTERNAL(name, arch, "latest", \
- virHashLookup(capslatest, arch), true, \
- __VA_ARGS__)
+ DO_TEST_CAPS_INTERNAL(name, arch, "latest", __VA_ARGS__)
# define DO_TEST_CAPS_ARCH_LATEST(name, arch) \
DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, ARG_END)
--
2.20.1
9:27 a.m.
New subject: [libvirt] [PATCH 17/21] tests: qemuxml2argv: build capsfile in DO_TEST_CAPS_INTERNAL
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
Rather than make callers do it. The operative info is just arch
and ver which we are passing in already.
Fold in stripmachinealiases too since it is just dependent on
ver value
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
9:44 a.m.
New subject: [libvirt] [PATCH 18/21] tests: qemuxml2argv: add testInfoClear
This is closer to the pattern of qemuxml2xml tests, and will make
things easier if we extend testInfo to contain more freeable data
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 496e33f4e5..813871d6b8 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -665,6 +665,12 @@ testInfoSetArgs(struct testInfo *info, ...)
return ret;
}
+static void
+testInfoClear(struct testInfo *info)
+{
+ virObjectUnref(info->qemuCaps);
+}
+
# define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XXXXXX"
static int
@@ -811,10 +817,9 @@ mymain(void)
if (virTestRun("QEMU XML-2-ARGV " _name "." arch
"-" ver, \
testCompareXMLToArgv, &info) < 0) \
ret = -1; \
- virObjectUnref(info.qemuCaps); \
+ testInfoClear(&info); \
} while (0)
-
# define DO_TEST_CAPS_ARCH_VER(name, arch, ver) \
DO_TEST_CAPS_INTERNAL(name, arch, ver, ARG_END)
@@ -853,7 +858,7 @@ mymain(void)
if (virTestRun("QEMU XML-2-ARGV " _name, \
testCompareXMLToArgv, &info) < 0) \
ret = -1; \
- virObjectUnref(info.qemuCaps); \
+ testInfoClear(&info); \
} while (0)
# define DO_TEST(name, ...) \
--
2.20.1
9:28 a.m.
New subject: [libvirt] [PATCH 18/21] tests: qemuxml2argv: add testInfoClear
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
This is closer to the pattern of qemuxml2xml tests, and will make
things easier if we extend testInfo to contain more freeable data
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
9:44 a.m.
New subject: [libvirt] [PATCH 19/21] tests: qemuxml2argv: move DO_TEST qemuCaps init
Move DO_TEST* qemuCaps init into testInfoSetArgs. This is a step
towards unifying the different test macro implementations
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 813871d6b8..ff74892944 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -621,19 +621,21 @@ testInfoSetArgs(struct testInfo *info, ...)
{
va_list argptr;
testInfoArgNames argname;
+ virQEMUCapsPtr qemuCaps = NULL;
+ int gic = GIC_NONE;
int ret = -1;
va_start(argptr, info);
while ((argname = va_arg(argptr, int)) < ARG_END) {
switch (argname) {
case ARG_QEMU_CAPS:
- virQEMUCapsSetVList(info->qemuCaps, argptr);
+ if (qemuCaps || !(qemuCaps = virQEMUCapsNew()))
+ goto cleanup;
+ virQEMUCapsSetVList(qemuCaps, argptr);
break;
case ARG_GIC:
- if (testQemuCapsSetGIC(info->qemuCaps,
- va_arg(argptr, int)) < 0)
- goto cleanup;
+ gic = va_arg(argptr, int);
break;
case ARG_MIGRATE_FROM:
@@ -659,8 +661,20 @@ testInfoSetArgs(struct testInfo *info, ...)
}
}
+ if (!info->qemuCaps) {
+ if (!qemuCaps) {
+ fprintf(stderr, "No qemuCaps generated\n");
+ goto cleanup;
+ }
+ VIR_STEAL_PTR(info->qemuCaps, qemuCaps);
+ }
+
+ if (gic && testQemuCapsSetGIC(info->qemuCaps, gic) < 0)
+ goto cleanup;
+
ret = 0;
cleanup:
+ virObjectUnref(qemuCaps);
va_end(argptr);
return ret;
}
@@ -851,8 +865,6 @@ mymain(void)
static struct testInfo info = { \
.name = _name, \
}; \
- if (!(info.qemuCaps = virQEMUCapsNew())) \
- return EXIT_FAILURE; \
if (testInfoSetArgs(&info, __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \
return EXIT_FAILURE; \
if (virTestRun("QEMU XML-2-ARGV " _name, \
--
2.20.1
9:55 a.m.
New subject: [libvirt] [PATCH 19/21] tests: qemuxml2argv: move DO_TEST qemuCaps init
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
[...]
+ if (gic && testQemuCapsSetGIC(info->qemuCaps, gic)
< 0)
+ goto cleanup;
Please change the first part of the condition to 'gic != GIC_NONE'
so that it doesn't implicitly rely on the actual value.
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
9:44 a.m.
New subject: [libvirt] [PATCH 20/21] tests: qemuxml2argv: move DO_CAPS_TEST* qemuCaps init
Move DO_CAPS_TEST* qemuCaps init and all the associated setup
into testInfoSetArgs, adding ARG_CAPS_ARCH and ARG_CAPS_VER
options and using those to build the capsfile path locally
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 68 +++++++++++++++++++++++++++-------------
1 file changed, 47 insertions(+), 21 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index ff74892944..d5e02c26d8 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -605,6 +605,8 @@ testCompareXMLToArgv(const void *data)
return ret;
}
+# define TEST_CAPS_PATH abs_srcdir "/qemucapabilitiesdata/caps_"
+
typedef enum {
ARG_QEMU_CAPS = 1,
ARG_GIC,
@@ -612,20 +614,25 @@ typedef enum {
ARG_MIGRATE_FD,
ARG_FLAGS,
ARG_PARSEFLAGS,
+ ARG_CAPS_ARCH,
+ ARG_CAPS_VER,
ARG_END = QEMU_CAPS_LAST,
} testInfoArgNames;
static int
-testInfoSetArgs(struct testInfo *info, ...)
+testInfoSetArgs(struct testInfo *info, virHashTablePtr capslatest, ...)
{
va_list argptr;
testInfoArgNames argname;
virQEMUCapsPtr qemuCaps = NULL;
int gic = GIC_NONE;
int ret = -1;
+ char *capsarch = NULL;
+ char *capsver = NULL;
+ VIR_AUTOFREE(char *) capsfile = NULL;
- va_start(argptr, info);
+ va_start(argptr, capslatest);
while ((argname = va_arg(argptr, int)) < ARG_END) {
switch (argname) {
case ARG_QEMU_CAPS:
@@ -654,6 +661,14 @@ testInfoSetArgs(struct testInfo *info, ...)
info->parseFlags = va_arg(argptr, int);
break;
+ case ARG_CAPS_ARCH:
+ capsarch = va_arg(argptr, char *);
+ break;
+
+ case ARG_CAPS_VER:
+ capsver = va_arg(argptr, char *);
+ break;
+
case ARG_END:
default:
fprintf(stderr, "Unexpected test info argument");
@@ -661,13 +676,33 @@ testInfoSetArgs(struct testInfo *info, ...)
}
}
- if (!info->qemuCaps) {
- if (!qemuCaps) {
- fprintf(stderr, "No qemuCaps generated\n");
+ if (!qemuCaps && capsarch && capsver) {
+ bool stripmachinealiases = false;
+
+ if (STREQ(capsver, "latest")) {
+ if (VIR_STRDUP(capsfile, virHashLookup(capslatest, capsarch)) < 0)
+ goto cleanup;
+ stripmachinealiases = true;
+ } else if (virAsprintf(&capsfile,
+ TEST_CAPS_PATH "%s.%s.xml",
+ capsver, capsarch) < 0) {
goto cleanup;
}
- VIR_STEAL_PTR(info->qemuCaps, qemuCaps);
+
+ if (!(qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(capsarch),
+ capsfile)))
+ goto cleanup;
+
+ if (stripmachinealiases)
+ virQEMUCapsStripMachineAliases(qemuCaps);
+ info->flags |= FLAG_REAL_CAPS;
+ }
+
+ if (!qemuCaps) {
+ fprintf(stderr, "No qemuCaps generated\n");
+ goto cleanup;
}
+ VIR_STEAL_PTR(info->qemuCaps, qemuCaps);
if (gic && testQemuCapsSetGIC(info->qemuCaps, gic) < 0)
goto cleanup;
@@ -806,7 +841,6 @@ mymain(void)
* the test cases should be forked using DO_TEST_CAPS_VER with the appropriate
* version.
*/
-# define TEST_CAPS_PATH abs_srcdir "/qemucapabilitiesdata/caps_"
# define DO_TEST_CAPS_INTERNAL(_name, arch, ver, ...) \
do { \
@@ -814,20 +848,11 @@ mymain(void)
.name = _name, \
.suffix = "." arch "-" ver, \
}; \
- static const char *capsfile = TEST_CAPS_PATH ver "." arch
".xml"; \
- static bool stripmachinealiases; \
- if (STREQ(ver, "latest")) { \
- capsfile = virHashLookup(capslatest, arch); \
- stripmachinealiases = true; \
- } \
- if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \
- capsfile))) \
- return EXIT_FAILURE; \
- if (stripmachinealiases) \
- virQEMUCapsStripMachineAliases(info.qemuCaps); \
- if (testInfoSetArgs(&info, __VA_ARGS__, ARG_END) < 0) \
+ if (testInfoSetArgs(&info, capslatest, \
+ ARG_CAPS_ARCH, arch, \
+ ARG_CAPS_VER, ver, \
+ __VA_ARGS__, ARG_END) < 0) \
return EXIT_FAILURE; \
- info.flags |= FLAG_REAL_CAPS; \
if (virTestRun("QEMU XML-2-ARGV " _name "." arch
"-" ver, \
testCompareXMLToArgv, &info) < 0) \
ret = -1; \
@@ -865,7 +890,8 @@ mymain(void)
static struct testInfo info = { \
.name = _name, \
}; \
- if (testInfoSetArgs(&info, __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \
+ if (testInfoSetArgs(&info, capslatest, \
+ __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \
return EXIT_FAILURE; \
if (virTestRun("QEMU XML-2-ARGV " _name, \
testCompareXMLToArgv, &info) < 0) \
--
2.20.1
10:18 a.m.
New subject: [libvirt] [PATCH 20/21] tests: qemuxml2argv: move DO_CAPS_TEST* qemuCaps init
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
[...]
@@ -612,20 +614,25 @@ typedef enum {
ARG_MIGRATE_FD,
ARG_FLAGS,
ARG_PARSEFLAGS,
+ ARG_CAPS_ARCH,
+ ARG_CAPS_VER,
I guess these could be simply ARG_ARCH and ARG_VER, but I don't
have a terribly strong opinion on the subject.
[...]
+testInfoSetArgs(struct testInfo *info, virHashTablePtr capslatest,
...)
One argument per line, please.
{
va_list argptr;
testInfoArgNames argname;
virQEMUCapsPtr qemuCaps = NULL;
int gic = GIC_NONE;
int ret = -1;
+ char *capsarch = NULL;
+ char *capsver = NULL;
+ VIR_AUTOFREE(char *) capsfile = NULL;
ret should be the last variable in the list.
[...]
+ if (!qemuCaps && capsarch && capsver) {
+ bool stripmachinealiases = false;
I think it would be a good idea to perform some more validation on
the input here: for example we should error out if ARG_CAPS_ARCH has
been provided but ARG_CAPS_VER hasn't or viceversa, or if both
ARG_QEMU_CAPS and ARG_CAPS_ARCH+ARG_CAPS_VER have been provided,
because in both scenarios the user is probably not getting whatever
result they were expecting.
+ if (STREQ(capsver, "latest")) {
+ if (VIR_STRDUP(capsfile, virHashLookup(capslatest, capsarch)) < 0)
+ goto cleanup;
+ stripmachinealiases = true;
+ } else if (virAsprintf(&capsfile,
+ TEST_CAPS_PATH "%s.%s.xml",
+ capsver, capsarch) < 0) {
I'd prefer this as
virAsprintf(&capsfile, "%s/caps_%s.%s.xml",
TEST_CAPS_PATH, capsver, capsarch)
with TEST_CAPS_PATH being redefined as
abs_srcdir "/qemucapabilitiesdata"
of course: according to the name, it's supposed to be a path and
not a prefix after all.
Everything else looks good.
--
Andrea Bolognani / Red Hat / Virtualization
2:54 p.m.
New subject: [libvirt] [PATCH 20/21] tests: qemuxml2argv: move DO_CAPS_TEST* qemuCaps init
On 3/19/19 11:18 AM, Andrea Bolognani wrote:
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
[...]
> @@ -612,20 +614,25 @@ typedef enum {
> ARG_MIGRATE_FD,
> ARG_FLAGS,
> ARG_PARSEFLAGS,
> + ARG_CAPS_ARCH,
> + ARG_CAPS_VER,
I guess these could be simply ARG_ARCH and ARG_VER, but I don't
have a terribly strong opinion on the subject.
I figured ARG_ARCH/ARG_VER are ambiguous enough, the CAPS was to make
it clear this was specifically for caps file lookups
[...]
> +testInfoSetArgs(struct testInfo *info, virHashTablePtr capslatest, ...)
One argument per line, please.
> {
> va_list argptr;
> testInfoArgNames argname;
> virQEMUCapsPtr qemuCaps = NULL;
> int gic = GIC_NONE;
> int ret = -1;
> + char *capsarch = NULL;
> + char *capsver = NULL;
> + VIR_AUTOFREE(char *) capsfile = NULL;
ret should be the last variable in the list.
[...]
> + if (!qemuCaps && capsarch && capsver) {
> + bool stripmachinealiases = false;
I think it would be a good idea to perform some more validation on
the input here: for example we should error out if ARG_CAPS_ARCH has
been provided but ARG_CAPS_VER hasn't or viceversa, or if both
ARG_QEMU_CAPS and ARG_CAPS_ARCH+ARG_CAPS_VER have been provided,
because in both scenarios the user is probably not getting whatever
result they were expecting.
I'll handle this in a separate patch
> + if (STREQ(capsver, "latest")) {
> + if (VIR_STRDUP(capsfile, virHashLookup(capslatest, capsarch)) < 0)
> + goto cleanup;
> + stripmachinealiases = true;
> + } else if (virAsprintf(&capsfile,
> + TEST_CAPS_PATH "%s.%s.xml",
> + capsver, capsarch) < 0) {
I'd prefer this as
virAsprintf(&capsfile, "%s/caps_%s.%s.xml",
TEST_CAPS_PATH, capsver, capsarch)
with TEST_CAPS_PATH being redefined as
abs_srcdir "/qemucapabilitiesdata"
of course: according to the name, it's supposed to be a path and
not a prefix after all.
Sure, I'll break that change out into a prep patch. v2 incoming
Thanks,
Cole
9:44 a.m.
New subject: [libvirt] [PATCH 21/21] tests: qemuxml2argv: add TEST_INTERNAL
Base macro to unify the actual testCompareXMLToArgv test calls
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
tests/qemuxml2argvtest.c | 29 +++++++++++------------------
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d5e02c26d8..a901582ef6 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -842,23 +842,27 @@ mymain(void)
* version.
*/
-# define DO_TEST_CAPS_INTERNAL(_name, arch, ver, ...) \
+# define TEST_INTERNAL(_name, _suffix, ...) \
do { \
static struct testInfo info = { \
.name = _name, \
- .suffix = "." arch "-" ver, \
+ .suffix = _suffix, \
}; \
if (testInfoSetArgs(&info, capslatest, \
- ARG_CAPS_ARCH, arch, \
- ARG_CAPS_VER, ver, \
__VA_ARGS__, ARG_END) < 0) \
return EXIT_FAILURE; \
- if (virTestRun("QEMU XML-2-ARGV " _name "." arch
"-" ver, \
+ if (virTestRun("QEMU XML-2-ARGV " _name _suffix, \
testCompareXMLToArgv, &info) < 0) \
ret = -1; \
testInfoClear(&info); \
} while (0)
+# define DO_TEST_CAPS_INTERNAL(name, arch, ver, ...) \
+ TEST_INTERNAL(name, "." arch "-" ver, \
+ ARG_CAPS_ARCH, arch, \
+ ARG_CAPS_VER, ver, \
+ __VA_ARGS__)
+
# define DO_TEST_CAPS_ARCH_VER(name, arch, ver) \
DO_TEST_CAPS_INTERNAL(name, arch, ver, ARG_END)
@@ -885,19 +889,8 @@ mymain(void)
/* All the following macros require an explicit QEMU_CAPS_* list
* at the end of the argument list, or the NONE placeholder */
-# define DO_TEST_FULL(_name, ...) \
- do { \
- static struct testInfo info = { \
- .name = _name, \
- }; \
- if (testInfoSetArgs(&info, capslatest, \
- __VA_ARGS__, QEMU_CAPS_LAST, ARG_END) < 0) \
- return EXIT_FAILURE; \
- if (virTestRun("QEMU XML-2-ARGV " _name, \
- testCompareXMLToArgv, &info) < 0) \
- ret = -1; \
- testInfoClear(&info); \
- } while (0)
+# define DO_TEST_FULL(name, ...) \
+ TEST_INTERNAL(name, "", __VA_ARGS__, QEMU_CAPS_LAST)
# define DO_TEST(name, ...) \
DO_TEST_FULL(name, \
--
2.20.1
10:24 a.m.
New subject: [libvirt] [PATCH 21/21] tests: qemuxml2argv: add TEST_INTERNAL
On Thu, 2019-03-14 at 10:44 -0400, Cole Robinson wrote:
[...]
+# define TEST_INTERNAL(_name, _suffix, ...) \
This macro should be called DO_TEST_INTERNAL() - you're still
running the test from it, so the DO_ prefix is fully warranted.
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
10:17 a.m.
On 3/14/19 9:43 AM, Cole Robinson wrote:
Right now in qemuxml2argv we have a proliferation of DO_*TEST*
macros.
They essentially fill in data in the testInfo struct and invoke the
test function.
There's several bits of data that we want to specify for a small
subset of tests: flags, parseFlags, migrateFrom/migrateFd, gic. The
base macros need to handle these in their argument lists, and we
provide many convenience macros that fill in default values for the
less common parameters. Any time we want to add a new bit of data
though, the base macros are extended, and every caller of those
needs to be adjusted, and we are faced with the question of whether
to extend the combinatorial explosion of convenience macros which
have only a subset of options exposed.
This series adds a testInfoSetArgs which uses va_args to make these
mandatory parameters optional. The general format is:
DO_TEST_FULL(...
ARG_FOO, foovalue,
ARG_BAR, barvalue, ...)
Specifically, one of the migrate tests went from:
DO_TEST_FULL("restore-v2", "exec:cat", 7, 0, 0, GIC_NONE, NONE);
to:
DO_TEST_FULL("restore-v2",
ARG_MIGRATE_FROM, "exec:cat",
ARG_MIGRATE_FD, 7,
ARG_QEMU_CAPS, NONE);
Umm, what's your sentinel value here? It is undefined behavior to call
va_arg more times than there are arguments present, but without some
sort of sentinel, you don't know when to stop calling va_arg.
The overall idea is nice, but I think you need an ARG_END sentinel.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
2:36 p.m.
On 3/14/19 11:17 AM, Eric Blake wrote:
On 3/14/19 9:43 AM, Cole Robinson wrote:
> Right now in qemuxml2argv we have a proliferation of DO_*TEST* macros.
> They essentially fill in data in the testInfo struct and invoke the
> test function.
>
> There's several bits of data that we want to specify for a small
> subset of tests: flags, parseFlags, migrateFrom/migrateFd, gic. The
> base macros need to handle these in their argument lists, and we
> provide many convenience macros that fill in default values for the
> less common parameters. Any time we want to add a new bit of data
> though, the base macros are extended, and every caller of those
> needs to be adjusted, and we are faced with the question of whether
> to extend the combinatorial explosion of convenience macros which
> have only a subset of options exposed.
>
> This series adds a testInfoSetArgs which uses va_args to make these
> mandatory parameters optional. The general format is:
>
> DO_TEST_FULL(...
> ARG_FOO, foovalue,
> ARG_BAR, barvalue, ...)
>
> Specifically, one of the migrate tests went from:
>
> DO_TEST_FULL("restore-v2", "exec:cat", 7, 0, 0, GIC_NONE,
NONE);
>
> to:
>
> DO_TEST_FULL("restore-v2",
> ARG_MIGRATE_FROM, "exec:cat",
> ARG_MIGRATE_FD, 7,
> ARG_QEMU_CAPS, NONE);
Umm, what's your sentinel value here? It is undefined behavior to call
va_arg more times than there are arguments present, but without some
sort of sentinel, you don't know when to stop calling va_arg.
The overall idea is nice, but I think you need an ARG_END sentinel.
You saw it in the later patches, but just to respond here if anyone else
is watching: there is an ARG_END but it's hidden in the macro definitions.
- Cole
- Cole
2073
days inactive
2080
days old
56 comments
4 participants
participants (4)
-
Andrea Bolognani -
Cole Robinson -
Eric Blake -
Ján Tomko