[libvirt] [PATCH v3 1/3] qemuxml2argvtest: Set more fake drivers

[John Ferlan]

On 07/27/2018 12:06 PM, Michal Privoznik wrote:
> On 07/27/2018 05:29 PM, Daniel P. Berrangé wrote:
>> On Fri, Jul 27, 2018 at 05:24:45PM +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.
>>
>> Is this last paragraph stale ? you're not seeing a dummy error
>> func in this patch ...
> 
> Oh yes, I am:
> 
> libvirt.git/tests $ VIR_TEST_DEBUG=1 ./qemuxml2argvtest 2>&1 | grep "this function is not supported"
> 441) QEMU XML-2-ARGV net-vhostuser-multiq                              ... libvirt: Network Filter Driver error : this function is not supported by the connection driver: virNWFilterBindingLookupByPortDev
> libvirt: Network Filter Driver error : this function is not supported by the connection driver: virNWFilterBindingLookupByPortDev
> libvirt: Network Filter Driver error : this function is not supported by the connection driver: virNWFilterBindingLookupByPortDev
> 2018-07-27 16:02:50.689+0000: 11166: error : virNWFilterBindingLookupByPortDev:599 : this function is not supported by the connection driver: virNWFilterBindingLookupByPortDev
> 2018-07-27 16:02:50.689+0000: 11166: error : virNWFilterBindingLookupByPortDev:599 : this function is not supported by the connection driver: virNWFilterBindingLookupByPortDev
> 2018-07-27 16:02:50.689+0000: 11166: error : virNWFilterBindingLookupByPortDev:599 : this function is not supported by the connection driver: virNWFilterBindingLookupByPortDev
> 
> (Or even without DEBUG)
> But as I told Peter, at this point stopping touching live user data is 
> more important to me than silencing some error message.
> 

The above error is only seen when the virSetConnectNWFilter is used... 
So, let's take the extra step to be more like fakeSecretDriver and
fakeStorageDriver.

Squash the following in and you can remove that last paragraph:

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index b905cc943c..cedef5c9ad 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -262,6 +262,33 @@ static virStorageDriver fakeStorageDriver = {
     .storagePoolIsActive = fakeStoragePoolIsActive,
 };
 
+
+/* virNetDevOpenvswitchGetVhostuserIfname mocks a portdev name - handle that */
+static virNWFilterBindingPtr
+fakeNWFilterBindingLookupByPortDev(virConnectPtr conn,
+                                   const char *portdev)
+{
+    if (STREQ(portdev, "vhost-user0"))
+        return virGetNWFilterBinding(conn, "fake_vnet0", "fakeFilterName");
+
+    virReportError(VIR_ERR_NO_NWFILTER_BINDING,
+                   "no nwfilter binding for port dev '%s'", portdev);
+    return NULL;
+}
+
+
+static int
+fakeNWFilterBindingDelete(virNWFilterBindingPtr binding ATTRIBUTE_UNUSED)
+{
+    return 0;
+}
+
+
+static virNWFilterDriver fakeNWFilterDriver = {
+    .nwfilterBindingLookupByPortDev = fakeNWFilterBindingLookupByPortDev,
+    .nwfilterBindingDelete = fakeNWFilterBindingDelete,
+};
+
 typedef enum {
     FLAG_EXPECT_FAILURE     = 1 << 0,
     FLAG_EXPECT_PARSE_ERROR = 1 << 1,
@@ -470,6 +497,7 @@ testCompareXMLToArgv(const void *data)
 
     conn->secretDriver = &fakeSecretDriver;
     conn->storageDriver = &fakeStorageDriver;
+    conn->nwfilterDriver = &fakeNWFilterDriver;
 
     virSetConnectInterface(conn);
     virSetConnectNetwork(conn);

------------

What's really interesting (perhaps to me) is that as long as this patch
(and the squashed hunk) are there, the error message you're trying to
avoid in the next two patches no longer appears.

That is - if just applying this patch, I don't see the Failed to connect
socket to '/run/user/1000...'.  If I comment out virSetConnectNWFilter,
then the message reappears.

If I flip-flop the patches (eg last 2 first) and rebuild, I don't see
the changed error message. But that could be because some file isn't
being rebuilt - I forget I think I tripped across it before, but cannot
remember how.

I did at least figure out why virSetConnectNWFilter triggers the calls you're
trying to avoid from the commit message for/from the net-vhostuser-multiq.

If qemuBuildVhostuserCommandLine fails, then we goto cleanup which
calls virDomainConfNWFilterTeardown sending us down the path to
calling virDomainConfNWFilterTeardownImpl because @conn is no longer
NULL. This calls in succession virNWFilterBindingLookupByPortDev and
virNWFilterBindingDelete before returning.

It's still not clear to me why when using virSetConnectNWFilter does
the "Failed to connect socket..." message not appear and I don't have
the energy to chase that right now.

So of course this is a really long way to say, I'm wondering whether
the subsequent patches are really necessary or not. I'll leave that
to you to decide.

In any case, with the squashed bit applied...

Reviewed-by: John Ferlan <jferlan at redhat.com>

John