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

[Michal Privoznik]

On 03.02.2014 15:21, Laine Stump wrote:
> 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 at redhat.com>
>>>> ---
>>>>   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?)
>

What a clever idea! If users are to use hooks for their own QoS 
implementation they certainly need this.

Then, I'm for network + whole domain XML - it doesn't hurt to have more 
information, it can hurt if we now introduce abbreviated XML => sooner 
or later somebody will come and say the domain XML is shortened too much.

Michal