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

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



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


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