[libvirt] [PATCH v2 3/3] network: Taint networks that are using hook script

[Daniel P. Berrange]

On Mon, Feb 10, 2014 at 03:52:50PM +0100, Michal Privoznik wrote:
> On 08.02.2014 11:51, Laine Stump wrote:
> >On 02/07/2014 10:52 PM, Antoni Segura Puimedon wrote:
> >>
> >>----- Original Message -----
> >>>From: "Laine Stump" <laine at laine.org>
> >>>To: libvir-list at redhat.com
> >>>Cc: "Michal Privoznik" <mprivozn at redhat.com>
> >>>Sent: Friday, February 7, 2014 1:17:10 PM
> >>>Subject: Re: [libvirt] [PATCH v2 3/3] network: Taint networks that are using hook script
> >>>
> >>>On 02/05/2014 12:11 PM, Michal Privoznik wrote:
> >>>>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.
> >>>What's missing here is that the network status XML doesn't include a
> >>><taint> element.
> >>>
> >>>Also, I think if a network is tainted, and domain that connects to that
> >>>network should be tainted as well.
> >>>
> >>>Of course what would make this more useful would be if would could
> >>>determine when a hook script actually *did* something for a particular
> >>>network/interface (since presumably people are usually going to write
> >>>their network hook scripts to only take action for particular networks
> >>>and/or domains, not for *all* networks). I don't know that there's a way
> >>>to do that without either 1) having a different hook script for each
> >>>network, or 2) trusting the hook script to return some sort of status
> >>>indicating whether or not it did anything. Obviously (2) is not a good
> >>>idea, but we may want to think about (1) in the future (for qemu and lxc
> >>>hook scripts as well) - instead of just looking for
> >>>/etc/libvirt/hook/network, we could first look for
> >>>/etc/libvirt/hook/network.${netname} and exec that instead if found (or
> >>>in addition). But I think that can be deferred until later.
> >>Actually I kind of like the option (2). I think it could make a lot of sense
> >>that the hook would be able to add an attribute to the network definition
> >>xml, e.g. <bandwidth hooked="1"> so that libvirt would know that that part
> >>has been taken care of by the hook. Of course, it might be a bad idea for
> >>libvirt to blindly accept any kind of modification, but something like what
> >>I propose does not seem eminently dangerous.
> >
> >The reason I don't like option (2) is that it requires trusting the hook
> >to leave its mark if it modifies anything, and that's exactly why we
> >want to taint the networks that call a hook - because we don't/can't
> >trust the hook.
> >
> >I wonder if there might be some way to allow a hook to add information
> >to the network's xml in some well-defined location, though. This
> >information would not be used/trusted by libvirt at all, but would only
> >be there, for example, so that a later "stop/unplug" hook could retrieve
> >it, rather than being required to keep its state externally.
> >
> 
> Well, we may make the hook script to return the network xml that
> libvirt will parse and startup. For example:
> 
> 1) network with <bandwidth/> is about to start. The network XML is
> passed to the script.
> 
> 2) The script sees <network> ... <bandwidth/> ... </network> and do
> all the tc magic. Then it produces the same XML minus <bandwidth/>
> 
> 3) Libvirt parses the <network> ... </network> without the bandwidth
> knowing that the script has taken care of it. If it doesn't we may
> error out because <bandwidth/> is not supported yet (assuming the
> right type of network for this little example). The whole network
> startup process would be aborted then.
> 
> On the other hand, if we go this way (and in some sense even if we
> don't), we are going to need <metadata/> for the networks so users
> may set some attributes that are unknown to libvirt but allows the
> script to make better decisions.

I really think this is all getting overly complex. IMHO we should just
treat anything the hook does as a black box and not try to intepret
its output or actions in any way.

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