[libvirt] [PATCH v4 2/3] network: Introduce network hooks

Michal Privoznik mprivozn at redhat.com
Tue Feb 18 13:53:03 UTC 2014


On 18.02.2014 12:20, Laine Stump wrote:
> On 02/13/2014 03:17 PM, Michal Privoznik wrote:
>> On 13.02.2014 12:10, Laine Stump wrote:
>>>
>>> On 02/12/2014 08:20 PM, Laine Stump wrote:
>>>> It all looks fine (aside from the few small grammar corrections in
>>>> the docs. I just want one last confirmation that we don't want both
>>>> "plug" and "plugged" events, and in that case ACK.
>>>
>>> A few marginally related things came to my mind since I sent that reply,
>>> leading me to rethink my conditional ACK:
>>>
>>> 1) ====================================================
>>>
>>> I looked back through the review comments of previous versions, and see
>>> that I mis-remembered about anyone objecting to both a "plug" and a
>>> "plugged" hook. I wasn't really thinking in terms of the "plug" hook
>>> being able to refuse plugging in an interface, but more that there my be
>>> some operations that must be performed prior to allocating the
>>> tap/macvtap device, and also that it makes it more consistent with the
>>> other hook groups (domains and networks both have "start" "started"
>>> "stopped", so to me it seems consistent for a network interface to have
>>> "plug" "plugged" "unplugged"). Daniel - care to reel in my
>>> over-ambitious spirit here? :-)
>>
>> Okay, we can ignore the hook return value in (un-)plug case. I don't
>> really care.
>
> (Actually, I hadn't said that I thought it was a bad idea to check the
> return value, only that I hadn't thought of it :-)
>
>>
>>>
>>>
>>> 2) ====================================================
>>>
>>> Beyond that, I was thinking about this last night as I fell asleep and
>>> realized that in these plug hooks, there is no simple way to determine
>>> which interface of the domain is being plugged/unplugged - we are
>>> sending the full domain XML and full network XML, but if the domain has
>>> multiple interfaces we can't easily figure out which is the one we're
>>> plugging (and as a matter of fact, due to the way
>>> qemuDomainAttachNetDevice() is organized, the new device being plugged
>>> in will not yet be in the domain XML *at all* at the time the "plugged"
>>> hook is called![*])
>>>
>>> So I think we need to add the specific <interface> to the XML sent to
>>> the hook, i.e.:
>>>
>>>        <hookData>
>>>          <interface>
>>>            ...
>>>          </interface>
>>>          <network>
>>>            ...
>>>          </network>
>>>          <domain>
>>>            ...
>>>          </domain>
>>>       <hookData>
>>>
>>> (Putting the lone <interface> first will make it simpler for luddites
>>> like me to just grep for the first occurence of "<mac address" on stdin
>>> rather than firing up a full-fledged XML parser.)
>>
>> Well, the hook script is now fed with XML - we can do whatever we want
>> - even after this is pushed :)
>
> Okay, since my main objection is the lack of XML for the specific
> interface in the "plugged" event, and since the interface XML anyway
> wouldn't be useful until I fix it to contain the actual allocated device
> info (as described in the remainder of my earlier message), I say ACK to
> this patch (with the couple of grammar fixes I'd mentioned earlier), so
> the entire series is now ACKed.
>
> After you've pushed these patches, I will fix the live <interface> XML
> (I'm working on that already, and when it is fixed I'll add it to the
> XML that is sent for the plugged and unplugged hooks.
>

Okay, I've pushed this. Thanks.

Michal




More information about the libvir-list mailing list