1. Patchew
  2. Libvirt
  3. View series

[PATCH] virsh: Make --type argument of detach-interface optional

Michal Privoznik via Devel posted 1 patch 6 days, 12 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/4252cfb16fadd1540a14a285d48f857c744d51f0.1781769497.git.mprivozn@redhat.com
docs/manpages/virsh.rst |  2 +-
tools/virsh-domain.c    | 13 ++++++++++---
2 files changed, 11 insertions(+), 4 deletions(-)
[PATCH] virsh: Make --type argument of detach-interface optional
Posted by Michal Privoznik via Devel 6 days, 12 hours ago 
From: Michal Privoznik <mprivozn@redhat.com>

The detach-interface virsh command requires domain (obviously)
and --type to identify <interface/>. Optionally, --mac can be
provided to chose from multiple interfaces. Well, that renders
--type argument redundant. I mean, if there are but unique MACs
within domain XML, then interface type is implied. If there are
duplicate MACs then --type can help to differentiate, though at
that point detach-device seems like a better fit.

Long story short, make --type optional.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 docs/manpages/virsh.rst |  2 +-
 tools/virsh-domain.c    | 13 ++++++++++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 39d91d8375..2992495ecb 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -5586,7 +5586,7 @@ detach-interface
 
 ::
 
-   detach-interface domain type [--mac mac]
+   detach-interface domain [--type type] [--mac mac]
       [[[--live] [--config] | [--current]] | [--persistent]] [--print-xml]
 
 Detach a network interface from a domain.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f9de3b42a9..8a6f868e34 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -12782,7 +12782,6 @@ static const vshCmdOptDef opts_detach_interface[] = {
     {.name = "type",
      .type = VSH_OT_STRING,
      .positional = true,
-     .required = true,
      .help = N_("network interface type")
     },
     {.name = "mac",
@@ -12816,7 +12815,7 @@ virshDomainDetachInterface(char *doc,
     g_autoptr(xmlDoc) xml = NULL;
     g_autoptr(xmlXPathContext) ctxt = NULL;
     g_autofree char *detach_xml = NULL;
-    g_autofree char *xpath = g_strdup_printf("/domain/devices/interface[@type='%s']", type);
+    g_autofree char *xpath = NULL;
     g_autofree xmlNodePtr *nodes = NULL;
     ssize_t nnodes;
     xmlNodePtr matchNode = NULL;
@@ -12827,8 +12826,16 @@ virshDomainDetachInterface(char *doc,
         return false;
     }
 
+    if (type)
+        xpath = g_strdup_printf("/domain/devices/interface[@type='%s']", type);
+    else
+        xpath = g_strdup("/domain/devices/interface");
+
     if ((nnodes = virXPathNodeSet(xpath, ctxt, &nodes)) <= 0) {
-        vshError(ctl, _("No interface found whose type is %1$s"), type);
+        if (type)
+            vshError(ctl, _("No interface found whose type is %1$s"), type);
+        else
+            vshError(ctl, "%s", _("Domain has no interfaces"));
         return false;
     }
 
-- 
2.53.0
Re: [PATCH] virsh: Make --type argument of detach-interface optional
Posted by Peter Krempa via Devel 6 days, 11 hours ago 
On Thu, Jun 18, 2026 at 09:58:17 +0200, Michal Privoznik via Devel wrote:
> From: Michal Privoznik <mprivozn@redhat.com>
> 
> The detach-interface virsh command requires domain (obviously)
> and --type to identify <interface/>. Optionally, --mac can be
> provided to chose from multiple interfaces. Well, that renders
> --type argument redundant. I mean, if there are but unique MACs
> within domain XML, then interface type is implied. If there are
> duplicate MACs then --type can help to differentiate, though at
> that point detach-device seems like a better fit.
> 
> Long story short, make --type optional.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  docs/manpages/virsh.rst |  2 +-
>  tools/virsh-domain.c    | 13 ++++++++++---
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index 39d91d8375..2992495ecb 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -5586,7 +5586,7 @@ detach-interface
>  
>  ::
>  
> -   detach-interface domain type [--mac mac]
> +   detach-interface domain [--type type] [--mac mac]

The syntax for an optional positional argument is: [<type>] per what our
help prints:

 SYNOPSIS
    detach-interface <domain> [<type>] [--mac <string>] [--persistent] [--config] [--live] [--current] [--print-xml]



>        [[[--live] [--config] | [--current]] | [--persistent]] [--print-xml]
>  
>  Detach a network interface from a domain.
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index f9de3b42a9..8a6f868e34 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -12782,7 +12782,6 @@ static const vshCmdOptDef opts_detach_interface[] = {
>      {.name = "type",
>       .type = VSH_OT_STRING,
>       .positional = true,
> -     .required = true,
>       .help = N_("network interface type")
>      },
>      {.name = "mac",
> @@ -12816,7 +12815,7 @@ virshDomainDetachInterface(char *doc,
>      g_autoptr(xmlDoc) xml = NULL;
>      g_autoptr(xmlXPathContext) ctxt = NULL;
>      g_autofree char *detach_xml = NULL;
> -    g_autofree char *xpath = g_strdup_printf("/domain/devices/interface[@type='%s']", type);
> +    g_autofree char *xpath = NULL;
>      g_autofree xmlNodePtr *nodes = NULL;
>      ssize_t nnodes;
>      xmlNodePtr matchNode = NULL;
> @@ -12827,8 +12826,16 @@ virshDomainDetachInterface(char *doc,
>          return false;
>      }
>  
> +    if (type)
> +        xpath = g_strdup_printf("/domain/devices/interface[@type='%s']", type);
> +    else
> +        xpath = g_strdup("/domain/devices/interface");
> +
>      if ((nnodes = virXPathNodeSet(xpath, ctxt, &nodes)) <= 0) {
> -        vshError(ctl, _("No interface found whose type is %1$s"), type);
> +        if (type)
> +            vshError(ctl, _("No interface found whose type is %1$s"), type);
> +        else
> +            vshError(ctl, "%s", _("Domain has no interfaces"));

Looking at the code, if neither 'mac' nor 'type' are specified it
detaches the only interface if there's exactly one.

I think I'd reword the man page section:

 Detach a network interface from a domain.
 *type* can be either *network* to indicate a physical network device or
 *bridge* to indicate a bridge to a device. It is recommended to use the
 *mac* option to distinguish between the interfaces if more than one are
 present on the domain.

to something like:

*type* and/or *mac* can be used to narrow

 Detach a network interface from a domain.

 In case when multiple interfaces are present in the *type* and/or
 *--mac* must be used to narrow down the selection to 1 interface.

 *type* can be either *network* to indicate a physical network device
 *bridge* to indicate a bridge to a device.

 *--mac* can select the interface based on the configured MAC address.




Reviewed-by: Peter Krempa <pkrempa@redhat.com>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.