Re: [PATCH] qemuDomainDeviceNetDefPostParse: Switch order of conditions

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(a)redhat.com&gt; > --- > > 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.