Re: [libvirt] [PATCH v1 2/2] network: Introduce start and shutdown hooks

On 02/03/2014 01:40 PM, Daniel P. Berrange wrote: > On Mon, Feb 03, 2014 at 12:36:32PM +0100, Michal Privoznik wrote: >> On 31.01.2014 17:43, Michal Privoznik wrote: >>> There might be some use cases, where user wants to prepare the host or >>> its environment prior to starting a network and do some cleanup after >>> the network has been shut down. Consider all the functionality that >>> libvirt doesn't currently have as an example what a hook script can >>> possibly do. >>> >>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >>> --- >>> docs/hooks.html.in | 43 +++++++++++++++++++++++++++++-------------- >>> src/network/bridge_driver.c | 29 +++++++++++++++++++++++++++++ >>> src/util/virhook.c | 10 +++++++++- >>> src/util/virhook.h | 8 ++++++++ >>> 4 files changed, 75 insertions(+), 15 deletions(-) >>> >>> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c >>> index 53c2274..2bca5bc 100644 >>> --- a/src/network/bridge_driver.c >>> +++ b/src/network/bridge_driver.c >>> @@ -71,6 +71,7 @@ >>> #include "virstring.h" >>> #include "viraccessapicheck.h" >>> #include "network_event.h" >>> +#include "virhook.h" >>> >>> #define VIR_FROM_THIS VIR_FROM_NETWORK >>> >>> @@ -2011,6 +2012,23 @@ networkStartNetwork(virNetworkDriverStatePtr driver, >>> if (virNetworkObjSetDefTransient(network, true) < 0) >>> goto cleanup; >>> >>> + /* Run an early hook to set-up missing devices */ >>> + if (virHookPresent(VIR_HOOK_DRIVER_NETWORK)) { >>> + char *xml = virNetworkDefFormat(network->def, 0); >>> + int hookret; >>> + >>> + hookret = virHookCall(VIR_HOOK_DRIVER_NETWORK, network->def->name, >>> + VIR_HOOK_NETWORK_OP_START, VIR_HOOK_SUBOP_BEGIN, >>> + NULL, xml, NULL); >>> + VIR_FREE(xml); >>> + >>> + /* >>> + * If the script raised an error abort the launch >>> + */ >>> + if (hookret < 0) >>> + goto cleanup; >>> + } >>> + >>> switch (network->def->forward.type) { >> I've just realized, that if the hook is going to be used to >> insert/delete some iptables rules or some tc work, maybe it's >> desired to have yet another hook that is executed *after* >> networkStartNetworkVirtual or networkStartNetworkExternal. Moreover, >> do we want to taint such networks that use hook scripts (bearing in >> mind that we don't do nothing like that for domains)? Any thoughts? Yes, this is very important - there is a big difference between adding an iptables rule before libvirt starts a network and after it starts the network, and either may be a valid choice depending on the situation. Additionally, while we're adding hooks, should there also be hooks pre/post adding a connection to a network and pre/post removing a connection from a network? (and in that case, what exactly should stdin receive? Perhaps the network XML + the domain XML, or maybe the network XML + an abbreviated domain XML that contains the domain name/uuid, and the particular <interface> that is being added/removed?)