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

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).

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