1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/6] Couple of important fixes
  4. View patch

[libvirt] [PATCH 2/6] qemuxml2argvtest: Don't spawn session daemon

Michal Privoznik posted 6 patches 7 years, 7 months ago
[libvirt] [PATCH 2/6] qemuxml2argvtest: Don't spawn session daemon
Posted by Michal Privoznik 7 years, 7 months ago 
After f14c37ce4c2ccd111 the cleanup path for
qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine()
tries to connect to nwfilter driver in order to tear down any
NWFilter that was brought up during cmd line construction. Since
we also have negative test cases where errors during cmd line
build are expected the cleanup paths are executed and NWFilter
removal is attempted.

Fortunately, there is another bug that by pure luck prevented us
from actually spawning the daemon and thus modifying actual user
data. See next commit for explanation.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/driver.h             | 2 +-
 tests/qemuxml2argvmock.c | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/driver.h b/src/driver.h
index 0b1f7a2269..0a0d8facee 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -113,7 +113,7 @@ int virDriverLoadModule(const char *name,
 
 virConnectPtr virGetConnectInterface(void);
 virConnectPtr virGetConnectNetwork(void);
-virConnectPtr virGetConnectNWFilter(void);
+virConnectPtr virGetConnectNWFilter(void) ATTRIBUTE_NOINLINE;
 virConnectPtr virGetConnectNodeDev(void);
 virConnectPtr virGetConnectSecret(void);
 virConnectPtr virGetConnectStorage(void);
diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index 4df92cf396..13ccfb855d 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -233,3 +233,10 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev ATTRIBUTE_UNUSED)
         abort();
     return 1729;
 }
+
+
+virConnectPtr
+virGetConnectNWFilter(void)
+{
+    return NULL;
+}
-- 
2.16.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] qemuxml2argvtest: Don't spawn session daemon
Posted by Daniel P. Berrangé 7 years, 7 months ago 
On Mon, Jul 09, 2018 at 11:56:10AM +0200, Michal Privoznik wrote:
> After f14c37ce4c2ccd111 the cleanup path for
> qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine()
> tries to connect to nwfilter driver in order to tear down any
> NWFilter that was brought up during cmd line construction. Since
> we also have negative test cases where errors during cmd line
> build are expected the cleanup paths are executed and NWFilter
> removal is attempted.
> 
> Fortunately, there is another bug that by pure luck prevented us
> from actually spawning the daemon and thus modifying actual user
> data. See next commit for explanation.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/driver.h             | 2 +-
>  tests/qemuxml2argvmock.c | 7 +++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/driver.h b/src/driver.h
> index 0b1f7a2269..0a0d8facee 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -113,7 +113,7 @@ int virDriverLoadModule(const char *name,
>  
>  virConnectPtr virGetConnectInterface(void);
>  virConnectPtr virGetConnectNetwork(void);
> -virConnectPtr virGetConnectNWFilter(void);
> +virConnectPtr virGetConnectNWFilter(void) ATTRIBUTE_NOINLINE;
>  virConnectPtr virGetConnectNodeDev(void);
>  virConnectPtr virGetConnectSecret(void);
>  virConnectPtr virGetConnectStorage(void);
> diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
> index 4df92cf396..13ccfb855d 100644
> --- a/tests/qemuxml2argvmock.c
> +++ b/tests/qemuxml2argvmock.c
> @@ -233,3 +233,10 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev ATTRIBUTE_UNUSED)
>          abort();
>      return 1729;
>  }
> +
> +
> +virConnectPtr
> +virGetConnectNWFilter(void)
> +{
> +    return NULL;
> +}

In qemuxml2argvtest.c we actally set a fake shared connection, but I only
set it for two of the drivers. We should just register it for all the
drivers. eg expand these lines:

    virSetConnectSecret(conn);
    virSetConnectStorage(conn);

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] qemuxml2argvtest: Don't spawn session daemon
Posted by Michal Privoznik 7 years, 7 months ago 
On 07/09/2018 12:44 PM, Daniel P. Berrangé wrote:
> On Mon, Jul 09, 2018 at 11:56:10AM +0200, Michal Privoznik wrote:
>> After f14c37ce4c2ccd111 the cleanup path for
>> qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine()
>> tries to connect to nwfilter driver in order to tear down any
>> NWFilter that was brought up during cmd line construction. Since
>> we also have negative test cases where errors during cmd line
>> build are expected the cleanup paths are executed and NWFilter
>> removal is attempted.
>>
>> Fortunately, there is another bug that by pure luck prevented us
>> from actually spawning the daemon and thus modifying actual user
>> data. See next commit for explanation.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/driver.h             | 2 +-
>>  tests/qemuxml2argvmock.c | 7 +++++++
>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/driver.h b/src/driver.h
>> index 0b1f7a2269..0a0d8facee 100644
>> --- a/src/driver.h
>> +++ b/src/driver.h
>> @@ -113,7 +113,7 @@ int virDriverLoadModule(const char *name,
>>  
>>  virConnectPtr virGetConnectInterface(void);
>>  virConnectPtr virGetConnectNetwork(void);
>> -virConnectPtr virGetConnectNWFilter(void);
>> +virConnectPtr virGetConnectNWFilter(void) ATTRIBUTE_NOINLINE;
>>  virConnectPtr virGetConnectNodeDev(void);
>>  virConnectPtr virGetConnectSecret(void);
>>  virConnectPtr virGetConnectStorage(void);
>> diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
>> index 4df92cf396..13ccfb855d 100644
>> --- a/tests/qemuxml2argvmock.c
>> +++ b/tests/qemuxml2argvmock.c
>> @@ -233,3 +233,10 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev ATTRIBUTE_UNUSED)
>>          abort();
>>      return 1729;
>>  }
>> +
>> +
>> +virConnectPtr
>> +virGetConnectNWFilter(void)
>> +{
>> +    return NULL;
>> +}
> 
> In qemuxml2argvtest.c we actally set a fake shared connection, but I only
> set it for two of the drivers. We should just register it for all the
> drivers. eg expand these lines:
> 
>     virSetConnectSecret(conn);
>     virSetConnectStorage(conn);

Oh, that means I have to provide some basic implementation. I can't just
set conn->nwFilterDriver = NULL; because then virGetConnectNWFilte()
would still try to connect.

Okay, I think I can do it.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] qemuxml2argvtest: Don't spawn session daemon
Posted by Daniel P. Berrangé 7 years, 7 months ago 
On Mon, Jul 09, 2018 at 02:00:06PM +0200, Michal Privoznik wrote:
> On 07/09/2018 12:44 PM, Daniel P. Berrangé wrote:
> > On Mon, Jul 09, 2018 at 11:56:10AM +0200, Michal Privoznik wrote:
> >> After f14c37ce4c2ccd111 the cleanup path for
> >> qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine()
> >> tries to connect to nwfilter driver in order to tear down any
> >> NWFilter that was brought up during cmd line construction. Since
> >> we also have negative test cases where errors during cmd line
> >> build are expected the cleanup paths are executed and NWFilter
> >> removal is attempted.
> >>
> >> Fortunately, there is another bug that by pure luck prevented us
> >> from actually spawning the daemon and thus modifying actual user
> >> data. See next commit for explanation.
> >>
> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> >> ---
> >>  src/driver.h             | 2 +-
> >>  tests/qemuxml2argvmock.c | 7 +++++++
> >>  2 files changed, 8 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/driver.h b/src/driver.h
> >> index 0b1f7a2269..0a0d8facee 100644
> >> --- a/src/driver.h
> >> +++ b/src/driver.h
> >> @@ -113,7 +113,7 @@ int virDriverLoadModule(const char *name,
> >>  
> >>  virConnectPtr virGetConnectInterface(void);
> >>  virConnectPtr virGetConnectNetwork(void);
> >> -virConnectPtr virGetConnectNWFilter(void);
> >> +virConnectPtr virGetConnectNWFilter(void) ATTRIBUTE_NOINLINE;
> >>  virConnectPtr virGetConnectNodeDev(void);
> >>  virConnectPtr virGetConnectSecret(void);
> >>  virConnectPtr virGetConnectStorage(void);
> >> diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
> >> index 4df92cf396..13ccfb855d 100644
> >> --- a/tests/qemuxml2argvmock.c
> >> +++ b/tests/qemuxml2argvmock.c
> >> @@ -233,3 +233,10 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev ATTRIBUTE_UNUSED)
> >>          abort();
> >>      return 1729;
> >>  }
> >> +
> >> +
> >> +virConnectPtr
> >> +virGetConnectNWFilter(void)
> >> +{
> >> +    return NULL;
> >> +}
> > 
> > In qemuxml2argvtest.c we actally set a fake shared connection, but I only
> > set it for two of the drivers. We should just register it for all the
> > drivers. eg expand these lines:
> > 
> >     virSetConnectSecret(conn);
> >     virSetConnectStorage(conn);
> 
> Oh, that means I have to provide some basic implementation. I can't just
> set conn->nwFilterDriver = NULL; because then virGetConnectNWFilte()
> would still try to connect.

You shouldn't have to provide any impl - virGetConnectNWFilter() merely
cares about the virConnectPtr being non-NULL - it doesn't check if the
conn->nwfilterDriver pointer is set.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] qemuxml2argvtest: Don't spawn session daemon
Posted by Michal Privoznik 7 years, 7 months ago 
On 07/09/2018 02:05 PM, Daniel P. Berrangé wrote:
> On Mon, Jul 09, 2018 at 02:00:06PM +0200, Michal Privoznik wrote:
>> On 07/09/2018 12:44 PM, Daniel P. Berrangé wrote:
>>> On Mon, Jul 09, 2018 at 11:56:10AM +0200, Michal Privoznik wrote:


>>> In qemuxml2argvtest.c we actally set a fake shared connection, but I only
>>> set it for two of the drivers. We should just register it for all the
>>> drivers. eg expand these lines:
>>>
>>>     virSetConnectSecret(conn);
>>>     virSetConnectStorage(conn);
>>
>> Oh, that means I have to provide some basic implementation. I can't just
>> set conn->nwFilterDriver = NULL; because then virGetConnectNWFilte()
>> would still try to connect.
> 
> You shouldn't have to provide any impl - virGetConnectNWFilter() merely
> cares about the virConnectPtr being non-NULL - it doesn't check if the
> conn->nwfilterDriver pointer is set.
> 

Well, in that case there are couple of errors reported (not sure why
they are being printed into stderr rather than respecting log settings?)


libvirt.git/tests $ VIR_TEST_RANGE=460  ./qemuxml2argvtest
TEST: qemuxml2argvtest
      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
.                                       1602 OK


Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.