[libvirt] [PATCH v1 0/2] Network hooks
With the latest noise I made on the list with QoS I'm coming forward with this idea. Network hooks can fill some holes in our functionality. For instance if somebody is willing to set a QoS that is not covered by libvirt, or just want to insert some additional iptables rules, etc. You name it. Michal Privoznik (2): networkStartNetwork: Be more verbose network: Introduce start and shutdown hooks docs/hooks.html.in | 43 ++++++++++++++++++++---------- src/network/bridge_driver.c | 64 +++++++++++++++++++++++++++++++++------------ src/util/virhook.c | 10 ++++++- src/util/virhook.h | 8 ++++++ 4 files changed, 94 insertions(+), 31 deletions(-) -- 1.8.5.2
The lack of debug printings might be frustrating in the future. Moreover, this function doesn't follow the usual pattern we have in the rest of the code: int ret = -1; /* do some work */ ret = 0; cleanup: /* some cleanup work */ return ret; Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/network/bridge_driver.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0b43a67..53c2274 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1995,23 +1995,29 @@ static int networkStartNetwork(virNetworkDriverStatePtr driver, virNetworkObjPtr network) { - int ret = 0; + int ret = -1; + + VIR_DEBUG("driver=%p, network=%p", driver, network); if (virNetworkObjIsActive(network)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("network is already active")); - return -1; + goto cleanup; } + VIR_DEBUG("Beginning network startup process"); + + VIR_DEBUG("Setting current network def as transient"); if (virNetworkObjSetDefTransient(network, true) < 0) - return -1; + goto cleanup; switch (network->def->forward.type) { case VIR_NETWORK_FORWARD_NONE: case VIR_NETWORK_FORWARD_NAT: case VIR_NETWORK_FORWARD_ROUTE: - ret = networkStartNetworkVirtual(driver, network); + if (networkStartNetworkVirtual(driver, network) < 0) + goto cleanup; break; case VIR_NETWORK_FORWARD_BRIDGE: @@ -2019,28 +2025,25 @@ networkStartNetwork(virNetworkDriverStatePtr driver, case VIR_NETWORK_FORWARD_VEPA: case VIR_NETWORK_FORWARD_PASSTHROUGH: case VIR_NETWORK_FORWARD_HOSTDEV: - ret = networkStartNetworkExternal(driver, network); + if (networkStartNetworkExternal(driver, network) < 0) + goto cleanup; break; } - if (ret < 0) { - virNetworkObjUnsetDefTransient(network); - return ret; - } - /* Persist the live configuration now that anything autogenerated * is setup. */ - if ((ret = virNetworkSaveStatus(driverState->stateDir, - network)) < 0) { - goto error; - } + VIR_DEBUG("Writing network status to disk"); + if (virNetworkSaveStatus(driverState->stateDir, network) < 0) + goto cleanup; - VIR_INFO("Starting up network '%s'", network->def->name); network->active = 1; + VIR_INFO("Network '%s' started up", network->def->name); + ret = 0; -error: +cleanup: if (ret < 0) { + virNetworkObjUnsetDefTransient(network); virErrorPtr save_err = virSaveLastError(); int save_errno = errno; networkShutdownNetwork(driver, network); -- 1.8.5.2
On 01/31/2014 06:43 PM, Michal Privoznik wrote:
The lack of debug printings might be frustrating in the future. Moreover, this function doesn't follow the usual pattern we have in the rest of the code:
int ret = -1; /* do some work */ ret = 0; cleanup: /* some cleanup work */ return ret;
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/network/bridge_driver.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0b43a67..53c2274 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1995,23 +1995,29 @@ static int networkStartNetwork(virNetworkDriverStatePtr driver, virNetworkObjPtr network) { - int ret = 0; + int ret = -1; + + VIR_DEBUG("driver=%p, network=%p", driver, network);
if (virNetworkObjIsActive(network)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("network is already active")); - return -1; + goto cleanup;
If the network is already active, and has a pending modification (e.g. made with "virsh net-edit"), the modified network definition will be stored in network->newDef. But if someone tries to start the network and it's already active, you're now jumping to cleanup, and cleanup now calls virNetworkObjUnsetDefTransient(), which will destroy network-def and *prematurely* move network->newDef into its place. From this point on, network->def will contain incorrect data, which could have bad consequences (for example, the wrong set of iptables rules may be deleted when the network is next destroyed, leaving undesired rules in effect). So, while I much prefer the new coding style in the function, you'll need to privately keep track of whether or not virNetworkObjSetDefTransient() has been called by this function, and only "UnsetDef" it if it was "SetDef"ed. Other than that I think it's okay.
}
+ VIR_DEBUG("Beginning network startup process"); + + VIR_DEBUG("Setting current network def as transient"); if (virNetworkObjSetDefTransient(network, true) < 0) - return -1; + goto cleanup;
switch (network->def->forward.type) {
case VIR_NETWORK_FORWARD_NONE: case VIR_NETWORK_FORWARD_NAT: case VIR_NETWORK_FORWARD_ROUTE: - ret = networkStartNetworkVirtual(driver, network); + if (networkStartNetworkVirtual(driver, network) < 0) + goto cleanup; break;
case VIR_NETWORK_FORWARD_BRIDGE: @@ -2019,28 +2025,25 @@ networkStartNetwork(virNetworkDriverStatePtr driver, case VIR_NETWORK_FORWARD_VEPA: case VIR_NETWORK_FORWARD_PASSTHROUGH: case VIR_NETWORK_FORWARD_HOSTDEV: - ret = networkStartNetworkExternal(driver, network); + if (networkStartNetworkExternal(driver, network) < 0) + goto cleanup; break; }
- if (ret < 0) { - virNetworkObjUnsetDefTransient(network); - return ret; - } - /* Persist the live configuration now that anything autogenerated * is setup. */ - if ((ret = virNetworkSaveStatus(driverState->stateDir, - network)) < 0) { - goto error; - } + VIR_DEBUG("Writing network status to disk"); + if (virNetworkSaveStatus(driverState->stateDir, network) < 0) + goto cleanup;
- VIR_INFO("Starting up network '%s'", network->def->name); network->active = 1; + VIR_INFO("Network '%s' started up", network->def->name); + ret = 0;
-error: +cleanup: if (ret < 0) { + virNetworkObjUnsetDefTransient(network); virErrorPtr save_err = virSaveLastError(); int save_errno = errno; networkShutdownNetwork(driver, network);
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@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/docs/hooks.html.in b/docs/hooks.html.in index f0f692b..b950031 100644 --- a/docs/hooks.html.in +++ b/docs/hooks.html.in @@ -16,6 +16,7 @@ configuration<br/><br/></li> <li>A QEMU guest is started or stopped<br/><br/></li> <li>An LXC guest is started or stopped<br/><br/></li> + <li>A network is started or stopped<br/><br/></li> </ul> <h2><a name="location">Script location</a></h2> @@ -44,6 +45,8 @@ Executed when a QEMU guest is started, stopped, or migrated<br/><br/></li> <li><code>/etc/libvirt/hooks/lxc</code><br /><br/> Executed when an LXC guest is started or stopped</li> + <li><code>/etc/libvirt/hooks/network</code><br /><br/> + Executed when a network is started or stopped</li> </ul> <br/> @@ -62,10 +65,11 @@ <h2><a name="arguments">Script arguments</a></h2> <p>The hook scripts are called with specific command line arguments, depending upon the script, and the operation being performed.</p> - <p>The guest hook scripts, qemu and lxc, are also given the <b>full</b> - XML description for the domain on their stdin. This includes items - such the UUID of the domain and its storage information, and is - intended to provide all the libvirt information the script needs.</p> + <p>The guest hook scripts, qemu and lxc, or network hook script are + also given the <b>full</b> XML description for the domain on their + stdin. This includes items such as the UUID of the domain or network, + domain storage information, etc. and is intended to provide all the + libvirt information the script needs.</p> <p>The command line arguments take this approach:</p> <ol> @@ -181,23 +185,34 @@ <pre>/etc/libvirt/hooks/lxc guest_name reconnect begin -</pre> </li> </ul> + + <h5><a name="network">/etc/libvirt/hooks/network</a></h5> + <ul> + <li><span class="since">Since 1.2.2</span>, when a network is started, + this script is called as:<br/> + <pre>/etc/libvirt/hooks/network network_name start - start</pre></li> + <li>When a network is shut down, this script is called as:<br/> + <pre>/etc/libvirt/hooks/network network_name shutdown - shutdown</pre></li> + </ul> + <br/> <h2><a name="execution">Script execution</a></h2> <ul> - <li>The "start" operation for the guest hook scripts, qemu and lxc, + <li>The "start" operation for the guest and network hook scripts, executes <b>prior</b> to the guest being created. This allows the - guest start operation to be aborted if the script returns indicating - failure.<br/><br/></li> - <li>The "shutdown" operation for the guest hook scripts, qemu and lxc, - executes <b>after</b> the guest has stopped. If the hook script - indicates failure in its return, the shut down of the guest cannot - be aborted because it has already been performed.<br/><br/></li> + guest or network start operation to be aborted if the script returns + indicating failure.<br/><br/></li> + <li>The "shutdown" operation for the guest and network hook scripts, + executes <b>after</b> the guest or network has stopped. If the hook + script indicates failure in its return, the shut down of the guest + or network cannot be aborted because it has already been performed. + <br/><br/></li> <li>Hook scripts execute in a synchronous fashion. Libvirt waits for them to return before continuing the given operation.<br/><br/> - This is most noticeable with the guest start operation, as a lengthy - operation in the hook script can mean an extended wait for the guest - to be available to end users.<br/><br/></li> + This is most noticeable with the guest or network start operation, + as a lengthy operation in the hook script can mean an extended wait + for the guest or network to be available to end users.<br/><br/></li> <li>For a hook script to be utilised, it must have its execute bit set (ie. chmod o+rx <i>qemu</i>), and must be present when the libvirt daemon is started.<br/><br/></li> 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) { case VIR_NETWORK_FORWARD_NONE: @@ -2090,6 +2108,17 @@ static int networkShutdownNetwork(virNetworkDriverStatePtr driver, break; } + /* now that we know it's stopped call the hook if present */ + if (virHookPresent(VIR_HOOK_DRIVER_NETWORK)) { + char *xml = virNetworkDefFormat(network->def, 0); + + /* we can't stop the operation even if the script raised an error */ + virHookCall(VIR_HOOK_DRIVER_NETWORK, network->def->name, + VIR_HOOK_NETWORK_OP_STOPPED, VIR_HOOK_SUBOP_END, + NULL, xml, NULL); + VIR_FREE(xml); + } + network->active = 0; virNetworkObjUnsetDefTransient(network); return ret; diff --git a/src/util/virhook.c b/src/util/virhook.c index 159efdb..5206810 100644 --- a/src/util/virhook.c +++ b/src/util/virhook.c @@ -48,12 +48,14 @@ VIR_ENUM_DECL(virHookDaemonOp) VIR_ENUM_DECL(virHookSubop) VIR_ENUM_DECL(virHookQemuOp) VIR_ENUM_DECL(virHookLxcOp) +VIR_ENUM_DECL(virHookNetworkOp) VIR_ENUM_IMPL(virHookDriver, VIR_HOOK_DRIVER_LAST, "daemon", "qemu", - "lxc") + "lxc", + "network") VIR_ENUM_IMPL(virHookDaemonOp, VIR_HOOK_DAEMON_OP_LAST, "start", @@ -83,6 +85,10 @@ VIR_ENUM_IMPL(virHookLxcOp, VIR_HOOK_LXC_OP_LAST, "started", "reconnect") +VIR_ENUM_IMPL(virHookNetworkOp, VIR_HOOK_NETWORK_OP_LAST, + "start", + "stopped") + static int virHooksFound = -1; /** @@ -246,6 +252,8 @@ virHookCall(int driver, case VIR_HOOK_DRIVER_LXC: opstr = virHookLxcOpTypeToString(op); break; + case VIR_HOOK_DRIVER_NETWORK: + opstr = virHookNetworkOpTypeToString(op); } if (opstr == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virhook.h b/src/util/virhook.h index 96bf4cf..05ed3b5 100644 --- a/src/util/virhook.h +++ b/src/util/virhook.h @@ -30,6 +30,7 @@ enum virHookDriverType { VIR_HOOK_DRIVER_DAEMON = 0, /* Daemon related events */ VIR_HOOK_DRIVER_QEMU, /* QEmu domains related events */ VIR_HOOK_DRIVER_LXC, /* LXC domains related events */ + VIR_HOOK_DRIVER_NETWORK, /* network related events */ VIR_HOOK_DRIVER_LAST, }; @@ -74,6 +75,13 @@ enum virHookLxcOpType { VIR_HOOK_LXC_OP_LAST, }; +enum virHookNetworkOpType { + VIR_HOOK_NETWORK_OP_START, /* network is about to start */ + VIR_HOOK_NETWORK_OP_STOPPED, /* network has stopped */ + + VIR_HOOK_NETWORK_OP_LAST, +}; + int virHookInitialize(void); int virHookPresent(int driver); -- 1.8.5.2
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@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? Michal
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@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?
We use 'tainting' as a way to identify anything which could alter the operation/behaviour of the VM in a way that isn't obvious from the XML config. As such I'd say that use of hooks should cause tainting, and likewise we should have a tainting concept for networks too. 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 :|
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@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?)
We use 'tainting' as a way to identify anything which could alter the operation/behaviour of the VM in a way that isn't obvious from the XML config. As such I'd say that use of hooks should cause tainting, and likewise we should have a tainting concept for networks too.
I also agree with the concept of tainting a network. In addition to tainting the network, probably any domain that connects to a tainted network should itself be tainted.
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@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
participants (3)
-
Daniel P. Berrange -
Laine Stump -
Michal Privoznik