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

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().