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 :|