On 05/17/2016 08:40 AM, Vasiliy Tolstov wrote:
2016-05-17 15:33 GMT+03:00 Cole Robinson
<crobinso(a)redhat.com>:
> I think Laine was saying 'I personally don't care about ->script so
I'm not
> planning on implementing it',
My position is a bit stronger than that. It's not that I don't want to
spend the effort. It's that everything added is just that much more to
support. A capability to call an arbitrary external script is especially
problematic, because nearly anything could happen during execution of
the script, making support of other parts of the system much more
difficult (at least with any level of certainty. It also gives at least
a feeling of greater vulnerability to potential security exploits
(although libvirtd is running as root already).
Also, keep in mind that the script capability was originally there just
to allow overriding the default script that qemu in order to create a
tap device and make it functional at all. Much (in most cases all?) of
the required setup is now done within libvirt, making me wonder if it's
necessary to do anything outside. If not, then we shouldn't put in
something that just invites Rube Goldberg-type contraptions. (I'm not
saying that's definitely what it would be; just saying "let's talk about
what this would be used for first to be sure we *really* want it)
Another thing - there was no design thought put into the script feature
for the qemu driver - it is just an approximate replacement for what
qemu itself does with a script; for example, the only input the script
gets is the name of the tap interface (as a command line arg), and is
only called once (just after the tap is created and MAC address is set)
which may be inadequate. If we're going to do this at all, perhaps we
should do it right, and send (in stdin) the full domain XML + an extra
copy of the interface XML for the interface being started (similar to
the libvirt network hook, but can be specified differently for each
interface). For that matter, we should also be calling the script both
before the tap/veth devices are created/configed, then again after they
are fully setup from libvirt's PoV - some things can only be done once
the interface exists (and some other things must be done before it's
created).
> so if it isn't implemented we should error
> explicitly rather than silently ignore it.
I agree with this.
>
> That said it's probably not hard to get ->script to work since we already
have
> the plumbing as you mention,
I also agree with this :-). It's trivial to all *all kinds of things* to
libvirt (but remember "just because you can do it, doesn't mean you
*should*"). I actually almost added support for script before posting,
then changed my mind and started to add an error condition when script
was specified. But since I was undecided I did neither. I don't intend
to push the patches without one or the other. If there really is a good
case for enabling script (maybe combined with officially tainting the
domain so that we can refuse to support anyone who has a script) then
I'll do that.
> so 'patches welcome' :)
Ok, after Laine patch merged i send script patch.
Rather than me spending time adding something to my patches that will
just be superceded by another patch immediately, I would rather just do
it the way everyone wants to begin with.
What type of things are usually done in the upscript? (and is a "down
script" needed?)
(Speaking of that - I had asked in my discussion of re-doing the
peer-address patches if you could supply some example address/route
configs (for both host and guest side) of what you're doing with the
peer address, to see just how generalized those need to be. Any chance
you can send me that?)