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 <mpriv...@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 :|
