[libvirt] [PATCH v3 0/2] Network hooks
Version three. The 1/3 from the previous round is already pushed (as it just reworks two functions - no interesting work really). The 1/2 from this patchset - I've moved the calling of the hook in networkStartNetwork() a bit forward, so now it's called before writing the network status file. This way it makes more sense. The 2/2 has been conditionally ACKed, but I'm sending it anyway just to make sure I got it right. Michal Privoznik (2): network: Introduce network hooks network: Taint networks that are using hook script docs/hooks.html.in | 70 +++++++++++---- src/conf/network_conf.c | 52 ++++++++++- src/conf/network_conf.h | 17 ++++ src/libvirt_private.syms | 3 + src/lxc/lxc_driver.c | 4 +- src/lxc/lxc_process.c | 6 +- src/network/bridge_driver.c | 212 +++++++++++++++++++++++++++++++++++++++++++- src/network/bridge_driver.h | 19 ++-- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_hotplug.c | 14 +-- src/qemu/qemu_process.c | 4 +- src/util/virhook.c | 13 ++- src/util/virhook.h | 11 +++ 13 files changed, 384 insertions(+), 43 deletions(-) -- 1.8.5.3
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 | 70 +++++++++++++---- src/lxc/lxc_driver.c | 4 +- src/lxc/lxc_process.c | 6 +- src/network/bridge_driver.c | 186 +++++++++++++++++++++++++++++++++++++++++++- src/network/bridge_driver.h | 19 +++-- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_hotplug.c | 14 ++-- src/qemu/qemu_process.c | 4 +- src/util/virhook.c | 13 +++- src/util/virhook.h | 11 +++ 10 files changed, 287 insertions(+), 42 deletions(-) diff --git a/docs/hooks.html.in b/docs/hooks.html.in index f0f692b..fd891bb 100644 --- a/docs/hooks.html.in +++ b/docs/hooks.html.in @@ -13,9 +13,15 @@ actions occur:</p> <ul> <li>The libvirt daemon starts, stops, or reloads its - 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> + configuration + (<span class="since">since 0.8.0</span>)<br/><br/></li> + <li>A QEMU guest is started or stopped + (<span class="since">since 0.8.0</span>)<br/><br/></li> + <li>An LXC guest is started or stopped + (<span class="since">since 0.8.0</span>)<br/><br/></li> + <li>A network is started or stopped or an interface is + un-/plugged from/to the network + (<span class="since">since 1.2.2</span>)<br/><br/></li> </ul> <h2><a name="location">Script location</a></h2> @@ -44,6 +50,9 @@ 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 or an + interface is un-/plugged from/to the network</li> </ul> <br/> @@ -66,6 +75,11 @@ 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>For all cases, stdin of the network hook script is provided with the + full XML description of the network status. In the case of an interface + being plugged/unplugged to the network, the network XML will be followed + with the full XML description of the domain containing the interface + that is being plugged/unplugged.</p> <p>The command line arguments take this approach:</p> <ol> @@ -181,25 +195,51 @@ <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>, before a network is started, + this script is called as:<br/> + <pre>/etc/libvirt/hooks/network network_name start begin -</pre></li> + <li>After the network is started, up ∧ running, the script is + called as:<br/> + <pre>/etc/libvirt/hooks/network network_name started begin -</pre></li> + <li>When a network is shut down, this script is called as:<br/> + <pre>/etc/libvirt/hooks/network network_name stopped end -</pre></li> + <li>Later, when network is started and there's an interface from a + domain to be plugged into the network (plugged may not be the correct + expression when it comes to bridgeless networks, perhaps allocated is + better one then), the hook script is called as:<br/> + <pre>/etc/libvirt/hooks/network network_name plugged begin -</pre> + Please note, that in this case, the script is passed both network and + domain XMLs on its stdin.</li> + <li>When the domain from previous case is shutting down, the interface + is unplugged. This leads to another script invocation:<br/> + <pre>/etc/libvirt/hooks/network network_name unplugged begin -</pre> + And again, as in previous case, both network and domain XMLs are passed + onto script's stdin.</li> + </ul> + <br/> <h2><a name="execution">Script execution</a></h2> <ul> - <li>The "start" operation for the guest hook scripts, qemu and lxc, - 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> + <li>The "start" operation for the guest and network hook scripts, + executes <b>prior</b> to the object (guest or network) being created. + This allows the object 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 object (guest or network) has stopped. If + the hook script indicates failure in its return, the shut down of the + object 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 + (e.g. chmod o+rx <i>qemu</i>), and must be present when the libvirt daemon is started.<br/><br/></li> <li>If a hook script is added to a host after the libvirt daemon is already running, it won't be used until the libvirt daemon diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 687046e..c48177c 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3772,7 +3772,7 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn, * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ - if (networkAllocateActualDevice(net) < 0) + if (networkAllocateActualDevice(vm->def, net) < 0) return -1; actualType = virDomainNetGetActualType(net); @@ -4427,7 +4427,7 @@ lxcDomainDetachDeviceNetLive(virDomainObjPtr vm, ret = 0; cleanup: if (!ret) { - networkReleaseActualDevice(detach); + networkReleaseActualDevice(vm->def, detach); virDomainNetRemove(vm->def, detachidx); virDomainNetDefFree(detach); } diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index ed729f6..8989245 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -198,7 +198,7 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver, iface->ifname)); ignore_value(virNetDevVethDelete(iface->ifname)); } - networkReleaseActualDevice(iface); + networkReleaseActualDevice(vm->def, iface); } virDomainConfVMNWFilterTeardown(vm); @@ -374,7 +374,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ - if (networkAllocateActualDevice(def->nets[i]) < 0) + if (networkAllocateActualDevice(def, def->nets[i]) < 0) goto cleanup; if (VIR_EXPAND_N(*veths, *nveths, 1) < 0) @@ -476,7 +476,7 @@ cleanup: ignore_value(virNetDevOpenvswitchRemovePort( virDomainNetGetActualBridgeName(iface), iface->ifname)); - networkReleaseActualDevice(iface); + networkReleaseActualDevice(def, iface); } } return ret; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0964350..90852d5 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 @@ -2008,6 +2009,26 @@ 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; + + if (!xml) + goto cleanup; + + 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: @@ -2027,6 +2048,26 @@ networkStartNetwork(virNetworkDriverStatePtr driver, break; } + /* finally we can call the 'started' hook script if any */ + if (virHookPresent(VIR_HOOK_DRIVER_NETWORK)) { + char *xml = virNetworkDefFormat(network->def, 0); + int hookret; + + if (!xml) + goto cleanup; + + hookret = virHookCall(VIR_HOOK_DRIVER_NETWORK, network->def->name, + VIR_HOOK_NETWORK_OP_STARTED, + VIR_HOOK_SUBOP_BEGIN, NULL, xml, NULL); + VIR_FREE(xml); + + /* + * If the script raised an error abort the launch + */ + if (hookret < 0) + goto cleanup; + } + /* Persist the live configuration now that anything autogenerated * is setup. */ @@ -2087,6 +2128,19 @@ 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); + + if (xml) { + /* 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; @@ -3239,6 +3293,7 @@ finish: } /* networkAllocateActualDevice: + * @dom: domain definition that @iface belongs to * @iface: the original NetDef from the domain * * Looks up the network reference by iface, allocates a physical @@ -3250,7 +3305,8 @@ finish: * Returns 0 on success, -1 on failure. */ int -networkAllocateActualDevice(virDomainNetDefPtr iface) +networkAllocateActualDevice(virDomainDefPtr dom, + virDomainNetDefPtr iface) { virNetworkDriverStatePtr driver = driverState; enum virDomainNetType actualType = iface->type; @@ -3583,6 +3639,48 @@ validate: } } + /* finally we can call the 'plugged' hook script if any */ + if (virHookPresent(VIR_HOOK_DRIVER_NETWORK)) { + /* the XML construction is a bit complicated here, + * as we want to pass both domain XML and network XML */ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *xml, *net_xml, *dom_xml; + int hookret; + + net_xml = virNetworkDefFormat(netdef, 0); + dom_xml = virDomainDefFormat(dom, 0); + + if (!net_xml || !dom_xml) { + VIR_FREE(net_xml); + VIR_FREE(dom_xml); + goto error; + } + + virBufferAdd(&buf, net_xml, -1); + virBufferAdd(&buf, dom_xml, -1); + + VIR_FREE(net_xml); + VIR_FREE(dom_xml); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + goto error; + } + + xml = virBufferContentAndReset(&buf); + + hookret = virHookCall(VIR_HOOK_DRIVER_NETWORK, network->def->name, + VIR_HOOK_NETWORK_OP_IFACE_PLUGGED, + VIR_HOOK_SUBOP_BEGIN, NULL, xml, NULL); + VIR_FREE(xml); + + /* + * If the script raised an error abort the allocation + */ + if (hookret < 0) + goto error; + } + if (dev) { /* we are now assured of success, so mark the allocation */ dev->connections++; @@ -3618,6 +3716,7 @@ error: } /* networkNotifyActualDevice: + * @dom: domain definition that @iface belongs to * @iface: the domain's NetDef with an "actual" device already filled in. * * Called to notify the network driver when libvirtd is restarted and @@ -3628,7 +3727,8 @@ error: * Returns 0 on success, -1 on failure. */ int -networkNotifyActualDevice(virDomainNetDefPtr iface) +networkNotifyActualDevice(virDomainDefPtr dom, + virDomainNetDefPtr iface) { virNetworkDriverStatePtr driver = driverState; enum virDomainNetType actualType = virDomainNetGetActualType(iface); @@ -3781,6 +3881,48 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) } success: + /* finally we can call the 'plugged' hook script if any */ + if (virHookPresent(VIR_HOOK_DRIVER_NETWORK)) { + /* the XML construction is a bit complicated here, + * as we want to pass both domain XML and network XML */ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *xml, *net_xml, *dom_xml; + int hookret; + + net_xml = virNetworkDefFormat(netdef, 0); + dom_xml = virDomainDefFormat(dom, 0); + + if (!net_xml || !dom_xml) { + VIR_FREE(net_xml); + VIR_FREE(dom_xml); + goto error; + } + + virBufferAdd(&buf, net_xml, -1); + virBufferAdd(&buf, dom_xml, -1); + + VIR_FREE(net_xml); + VIR_FREE(dom_xml); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + goto error; + } + + xml = virBufferContentAndReset(&buf); + + hookret = virHookCall(VIR_HOOK_DRIVER_NETWORK, network->def->name, + VIR_HOOK_NETWORK_OP_IFACE_PLUGGED, + VIR_HOOK_SUBOP_BEGIN, NULL, xml, NULL); + VIR_FREE(xml); + + /* + * If the script raised an error abort the allocation + */ + if (hookret < 0) + goto error; + } + netdef->connections++; VIR_DEBUG("Using network %s, %d connections", netdef->name, netdef->connections); @@ -3796,6 +3938,7 @@ error: /* networkReleaseActualDevice: + * @dom: domain definition that @iface belongs to * @iface: a domain's NetDef (interface definition) * * Given a domain <interface> element that previously had its <actual> @@ -3806,7 +3949,8 @@ error: * Returns 0 on success, -1 on failure. */ int -networkReleaseActualDevice(virDomainNetDefPtr iface) +networkReleaseActualDevice(virDomainDefPtr dom, + virDomainNetDefPtr iface) { virNetworkDriverStatePtr driver = driverState; enum virDomainNetType actualType = virDomainNetGetActualType(iface); @@ -3925,6 +4069,42 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) success: if (iface->data.network.actual) netdef->connections--; + + /* finally we can call the 'unplugged' hook script if any */ + if (virHookPresent(VIR_HOOK_DRIVER_NETWORK)) { + /* the XML construction is a bit complicated here, + * as we want to pass both domain XML and network XML */ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *xml, *net_xml, *dom_xml; + + net_xml = virNetworkDefFormat(netdef, 0); + dom_xml = virDomainDefFormat(dom, 0); + + if (!net_xml || !dom_xml) { + VIR_FREE(net_xml); + VIR_FREE(dom_xml); + goto error; + } + + virBufferAdd(&buf, net_xml, -1); + virBufferAdd(&buf, dom_xml, -1); + + VIR_FREE(net_xml); + VIR_FREE(dom_xml); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + goto error; + } + + xml = virBufferContentAndReset(&buf); + + virHookCall(VIR_HOOK_DRIVER_NETWORK, network->def->name, + VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED, + VIR_HOOK_SUBOP_BEGIN, NULL, xml, NULL); + VIR_FREE(xml); + } + VIR_DEBUG("Releasing network %s, %d connections", netdef->name, netdef->connections); ret = 0; diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h index 50258b5..0f80556 100644 --- a/src/network/bridge_driver.h +++ b/src/network/bridge_driver.h @@ -34,12 +34,15 @@ int networkRegister(void); # if WITH_NETWORK -int networkAllocateActualDevice(virDomainNetDefPtr iface) - ATTRIBUTE_NONNULL(1); -int networkNotifyActualDevice(virDomainNetDefPtr iface) - ATTRIBUTE_NONNULL(1); -int networkReleaseActualDevice(virDomainNetDefPtr iface) - ATTRIBUTE_NONNULL(1); +int networkAllocateActualDevice(virDomainDefPtr dom, + virDomainNetDefPtr iface) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int networkNotifyActualDevice(virDomainDefPtr dom, + virDomainNetDefPtr iface) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int networkReleaseActualDevice(virDomainDefPtr dom, + virDomainNetDefPtr iface) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int networkGetNetworkAddress(const char *netname, char **netaddr) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); @@ -51,9 +54,9 @@ int networkDnsmasqConfContents(virNetworkObjPtr network, dnsmasqCapsPtr caps); # else /* Define no-op replacements that don't drag in any link dependencies. */ -# define networkAllocateActualDevice(iface) 0 +# define networkAllocateActualDevice(dom, iface) 0 # define networkNotifyActualDevice(iface) (iface=iface, 0) -# define networkReleaseActualDevice(iface) (iface=iface, 0) +# define networkReleaseActualDevice(dom, iface) (dom=dom, iface=iface, 0) # define networkGetNetworkAddress(netname, netaddr) (-2) # define networkDnsmasqConfContents(network, pidfile, configstr, \ dctx, caps) 0 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7b2b91b..71f3ea4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -548,7 +548,7 @@ qemuNetworkPrepareDevices(virDomainDefPtr def) * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ - if (networkAllocateActualDevice(net) < 0) + if (networkAllocateActualDevice(def, net) < 0) goto cleanup; actualType = virDomainNetGetActualType(net); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7066be6..6703c92 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -837,7 +837,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ - if (networkAllocateActualDevice(net) < 0) + if (networkAllocateActualDevice(vm->def, net) < 0) goto cleanup; actualType = virDomainNetGetActualType(net); @@ -1082,7 +1082,7 @@ cleanup: virDomainNetRemoveHostdev(vm->def, net); - networkReleaseActualDevice(net); + networkReleaseActualDevice(vm->def, net); } VIR_FREE(nicstr); @@ -2017,7 +2017,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, * free it if we fail for any reason */ if (newdev->type == VIR_DOMAIN_NET_TYPE_NETWORK && - networkAllocateActualDevice(newdev) < 0) { + networkAllocateActualDevice(vm->def, newdev) < 0) { goto cleanup; } @@ -2204,7 +2204,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, /* this function doesn't work with HOSTDEV networks yet, thus * no need to change the pointer in the hostdev structure */ - networkReleaseActualDevice(olddev); + networkReleaseActualDevice(vm->def, olddev); virDomainNetDefFree(olddev); /* move newdev into the nets list, and NULL it out from the * virDomainDeviceDef that we were given so that the caller @@ -2236,7 +2236,7 @@ cleanup: * replace the entire device object. */ if (newdev) - networkReleaseActualDevice(newdev); + networkReleaseActualDevice(vm->def, newdev); return ret; } @@ -2649,7 +2649,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, virDomainHostdevDefFree(hostdev); if (net) { - networkReleaseActualDevice(net); + networkReleaseActualDevice(vm->def, net); virDomainNetDefFree(net); } virObjectUnref(cfg); @@ -2717,7 +2717,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, virDomainNetGetActualBridgeName(net), net->ifname)); - networkReleaseActualDevice(net); + networkReleaseActualDevice(vm->def, net); virDomainNetDefFree(net); virObjectUnref(cfg); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c1f60cd..82d2661 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2773,7 +2773,7 @@ qemuProcessNotifyNets(virDomainDefPtr def) for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; - if (networkNotifyActualDevice(net) < 0) + if (networkNotifyActualDevice(def, net) < 0) return -1; } return 0; @@ -4393,7 +4393,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, /* kick the device out of the hostdev list too */ virDomainNetRemoveHostdev(def, net); - networkReleaseActualDevice(net); + networkReleaseActualDevice(vm->def, net); } retry: diff --git a/src/util/virhook.c b/src/util/virhook.c index 159efdb..0f5d0c5 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,13 @@ VIR_ENUM_IMPL(virHookLxcOp, VIR_HOOK_LXC_OP_LAST, "started", "reconnect") +VIR_ENUM_IMPL(virHookNetworkOp, VIR_HOOK_NETWORK_OP_LAST, + "start", + "started", + "stopped", + "plugged", + "unplugged") + static int virHooksFound = -1; /** @@ -246,6 +255,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..7b09ac5 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,16 @@ enum virHookLxcOpType { VIR_HOOK_LXC_OP_LAST, }; +enum virHookNetworkOpType { + VIR_HOOK_NETWORK_OP_START, /* network is about to start */ + VIR_HOOK_NETWORK_OP_STARTED, /* network has start */ + VIR_HOOK_NETWORK_OP_STOPPED, /* network has stopped */ + VIR_HOOK_NETWORK_OP_IFACE_PLUGGED, /* an interface has been plugged into the network */ + VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED, /* an interface was unplugged from the network */ + + VIR_HOOK_NETWORK_OP_LAST, +}; + int virHookInitialize(void); int virHookPresent(int driver); -- 1.8.5.3
On Mon, Feb 10, 2014 at 07:52:34PM +0100, Michal Privoznik wrote:
@@ -3583,6 +3639,48 @@ validate: } }
+ /* finally we can call the 'plugged' hook script if any */ + if (virHookPresent(VIR_HOOK_DRIVER_NETWORK)) { + /* the XML construction is a bit complicated here, + * as we want to pass both domain XML and network XML */ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *xml, *net_xml, *dom_xml; + int hookret; + + net_xml = virNetworkDefFormat(netdef, 0); + dom_xml = virDomainDefFormat(dom, 0); + + if (!net_xml || !dom_xml) { + VIR_FREE(net_xml); + VIR_FREE(dom_xml); + goto error; + } + + virBufferAdd(&buf, net_xml, -1); + virBufferAdd(&buf, dom_xml, -1);
This isn't very easy for applications to consume. With all the other things you can just pass stdin straight to your XML parser and process it. When you concatenate 2 XML docs this way it becomes much harder, since you have to split the two docs which means parsing them without an XML parser. Usually you'd want to place the 2 docs within a parent XML doc so the overall result is still a single wellformed XML doc.
+ + VIR_FREE(net_xml); + VIR_FREE(dom_xml); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + goto error; + } + + xml = virBufferContentAndReset(&buf); + + hookret = virHookCall(VIR_HOOK_DRIVER_NETWORK, network->def->name, + VIR_HOOK_NETWORK_OP_IFACE_PLUGGED, + VIR_HOOK_SUBOP_BEGIN, NULL, xml, NULL); + VIR_FREE(xml); + + /* + * If the script raised an error abort the allocation + */ + if (hookret < 0) + goto error; + } + if (dev) { /* we are now assured of success, so mark the allocation */ dev->connections++; @@ -3618,6 +3716,7 @@ error: }
/* networkNotifyActualDevice: + * @dom: domain definition that @iface belongs to * @iface: the domain's NetDef with an "actual" device already filled in. * * Called to notify the network driver when libvirtd is restarted and @@ -3628,7 +3727,8 @@ error: * Returns 0 on success, -1 on failure. */ int -networkNotifyActualDevice(virDomainNetDefPtr iface) +networkNotifyActualDevice(virDomainDefPtr dom, + virDomainNetDefPtr iface) { virNetworkDriverStatePtr driver = driverState; enum virDomainNetType actualType = virDomainNetGetActualType(iface); @@ -3781,6 +3881,48 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) }
success: + /* finally we can call the 'plugged' hook script if any */ + if (virHookPresent(VIR_HOOK_DRIVER_NETWORK)) { + /* the XML construction is a bit complicated here, + * as we want to pass both domain XML and network XML */ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *xml, *net_xml, *dom_xml; + int hookret; + + net_xml = virNetworkDefFormat(netdef, 0); + dom_xml = virDomainDefFormat(dom, 0); + + if (!net_xml || !dom_xml) { + VIR_FREE(net_xml); + VIR_FREE(dom_xml); + goto error; + } + + virBufferAdd(&buf, net_xml, -1); + virBufferAdd(&buf, dom_xml, -1); + + VIR_FREE(net_xml); + VIR_FREE(dom_xml); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + goto error; + } + + xml = virBufferContentAndReset(&buf); + + hookret = virHookCall(VIR_HOOK_DRIVER_NETWORK, network->def->name, + VIR_HOOK_NETWORK_OP_IFACE_PLUGGED, + VIR_HOOK_SUBOP_BEGIN, NULL, xml, NULL); + VIR_FREE(xml); + + /* + * If the script raised an error abort the allocation + */ + if (hookret < 0) + goto error; + } + netdef->connections++; VIR_DEBUG("Using network %s, %d connections", netdef->name, netdef->connections); @@ -3796,6 +3938,7 @@ error:
/* networkReleaseActualDevice: + * @dom: domain definition that @iface belongs to * @iface: a domain's NetDef (interface definition) * * Given a domain <interface> element that previously had its <actual> @@ -3806,7 +3949,8 @@ error: * Returns 0 on success, -1 on failure. */ int -networkReleaseActualDevice(virDomainNetDefPtr iface) +networkReleaseActualDevice(virDomainDefPtr dom, + virDomainNetDefPtr iface) { virNetworkDriverStatePtr driver = driverState; enum virDomainNetType actualType = virDomainNetGetActualType(iface); @@ -3925,6 +4069,42 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) success: if (iface->data.network.actual) netdef->connections--; + + /* finally we can call the 'unplugged' hook script if any */ + if (virHookPresent(VIR_HOOK_DRIVER_NETWORK)) { + /* the XML construction is a bit complicated here, + * as we want to pass both domain XML and network XML */ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *xml, *net_xml, *dom_xml; + + net_xml = virNetworkDefFormat(netdef, 0); + dom_xml = virDomainDefFormat(dom, 0); + + if (!net_xml || !dom_xml) { + VIR_FREE(net_xml); + VIR_FREE(dom_xml); + goto error; + } + + virBufferAdd(&buf, net_xml, -1); + virBufferAdd(&buf, dom_xml, -1); + + VIR_FREE(net_xml); + VIR_FREE(dom_xml); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + goto error; + } + + xml = virBufferContentAndReset(&buf); + + virHookCall(VIR_HOOK_DRIVER_NETWORK, network->def->name, + VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED, + VIR_HOOK_SUBOP_BEGIN, NULL, xml, NULL); + VIR_FREE(xml); + } + VIR_DEBUG("Releasing network %s, %d connections", netdef->name, netdef->connections);
There's alot of repeated code patterns throughout this file. Seems this would be shorter if we had a helper method for doing all the hook presence check / xml formatting hook invocation. Each usage would then just be a single line function call. 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 11.02.2014 10:54, Daniel P. Berrange wrote:
On Mon, Feb 10, 2014 at 07:52:34PM +0100, Michal Privoznik wrote:
@@ -3583,6 +3639,48 @@ validate: } }
+ /* finally we can call the 'plugged' hook script if any */ + if (virHookPresent(VIR_HOOK_DRIVER_NETWORK)) { + /* the XML construction is a bit complicated here, + * as we want to pass both domain XML and network XML */ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *xml, *net_xml, *dom_xml; + int hookret; + + net_xml = virNetworkDefFormat(netdef, 0); + dom_xml = virDomainDefFormat(dom, 0); + + if (!net_xml || !dom_xml) { + VIR_FREE(net_xml); + VIR_FREE(dom_xml); + goto error; + } + + virBufferAdd(&buf, net_xml, -1); + virBufferAdd(&buf, dom_xml, -1);
This isn't very easy for applications to consume. With all the other things you can just pass stdin straight to your XML parser and process it. When you concatenate 2 XML docs this way it becomes much harder, since you have to split the two docs which means parsing them without an XML parser. Usually you'd want to place the 2 docs within a parent XML doc so the overall result is still a single wellformed XML doc.
Okay, but how should the XML look then? <hookData> <network/> <domain/> </hookData> I'm basically asking about the root element name. Then, if we do this for plug & unplug - should we follow the same scheme for network start & stop? Without the <domain/>. You know, once we require hook script to take network xml from /hookData/network we can do that for other cases too, so the XPath to select the network doesn't change. Or is that an overkill, because the hook script can easily do: if [ "$2" == "plugged" -o "$2" == "unplugged" ]; then XPATH="/hookData/network" else XPATH="/network" fi Michal
Basically, the idea is copied from domain code, where tainting exists for a while. Currently, only one taint reason exists - VIR_NETWORK_TAINT_HOOK to mark those networks which caused invoking of hook script. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/network_conf.c | 52 ++++++++++++++++++++++++++++++++++++++++++++- src/conf/network_conf.h | 17 +++++++++++++++ src/libvirt_private.syms | 3 +++ src/network/bridge_driver.c | 26 +++++++++++++++++++++++ 4 files changed, 97 insertions(+), 1 deletion(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index dd3fa19..341b24b 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -72,6 +72,22 @@ VIR_ENUM_IMPL(virNetworkDNSForwardPlainNames, "yes", "no") +VIR_ENUM_IMPL(virNetworkTaint, VIR_NETWORK_TAINT_LAST, + "hook-script"); + +bool +virNetworkObjTaint(virNetworkObjPtr obj, + enum virNetworkTaintFlags taint) +{ + unsigned int flag = (1 << taint); + + if (obj->taint & flag) + return false; + + obj->taint |= flag; + return true; +} + virNetworkObjPtr virNetworkFindByUUID(virNetworkObjListPtr nets, const unsigned char *uuid) { @@ -2784,6 +2800,7 @@ virNetworkObjFormat(virNetworkObjPtr net, { virBuffer buf = VIR_BUFFER_INITIALIZER; char *class_id = virBitmapFormat(net->class_id); + size_t i; if (!class_id) goto no_memory; @@ -2793,6 +2810,12 @@ virNetworkObjFormat(virNetworkObjPtr net, virBufferAsprintf(&buf, " <floor sum='%llu'/>\n", net->floor_sum); VIR_FREE(class_id); + for (i = 0; i < VIR_NETWORK_TAINT_LAST; i++) { + if (net->taint & (1 << i)) + virBufferAsprintf(&buf, " <taint flag='%s'/>\n", + virNetworkTaintTypeToString(i)); + } + virBufferAdjustIndent(&buf, 2); if (virNetworkDefFormatInternal(&buf, net->def, flags) < 0) goto error; @@ -2903,10 +2926,13 @@ virNetworkLoadState(virNetworkObjListPtr nets, virNetworkDefPtr def = NULL; virNetworkObjPtr net = NULL; xmlDocPtr xml = NULL; - xmlNodePtr node = NULL; + xmlNodePtr node = NULL, *nodes = NULL; xmlXPathContextPtr ctxt = NULL; virBitmapPtr class_id_map = NULL; unsigned long long floor_sum_val = 0; + unsigned int taint = 0; + int n; + size_t i; if ((configFile = virNetworkConfigFile(stateDir, name)) == NULL) @@ -2962,6 +2988,27 @@ virNetworkLoadState(virNetworkObjListPtr nets, goto error; } VIR_FREE(floor_sum); + + if ((n = virXPathNodeSet("./taint", ctxt, &nodes)) < 0) + goto error; + + for (i = 0; i < n; i++) { + char *str = virXMLPropString(nodes[i], "flag"); + if (str) { + int flag = virNetworkTaintTypeFromString(str); + if (flag < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown taint flag %s"), str); + VIR_FREE(str); + goto error; + } + VIR_FREE(str); + /* Compute taint mask here. The network object does not + * exist yet, so we can't use virNetworkObjtTaint. */ + taint |= (1 << flag); + } + } + VIR_FREE(nodes); } /* create the object */ @@ -2978,6 +3025,8 @@ virNetworkLoadState(virNetworkObjListPtr nets, if (floor_sum_val > 0) net->floor_sum = floor_sum_val; + net->taint = taint; + cleanup: VIR_FREE(configFile); xmlFreeDoc(xml); @@ -2985,6 +3034,7 @@ cleanup: return net; error: + VIR_FREE(nodes); virBitmapFree(class_id_map); virNetworkDefFree(def); goto cleanup; diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index b84762a..edcc49f 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -287,6 +287,8 @@ struct _virNetworkObj { virBitmapPtr class_id; /* bitmap of class IDs for QoS */ unsigned long long floor_sum; /* sum of all 'floor'-s of attached NICs */ + + unsigned int taint; }; typedef struct _virNetworkObjList virNetworkObjList; @@ -296,12 +298,26 @@ struct _virNetworkObjList { virNetworkObjPtr *objs; }; +enum virNetworkTaintFlags { + VIR_NETWORK_TAINT_HOOK, /* Hook script was executed over + network. We can't guarantee + connectivity or other settings + as the script may have played + with iptables, tc, you name it. + */ + + VIR_NETWORK_TAINT_LAST +}; + static inline int virNetworkObjIsActive(const virNetworkObj *net) { return net->active; } +bool virNetworkObjTaint(virNetworkObjPtr obj, + enum virNetworkTaintFlags taint); + virNetworkObjPtr virNetworkFindByUUID(virNetworkObjListPtr nets, const unsigned char *uuid); virNetworkObjPtr virNetworkFindByName(virNetworkObjListPtr nets, @@ -452,4 +468,5 @@ virNetworkDefUpdateSection(virNetworkDefPtr def, const char *xml, unsigned int flags); /* virNetworkUpdateFlags */ +VIR_ENUM_DECL(virNetworkTaint) #endif /* __NETWORK_CONF_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2c9536a..e1cfca0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -527,6 +527,7 @@ virNetworkObjListFree; virNetworkObjLock; virNetworkObjReplacePersistentDef; virNetworkObjSetDefTransient; +virNetworkObjTaint; virNetworkObjUnlock; virNetworkObjUnsetDefTransient; virNetworkObjUpdate; @@ -535,6 +536,8 @@ virNetworkSaveConfig; virNetworkSaveStatus; virNetworkSetBridgeMacAddr; virNetworkSetBridgeName; +virNetworkTaintTypeFromString; +virNetworkTaintTypeToString; virPortGroupFindByName; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 90852d5..285b26b 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -112,6 +112,9 @@ static int networkPlugBandwidth(virNetworkObjPtr net, static int networkUnplugBandwidth(virNetworkObjPtr net, virDomainNetDefPtr iface); +static void networkNetworkObjTaint(virNetworkObjPtr net, + enum virNetworkTaintFlags taint); + static virNetworkDriverStatePtr driverState = NULL; static virNetworkObjPtr @@ -2027,6 +2030,8 @@ networkStartNetwork(virNetworkDriverStatePtr driver, */ if (hookret < 0) goto cleanup; + + networkNetworkObjTaint(network, VIR_NETWORK_TAINT_HOOK); } switch (network->def->forward.type) { @@ -2066,6 +2071,8 @@ networkStartNetwork(virNetworkDriverStatePtr driver, */ if (hookret < 0) goto cleanup; + + networkNetworkObjTaint(network, VIR_NETWORK_TAINT_HOOK); } /* Persist the live configuration now that anything autogenerated @@ -3679,6 +3686,8 @@ validate: */ if (hookret < 0) goto error; + + networkNetworkObjTaint(network, VIR_NETWORK_TAINT_HOOK); } if (dev) { @@ -4103,6 +4112,8 @@ success: VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED, VIR_HOOK_SUBOP_BEGIN, NULL, xml, NULL); VIR_FREE(xml); + + networkNetworkObjTaint(network, VIR_NETWORK_TAINT_HOOK); } VIR_DEBUG("Releasing network %s, %d connections", @@ -4439,3 +4450,18 @@ networkUnplugBandwidth(virNetworkObjPtr net, cleanup: return ret; } + +static void +networkNetworkObjTaint(virNetworkObjPtr net, + enum virNetworkTaintFlags taint) +{ + if (virNetworkObjTaint(net, taint)) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(net->def->uuid, uuidstr); + + VIR_WARN("Network name='%s' uuid=%s is tainted: %s", + net->def->name, + uuidstr, + virNetworkTaintTypeToString(taint)); + } +} -- 1.8.5.3
participants (2)
-
Daniel P. Berrange -
Michal Privoznik