On 12/11/2013 11:16 AM, Michal Privoznik wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=956994 > > Currently, it is possible to start an interface that is already running: > > # virsh iface-start eth2 > Interface eth2 started > > # echo $? > 0 > > # virsh iface-start eth2 > Interface eth2 started > > # echo $? > 0 > > # virsh iface-start eth2 > Interface eth2 started > > # echo $? > 0 > > Same applies for destroying a dead interface. We should not allow such > state transitions. > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > src/interface/interface_backend_netcf.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c > index 2e681ec..c525ca9 100644 > --- a/src/interface/interface_backend_netcf.c > +++ b/src/interface/interface_backend_netcf.c > @@ -944,6 +944,7 @@ static int netcfInterfaceCreate(virInterfacePtr ifinfo, > struct netcf_if *iface = NULL; > virInterfaceDefPtr def = NULL; > int ret = -1; > + bool active; > > virCheckFlags(0, -1); > > @@ -962,6 +963,15 @@ static int netcfInterfaceCreate(virInterfacePtr ifinfo, > if (virInterfaceCreateEnsureACL(ifinfo->conn, def) < 0) > goto cleanup; > > + if (netcfInterfaceObjIsActive(iface, &active) < 0) > + goto cleanup; > + > + if (active) { > + virReportError(VIR_ERR_OPERATION_INVALID, "%s", > + _("interface is already running")); Do we want to use the word "running" here? The flags to virConnectListAllInterfaces() use the term "active" rather than "running". virConnectListAllNetworks() does likewise, and its error message when trying to start an active network is "network is already active". On the other hand, the flags for virConnectListAllDomains() also uses "active" in the flag name, but "domain is already running" for the error message. So there is precedence for either, but I like "active" better (and an interface is closer to a network than a domain :-) > + goto cleanup; > + } > + > ret = ncf_if_up(iface); > if (ret < 0) { > const char *errmsg, *details; > @@ -987,6 +997,7 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo, > struct netcf_if *iface = NULL; > virInterfaceDefPtr def = NULL; > int ret = -1; > + bool active; > > virCheckFlags(0, -1); > > @@ -1005,6 +1016,15 @@ static int netcfInterfaceDestroy(virInterfacePtr ifinfo, > if (virInterfaceDestroyEnsureACL(ifinfo->conn, def) < 0) > goto cleanup; > > + if (netcfInterfaceObjIsActive(iface, &active) < 0) > + goto cleanup; > + > + if (!active) { > + virReportError(VIR_ERR_OPERATION_INVALID, "%s", > + _("interface is not running")); Same comment here. > + goto cleanup; > + } > + > ret = ncf_if_down(iface); > if (ret < 0) { > const char *errmsg, *details; ACK with the changes to the error messages (or without them if you have a strong opinion in the other direction). P.S. Be prepared for any fallout from the change in semantics! -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list