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@xxxxxxxxx> > >>>To: libvir-list@xxxxxxxxxx > >>>Cc: "Michal Privoznik" <mprivozn@xxxxxxxxxx> > >>>Sent: Friday, February 7, 2014 1:17:10 PM > >>>Subject: Re: [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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list