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