[PATCH] qemuDomainDeviceNetDefPostParse: Switch order of conditions

Daniel P. Berrangé berrange at redhat.com
Thu Jun 25 08:38:24 UTC 2020


On Thu, Jun 25, 2020 at 09:48:56AM +0200, Michal Privoznik wrote:
> A few commits back (in v6.4.0-131-gbdb8f2e418) the post parse
> function for domain interface was changed so that it doesn't fill
> in model for hostdev types of interfaces (including network type
> interfaces which would end up hostdevs).
> 
> While the idea is sound, the execution can be a bit better:
> virDomainNetResolveActualType() which is used to determine
> runtime type of given interface is heavy gun - it connects to
> network driver, fetches network XML, parses it. This all is
> followed by check whether the interface doesn't already have
> model set (from domain XML).
> 
> If we switch the order of these two checks then the short circuit
> evaluation will ensure the expensive check is done only if really
> needed.
> 
> This commit in fact fixes qemuxml2xmltest which due to lacking
> fake network driver tries to connect to network:///session and
> start the virtnetworkd. Fortunately, because of
> v6.3.0-25-gf28fbb05d3 it fails to do so and
> virDomainNetResolveActualType() returns -1. The only reason we
> don't see the test failing is because our input XMLs have model
> and thus we are saved by the latter (now former) check.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> 
> While this patch is technically correct (the best way to be correct), it
> can also be viewed as papering over the real issue. Question is, how to
> address that. Nor xml2xml test nor xml2argv test are creating fake
> network driver. Is mocking virDomainNetResolveActualType() the way to go
> then? Or should we create the fake network driver with networks and
> everything?

The qemuxml2argtest calls virSetConnectNetwork() to provide a stub
network driver, likewise for other secondary drivers. This prevents
any risk of spawning daemons. We should probably do that for the
other tests too.

> 
>  src/qemu/qemu_domain.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 2efbf1a4b3..c5b8d91f9a 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5230,8 +5230,8 @@ qemuDomainDeviceNetDefPostParse(virDomainNetDefPtr net,
>                                  virQEMUCapsPtr qemuCaps)
>  {
>      if (net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
> -        virDomainNetResolveActualType(net) != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
> -        !virDomainNetGetModelString(net))
> +        !virDomainNetGetModelString(net) &&
> +        virDomainNetResolveActualType(net) != VIR_DOMAIN_NET_TYPE_HOSTDEV)
>          net->model = qemuDomainDefaultNetModel(def, qemuCaps);
>  
>      return 0;
> -- 
> 2.26.2
> 

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 :|




More information about the libvir-list mailing list