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
> ---
> 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
