[PATCH] qemuDomainDeviceNetDefPostParse: Switch order of conditions

Daniel P. Berrangé berrange at redhat.com
Thu Jun 25 13:59:18 UTC 2020


On Thu, Jun 25, 2020 at 03:54:18PM +0200, Michal Privoznik wrote:
> On 6/25/20 10:38 AM, Daniel P. Berrangé wrote:
> > 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.
> 
> What do you mean by "stub driver"? I can see conn->networkDriver = NULL. I
> mean, it works, but it's not a stub driver :-) Basically any virNetwork*
> call fails and we merely just let the code deal with it.

Yeah, by stub i mean something that prevents any network APIs from
working.

xs

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