[libvirt] [PATCH 0/2] Fix interface state transitions logic

[Daniel P. Berrange]

On Thu, Dec 12, 2013 at 12:23:48PM +0200, Laine Stump wrote:
> On 12/11/2013 11:16 AM, Michal Privoznik wrote:
> > Right now it's possible to start an interface that is already running, or
> > destroy an interface multiple times. Such state transitions are not allowed and
> > we check for such cases explicitly in other areas like qemu driver.
> 
> I recall that someone brought up this issue before, and we didn't change
> the code. If I remember correctly, the reason was that the
> virInterfaceCreate() API is basically just a wrapper around /sbin/ifup,
> and /sbin/ifup can be called (and returns success after doing something
> useful - see below) if the interface is already up.
> 
> Why would you want to do that? Well, calling ifup on an interface that
> is already up causes it to be "re-started", renewing its IP (and other)
> configuration from any changes that have been made, *without* needing to
> ifdown the interface in the process (and completely losing network
> connectivity, possibly making it impossible for the  program calling the
> API to re-start the interface). (It's not directly applicable in this
> case, but when a bridge device is added, subsuming an existing ethernet,
> we rely on allowing virInterfaceCreate() to be called for the new bridge
> device even though the subordinate ethernet is already up; this permits
> us to add a bridge to the host's config without losing network
> connectivity.)

The usage you describe here is not within the scope of the
virInterfaceCreate()  API IMHO. If we want users to have the
ability to "re start" an interface without taking it offline
first, then we should add another API that explicitly supports
that use case.

> I'm not convinced that it's a bad thing that virInterfaceCreate/Destroy
> can, by definition, be called when the device is already in the desired
> state, and wouldn't want to end up with other unforeseen problems just
> because we disabled it.

Overloading a single virInterfaceCreate to support two different use
cases puts applications in an impossible position if they *want* to
see an error from attempting to start an already active interface.
They can't just check virInterfaceIsActive before calling the
virInterfaceCreate API because that is racy from the apps POV. If
we explicitly have separate APIs for starting vs refreshing the config
then we can cleanly support all use scenarios.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|