Re: [PATCH] lxc: support <interface type='ethernet'>

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/17/2016 08:40 AM, Vasiliy Tolstov wrote:
2016-05-17 15:33 GMT+03:00 Cole Robinson <crobinso@xxxxxxxxxx>:
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?)


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]