On 21.02.2014 14:58, Laine Stump wrote:
> The network hook script gets called whenever an interface is plugged
> into or unplugged from a network, but even though the full XML of both
> the network and the domain is included, there is no reasonable way to
> determine what exact resources the plugged interface is using:
>
> 1) Prior to a recent patch which modified the status XML of interfaces
> to include the information about actual hardware resources used, it
> would be possible to scan through the domain XML output sent to the
> hook, and from there find the correct interface, but that interface
> definition would not include any runtime info (e.g. bandwidth or vlan
> taken from a portgroup, or which physdev was used in case of a macvtap
> network).
>
> 2) After the patch modifying the status XML of interfaces, the network
> name would no longer be included in the domain XML, so it would be
> completely impossible to determine which interface was the one being
> plugged.
>
> To solve that problem, this patch includes a single <interface>
> element at the beginning of the XML sent to the network hook for
> "plugged" and "unplugged" (just inside <hookData>) that is the status
> XML of the interface being plugged. This XML will include all info
> gathered from the chosen network and portgroup.
>
> NB: due to hardcoded spaces in all of the device *Format() functions,
> the <interface> element inside the <hookData> will be indented by 6
> spaces rather than 2. I had intended to fix this, but it turns out
> that to make virDomainNetDefFormat() indentation relative, I would
> have to do the same to virDomainDeviceInfoFormat(), and that function
> is called from 19 places - making that a prerequisite of this patch
> would cause too many merge difficulties if we needed to backport
> network hooks, so I chose to ignore the problem here and fix the
> problem for *all* devices in a followup later.
> ---
> src/network/bridge_driver.c | 48 ++++++++++++++++++++++++++++++---------------
> 1 file changed, 32 insertions(+), 16 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index a6c719d..152bd06 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -141,6 +141,7 @@ networkObjFromNetwork(virNetworkPtr net)
> static int
> networkRunHook(virNetworkObjPtr network,
> virDomainDefPtr dom,
> + virDomainNetDefPtr iface,
> int op,
> int sub_op)
> {
> @@ -158,6 +159,8 @@ networkRunHook(virNetworkObjPtr network,
>
> virBufferAddLit(&buf, "<hookData>\n");
> virBufferAdjustIndent(&buf, 2);
> + if (iface && virDomainNetDefFormat(&buf, iface, 0) < 0)
> + goto cleanup;
> if (virNetworkDefFormatBuf(&buf, network->def, 0) < 0)
> goto cleanup;
> if (dom && virDomainDefFormatInternal(dom, 0, &buf) < 0)
> @@ -2067,7 +2070,7 @@ networkStartNetwork(virNetworkDriverStatePtr driver,
>
> /* Run an early hook to set-up missing devices.
> * If the script raised an error abort the launch. */
> - if (networkRunHook(network, NULL,
> + if (networkRunHook(network, NULL, NULL,
> VIR_HOOK_NETWORK_OP_START,
> VIR_HOOK_SUBOP_BEGIN) < 0)
> goto cleanup;
> @@ -2092,7 +2095,7 @@ networkStartNetwork(virNetworkDriverStatePtr driver,
> }
>
> /* finally we can call the 'started' hook script if any */
> - if (networkRunHook(network, NULL,
> + if (networkRunHook(network, NULL, NULL,
> VIR_HOOK_NETWORK_OP_STARTED,
> VIR_HOOK_SUBOP_BEGIN) < 0)
> goto cleanup;
> @@ -2158,7 +2161,7 @@ static int networkShutdownNetwork(virNetworkDriverStatePtr driver,
> }
>
> /* now that we know it's stopped call the hook if present */
> - networkRunHook(network, NULL, VIR_HOOK_NETWORK_OP_STOPPED,
> + networkRunHook(network, NULL, NULL, VIR_HOOK_NETWORK_OP_STOPPED,
> VIR_HOOK_SUBOP_END);
>
> network->active = 0;
> @@ -3659,14 +3662,8 @@ validate:
> }
> }
>
> - /* finally we can call the 'plugged' hook script if any */
> - if (networkRunHook(network, dom,
> - VIR_HOOK_NETWORK_OP_IFACE_PLUGGED,
> - VIR_HOOK_SUBOP_BEGIN) < 0)
> - goto error;
> -
> if (dev) {
> - /* we are now assured of success, so mark the allocation */
> + /* mark the allocation */
> dev->connections++;
> if (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV) {
> VIR_DEBUG("Using physical device %s, %d connections",
> @@ -3684,6 +3681,19 @@ validate:
> VIR_DEBUG("Using network %s, %d connections",
> netdef->name, netdef->connections);
> }
> +
> + /* finally we can call the 'plugged' hook script if any */
> + if (networkRunHook(network, dom, iface,
> + VIR_HOOK_NETWORK_OP_IFACE_PLUGGED,
> + VIR_HOOK_SUBOP_BEGIN) < 0) {
> + /* adjust for failure */
> + if (dev)
> + dev->connections--;
> + if (netdef)
> + netdef->connections--;
> + goto error;
> + }
> +
> ret = 0;
>
> cleanup:
> @@ -3865,14 +3875,20 @@ networkNotifyActualDevice(virDomainDefPtr dom,
> }
>
> success:
> - /* finally we can call the 'plugged' hook script if any */
> - if (networkRunHook(network, dom, VIR_HOOK_NETWORK_OP_IFACE_PLUGGED,
> - VIR_HOOK_SUBOP_BEGIN) < 0)
> - goto error;
> -
> netdef->connections++;
> VIR_DEBUG("Using network %s, %d connections",
> netdef->name, netdef->connections);
> +
> + /* finally we can call the 'plugged' hook script if any */
> + if (networkRunHook(network, dom, iface, VIR_HOOK_NETWORK_OP_IFACE_PLUGGED,
> + VIR_HOOK_SUBOP_BEGIN) < 0) {
> + /* adjust for failure */
> + if (dev)
> + dev->connections--;
> + netdef->connections--;
> + goto error;
> + }
> +
> ret = 0;
> cleanup:
> if (network)
> @@ -4018,7 +4034,7 @@ success:
> netdef->connections--;
>
> /* finally we can call the 'unplugged' hook script if any */
> - networkRunHook(network, dom, VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED,
> + networkRunHook(network, dom, iface, VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED,
> VIR_HOOK_SUBOP_BEGIN);
>
> VIR_DEBUG("Releasing network %s, %d connections",
>
ACK although the indentation of XML we're passing to the hook script seems off:
<hookData>
<interface type='network'>
...
</interface>
<network>
...
</network>
<domain type='kvm'>
</domain>
</hookData>
This is not a show stopper for me. I wonder if we should push these patches now, even during the freeze as this is very closely related to the network hooks - one of the main features in this release.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list