[libvirt] [RFC][PATCH]QEMU: Parse -device vfio-pci commandline

Signed-off-by: Olivia Yin <Hong-Hua.Yin@freescale.com> --- src/qemu/qemu_command.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++ tests/qemuargv2xmltest.c | 2 +- 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e6acced..4db4a1d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10239,6 +10239,59 @@ qemuParseCommandLinePCI(const char *val) return NULL; } +/* + * Tries to parse a QEMU vfio-pci device + */ +static virDomainHostdevDefPtr +qemuParseCommandLineVFIOPCI(const char *val) +{ + int bus = 0, slot = 0, func = 0; + const char *start; + char *end; + virDomainHostdevDefPtr def = virDomainHostdevDefAlloc(); + + if (!def) + goto error; + + if (!STRPREFIX(val, "host=")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown PCI device syntax '%s'"), val); + goto error; + } + + start = val + strlen("host="); + if (virStrToLong_i(start, &end, 16, &bus) < 0 || *end != ':') { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot extract PCI device bus '%s'"), val); + goto error; + } + start = end + 1; + if (virStrToLong_i(start, &end, 16, &slot) < 0 || *end != '.') { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot extract PCI device slot '%s'"), val); + goto error; + } + start = end + 1; + if (virStrToLong_i(start, &end, 16, &func) < 0 || *end != ',') { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot extract PCI device function '%s'"), val); + goto error; + } + + def->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; + def->managed = true; + def->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI; + def->source.subsys.u.pci.backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; + def->source.subsys.u.pci.addr.bus = bus; + def->source.subsys.u.pci.addr.slot = slot; + def->source.subsys.u.pci.addr.function = func; + return def; + + error: + virDomainHostdevDefFree(def); + return NULL; +} + /* * Tries to parse a QEMU USB device @@ -11351,6 +11404,20 @@ qemuParseCommandLine(virCapsPtr qemuCaps, virDomainHostdevDefFree(hostdev); goto error; } + } else if (STREQ(arg, "-device")) { + WANT_VALUE(); + if (STRPREFIX(val, "vfio-pci,")) { + const char *start; + start = val; + virDomainHostdevDefPtr hostdev; + start += strlen("vfio-pci,"); + if (!(hostdev = qemuParseCommandLineVFIOPCI(start))) + goto error; + if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0) { + virDomainHostdevDefFree(hostdev); + goto error; + } + } } else if (STREQ(arg, "-soundhw")) { const char *start; WANT_VALUE(); diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 0fc9fcb..b4ba97a 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -251,8 +251,8 @@ mymain(void) DO_TEST("watchdog"); DO_TEST("hostdev-usb-address"); - DO_TEST("hostdev-pci-address"); + DO_TEST("hostdev-vfio"); DO_TEST("smp"); -- 1.8.5

The test case failed with the patch. tests]$ VIR_TEST_DEBUG=1 ./qemuargv2xmltest ... 78) QEMU ARGV-2-XML hostdev-vfio ... FAILED ... tests]$ ../tools/virsh domxml-from-native qemu-argv qemuxml2argvdata/qemuxml2argv-hostdev-vfio.args <domain type='qemu' xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'> <name>unnamed</name> <uuid>4f190a56-f64b-4db4-b326-c4895dd1bae5</uuid> <memory unit='KiB'>219136</memory> <currentMemory unit='KiB'>219136</currentMemory> <vcpu placement='static'>1</vcpu> <os> <type arch='i686' machine='\ pc'>hvm</type> <boot dev='hd'/> </os> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>\ /usr/bin/qemu</emulator> <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='\ /dev/HostVG/QEMUGuest2'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='usb' index='0'/> <controller type='ide' index='0'/> <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> <address domain='0x0000' bus='0x06' slot='0x12' function='0x5'/> </source> </hostdev> <memballoon model='virtio'/> </devices> <qemu:commandline> <qemu:arg value='-nodefconfig'/> <qemu:arg value='-nodefaults'/> </qemu:commandline> </domain> It seemed that <hostdev> section is correct as expected. Why does the case fail? How to check more detailed debug information? Best Regards, Olivia
-----Original Message----- From: Olivia Yin [mailto:Hong-Hua.Yin@freescale.com] Sent: Thursday, June 05, 2014 7:43 PM To: libvir-list@redhat.com Cc: Yin Olivia-R63875 Subject: [RFC][PATCH]QEMU: Parse -device vfio-pci commandline
Signed-off-by: Olivia Yin <Hong-Hua.Yin@freescale.com> --- src/qemu/qemu_command.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++ tests/qemuargv2xmltest.c | 2 +- 2 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e6acced..4db4a1d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10239,6 +10239,59 @@ qemuParseCommandLinePCI(const char *val) return NULL; }
+/* + * Tries to parse a QEMU vfio-pci device */ static +virDomainHostdevDefPtr qemuParseCommandLineVFIOPCI(const char *val) { + int bus = 0, slot = 0, func = 0; + const char *start; + char *end; + virDomainHostdevDefPtr def = virDomainHostdevDefAlloc(); + + if (!def) + goto error; + + if (!STRPREFIX(val, "host=")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown PCI device syntax '%s'"), val); + goto error; + } + + start = val + strlen("host="); + if (virStrToLong_i(start, &end, 16, &bus) < 0 || *end != ':') { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot extract PCI device bus '%s'"), val); + goto error; + } + start = end + 1; + if (virStrToLong_i(start, &end, 16, &slot) < 0 || *end != '.') { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot extract PCI device slot '%s'"), val); + goto error; + } + start = end + 1; + if (virStrToLong_i(start, &end, 16, &func) < 0 || *end != ',') { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot extract PCI device function '%s'"), val); + goto error; + } + + def->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; + def->managed = true; + def->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI; + def->source.subsys.u.pci.backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; + def->source.subsys.u.pci.addr.bus = bus; + def->source.subsys.u.pci.addr.slot = slot; + def->source.subsys.u.pci.addr.function = func; + return def; + + error: + virDomainHostdevDefFree(def); + return NULL; +} +
/* * Tries to parse a QEMU USB device @@ -11351,6 +11404,20 @@ qemuParseCommandLine(virCapsPtr qemuCaps, virDomainHostdevDefFree(hostdev); goto error; } + } else if (STREQ(arg, "-device")) { + WANT_VALUE(); + if (STRPREFIX(val, "vfio-pci,")) { + const char *start; + start = val; + virDomainHostdevDefPtr hostdev; + start += strlen("vfio-pci,"); + if (!(hostdev = qemuParseCommandLineVFIOPCI(start))) + goto error; + if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0) { + virDomainHostdevDefFree(hostdev); + goto error; + } + } } else if (STREQ(arg, "-soundhw")) { const char *start; WANT_VALUE(); diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 0fc9fcb..b4ba97a 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -251,8 +251,8 @@ mymain(void) DO_TEST("watchdog");
DO_TEST("hostdev-usb-address"); - DO_TEST("hostdev-pci-address"); + DO_TEST("hostdev-vfio");
DO_TEST("smp");
-- 1.8.5

On 06/05/2014 05:51 AM, Hong-Hua.Yin@freescale.com wrote:
The test case failed with the patch.
tests]$ VIR_TEST_DEBUG=1 ./qemuargv2xmltest ... 78) QEMU ARGV-2-XML hostdev-vfio ... FAILED ...
It seemed that <hostdev> section is correct as expected. Why does the case fail? How to check more detailed debug information?
make check -C tests TESTS=qemuargv2xmltest VIR_TEST_DEBUG=1 VERBOSE=1 (or bump VIR_TEST_DEBUG=2 for even more details) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Hi Eric, I've tried bump VIR_TEST_DEBUG=2, but there's not more information. $ VIR_TEST_DEBUG=2 ./qemuargv2xmltest ... 76) QEMU ARGV-2-XML hostdev-usb-address ... OK 77) QEMU ARGV-2-XML hostdev-pci-address ... OK 78) QEMU ARGV-2-XML hostdev-vfio ... FAILED 79) QEMU ARGV-2-XML smp ... OK 80) QEMU ARGV-2-XML hyperv ... OK 81) QEMU ARGV-2-XML pseries-nvram ... OK 82) QEMU ARGV-2-XML pseries-disk ... OK 83) QEMU ARGV-2-XML nosharepages ... OK 84) QEMU ARGV-2-XML restore-v1 ... OK 85) QEMU ARGV-2-XML restore-v2 ... OK 86) QEMU ARGV-2-XML migrate ... OK 87) QEMU ARGV-2-XML qemu-ns-no-env ... OK The result is also same with 'make check -C tests TESTS=qemuargv2xmltest VIR_TEST_DEBUG=1 VERBOSE=1'. Best Regards, Olivia
-----Original Message----- From: Eric Blake [mailto:eblake@redhat.com] Sent: Thursday, June 05, 2014 9:03 PM To: Yin Olivia-R63875; libvir-list@redhat.com Subject: Re: [libvirt] [RFC][PATCH]QEMU: Parse -device vfio-pci commandline
On 06/05/2014 05:51 AM, Hong-Hua.Yin@freescale.com wrote:
The test case failed with the patch.
tests]$ VIR_TEST_DEBUG=1 ./qemuargv2xmltest ... 78) QEMU ARGV-2-XML hostdev-vfio ... FAILED ...
It seemed that <hostdev> section is correct as expected. Why does the case fail? How to check more detailed debug information?
make check -C tests TESTS=qemuargv2xmltest VIR_TEST_DEBUG=1 VERBOSE=1
(or bump VIR_TEST_DEBUG=2 for even more details)
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 06/05/2014 04:03 PM, Eric Blake wrote:
On 06/05/2014 05:51 AM, Hong-Hua.Yin@freescale.com wrote:
The test case failed with the patch.
tests]$ VIR_TEST_DEBUG=1 ./qemuargv2xmltest ... 78) QEMU ARGV-2-XML hostdev-vfio ... FAILED ... It seemed that <hostdev> section is correct as expected. Why does the case fail? How to check more detailed debug information?
make check -C tests TESTS=qemuargv2xmltest VIR_TEST_DEBUG=1 VERBOSE=1
(or bump VIR_TEST_DEBUG=2 for even more details)
None of those convince it to cough up the warning message that shows what is wrong, but running it under gdb I found that qemuParseommandLine() is producing this warning: unknown QEMU argument '-nodefconfig', adding to the qemu namespace When a warning it logged, it is noticed by testCompareXMLToArgvFiles, and unless that function is called with expect_warning = true, this log warning will cause the test to fail (unfortunately *without* logging the warning that caused the failure). It turns out that expect_warning is set in the test struct's extraFlags, but only if the macro DO_TEST_FULL is used: DO_TEST_FULL("hostdev-vfio", 1); HOWEVER, even if you do that, it will *still* fail, because the XML resulting from qemuParseCommandLine will have -nodefconfig and -nodefaults added to the qemu commandline passthrough part: <qemu:commandline> <qemu:arg value='-nodefconfig'/> <qemu:arg value='-nodefaults'/> </qemu:commandline> (because it doesn't recognize that option). So it *still* fails (although at least this time you can see the reason if you've set VIR_TEST_DEBUG=2). What is really needed is the have qemuParseCommandLine silently discard those two args. Interestingly, this is the very first of the qemuxml2argvdata cases containing -nodefconfig in the commandline data that has ever been added to qemuargv2xmltest.c (which is why we've never seen this problem before). Once that is fixed, you'll also notice that qemuParseCommandLine adds this to the <disk> element: <driver name='qemu' type='raw'/> (that's the default, but it gets spelled out in the conversion), so that needs to be added to the .xml file before the hostdev-vfio test can be added to argv2xmltest. Beyond that, the original failure case (unrecognized option in parse) needs to be enhanced so that it actually outputs the reason for the failure. I'm finishing up patches to fix these problems, and will post them to this thread shortly.

Hi Laine, The problem with my case is fixed after applying your patch. Thank you very much. Eric had ACKed your patches. I hope my patch could also be accepted soon. Best Regards, Olivia
-----Original Message----- From: sendmail [mailto:justsendmailnothingelse@gmail.com] On Behalf Of Laine Stump Sent: Friday, June 06, 2014 8:37 PM To: libvir-list@redhat.com Cc: Eric Blake; Yin Olivia-R63875 Subject: Re: [libvirt] [RFC][PATCH]QEMU: Parse -device vfio-pci commandline
On 06/05/2014 04:03 PM, Eric Blake wrote:
On 06/05/2014 05:51 AM, Hong-Hua.Yin@freescale.com wrote:
The test case failed with the patch.
tests]$ VIR_TEST_DEBUG=1 ./qemuargv2xmltest ... 78) QEMU ARGV-2-XML hostdev-vfio ... FAILED ... It seemed that <hostdev> section is correct as expected. Why does the case fail? How to check more detailed debug information?
make check -C tests TESTS=qemuargv2xmltest VIR_TEST_DEBUG=1 VERBOSE=1
(or bump VIR_TEST_DEBUG=2 for even more details)
None of those convince it to cough up the warning message that shows what is wrong, but running it under gdb I found that qemuParseommandLine() is producing this warning:
unknown QEMU argument '-nodefconfig', adding to the qemu namespace
When a warning it logged, it is noticed by testCompareXMLToArgvFiles, and unless that function is called with expect_warning = true, this log warning will cause the test to fail (unfortunately *without* logging the warning that caused the failure).
It turns out that expect_warning is set in the test struct's extraFlags, but only if the macro DO_TEST_FULL is used:
DO_TEST_FULL("hostdev-vfio", 1);
HOWEVER, even if you do that, it will *still* fail, because the XML resulting from qemuParseCommandLine will have -nodefconfig and -nodefaults added to the qemu commandline passthrough part:
<qemu:commandline> <qemu:arg value='-nodefconfig'/> <qemu:arg value='-nodefaults'/> </qemu:commandline>
(because it doesn't recognize that option).
So it *still* fails (although at least this time you can see the reason if you've set VIR_TEST_DEBUG=2). What is really needed is the have qemuParseCommandLine silently discard those two args.
Interestingly, this is the very first of the qemuxml2argvdata cases containing -nodefconfig in the commandline data that has ever been added to qemuargv2xmltest.c (which is why we've never seen this problem before).
Once that is fixed, you'll also notice that qemuParseCommandLine adds this to the <disk> element:
<driver name='qemu' type='raw'/>
(that's the default, but it gets spelled out in the conversion), so that needs to be added to the .xml file before the hostdev-vfio test can be added to argv2xmltest.
Beyond that, the original failure case (unrecognized option in parse) needs to be enhanced so that it actually outputs the reason for the failure.
I'm finishing up patches to fix these problems, and will post them to this thread shortly.

Patches 1/3 and 2/3 are prerequisites the the patch that started this thread. Patch 3/3 should be squashed into the original patch. I also noticed that the original patch causes all unrecognized "-device" options to now be ignored rather than being added to the qemu namespace (with a warning). This needs to be fixed before resubmitting that patch too, but I didn't have the time/interest to do it. (If needed/desired, all three of these new patches can be pushed separately before the patch at the top of this thread). Laine Stump (3): test: display qemuParseCommandline warnings when VIR_TEST_DEBUG > 0 qemu: ignore -nodefconfig and -nodefaults in qemuParseCommandLineString test: make hostdev-vfio test able to pass qemuargv2xmltest src/qemu/qemu_command.c | 4 +- tests/qemuargv2xmltest.c | 45 ++++++++++++++++------ .../qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml | 1 + 3 files changed, 38 insertions(+), 12 deletions(-) -- 1.9.3

qmeuargv2xmltest.c would fail any test that logged anything during qemuParseCommandline(), but then discard the log message, even with VIR_TEST_DEBUG=2. This patch outputs the log messages with fprintf(stderr,...) when debug logging is on. In the process of modifying that logic, the testInfo data was made more similar to that of qemuxml2argvtest.c - rather than turning info->extraFlags into a bool, an enum of flags is defined, the info struct is given an "unsigned int flags", and FLAG_EXPECT_WARNING is saved into info->flags, to be checked during the test; this will make it easier to add other FLAG_EXPECT_* items in the future. --- tests/qemuargv2xmltest.c | 45 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 0fc9fcb..2cbbe3d 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -34,13 +34,18 @@ static int blankProblemElements(char *data) return 0; } +typedef enum { + FLAG_EXPECT_WARNING = 1 << 0, +} virQemuXML2ArgvTestFlags; + static int testCompareXMLToArgvFiles(const char *xml, const char *cmdfile, - bool expect_warning) + virQemuXML2ArgvTestFlags flags) { char *expectxml = NULL; char *actualxml = NULL; char *cmd = NULL; + char *log = NULL; int ret = -1; virDomainDefPtr vmdef = NULL; @@ -54,14 +59,31 @@ static int testCompareXMLToArgvFiles(const char *xml, goto fail; if (!virtTestOOMActive()) { - char *log; if ((log = virtTestLogContentAndReset()) == NULL) goto fail; - if ((*log != '\0') != expect_warning) { - VIR_FREE(log); - goto fail; + if (flags & FLAG_EXPECT_WARNING) { + if (*log) { + if (virTestGetDebug() > 1) + fprintf(stderr, + "Got expected warning from " + "qemuParseCommandLineString:\n%s", + log); + } else { + if (virTestGetDebug()) + fprintf(stderr, "qemuParseCommandLineString " + "should have logged a warning\n"); + goto fail; + } + } else { /* didn't expect a warning */ + if (*log) { + if (virTestGetDebug()) + fprintf(stderr, + "Got unexpected warning from " + "qemuParseCommandLineString:\n%s", + log); + goto fail; + } } - VIR_FREE(log); } if (!virDomainDefCheckABIStability(vmdef, vmdef)) { @@ -87,6 +109,7 @@ static int testCompareXMLToArgvFiles(const char *xml, VIR_FREE(expectxml); VIR_FREE(actualxml); VIR_FREE(cmd); + VIR_FREE(log); virDomainDefFree(vmdef); return ret; } @@ -94,7 +117,7 @@ static int testCompareXMLToArgvFiles(const char *xml, struct testInfo { const char *name; - unsigned long long extraFlags; + unsigned int flags; }; static int @@ -111,7 +134,7 @@ testCompareXMLToArgvHelper(const void *data) abs_srcdir, info->name) < 0) goto cleanup; - result = testCompareXMLToArgvFiles(xml, args, !!info->extraFlags); + result = testCompareXMLToArgvFiles(xml, args, info->flags); cleanup: VIR_FREE(xml); @@ -136,9 +159,9 @@ mymain(void) if (!(driver.xmlopt = virQEMUDriverCreateXMLConf(&driver))) return EXIT_FAILURE; -# define DO_TEST_FULL(name, extraFlags) \ +# define DO_TEST_FULL(name, flags) \ do { \ - const struct testInfo info = { name, extraFlags }; \ + const struct testInfo info = { name, (flags) }; \ if (virtTestRun("QEMU ARGV-2-XML " name, \ testCompareXMLToArgvHelper, &info) < 0) \ ret = -1; \ @@ -267,7 +290,7 @@ mymain(void) DO_TEST("restore-v2"); DO_TEST("migrate"); - DO_TEST_FULL("qemu-ns-no-env", 1); + DO_TEST_FULL("qemu-ns-no-env", FLAG_EXPECT_WARNING); virObjectUnref(driver.config); virObjectUnref(driver.caps); -- 1.9.3

On 06/06/2014 07:54 AM, Laine Stump wrote:
qmeuargv2xmltest.c would fail any test that logged anything during qemuParseCommandline(), but then discard the log message, even with VIR_TEST_DEBUG=2. This patch outputs the log messages with fprintf(stderr,...) when debug logging is on.
In the process of modifying that logic, the testInfo data was made more similar to that of qemuxml2argvtest.c - rather than turning info->extraFlags into a bool, an enum of flags is defined, the info struct is given an "unsigned int flags", and FLAG_EXPECT_WARNING is saved into info->flags, to be checked during the test; this will make it easier to add other FLAG_EXPECT_* items in the future. --- tests/qemuargv2xmltest.c | 45 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 11 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

the qemu driver always adds these options to the qemu commandlines, but the commandline parser didn't recognize them, so sending a libvirt-generated qemu commandline to its own argvtoxml would always result in a warning message and a qemu namespace added to the xml. Since the options don't add any functionality to the domain, they should just be ignored (similar to -S). --- src/qemu/qemu_command.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e6acced..8ea5f2e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11513,7 +11513,9 @@ qemuParseCommandLine(virCapsPtr qemuCaps, _("cannot parse nvram's address '%s'"), val); goto error; } - } else if (STREQ(arg, "-S")) { + } else if (STREQ(arg, "-S") || + STREQ(arg, "-nodefaults") || + STREQ(arg, "-nodefconfig")) { /* ignore, always added by libvirt */ } else { char *tmp = NULL; -- 1.9.3

On 06/06/2014 07:54 AM, Laine Stump wrote:
the qemu driver always adds these options to the qemu commandlines, but the commandline parser didn't recognize them, so sending a libvirt-generated qemu commandline to its own argvtoxml would always result in a warning message and a qemu namespace added to the xml. Since the options don't add any functionality to the domain, they should just be ignored (similar to -S). --- src/qemu/qemu_command.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Before we can add the hostdev-vfio test to the list in qemuargv2xmltest, we need to add in the <driver> element that is automatically put in the <disk> by qemuParseCommandLineString. This will be squashed into the commit that adds hostdev-vfio to qemuargv2xmltest. --- tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml index 8daa53a..b99f798 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml @@ -15,6 +15,7 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMUGuest2'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> -- 1.9.3

On 06/06/2014 07:54 AM, Laine Stump wrote:
Patches 1/3 and 2/3 are prerequisites the the patch that started this thread. Patch 3/3 should be squashed into the original patch.
I also noticed that the original patch causes all unrecognized "-device" options to now be ignored rather than being added to the qemu namespace (with a warning). This needs to be fixed before resubmitting that patch too, but I didn't have the time/interest to do it.
(If needed/desired, all three of these new patches can be pushed separately before the patch at the top of this thread).
So for 3/3, which is it? Push now, or squash into the respin of the patch that started this thread?
Laine Stump (3): test: display qemuParseCommandline warnings when VIR_TEST_DEBUG > 0 qemu: ignore -nodefconfig and -nodefaults in qemuParseCommandLineString test: make hostdev-vfio test able to pass qemuargv2xmltest
src/qemu/qemu_command.c | 4 +- tests/qemuargv2xmltest.c | 45 ++++++++++++++++------ .../qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml | 1 + 3 files changed, 38 insertions(+), 12 deletions(-)
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 06/06/2014 05:32 PM, Eric Blake wrote:
Patches 1/3 and 2/3 are prerequisites the the patch that started this thread. Patch 3/3 should be squashed into the original patch.
I also noticed that the original patch causes all unrecognized "-device" options to now be ignored rather than being added to the qemu namespace (with a warning). This needs to be fixed before resubmitting that patch too, but I didn't have the time/interest to do it.
(If needed/desired, all three of these new patches can be pushed separately before the patch at the top of this thread). So for 3/3, which is it? Push now, or squash into the respin of the
On 06/06/2014 07:54 AM, Laine Stump wrote: patch that started this thread?
I think it's too small, and not of any use by itself, so I recommended squashing it into the new version of the original patch that adds vfio-pci parsing.

On 06/06/2014 04:54 PM, Laine Stump wrote:
Patches 1/3 and 2/3 are prerequisites the the patch that started this thread. Patch 3/3 should be squashed into the original patch.
I also noticed that the original patch causes all unrecognized "-device" options to now be ignored rather than being added to the qemu namespace (with a warning). This needs to be fixed before resubmitting that patch too, but I didn't have the time/interest to do it.
(If needed/desired, all three of these new patches can be pushed separately before the patch at the top of this thread).
Laine Stump (3): test: display qemuParseCommandline warnings when VIR_TEST_DEBUG > 0 qemu: ignore -nodefconfig and -nodefaults in qemuParseCommandLineString test: make hostdev-vfio test able to pass qemuargv2xmltest
src/qemu/qemu_command.c | 4 +- tests/qemuargv2xmltest.c | 45 ++++++++++++++++------ .../qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml | 1 + 3 files changed, 38 insertions(+), 12 deletions(-)
I pushed 1/3 and 2/3, and have sent another patch to add parsing for <memballoon> (which is also required for all the tests to patch once the original is done correctly). I left 3/3 to be squashed into the original patch of the thread. Thanks for the reviews!
participants (4)
-
Eric Blake
-
Hong-Hua.Yin@freescale.com
-
Laine Stump
-
Olivia Yin