[libvirt] [PATCH] interface: Check for interface (in-)activity on some operations

[Michal Privoznik]

On 18.07.2011 19:21, Laine Stump wrote:
> On 07/15/2011 10:36 AM, Michal Privoznik wrote:
>> On 15.07.2011 16:29, Eric Blake wrote:
>>> On 07/15/2011 07:58 AM, Michal Privoznik wrote:
>>>> Right now it is possible to undefine an active interface, or
>>>> destroy inactive. This patch add some checking to these operations
>>>> to prevent this. Also fix test driver.
>>>
>>> I'm inclined to NACK this on design principles (I haven't read the patch
>>> itself, though). Given the discussion about domains and undefine, the
>>> ability to undefine an active interface is a feature, provided we
>>> support the concept of a transient interface like we do for transient
>>> domains.
>>>
>>> That is, we have the following transitions:
>>>
>>> nothing -> transient/running via Create
>>> nothing -> persistent/inactive via Define
>>>
>>> persistent/inactive -> persistent/active via Start
>>> persistent/inactive -> gone via Undefine
>>>
>>> persistent/running -> persistent/inactive via Destroy
>>> persistent/running -> transient/running via Undefine
>>>
>>> transient/running -> gone via Destroy
>>> transient/running -> persistent/running via Define
>>>
>>> and rejecting Undefine on a running interface would prevent the ability
>>> to transistion a persistent over to a transient interface.
>>>
>>> On the other hand, if we don't support transient interfaces, then the
>>> above analysis which works for domains would have to be adjusted for
>>> interfaces, so you may have something to patch after all.
>>>
>> Well, although we have function interfaceCreate, it is actually (from
>> semantic POV) interfaceStart, because it just start inactive but
>> defined interface. So we do not support transient interfaces.
>> Therefore transitions for interfaces are slightly different from
>> transitions for domains. That's why I think we do need this patch.
>
> Since I was the original author of this file, I guess I'd better get
> into the conversation :-)
>
> The odd part of this is that I recall having a conversation about
> allowing/not allowing undefine of an interface that is active, and I
> *thought* that it didn't allow it (but obviously it does).
>
> A couple of points:
>
> 1) The fact that we don't support transient interfaces now doesn't
> preclude supporting them in the future (although we may use some method
> other than netcf to do it).

Agree. So what about - letting this in, and once we decide to support 
transient interfaces, we can (can't we?) remove this. I think it is a 
bug to allow interface go transient as we don't support them now.

>
> 2) We should be careful changing this, in case it has any effects on
> virt-manager's use of the API.

Agree in being careful. But as I've said, it is from my POV a bug which 
in this case might expose bug in there.

Michal