On Fri, Sep 13, 2019 at 12:10:34PM -0400, Laine Stump wrote:
On 9/13/19 10:59 AM, Daniel P. Berrangé wrote:
> Since the introduction of the virNetworkPort object, the network driver
> has a persistent record of ports that have been created against the
> networks. Thus the hypervisor drivers no longer communicate to the
> network driver during libvirtd restart.
>
> This change, however, meant that the connection usage counts were
> no longer re-initialized during a libvirtd restart. To deal with this we
> must iterate over all virNetworkPortDefPtr objects we have and invoke
> the notify callback to record the connection usage count.
>
> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
Reviewed-by: Laine Stump <laine(a)laine.org>
(also tested for direct/bridge, direct/passthrough, hostdev, and normal
tap-based virtual networks)
There is one issue with this though - it only re-adds connections that were
in the port list, while previously (before introduction of
virNetworkPortDef) we had iterated through all interfaces of all active
domains when libvirtd started - this would catch those interfaces that had
been "lost" by the network driver when a network with active domains was
destroyed and then restarted. Now that we're only iterating through the list
of what the network driver knows about, we're not restoring those on
libvirtd restart. Of course what we *really* want to have happen is for
those connections to be restored when the *network* is restarted, not
require a libvirtd restart (that wasn't done in the past because there
wasn't any avenue for the network driver to get a list of domains/interfaces
that *should* be connected to a particular network).
The restart problem is with this code:
void
virDomainNetNotifyActualDevice(virConnectPtr conn,
virDomainDefPtr dom,
virDomainNetDefPtr iface)
{
if (!virUUIDIsValid(iface->data.network.portid)) {
if (virDomainNetCreatePort(conn, dom, iface,
VIR_NETWORK_PORT_CREATE_RECLAIM) < 0)
return;
}
....
}
we're assuming the portid exists if we have it recorded. This is not
valid if the network has been destroyed and recreated.
We could easily make this check if the port needs re-creating
which will fix restart handling.
I'm in two minds about the idea of preserving ports across restart
of the network. If anything I wish we simply rejected restarts
with OPERATION_INVALID if anything is connected still, but that
might be too annoying for people who've come to rely on this
hack.
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 :|