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>
>>> ---
>>> 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.
Michal