Re: [libvirt] [PATCH v2 1/5] qemuxml2argvtest: Set more fake drivers

On 07/12/2018 10:56 AM, Peter Krempa wrote: > On Thu, Jul 12, 2018 at 10:51:33 +0200, Michal Privoznik wrote: >> On 07/12/2018 10:01 AM, Peter Krempa wrote: >>> On Thu, Jul 12, 2018 at 09:37:47 +0200, Michal Privoznik wrote: >>>> So far we are setting only fake secret and storage drivers. >>>> Therefore if the code wants to call a public NWFilter API (like >>>> qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() are >>>> doing) the virGetConnectNWFilter() function will try to actually >>>> spawn session daemon because there's no connection object set to >>>> handle NWFilter driver. >>>> >>>> Even though I haven't experienced the same problem with the rest >>>> of the drivers (interface, network and node dev), the reasoning >>>> above can be applied to them as well. >>>> >>>> At the same time, now that connection object is registered for >>>> the drivers, the public APIs will throw >>>> virReportUnsupportedError(). And since we don't provide any error >>>> func the error is printed to stderr. Fix this by setting dummy >>>> error func. >>>> >>>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >>>> --- >>>> tests/qemuxml2argvtest.c | 6 ++++++ >>>> 1 file changed, 6 insertions(+) >>> >>> [...] >>> >>>> >>>> @@ -652,6 +656,8 @@ mymain(void) >>>> return EXIT_FAILURE; >>>> } >>>> >>>> + virTestQuiesceLibvirtErrors(true); >>>> + >>> >>> NACK, this suppresses legitimate errors in the testsuite. >>> >>> I've mangled one of the XML files and ran the qemuxml2argvtest with >>> VIR_TEST_DEBUG=1 and got: >>> >>> 249) QEMU XML-2-startup-XML disk-drive-cache-directsync ... SKIP >>> 250) QEMU XML-2-ARGV disk-drive-cache-unsafe ... FAILED >>> 251) QEMU XML-2-startup-XML disk-drive-cache-unsafe ... SKIP >>> >>> Without this patch I'd get: >>> >>> 249) QEMU XML-2-startup-XML disk-drive-cache-directsync ... SKIP >>> 250) QEMU XML-2-ARGV disk-drive-cache-unsafe ... libvirt: Domain Config error : unsupported configuration: unknown disk cache mode 'unafe' >>> FAILED >>> 251) QEMU XML-2-startup-XML disk-drive-cache-unsafe ... SKIP >>> >> >> Well, without it I get: >> >> tests $ ./qemuxml2argvtest >> TEST: qemuxml2argvtest >> ._._._._..._._._._._._._._._._._._._._._ 40 >> ._._._._._._._._._._._._._._._._._._._._ 80 >> ._._._._._._._._._._._._._._._._._._._._ 120 >> ._._._._._._._._._._._._._._._._._._._._ 160 >> ._._._._._._._._._._._._._._._._._._._._ 200 >> ._._._._._._._._._._._._._._._._..._._._ 240 >> .._._._._._...._._._._._._._._._._._._._ 280 >> ._._._._._._._._._._._._._._._._._._._._ 320 >> ._._._._._._._._._._._._._._._._._._._._ 360 >> ._._._._._._._._._._._._._._._._._._._._ 400 >> ._._._._._._._._._._._._._._._._._._._._ 440 >> ._._._._._._._._libvirt: Network Filter Driver error : internal >> error: unexpected nwfilter URI path '/session', try nwfilter:///system >> libvirt: Network Filter Driver error : internal error: unexpected >> nwfilter URI path '/session', try nwfilter:///system >> libvirt: Network Filter Driver error : internal error: unexpected >> nwfilter URI path '/session', try nwfilter:///system >> libvirt: Network Filter Driver error : internal error: unexpected >> nwfilter URI path '/session', try nwfilter:///system >> libvirt: Network Filter Driver error : internal error: unexpected >> nwfilter URI path '/session', try nwfilter:///system >> ._._._._._._._._._._._._ 480 >>

>> >> So do you have any other idea? I came up with two already and neither of >> them got through review. Just to remind everybody, we are possibly >> touching live user data here so we need a resolution rather sooner than >> later. > > I specifically NACKd the part that installs the callback for suppressing > error messages. The messages here need to be suppressed by some other > way, but we should not decrease the debugability of tests. > > The error handler installation does not seem to have to do with live > user data touching. > > I did not try to see what the other part of this patch does. > Okay, I will drop it. The output of the test would be ugly then, but I don't care anymore. Also, can I get review on the rest of the patches please? I noticed some people started replying only to a single patch in a series leaving the rest unreviewed. I don't think that's good.