[libvirt] [PATCH 0/3] Prevent removing a in-used static bridge and destroying a in-used virtual network

[Daniel P. Berrange]

On Wed, Feb 04, 2015 at 12:39:56PM -0500, Laine Stump wrote:
> On 02/04/2015 09:58 AM, Daniel P. Berrange wrote:
> > On Wed, Feb 04, 2015 at 02:21:18AM -0500, Laine Stump wrote:
> >> On 02/03/2015 11:47 AM, Michal Privoznik wrote:
> >>> On 02.02.2015 15:08, Lin Ma wrote:
> >>>> * Get the live state info of a virtual network through netcf in networkGetXMLDesc.
> >>>> * Add --system flag for net-dumpxml to show the live state info.
> >>>> * Check the live state info in net-destroy.
> >>>> * Add --force flag for net-destroy to forcibly destroy the virtual network.
> >>>> * Check the transient interfaces info in iface-unbridge.
> >>>>
> >>>> ---
> >>>> Lin Ma (3):
> >>>>   bridge_driver: Return the live state info of a given virtual network
> >>>>   virsh: prevent destroying a in-used network for net-destroy
> >>>>   virsh: prevent removing a in-used bridge for iface-unbridge
> >>>>
> >>>>  include/libvirt/libvirt-network.h    |   1 +
> >>>>  src/Makefile.am                      |   3 +
> >>>>  src/network/bridge_driver.c          | 141 ++++++++++++++++++++++++++++++++++-
> >>>>  src/network/bridge_driver_platform.h |   7 ++
> >>>>  tests/Makefile.am                    |   4 +
> >>>>  tools/virsh-interface.c              |  25 ++++++-
> >>>>  tools/virsh-network.c                |  62 ++++++++++++++-
> >>>>  tools/virsh.pod                      |   8 +-
> >>>>  8 files changed, 241 insertions(+), 10 deletions(-)
> >>>>
> >>> So I've spent some time thinking about this. I don't really like the
> >>> idea of producing completely different XML (it doesn't even have
> >>> <network/> as its root element!). But I see what you're trying to
> >>> achieve. How about putting the bridged interfaces into the network
> >>> definition (on request signalized by a flag, of course).
> >> I think that if we're going to add the list of connected guest devices
> >> to a network's status, that it should just always be there - adding a
> >> new flag for every little bit of different status sets a bad precedent
> >> and sets us up to have an infinitely increasing number of flags.
> >>
> >> Like I mentioned in my response to one of the patches, I think this
> >> information is better gathered and maintained by the bridge driver as
> >> guests connect to / disconnect from a network; this avoids the necessity
> >> of dragging netcf into the picture and allows much better information -
> >> the name of the domain using each interface can be included.
> > You know if this checking for guests were done in the virNetworkDestroy
> > API implementation in src/network/bridge_driver.c, instead of in
> > virsh, then we would not need any of this code for extending the XML
> > nor parsing it in virsh. We could simply check the number of connections
> > which is something we have directly available internally. So 95% of this
> > patch series would go away
> 
> Except that the virNetworkDestroy() API was created before we realized
> it was a good idea to have a flags argument to every new API. So that
> would require creating a new virNetworkDestroyFlags() API (and the first
> flag to be defined would be VIR_NETWORK_DESTROY_GRACEFUL).
> 
> Note that a network's connections count *is* returned in the status XML,
> so virsh could check that and it would work even when connecting to
> older servers. I think I prefer your idea, though, because it is more
> consistent with what is done for the virDomain API (it won't work when
> talking to older servers though, and virsh will have to be written to
> fall back to virNetworkDestroy() when the server is too old to have
> virNetworkDestroyFlags().

Yep, adding a virNetworkDestroyFlags() is no big deal - we'll inevitably
need it at some point. There's probably an argument for identifying all
our remaining APIs which lack a flags parameter and fixing it once and
for all :-)

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|