[libvirt] [PATCH 0/8] virsh: Introduce more completers
It'd be nice if we could autocomplete the basic types like pools, volumes, networks, ... before the release. Michal Privoznik (8): virsh: Introduce virshStoragePoolNameCompleter virsh: Introduce virshStorageVolNameCompleter virsh: Introduce virshInterfaceNameCompleter virsh: Introduce virshNetworkNameCompleter virsh: Introduce virshNodeDeviceNameCompleter virsh: Introduce virshNWFilterNameCompleter virsh: Introduce virshSecretUUIDCompleter virsh: Introduce virshSnapshotNameCompleter tools/virsh-completer.c | 388 ++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 32 ++++ tools/virsh-interface.c | 16 +- tools/virsh-network.c | 24 +-- tools/virsh-nodedev.c | 16 +- tools/virsh-nwfilter.c | 9 +- tools/virsh-pool.c | 28 ++-- tools/virsh-secret.c | 15 +- tools/virsh-snapshot.c | 21 ++- tools/virsh-volume.c | 45 +++--- tools/virsh.h | 6 +- 11 files changed, 525 insertions(+), 75 deletions(-) -- 2.13.6
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-pool.c | 28 +++++++++++++-------------- tools/virsh-volume.c | 42 +++++++++++++++++++++------------------- tools/virsh.h | 6 ++++-- 5 files changed, 95 insertions(+), 36 deletions(-) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index 48dd9fbc2..8ca2fffd9 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -147,3 +147,54 @@ virshDomainInterfaceCompleter(vshControl *ctl, virStringListFree(ret); return NULL; } + + +char ** +virshStoragePoolNameCompleter(vshControl *ctl, + const vshCmd *cmd ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + virStoragePoolPtr *pools = NULL; + int npools = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE | + VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | + VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT | + VIR_CONNECT_LIST_STORAGE_POOLS_TRANSIENT | + VIR_CONNECT_LIST_STORAGE_POOLS_AUTOSTART | + VIR_CONNECT_LIST_STORAGE_POOLS_NO_AUTOSTART, + NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if ((npools = virConnectListAllStoragePools(priv->conn, &pools, flags)) < 0) + return NULL; + + if (VIR_ALLOC_N(ret, npools + 1) < 0) + goto error; + + for (i = 0; i < npools; i++) { + const char *name = virStoragePoolGetName(pools[i]); + + if (VIR_STRDUP(ret[i], name) < 0) + goto error; + + virStoragePoolFree(pools[i]); + } + VIR_FREE(pools); + + return ret; + + error: + for (; i < npools; i++) + virStoragePoolFree(pools[i]); + VIR_FREE(pools); + for (i = 0; i < npools; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 1a2dd685f..249e793b9 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -38,4 +38,8 @@ char ** virshDomainInterfaceCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); +char ** virshStoragePoolNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + #endif diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 094874b64..cea4cfc12 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -34,8 +34,8 @@ #include "virstring.h" #include "virtime.h" -#define VIRSH_COMMON_OPT_POOL_FULL \ - VIRSH_COMMON_OPT_POOL(N_("pool name or uuid")) +#define VIRSH_COMMON_OPT_POOL_FULL(cflags) \ + VIRSH_COMMON_OPT_POOL(N_("pool name or uuid"), cflags) #define VIRSH_COMMON_OPT_POOL_BUILD \ {.name = "build", \ @@ -182,7 +182,7 @@ static const vshCmdInfo info_pool_autostart[] = { }; static const vshCmdOptDef opts_pool_autostart[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = "disable", .type = VSH_OT_BOOL, @@ -575,7 +575,7 @@ static const vshCmdInfo info_pool_build[] = { }; static const vshCmdOptDef opts_pool_build[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), VIRSH_COMMON_OPT_POOL_NO_OVERWRITE, VIRSH_COMMON_OPT_POOL_OVERWRITE, @@ -625,7 +625,7 @@ static const vshCmdInfo info_pool_destroy[] = { }; static const vshCmdOptDef opts_pool_destroy[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE), {.name = NULL} }; @@ -665,7 +665,7 @@ static const vshCmdInfo info_pool_delete[] = { }; static const vshCmdOptDef opts_pool_delete[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = NULL} }; @@ -705,7 +705,7 @@ static const vshCmdInfo info_pool_refresh[] = { }; static const vshCmdOptDef opts_pool_refresh[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = NULL} }; @@ -745,7 +745,7 @@ static const vshCmdInfo info_pool_dumpxml[] = { }; static const vshCmdOptDef opts_pool_dumpxml[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = "inactive", .type = VSH_OT_BOOL, @@ -1636,7 +1636,7 @@ static const vshCmdInfo info_pool_info[] = { }; static const vshCmdOptDef opts_pool_info[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = "bytes", .type = VSH_OT_BOOL, @@ -1726,7 +1726,7 @@ static const vshCmdInfo info_pool_name[] = { }; static const vshCmdOptDef opts_pool_name[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = NULL} }; @@ -1758,7 +1758,7 @@ static const vshCmdInfo info_pool_start[] = { }; static const vshCmdOptDef opts_pool_start[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE), VIRSH_COMMON_OPT_POOL_BUILD, VIRSH_COMMON_OPT_POOL_NO_OVERWRITE, VIRSH_COMMON_OPT_POOL_OVERWRITE, @@ -1819,7 +1819,7 @@ static const vshCmdInfo info_pool_undefine[] = { }; static const vshCmdOptDef opts_pool_undefine[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT), {.name = NULL} }; @@ -1859,7 +1859,7 @@ static const vshCmdInfo info_pool_uuid[] = { }; static const vshCmdOptDef opts_pool_uuid[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = NULL} }; @@ -1896,7 +1896,7 @@ static const vshCmdInfo info_pool_edit[] = { }; static const vshCmdOptDef opts_pool_edit[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = NULL} }; diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 8265a3979..b96e205f7 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -43,16 +43,18 @@ #include "virxml.h" #include "virstring.h" -#define VIRSH_COMMON_OPT_POOL_FULL \ - VIRSH_COMMON_OPT_POOL(N_("pool name or uuid")) +#define VIRSH_COMMON_OPT_POOL_FULL(cflags) \ + VIRSH_COMMON_OPT_POOL(N_("pool name or uuid"), cflags) -#define VIRSH_COMMON_OPT_POOL_NAME \ - VIRSH_COMMON_OPT_POOL(N_("pool name")) +#define VIRSH_COMMON_OPT_POOL_NAME(cflags) \ + VIRSH_COMMON_OPT_POOL(N_("pool name"), cflags) -#define VIRSH_COMMON_OPT_POOL_OPTIONAL \ +#define VIRSH_COMMON_OPT_POOL_OPTIONAL(cflags) \ {.name = "pool", \ .type = VSH_OT_STRING, \ - .help = N_("pool name or uuid") \ + .help = N_("pool name or uuid"), \ + .completer = virshStoragePoolNameCompleter, \ + .completer_flags = cflags, \ } #define VIRSH_COMMON_OPT_VOLUME_VOL \ @@ -165,7 +167,7 @@ static const vshCmdInfo info_vol_create_as[] = { }; static const vshCmdOptDef opts_vol_create_as[] = { - VIRSH_COMMON_OPT_POOL_NAME, + VIRSH_COMMON_OPT_POOL_NAME(0), {.name = "name", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -378,7 +380,7 @@ static const vshCmdInfo info_vol_create[] = { }; static const vshCmdOptDef opts_vol_create[] = { - VIRSH_COMMON_OPT_POOL_NAME, + VIRSH_COMMON_OPT_POOL_NAME(0), VIRSH_COMMON_OPT_FILE(N_("file containing an XML vol description")), {.name = "prealloc-metadata", .type = VSH_OT_BOOL, @@ -440,7 +442,7 @@ static const vshCmdInfo info_vol_create_from[] = { }; static const vshCmdOptDef opts_vol_create_from[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), VIRSH_COMMON_OPT_FILE(N_("file containing an XML vol description")), VIRSH_COMMON_OPT_VOLUME_VOL, {.name = "inputpool", @@ -559,7 +561,7 @@ static const vshCmdOptDef opts_vol_clone[] = { .flags = VSH_OFLAG_REQ, .help = N_("clone name") }, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = "prealloc-metadata", .type = VSH_OT_BOOL, .help = N_("preallocate metadata (for qcow2 instead of full allocation)") @@ -651,7 +653,7 @@ static const vshCmdInfo info_vol_upload[] = { static const vshCmdOptDef opts_vol_upload[] = { VIRSH_COMMON_OPT_VOLUME_VOL, VIRSH_COMMON_OPT_FILE(N_("file")), - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = "offset", .type = VSH_OT_INT, .help = N_("volume offset to upload to") @@ -766,7 +768,7 @@ static const vshCmdInfo info_vol_download[] = { static const vshCmdOptDef opts_vol_download[] = { VIRSH_COMMON_OPT_VOLUME_VOL, VIRSH_COMMON_OPT_FILE(N_("file")), - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = "offset", .type = VSH_OT_INT, .help = N_("volume offset to download from") @@ -875,7 +877,7 @@ static const vshCmdInfo info_vol_delete[] = { static const vshCmdOptDef opts_vol_delete[] = { VIRSH_COMMON_OPT_VOLUME_VOL, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = "delete-snapshots", .type = VSH_OT_BOOL, .help = N_("delete snapshots associated with volume (must be " @@ -925,7 +927,7 @@ static const vshCmdInfo info_vol_wipe[] = { static const vshCmdOptDef opts_vol_wipe[] = { VIRSH_COMMON_OPT_VOLUME_VOL, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = "algorithm", .type = VSH_OT_STRING, .help = N_("perform selected wiping algorithm") @@ -1012,7 +1014,7 @@ static const vshCmdInfo info_vol_info[] = { static const vshCmdOptDef opts_vol_info[] = { VIRSH_COMMON_OPT_VOLUME_VOL, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = "bytes", .type = VSH_OT_BOOL, .help = N_("sizes are represented in bytes rather than pretty units") @@ -1107,7 +1109,7 @@ static const vshCmdOptDef opts_vol_resize[] = { .flags = VSH_OFLAG_REQ, .help = N_("new capacity for the vol, as scaled integer (default bytes)") }, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = "allocate", .type = VSH_OT_BOOL, .help = N_("allocate the new capacity, rather than leaving it sparse") @@ -1199,7 +1201,7 @@ static const vshCmdInfo info_vol_dumpxml[] = { static const vshCmdOptDef opts_vol_dumpxml[] = { VIRSH_COMMON_OPT_VOLUME_VOL, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = NULL} }; @@ -1363,7 +1365,7 @@ static const vshCmdInfo info_vol_list[] = { }; static const vshCmdOptDef opts_vol_list[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = "details", .type = VSH_OT_BOOL, .help = N_("display extended details for volumes") @@ -1705,7 +1707,7 @@ static const vshCmdOptDef opts_vol_key[] = { .flags = VSH_OFLAG_REQ, .help = N_("volume name or path") }, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = NULL} }; @@ -1741,7 +1743,7 @@ static const vshCmdOptDef opts_vol_path[] = { .flags = VSH_OFLAG_REQ, .help = N_("volume name or key") }, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = NULL} }; diff --git a/tools/virsh.h b/tools/virsh.h index 528e04558..f2213ebb5 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -64,11 +64,13 @@ /* * Common command options */ -# define VIRSH_COMMON_OPT_POOL(_helpstr) \ +# define VIRSH_COMMON_OPT_POOL(_helpstr, cflags) \ {.name = "pool", \ .type = VSH_OT_DATA, \ .flags = VSH_OFLAG_REQ, \ - .help = _helpstr \ + .help = _helpstr, \ + .completer = virshStoragePoolNameCompleter, \ + .completer_flags = cflags, \ } # define VIRSH_COMMON_OPT_DOMAIN(_helpstr, cflags) \ -- 2.13.6
On 01/12/2018 09:37 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-pool.c | 28 +++++++++++++-------------- tools/virsh-volume.c | 42 +++++++++++++++++++++------------------- tools/virsh.h | 6 ++++-- 5 files changed, 95 insertions(+), 36 deletions(-)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index 48dd9fbc2..8ca2fffd9 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -147,3 +147,54 @@ virshDomainInterfaceCompleter(vshControl *ctl, virStringListFree(ret); return NULL; } + + +char ** +virshStoragePoolNameCompleter(vshControl *ctl, + const vshCmd *cmd ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + virStoragePoolPtr *pools = NULL; + int npools = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE | + VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | + VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT | + VIR_CONNECT_LIST_STORAGE_POOLS_TRANSIENT | + VIR_CONNECT_LIST_STORAGE_POOLS_AUTOSTART | + VIR_CONNECT_LIST_STORAGE_POOLS_NO_AUTOSTART, + NULL);
So for this and other patches, is this list right? For various storage consumers, all I see necessary is 0, ACTIVE, INACTIVE, and PERSISTENT. For that matter VIRSH_COMMON_OPT_DOMAIN_FULL doesn't seem to use the full set from virshDomainNameCompleter either... I see 0, ACTIVE, RUNNING, OTHER, PERSISTENT, PAUSED, and INACTIVE. Also, it seems there's a dual usage model where either various CONNECT flags are used or the Completer function could "roll it's own"? That is the virshDomainInterfaceCompleter reads the XML files and checks flags against VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC (e.g. 1 << 0) which is the same as VIR_CONNECT_LIST_INTERFACES_INACTIVE. A few patches later we have virshInterfaceNameCompleter that's using the same CONNECT flags. It could be construed as confusing... Took me a few to realize the usage. FWIW: in the IIRC the "odd" thing about MAC's is that they could be added once the vNIC is active - that is they'll eventually show up. So just because it's not in the config/inactive XML doesn't mean it won't be present. So should the code be only reading the XML? Maybe it'd be better to add a CONNECT flag that returns when a MAC is present and then let this code choose the active/inactive + new connect mac flag. I dunno. I know separate issue, patches welcome, yadda yadda, but figured I'd bring it up.
+ + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL;
Since this same check is copied everywhere, why not move it to the caller (e.g. vshReadlineParse) ?
+ + if ((npools = virConnectListAllStoragePools(priv->conn, &pools, flags)) < 0) + return NULL; + + if (VIR_ALLOC_N(ret, npools + 1) < 0) + goto error; + + for (i = 0; i < npools; i++) { + const char *name = virStoragePoolGetName(pools[i]); + + if (VIR_STRDUP(ret[i], name) < 0) + goto error; + + virStoragePoolFree(pools[i]); + } + VIR_FREE(pools); + + return ret; + + error: + for (; i < npools; i++) + virStoragePoolFree(pools[i]); + VIR_FREE(pools); + for (i = 0; i < npools; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 1a2dd685f..249e793b9 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -38,4 +38,8 @@ char ** virshDomainInterfaceCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags);
+char ** virshStoragePoolNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + #endif diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 094874b64..cea4cfc12 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -34,8 +34,8 @@ #include "virstring.h" #include "virtime.h"
-#define VIRSH_COMMON_OPT_POOL_FULL \ - VIRSH_COMMON_OPT_POOL(N_("pool name or uuid")) +#define VIRSH_COMMON_OPT_POOL_FULL(cflags) \ + VIRSH_COMMON_OPT_POOL(N_("pool name or uuid"), cflags)
#define VIRSH_COMMON_OPT_POOL_BUILD \ {.name = "build", \ @@ -182,7 +182,7 @@ static const vshCmdInfo info_pool_autostart[] = { };
static const vshCmdOptDef opts_pool_autostart[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
{.name = "disable", .type = VSH_OT_BOOL, @@ -575,7 +575,7 @@ static const vshCmdInfo info_pool_build[] = { };
static const vshCmdOptDef opts_pool_build[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), VIRSH_COMMON_OPT_POOL_NO_OVERWRITE, VIRSH_COMMON_OPT_POOL_OVERWRITE,
@@ -625,7 +625,7 @@ static const vshCmdInfo info_pool_destroy[] = { };
static const vshCmdOptDef opts_pool_destroy[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE),
{.name = NULL} }; @@ -665,7 +665,7 @@ static const vshCmdInfo info_pool_delete[] = { };
static const vshCmdOptDef opts_pool_delete[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
Delete does not work if ACTIVE... Perhaps this should be INACTIVE (which also I think would mean PERSISTENT).
{.name = NULL} }; @@ -705,7 +705,7 @@ static const vshCmdInfo info_pool_refresh[] = { };
static const vshCmdOptDef opts_pool_refresh[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
{.name = NULL} }; @@ -745,7 +745,7 @@ static const vshCmdInfo info_pool_dumpxml[] = { };
static const vshCmdOptDef opts_pool_dumpxml[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
{.name = "inactive", .type = VSH_OT_BOOL, @@ -1636,7 +1636,7 @@ static const vshCmdInfo info_pool_info[] = { };
static const vshCmdOptDef opts_pool_info[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
{.name = "bytes", .type = VSH_OT_BOOL, @@ -1726,7 +1726,7 @@ static const vshCmdInfo info_pool_name[] = { };
static const vshCmdOptDef opts_pool_name[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
{.name = NULL} }; @@ -1758,7 +1758,7 @@ static const vshCmdInfo info_pool_start[] = { };
static const vshCmdOptDef opts_pool_start[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE), VIRSH_COMMON_OPT_POOL_BUILD, VIRSH_COMMON_OPT_POOL_NO_OVERWRITE, VIRSH_COMMON_OPT_POOL_OVERWRITE, @@ -1819,7 +1819,7 @@ static const vshCmdInfo info_pool_undefine[] = { };
static const vshCmdOptDef opts_pool_undefine[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT),
{.name = NULL} }; @@ -1859,7 +1859,7 @@ static const vshCmdInfo info_pool_uuid[] = { };
static const vshCmdOptDef opts_pool_uuid[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
{.name = NULL} }; @@ -1896,7 +1896,7 @@ static const vshCmdInfo info_pool_edit[] = { };
static const vshCmdOptDef opts_pool_edit[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
{.name = NULL} }; diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 8265a3979..b96e205f7 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -43,16 +43,18 @@ #include "virxml.h" #include "virstring.h"
-#define VIRSH_COMMON_OPT_POOL_FULL \ - VIRSH_COMMON_OPT_POOL(N_("pool name or uuid")) +#define VIRSH_COMMON_OPT_POOL_FULL(cflags) \ + VIRSH_COMMON_OPT_POOL(N_("pool name or uuid"), cflags)
-#define VIRSH_COMMON_OPT_POOL_NAME \ - VIRSH_COMMON_OPT_POOL(N_("pool name")) +#define VIRSH_COMMON_OPT_POOL_NAME(cflags) \ + VIRSH_COMMON_OPT_POOL(N_("pool name"), cflags)
-#define VIRSH_COMMON_OPT_POOL_OPTIONAL \ +#define VIRSH_COMMON_OPT_POOL_OPTIONAL(cflags) \ {.name = "pool", \ .type = VSH_OT_STRING, \ - .help = N_("pool name or uuid") \ + .help = N_("pool name or uuid"), \ + .completer = virshStoragePoolNameCompleter, \ + .completer_flags = cflags, \ }
#define VIRSH_COMMON_OPT_VOLUME_VOL \ @@ -165,7 +167,7 @@ static const vshCmdInfo info_vol_create_as[] = { };
Will any of the vol functions work if the pool is not ACTIVE? John
static const vshCmdOptDef opts_vol_create_as[] = { - VIRSH_COMMON_OPT_POOL_NAME, + VIRSH_COMMON_OPT_POOL_NAME(0), {.name = "name", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -378,7 +380,7 @@ static const vshCmdInfo info_vol_create[] = { };
static const vshCmdOptDef opts_vol_create[] = { - VIRSH_COMMON_OPT_POOL_NAME, + VIRSH_COMMON_OPT_POOL_NAME(0), VIRSH_COMMON_OPT_FILE(N_("file containing an XML vol description")), {.name = "prealloc-metadata", .type = VSH_OT_BOOL, @@ -440,7 +442,7 @@ static const vshCmdInfo info_vol_create_from[] = { };
static const vshCmdOptDef opts_vol_create_from[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), VIRSH_COMMON_OPT_FILE(N_("file containing an XML vol description")), VIRSH_COMMON_OPT_VOLUME_VOL, {.name = "inputpool", @@ -559,7 +561,7 @@ static const vshCmdOptDef opts_vol_clone[] = { .flags = VSH_OFLAG_REQ, .help = N_("clone name") }, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = "prealloc-metadata", .type = VSH_OT_BOOL, .help = N_("preallocate metadata (for qcow2 instead of full allocation)") @@ -651,7 +653,7 @@ static const vshCmdInfo info_vol_upload[] = { static const vshCmdOptDef opts_vol_upload[] = { VIRSH_COMMON_OPT_VOLUME_VOL, VIRSH_COMMON_OPT_FILE(N_("file")), - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = "offset", .type = VSH_OT_INT, .help = N_("volume offset to upload to") @@ -766,7 +768,7 @@ static const vshCmdInfo info_vol_download[] = { static const vshCmdOptDef opts_vol_download[] = { VIRSH_COMMON_OPT_VOLUME_VOL, VIRSH_COMMON_OPT_FILE(N_("file")), - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = "offset", .type = VSH_OT_INT, .help = N_("volume offset to download from") @@ -875,7 +877,7 @@ static const vshCmdInfo info_vol_delete[] = {
static const vshCmdOptDef opts_vol_delete[] = { VIRSH_COMMON_OPT_VOLUME_VOL, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = "delete-snapshots", .type = VSH_OT_BOOL, .help = N_("delete snapshots associated with volume (must be " @@ -925,7 +927,7 @@ static const vshCmdInfo info_vol_wipe[] = {
static const vshCmdOptDef opts_vol_wipe[] = { VIRSH_COMMON_OPT_VOLUME_VOL, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = "algorithm", .type = VSH_OT_STRING, .help = N_("perform selected wiping algorithm") @@ -1012,7 +1014,7 @@ static const vshCmdInfo info_vol_info[] = {
static const vshCmdOptDef opts_vol_info[] = { VIRSH_COMMON_OPT_VOLUME_VOL, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = "bytes", .type = VSH_OT_BOOL, .help = N_("sizes are represented in bytes rather than pretty units") @@ -1107,7 +1109,7 @@ static const vshCmdOptDef opts_vol_resize[] = { .flags = VSH_OFLAG_REQ, .help = N_("new capacity for the vol, as scaled integer (default bytes)") }, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = "allocate", .type = VSH_OT_BOOL, .help = N_("allocate the new capacity, rather than leaving it sparse") @@ -1199,7 +1201,7 @@ static const vshCmdInfo info_vol_dumpxml[] = {
static const vshCmdOptDef opts_vol_dumpxml[] = { VIRSH_COMMON_OPT_VOLUME_VOL, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = NULL} };
@@ -1363,7 +1365,7 @@ static const vshCmdInfo info_vol_list[] = { };
static const vshCmdOptDef opts_vol_list[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = "details", .type = VSH_OT_BOOL, .help = N_("display extended details for volumes") @@ -1705,7 +1707,7 @@ static const vshCmdOptDef opts_vol_key[] = { .flags = VSH_OFLAG_REQ, .help = N_("volume name or path") }, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = NULL} };
@@ -1741,7 +1743,7 @@ static const vshCmdOptDef opts_vol_path[] = { .flags = VSH_OFLAG_REQ, .help = N_("volume name or key") }, - VIRSH_COMMON_OPT_POOL_OPTIONAL, + VIRSH_COMMON_OPT_POOL_OPTIONAL(0), {.name = NULL} };
diff --git a/tools/virsh.h b/tools/virsh.h index 528e04558..f2213ebb5 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -64,11 +64,13 @@ /* * Common command options */ -# define VIRSH_COMMON_OPT_POOL(_helpstr) \ +# define VIRSH_COMMON_OPT_POOL(_helpstr, cflags) \ {.name = "pool", \ .type = VSH_OT_DATA, \ .flags = VSH_OFLAG_REQ, \ - .help = _helpstr \ + .help = _helpstr, \ + .completer = virshStoragePoolNameCompleter, \ + .completer_flags = cflags, \ }
# define VIRSH_COMMON_OPT_DOMAIN(_helpstr, cflags) \
On 01/22/2018 07:22 PM, John Ferlan wrote:
On 01/12/2018 09:37 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-pool.c | 28 +++++++++++++-------------- tools/virsh-volume.c | 42 +++++++++++++++++++++------------------- tools/virsh.h | 6 ++++-- 5 files changed, 95 insertions(+), 36 deletions(-)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index 48dd9fbc2..8ca2fffd9 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -147,3 +147,54 @@ virshDomainInterfaceCompleter(vshControl *ctl, virStringListFree(ret); return NULL; } + + +char ** +virshStoragePoolNameCompleter(vshControl *ctl, + const vshCmd *cmd ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + virStoragePoolPtr *pools = NULL; + int npools = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE | + VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | + VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT | + VIR_CONNECT_LIST_STORAGE_POOLS_TRANSIENT | + VIR_CONNECT_LIST_STORAGE_POOLS_AUTOSTART | + VIR_CONNECT_LIST_STORAGE_POOLS_NO_AUTOSTART, + NULL);
So for this and other patches, is this list right? For various storage consumers, all I see necessary is 0, ACTIVE, INACTIVE, and PERSISTENT.
Oh right. I was writing these in a hurry as I wanted to have them in the release. But you're right. We don't need all the flags.
For that matter VIRSH_COMMON_OPT_DOMAIN_FULL doesn't seem to use the full set from virshDomainNameCompleter either... I see 0, ACTIVE, RUNNING, OTHER, PERSISTENT, PAUSED, and INACTIVE.
Also, it seems there's a dual usage model where either various CONNECT flags are used or the Completer function could "roll its own"?
Yes. There are no restrictions places on the @flags.
That is the virshDomainInterfaceCompleter reads the XML files and checks flags against VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC (e.g. 1 << 0) which is the same as VIR_CONNECT_LIST_INTERFACES_INACTIVE.
That is just a coincidence. The flags are distinct in meaning, they only share the same integer value. Just like VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE and VIR_DOMAIN_STATS_CPU_TOTAL (both 1 << 0).
A few patches later we have virshInterfaceNameCompleter that's using the same CONNECT flags. It could be construed as confusing... Took me a few to realize the usage.
Okay, I can document it. Although I think I covered it in the commit message for virshDomainInterfaceCompleter. But it's better to have it in the code apparently.
FWIW: in the IIRC the "odd" thing about MAC's is that they could be added once the vNIC is active - that is they'll eventually show up. So just because it's not in the config/inactive XML doesn't mean it won't be present. So should the code be only reading the XML? Maybe it'd be better to add a CONNECT flag that returns when a MAC is present and then let this code choose the active/inactive + new connect mac flag. I dunno. I know separate issue, patches welcome, yadda yadda, but figured I'd bring it up.
Hm. I don't think that MACs can be left out from inactive XML. In the virDomainNetDefParseXML() function, if MAC is not present it's generated (regardless of the type of vNIC).
+ + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL;
Since this same check is copied everywhere, why not move it to the caller (e.g. vshReadlineParse) ?
Because not every callback needs connection. I mean, yes all current do, but in theory they don't. For instance "attach-disk --sourcetype" where we can have only two options ("block", "file") we don't need a connection.
+ + if ((npools = virConnectListAllStoragePools(priv->conn, &pools, flags)) < 0) + return NULL; + + if (VIR_ALLOC_N(ret, npools + 1) < 0) + goto error; + + for (i = 0; i < npools; i++) { + const char *name = virStoragePoolGetName(pools[i]); + + if (VIR_STRDUP(ret[i], name) < 0) + goto error; + + virStoragePoolFree(pools[i]); + } + VIR_FREE(pools); + + return ret; + + error: + for (; i < npools; i++) + virStoragePoolFree(pools[i]); + VIR_FREE(pools); + for (i = 0; i < npools; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 1a2dd685f..249e793b9 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -38,4 +38,8 @@ char ** virshDomainInterfaceCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags);
+char ** virshStoragePoolNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + #endif diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 094874b64..cea4cfc12 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -34,8 +34,8 @@ #include "virstring.h" #include "virtime.h"
-#define VIRSH_COMMON_OPT_POOL_FULL \ - VIRSH_COMMON_OPT_POOL(N_("pool name or uuid")) +#define VIRSH_COMMON_OPT_POOL_FULL(cflags) \ + VIRSH_COMMON_OPT_POOL(N_("pool name or uuid"), cflags)
#define VIRSH_COMMON_OPT_POOL_BUILD \ {.name = "build", \ @@ -182,7 +182,7 @@ static const vshCmdInfo info_pool_autostart[] = { };
static const vshCmdOptDef opts_pool_autostart[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
{.name = "disable", .type = VSH_OT_BOOL, @@ -575,7 +575,7 @@ static const vshCmdInfo info_pool_build[] = { };
static const vshCmdOptDef opts_pool_build[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), VIRSH_COMMON_OPT_POOL_NO_OVERWRITE, VIRSH_COMMON_OPT_POOL_OVERWRITE,
@@ -625,7 +625,7 @@ static const vshCmdInfo info_pool_destroy[] = { };
static const vshCmdOptDef opts_pool_destroy[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE),
{.name = NULL} }; @@ -665,7 +665,7 @@ static const vshCmdInfo info_pool_delete[] = { };
static const vshCmdOptDef opts_pool_delete[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
Delete does not work if ACTIVE... Perhaps this should be INACTIVE (which also I think would mean PERSISTENT).
Ah, good catch.
{.name = NULL} }; @@ -705,7 +705,7 @@ static const vshCmdInfo info_pool_refresh[] = { };
static const vshCmdOptDef opts_pool_refresh[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
{.name = NULL} }; @@ -745,7 +745,7 @@ static const vshCmdInfo info_pool_dumpxml[] = { };
static const vshCmdOptDef opts_pool_dumpxml[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
{.name = "inactive", .type = VSH_OT_BOOL, @@ -1636,7 +1636,7 @@ static const vshCmdInfo info_pool_info[] = { };
static const vshCmdOptDef opts_pool_info[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
{.name = "bytes", .type = VSH_OT_BOOL, @@ -1726,7 +1726,7 @@ static const vshCmdInfo info_pool_name[] = { };
static const vshCmdOptDef opts_pool_name[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
{.name = NULL} }; @@ -1758,7 +1758,7 @@ static const vshCmdInfo info_pool_start[] = { };
static const vshCmdOptDef opts_pool_start[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE), VIRSH_COMMON_OPT_POOL_BUILD, VIRSH_COMMON_OPT_POOL_NO_OVERWRITE, VIRSH_COMMON_OPT_POOL_OVERWRITE, @@ -1819,7 +1819,7 @@ static const vshCmdInfo info_pool_undefine[] = { };
static const vshCmdOptDef opts_pool_undefine[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT),
{.name = NULL} }; @@ -1859,7 +1859,7 @@ static const vshCmdInfo info_pool_uuid[] = { };
static const vshCmdOptDef opts_pool_uuid[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
{.name = NULL} }; @@ -1896,7 +1896,7 @@ static const vshCmdInfo info_pool_edit[] = { };
static const vshCmdOptDef opts_pool_edit[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0),
{.name = NULL} }; diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 8265a3979..b96e205f7 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -43,16 +43,18 @@ #include "virxml.h" #include "virstring.h"
-#define VIRSH_COMMON_OPT_POOL_FULL \ - VIRSH_COMMON_OPT_POOL(N_("pool name or uuid")) +#define VIRSH_COMMON_OPT_POOL_FULL(cflags) \ + VIRSH_COMMON_OPT_POOL(N_("pool name or uuid"), cflags)
-#define VIRSH_COMMON_OPT_POOL_NAME \ - VIRSH_COMMON_OPT_POOL(N_("pool name")) +#define VIRSH_COMMON_OPT_POOL_NAME(cflags) \ + VIRSH_COMMON_OPT_POOL(N_("pool name"), cflags)
-#define VIRSH_COMMON_OPT_POOL_OPTIONAL \ +#define VIRSH_COMMON_OPT_POOL_OPTIONAL(cflags) \ {.name = "pool", \ .type = VSH_OT_STRING, \ - .help = N_("pool name or uuid") \ + .help = N_("pool name or uuid"), \ + .completer = virshStoragePoolNameCompleter, \ + .completer_flags = cflags, \ }
#define VIRSH_COMMON_OPT_VOLUME_VOL \ @@ -165,7 +167,7 @@ static const vshCmdInfo info_vol_create_as[] = { };
Will any of the vol functions work if the pool is not ACTIVE?
Yeah probably not. I'll change it to ACTIVE. The advantage of listing all pools is that it's either exactly what we need or a superset of it :-) Michal
This one is a bit simpler since virStoragePoolListAllVolumes() has no flags yet. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-volume.c | 3 ++- 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index 8ca2fffd9..d6ac0ccb8 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -24,6 +24,7 @@ #include "virsh-completer.h" #include "virsh.h" +#include "virsh-pool.h" #include "virsh-util.h" #include "internal.h" #include "viralloc.h" @@ -198,3 +199,54 @@ virshStoragePoolNameCompleter(vshControl *ctl, VIR_FREE(ret); return NULL; } + + +char ** +virshStorageVolNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + virStoragePoolPtr pool = NULL; + virStorageVolPtr *vols = NULL; + int nvols = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(0, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if (!(pool = virshCommandOptPool(ctl, cmd, "pool", NULL))) + return false; + + if ((nvols = virStoragePoolListAllVolumes(pool, &vols, flags)) < 0) + goto error; + + if (VIR_ALLOC_N(ret, nvols + 1) < 0) + goto error; + + for (i = 0; i < nvols; i++) { + const char *name = virStorageVolGetName(vols[i]); + + if (VIR_STRDUP(ret[i], name) < 0) + goto error; + + virStorageVolFree(vols[i]); + } + VIR_FREE(vols); + virStoragePoolFree(pool); + + return ret; + + error: + for (; i < nvols; i++) + virStorageVolFree(vols[i]); + VIR_FREE(vols); + for (i = 0; i < nvols; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + virStoragePoolFree(pool); + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 249e793b9..1e4ce5932 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -42,4 +42,8 @@ char ** virshStoragePoolNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); +char ** virshStorageVolNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + #endif diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index b96e205f7..c1f36d9b9 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -61,7 +61,8 @@ {.name = "vol", \ .type = VSH_OT_DATA, \ .flags = VSH_OFLAG_REQ, \ - .help = N_("vol name, key or path") \ + .help = N_("vol name, key or path"), \ + .completer = virshStorageVolNameCompleter, \ } virStorageVolPtr -- 2.13.6
On 01/12/2018 09:37 AM, Michal Privoznik wrote:
This one is a bit simpler since virStoragePoolListAllVolumes() has no flags yet.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-volume.c | 3 ++- 3 files changed, 58 insertions(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-interface.c | 16 +++++++++------- 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index d6ac0ccb8..f5b1e4261 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -250,3 +250,50 @@ virshStorageVolNameCompleter(vshControl *ctl, virStoragePoolFree(pool); return NULL; } + + +char ** +virshInterfaceNameCompleter(vshControl *ctl, + const vshCmd *cmd ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + virInterfacePtr *ifaces = NULL; + int nifaces = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(VIR_CONNECT_LIST_INTERFACES_ACTIVE | + VIR_CONNECT_LIST_INTERFACES_INACTIVE, + NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if ((nifaces = virConnectListAllInterfaces(priv->conn, &ifaces, flags)) < 0) + return NULL; + + if (VIR_ALLOC_N(ret, nifaces + 1) < 0) + goto error; + + for (i = 0; i < nifaces; i++) { + const char *name = virInterfaceGetName(ifaces[i]); + + if (VIR_STRDUP(ret[i], name) < 0) + goto error; + + virInterfaceFree(ifaces[i]); + } + VIR_FREE(ifaces); + + return ret; + + error: + for (; i < nifaces; i++) + virInterfaceFree(ifaces[i]); + VIR_FREE(ifaces); + for (i = 0; i < nifaces; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 1e4ce5932..2323aaba3 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -46,4 +46,8 @@ char ** virshStorageVolNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); +char ** virshInterfaceNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + #endif diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index c1a2b21de..50518c667 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -23,11 +23,13 @@ * */ -#define VIRSH_COMMON_OPT_INTERFACE \ +#define VIRSH_COMMON_OPT_INTERFACE(cflags) \ {.name = "interface", \ .type = VSH_OT_DATA, \ .flags = VSH_OFLAG_REQ, \ - .help = N_("interface name or MAC address") \ + .help = N_("interface name or MAC address"), \ + .completer = virshInterfaceNameCompleter, \ + .completer_flags = cflags, \ } #include <config.h> @@ -107,7 +109,7 @@ static const vshCmdInfo info_interface_edit[] = { }; static const vshCmdOptDef opts_interface_edit[] = { - VIRSH_COMMON_OPT_INTERFACE, + VIRSH_COMMON_OPT_INTERFACE(0), {.name = NULL} }; @@ -467,7 +469,7 @@ static const vshCmdInfo info_interface_dumpxml[] = { }; static const vshCmdOptDef opts_interface_dumpxml[] = { - VIRSH_COMMON_OPT_INTERFACE, + VIRSH_COMMON_OPT_INTERFACE(0), {.name = "inactive", .type = VSH_OT_BOOL, .help = N_("show inactive defined XML") @@ -564,7 +566,7 @@ static const vshCmdInfo info_interface_undefine[] = { }; static const vshCmdOptDef opts_interface_undefine[] = { - VIRSH_COMMON_OPT_INTERFACE, + VIRSH_COMMON_OPT_INTERFACE(0), {.name = NULL} }; @@ -603,7 +605,7 @@ static const vshCmdInfo info_interface_start[] = { }; static const vshCmdOptDef opts_interface_start[] = { - VIRSH_COMMON_OPT_INTERFACE, + VIRSH_COMMON_OPT_INTERFACE(VIR_CONNECT_LIST_INTERFACES_INACTIVE), {.name = NULL} }; @@ -642,7 +644,7 @@ static const vshCmdInfo info_interface_destroy[] = { }; static const vshCmdOptDef opts_interface_destroy[] = { - VIRSH_COMMON_OPT_INTERFACE, + VIRSH_COMMON_OPT_INTERFACE(VIR_CONNECT_LIST_INTERFACES_ACTIVE), {.name = NULL} }; -- 2.13.6
On 01/12/2018 09:37 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-interface.c | 16 +++++++++------- 3 files changed, 60 insertions(+), 7 deletions(-)
Noted a couple of patches ago about the oddness with virshDomainInterfaceCompleter... Doesn't stop this, but it may alter this one if the other interface is changed. Reviewed-by: John Ferlan <jferlan@redhat.com> John
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-network.c | 24 ++++++++++++----------- 3 files changed, 68 insertions(+), 11 deletions(-) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index f5b1e4261..2c0d4f640 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -297,3 +297,54 @@ virshInterfaceNameCompleter(vshControl *ctl, VIR_FREE(ret); return NULL; } + + +char ** +virshNetworkNameCompleter(vshControl *ctl, + const vshCmd *cmd ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + virNetworkPtr *nets = NULL; + int nnets = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(VIR_CONNECT_LIST_NETWORKS_INACTIVE | + VIR_CONNECT_LIST_NETWORKS_ACTIVE | + VIR_CONNECT_LIST_NETWORKS_PERSISTENT | + VIR_CONNECT_LIST_NETWORKS_TRANSIENT | + VIR_CONNECT_LIST_NETWORKS_AUTOSTART | + VIR_CONNECT_LIST_NETWORKS_NO_AUTOSTART, + NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if ((nnets = virConnectListAllNetworks(priv->conn, &nets, flags)) < 0) + return NULL; + + if (VIR_ALLOC_N(ret, nnets + 1) < 0) + goto error; + + for (i = 0; i < nnets; i++) { + const char *name = virNetworkGetName(nets[i]); + + if (VIR_STRDUP(ret[i], name) < 0) + goto error; + + virNetworkFree(nets[i]); + } + VIR_FREE(nets); + + return ret; + + error: + for (; i < nnets; i++) + virNetworkFree(nets[i]); + VIR_FREE(nets); + for (i = 0; i < nnets; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 2323aaba3..20ba4cb55 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -50,4 +50,8 @@ char ** virshInterfaceNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); +char ** virshNetworkNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + #endif diff --git a/tools/virsh-network.c b/tools/virsh-network.c index cd55e384f..3b472ea67 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -34,11 +34,13 @@ #include "virtime.h" #include "conf/network_conf.h" -#define VIRSH_COMMON_OPT_NETWORK \ +#define VIRSH_COMMON_OPT_NETWORK(cflags) \ {.name = "network", \ .type = VSH_OT_DATA, \ .flags = VSH_OFLAG_REQ, \ - .help = N_("network name or uuid") \ + .help = N_("network name or uuid"), \ + .completer = virshNetworkNameCompleter, \ + .completer_flags = cflags, \ } virNetworkPtr @@ -93,7 +95,7 @@ static const vshCmdInfo info_network_autostart[] = { }; static const vshCmdOptDef opts_network_autostart[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(0), {.name = "disable", .type = VSH_OT_BOOL, .help = N_("disable autostarting") @@ -240,7 +242,7 @@ static const vshCmdInfo info_network_destroy[] = { }; static const vshCmdOptDef opts_network_destroy[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(VIR_CONNECT_LIST_NETWORKS_ACTIVE), {.name = NULL} }; @@ -279,7 +281,7 @@ static const vshCmdInfo info_network_dumpxml[] = { }; static const vshCmdOptDef opts_network_dumpxml[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(0), {.name = "inactive", .type = VSH_OT_BOOL, .help = N_("show inactive defined XML") @@ -330,7 +332,7 @@ static const vshCmdInfo info_network_info[] = { }; static const vshCmdOptDef opts_network_info[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(0), {.name = NULL} }; @@ -779,7 +781,7 @@ static const vshCmdInfo info_network_start[] = { }; static const vshCmdOptDef opts_network_start[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(VIR_CONNECT_LIST_NETWORKS_INACTIVE), {.name = NULL} }; @@ -817,7 +819,7 @@ static const vshCmdInfo info_network_undefine[] = { }; static const vshCmdOptDef opts_network_undefine[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(0), {.name = NULL} }; @@ -856,7 +858,7 @@ static const vshCmdInfo info_network_update[] = { }; static const vshCmdOptDef opts_network_update[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(0), {.name = "command", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -1057,7 +1059,7 @@ static const vshCmdInfo info_network_edit[] = { }; static const vshCmdOptDef opts_network_edit[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(0), {.name = NULL} }; @@ -1304,7 +1306,7 @@ static const vshCmdInfo info_network_dhcp_leases[] = { }; static const vshCmdOptDef opts_network_dhcp_leases[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(0), {.name = "mac", .type = VSH_OT_STRING, .flags = VSH_OFLAG_NONE, -- 2.13.6
On 01/12/2018 09:37 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-network.c | 24 ++++++++++++----------- 3 files changed, 68 insertions(+), 11 deletions(-)
For as much as I recall there being complaints when I added the various VIRSH macros for various comments, I would think that made this task easier!
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index f5b1e4261..2c0d4f640 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -297,3 +297,54 @@ virshInterfaceNameCompleter(vshControl *ctl, VIR_FREE(ret); return NULL; } + + +char ** +virshNetworkNameCompleter(vshControl *ctl, + const vshCmd *cmd ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + virNetworkPtr *nets = NULL; + int nnets = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(VIR_CONNECT_LIST_NETWORKS_INACTIVE | + VIR_CONNECT_LIST_NETWORKS_ACTIVE | + VIR_CONNECT_LIST_NETWORKS_PERSISTENT | + VIR_CONNECT_LIST_NETWORKS_TRANSIENT | + VIR_CONNECT_LIST_NETWORKS_AUTOSTART | + VIR_CONNECT_LIST_NETWORKS_NO_AUTOSTART, + NULL);
Only 0, ACTIVE, and INACTIVE are used, but I think PERSISTENT needs to be added (per below).
+ + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if ((nnets = virConnectListAllNetworks(priv->conn, &nets, flags)) < 0) + return NULL; + + if (VIR_ALLOC_N(ret, nnets + 1) < 0) + goto error; + + for (i = 0; i < nnets; i++) { + const char *name = virNetworkGetName(nets[i]); + + if (VIR_STRDUP(ret[i], name) < 0) + goto error; + + virNetworkFree(nets[i]); + } + VIR_FREE(nets); + + return ret; + + error: + for (; i < nnets; i++) + virNetworkFree(nets[i]); + VIR_FREE(nets); + for (i = 0; i < nnets; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 2323aaba3..20ba4cb55 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -50,4 +50,8 @@ char ** virshInterfaceNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags);
+char ** virshNetworkNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + #endif diff --git a/tools/virsh-network.c b/tools/virsh-network.c index cd55e384f..3b472ea67 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -34,11 +34,13 @@ #include "virtime.h" #include "conf/network_conf.h"
-#define VIRSH_COMMON_OPT_NETWORK \ +#define VIRSH_COMMON_OPT_NETWORK(cflags) \ {.name = "network", \ .type = VSH_OT_DATA, \ .flags = VSH_OFLAG_REQ, \ - .help = N_("network name or uuid") \ + .help = N_("network name or uuid"), \ + .completer = virshNetworkNameCompleter, \ + .completer_flags = cflags, \ }
virNetworkPtr @@ -93,7 +95,7 @@ static const vshCmdInfo info_network_autostart[] = { };
static const vshCmdOptDef opts_network_autostart[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(0),
Light dawns... Would only be reasonable for PERSISTENT networks right? Not going to autostart a transient one... Perhaps this should be true for previous ones already pushed and reviewed... That is I won't go back and I won't bring it up again.
{.name = "disable", .type = VSH_OT_BOOL, .help = N_("disable autostarting") @@ -240,7 +242,7 @@ static const vshCmdInfo info_network_destroy[] = { };
static const vshCmdOptDef opts_network_destroy[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(VIR_CONNECT_LIST_NETWORKS_ACTIVE), {.name = NULL} };
@@ -279,7 +281,7 @@ static const vshCmdInfo info_network_dumpxml[] = { };
static const vshCmdOptDef opts_network_dumpxml[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(0), {.name = "inactive", .type = VSH_OT_BOOL, .help = N_("show inactive defined XML") @@ -330,7 +332,7 @@ static const vshCmdInfo info_network_info[] = { };
static const vshCmdOptDef opts_network_info[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(0), {.name = NULL} };
@@ -779,7 +781,7 @@ static const vshCmdInfo info_network_start[] = { };
static const vshCmdOptDef opts_network_start[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(VIR_CONNECT_LIST_NETWORKS_INACTIVE), {.name = NULL} };
@@ -817,7 +819,7 @@ static const vshCmdInfo info_network_undefine[] = { };
static const vshCmdOptDef opts_network_undefine[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(0),
PERSISTENT Reviewed-by: John Ferlan <jferlan@redhat.com> John
{.name = NULL} };
@@ -856,7 +858,7 @@ static const vshCmdInfo info_network_update[] = { };
static const vshCmdOptDef opts_network_update[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(0), {.name = "command", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -1057,7 +1059,7 @@ static const vshCmdInfo info_network_edit[] = { };
static const vshCmdOptDef opts_network_edit[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(0), {.name = NULL} };
@@ -1304,7 +1306,7 @@ static const vshCmdInfo info_network_dhcp_leases[] = { };
static const vshCmdOptDef opts_network_dhcp_leases[] = { - VIRSH_COMMON_OPT_NETWORK, + VIRSH_COMMON_OPT_NETWORK(0), {.name = "mac", .type = VSH_OT_STRING, .flags = VSH_OFLAG_NONE,
Yet again, we don't need listing by device capabilities, so flags are unused. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-nodedev.c | 16 +++++++++++----- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index 2c0d4f640..c50143142 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -348,3 +348,48 @@ virshNetworkNameCompleter(vshControl *ctl, VIR_FREE(ret); return NULL; } + + +char ** +virshNodeDeviceNameCompleter(vshControl *ctl, + const vshCmd *cmd ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + virNodeDevicePtr *devs = NULL; + int ndevs = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(0, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if ((ndevs = virConnectListAllNodeDevices(priv->conn, &devs, flags)) < 0) + return NULL; + + if (VIR_ALLOC_N(ret, ndevs + 1) < 0) + goto error; + + for (i = 0; i < ndevs; i++) { + const char *name = virNodeDeviceGetName(devs[i]); + + if (VIR_STRDUP(ret[i], name) < 0) + goto error; + + virNodeDeviceFree(devs[i]); + } + VIR_FREE(devs); + + return ret; + + error: + for (; i < ndevs; i++) + virNodeDeviceFree(devs[i]); + VIR_FREE(devs); + for (i = 0; i < ndevs; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 20ba4cb55..19fa2113d 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -54,4 +54,8 @@ char ** virshNetworkNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); +char ** virshNodeDeviceNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + #endif diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index c7ef6bfde..d25fe0e09 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -109,7 +109,8 @@ static const vshCmdOptDef opts_node_device_destroy[] = { {.name = "device", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, - .help = N_("device name or wwn pair in 'wwnn,wwpn' format") + .help = N_("device name or wwn pair in 'wwnn,wwpn' format"), + .completer = virshNodeDeviceNameCompleter, }, {.name = NULL} }; @@ -534,6 +535,7 @@ static const vshCmdOptDef opts_node_device_dumpxml[] = { .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, .help = N_("device name or wwn pair in 'wwnn,wwpn' format"), + .completer = virshNodeDeviceNameCompleter, }, {.name = NULL} }; @@ -604,7 +606,8 @@ static const vshCmdOptDef opts_node_device_detach[] = { {.name = "device", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, - .help = N_("device key") + .help = N_("device key"), + .completer = virshNodeDeviceNameCompleter, }, {.name = "driver", .type = VSH_OT_STRING, @@ -670,7 +673,8 @@ static const vshCmdOptDef opts_node_device_reattach[] = { {.name = "device", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, - .help = N_("device key") + .help = N_("device key"), + .completer = virshNodeDeviceNameCompleter, }, {.name = NULL} }; @@ -720,7 +724,8 @@ static const vshCmdOptDef opts_node_device_reset[] = { {.name = "device", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, - .help = N_("device key") + .help = N_("device key"), + .completer = virshNodeDeviceNameCompleter, }, {.name = NULL} }; @@ -866,7 +871,8 @@ static const vshCmdInfo info_node_device_event[] = { static const vshCmdOptDef opts_node_device_event[] = { {.name = "device", .type = VSH_OT_STRING, - .help = N_("filter by node device name") + .help = N_("filter by node device name"), + .completer = virshNodeDeviceNameCompleter, }, {.name = "event", .type = VSH_OT_STRING, -- 2.13.6
On 01/12/2018 09:37 AM, Michal Privoznik wrote:
Yet again, we don't need listing by device capabilities, so flags are unused.
The CONNECT flags wouldn't make sense anyway ;-)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-nodedev.c | 16 +++++++++++----- 3 files changed, 60 insertions(+), 5 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
The virConnectListAllNWFilters() has no extra flags yet, which simplifies things a bit. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-nwfilter.c | 9 ++++++--- 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index c50143142..9e6f086c0 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -393,3 +393,48 @@ virshNodeDeviceNameCompleter(vshControl *ctl, VIR_FREE(ret); return NULL; } + + +char ** +virshNWFilterNameCompleter(vshControl *ctl, + const vshCmd *cmd ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + virNWFilterPtr *nwfilters = NULL; + int nnwfilters = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(0, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if ((nnwfilters = virConnectListAllNWFilters(priv->conn, &nwfilters, flags)) < 0) + return NULL; + + if (VIR_ALLOC_N(ret, nnwfilters + 1) < 0) + goto error; + + for (i = 0; i < nnwfilters; i++) { + const char *name = virNWFilterGetName(nwfilters[i]); + + if (VIR_STRDUP(ret[i], name) < 0) + goto error; + + virNWFilterFree(nwfilters[i]); + } + VIR_FREE(nwfilters); + + return ret; + + error: + for (; i < nnwfilters; i++) + virNWFilterFree(nwfilters[i]); + VIR_FREE(nwfilters); + for (i = 0; i < nnwfilters; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 19fa2113d..3c3b17f1e 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -58,4 +58,8 @@ char ** virshNodeDeviceNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); +char ** virshNWFilterNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + #endif diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 40bc193ad..06a002dff 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -136,7 +136,8 @@ static const vshCmdOptDef opts_nwfilter_undefine[] = { {.name = "nwfilter", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, - .help = N_("network filter name or uuid") + .help = N_("network filter name or uuid"), + .completer = virshNWFilterNameCompleter, }, {.name = NULL} }; @@ -179,7 +180,8 @@ static const vshCmdOptDef opts_nwfilter_dumpxml[] = { {.name = "nwfilter", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, - .help = N_("network filter name or uuid") + .help = N_("network filter name or uuid"), + .completer = virshNWFilterNameCompleter, }, {.name = NULL} }; @@ -396,7 +398,8 @@ static const vshCmdOptDef opts_nwfilter_edit[] = { {.name = "nwfilter", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, - .help = N_("network filter name or uuid") + .help = N_("network filter name or uuid"), + .completer = virshNWFilterNameCompleter, }, {.name = NULL} }; -- 2.13.6
On 01/12/2018 09:37 AM, Michal Privoznik wrote:
The virConnectListAllNWFilters() has no extra flags yet, which simplifies things a bit.
CONNECT flags wouldn't make sense here either especially for undefine where things either are or aren't.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-nwfilter.c | 9 ++++++--- 3 files changed, 55 insertions(+), 3 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
This is a slight change from previous patches since virSecret does not have a name only UUID strings. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-secret.c | 15 ++++++++++----- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index 9e6f086c0..7332fa97a 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -438,3 +438,49 @@ virshNWFilterNameCompleter(vshControl *ctl, VIR_FREE(ret); return NULL; } + + +char ** +virshSecretUUIDCompleter(vshControl *ctl, + const vshCmd *cmd ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + virSecretPtr *secrets = NULL; + int nsecrets = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(0, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if ((nsecrets = virConnectListAllSecrets(priv->conn, &secrets, flags)) < 0) + return NULL; + + if (VIR_ALLOC_N(ret, nsecrets + 1) < 0) + goto error; + + for (i = 0; i < nsecrets; i++) { + char uuid[VIR_UUID_STRING_BUFLEN]; + + if (virSecretGetUUIDString(secrets[i], uuid) < 0 || + VIR_STRDUP(ret[i], uuid) < 0) + goto error; + + virSecretFree(secrets[i]); + } + VIR_FREE(secrets); + + return ret; + + error: + for (; i < nsecrets; i++) + virSecretFree(secrets[i]); + VIR_FREE(secrets); + for (i = 0; i < nsecrets; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 3c3b17f1e..0e518873c 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -62,4 +62,8 @@ char ** virshNWFilterNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); +char ** virshSecretUUIDCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + #endif diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c index 52f067652..9e4ec61a8 100644 --- a/tools/virsh-secret.c +++ b/tools/virsh-secret.c @@ -132,7 +132,8 @@ static const vshCmdOptDef opts_secret_dumpxml[] = { {.name = "secret", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, - .help = N_("secret UUID") + .help = N_("secret UUID"), + .completer = virshSecretUUIDCompleter, }, {.name = NULL} }; @@ -177,7 +178,8 @@ static const vshCmdOptDef opts_secret_set_value[] = { {.name = "secret", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, - .help = N_("secret UUID") + .help = N_("secret UUID"), + .completer = virshSecretUUIDCompleter, }, {.name = "base64", .type = VSH_OT_DATA, @@ -245,7 +247,8 @@ static const vshCmdOptDef opts_secret_get_value[] = { {.name = "secret", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, - .help = N_("secret UUID") + .help = N_("secret UUID"), + .completer = virshSecretUUIDCompleter, }, {.name = NULL} }; @@ -297,7 +300,8 @@ static const vshCmdOptDef opts_secret_undefine[] = { {.name = "secret", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, - .help = N_("secret UUID") + .help = N_("secret UUID"), + .completer = virshSecretUUIDCompleter, }, {.name = NULL} }; @@ -667,7 +671,8 @@ static const vshCmdInfo info_secret_event[] = { static const vshCmdOptDef opts_secret_event[] = { {.name = "secret", .type = VSH_OT_STRING, - .help = N_("filter by secret name or uuid") + .help = N_("filter by secret name or uuid"), + .completer = virshSecretUUIDCompleter, }, {.name = "event", .type = VSH_OT_STRING, -- 2.13.6
On 01/12/2018 09:37 AM, Michal Privoznik wrote:
This is a slight change from previous patches since virSecret does not have a name only UUID strings.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-secret.c | 15 ++++++++++----- 3 files changed, 60 insertions(+), 5 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John [...]
}; @@ -667,7 +671,8 @@ static const vshCmdInfo info_secret_event[] = { static const vshCmdOptDef opts_secret_event[] = { {.name = "secret", .type = VSH_OT_STRING, - .help = N_("filter by secret name or uuid") + .help = N_("filter by secret name or uuid"),
Wonder what a secret name is? ;-) I know separate issue... This and the virsh.pod should be adjusted.
+ .completer = virshSecretUUIDCompleter, }, {.name = "event", .type = VSH_OT_STRING,
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-snapshot.c | 21 +++++++++++++------- 3 files changed, 69 insertions(+), 7 deletions(-) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index 7332fa97a..9db7c59d2 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -484,3 +484,54 @@ virshSecretUUIDCompleter(vshControl *ctl, VIR_FREE(ret); return NULL; } + + +char ** +virshSnapshotNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + virDomainPtr dom = NULL; + virDomainSnapshotPtr *snapshots = NULL; + int nsnapshots = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(0, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return NULL; + + if ((nsnapshots = virDomainListAllSnapshots(dom, &snapshots, flags)) < 0) + goto error; + + if (VIR_ALLOC_N(ret, nsnapshots + 1) < 0) + goto error; + + for (i = 0; i < nsnapshots; i++) { + const char *name = virDomainSnapshotGetName(snapshots[i]); + + if (VIR_STRDUP(ret[i], name) < 0) + goto error; + + virshDomainSnapshotFree(snapshots[i]); + } + VIR_FREE(snapshots); + virshDomainFree(dom); + + return ret; + + error: + for (; i < nsnapshots; i++) + virshDomainSnapshotFree(snapshots[i]); + VIR_FREE(snapshots); + for (i = 0; i < nsnapshots; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + virshDomainFree(dom); + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 0e518873c..fa443d3ad 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -66,4 +66,8 @@ char ** virshSecretUUIDCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); +char ** virshSnapshotNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + #endif diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index c44a36f98..e4908eea7 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -511,7 +511,8 @@ static const vshCmdOptDef opts_snapshot_edit[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "snapshotname", .type = VSH_OT_STRING, - .help = N_("snapshot name") + .help = N_("snapshot name"), + .completer = virshSnapshotNameCompleter, }, VIRSH_COMMON_OPT_CURRENT(N_("also set edited snapshot as current")), {.name = "rename", @@ -631,7 +632,8 @@ static const vshCmdOptDef opts_snapshot_current[] = { }, {.name = "snapshotname", .type = VSH_OT_STRING, - .help = N_("name of existing snapshot to make current") + .help = N_("name of existing snapshot to make current"), + .completer = virshSnapshotNameCompleter, }, {.name = NULL} }; @@ -854,7 +856,8 @@ static const vshCmdOptDef opts_snapshot_info[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "snapshotname", .type = VSH_OT_STRING, - .help = N_("snapshot name") + .help = N_("snapshot name"), + .completer = virshSnapshotNameCompleter, }, VIRSH_COMMON_OPT_CURRENT(N_("info on current snapshot")), {.name = NULL} @@ -1661,7 +1664,8 @@ static const vshCmdOptDef opts_snapshot_dumpxml[] = { {.name = "snapshotname", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, - .help = N_("snapshot name") + .help = N_("snapshot name"), + .completer = virshSnapshotNameCompleter, }, {.name = "security-info", .type = VSH_OT_BOOL, @@ -1723,7 +1727,8 @@ static const vshCmdOptDef opts_snapshot_parent[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "snapshotname", .type = VSH_OT_STRING, - .help = N_("find parent of snapshot name") + .help = N_("find parent of snapshot name"), + .completer = virshSnapshotNameCompleter, }, VIRSH_COMMON_OPT_CURRENT(N_("find parent of current snapshot")), {.name = NULL} @@ -1782,7 +1787,8 @@ static const vshCmdOptDef opts_snapshot_revert[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "snapshotname", .type = VSH_OT_STRING, - .help = N_("snapshot name") + .help = N_("snapshot name"), + .completer = virshSnapshotNameCompleter, }, VIRSH_COMMON_OPT_CURRENT(N_("revert to current snapshot")), {.name = "running", @@ -1866,7 +1872,8 @@ static const vshCmdOptDef opts_snapshot_delete[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "snapshotname", .type = VSH_OT_STRING, - .help = N_("snapshot name") + .help = N_("snapshot name"), + .completer = virshSnapshotNameCompleter, }, VIRSH_COMMON_OPT_CURRENT(N_("delete current snapshot")), {.name = "children", -- 2.13.6
On 01/12/2018 09:37 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-snapshot.c | 21 +++++++++++++------- 3 files changed, 69 insertions(+), 7 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 064b9ae83..e5ed89504 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -47,6 +47,17 @@ qemu: Add support for hot unplugging redirdev device </summary> </change> + <change> + <summary> + virsh: Enhance bash completion + </summary> + <description> + New bash completion script is introduced to enable completion even + for non-interactive virsh. At the same time, virsh offers completion + of some basic libvirt objects like domains, networks, storage pools, + etc. to virsh commands accepting them. + </description> + </change> </section> <section title="Bug fixes"> </section> -- 2.13.6
On 01/12/2018 10:30 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+)
I think you were planning to change this one anyway considering 4.0.0 got some of this added already. John
diff --git a/docs/news.xml b/docs/news.xml index 064b9ae83..e5ed89504 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -47,6 +47,17 @@ qemu: Add support for hot unplugging redirdev device </summary> </change> + <change> + <summary> + virsh: Enhance bash completion + </summary> + <description> + New bash completion script is introduced to enable completion even + for non-interactive virsh. At the same time, virsh offers completion + of some basic libvirt objects like domains, networks, storage pools, + etc. to virsh commands accepting them. + </description> + </change> </section> <section title="Bug fixes"> </section>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- Diff to v1: - Pruned the list of flags accepted in the callback - Pools for vol_* are ACTIVE only - Other small nits raised in the review tools/virsh-completer.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-pool.c | 28 ++++++++++++++-------------- tools/virsh-volume.c | 10 +++++++--- tools/virsh.h | 6 ++++-- 5 files changed, 77 insertions(+), 19 deletions(-) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index 48dd9fbc2..947c326fc 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -147,3 +147,51 @@ virshDomainInterfaceCompleter(vshControl *ctl, virStringListFree(ret); return NULL; } + + +char ** +virshStoragePoolNameCompleter(vshControl *ctl, + const vshCmd *cmd ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + virStoragePoolPtr *pools = NULL; + int npools = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE | + VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | + VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT, + NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if ((npools = virConnectListAllStoragePools(priv->conn, &pools, flags)) < 0) + return NULL; + + if (VIR_ALLOC_N(ret, npools + 1) < 0) + goto error; + + for (i = 0; i < npools; i++) { + const char *name = virStoragePoolGetName(pools[i]); + + if (VIR_STRDUP(ret[i], name) < 0) + goto error; + + virStoragePoolFree(pools[i]); + } + VIR_FREE(pools); + + return ret; + + error: + for (; i < npools; i++) + virStoragePoolFree(pools[i]); + VIR_FREE(pools); + for (i = 0; i < npools; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 1a2dd685f..249e793b9 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -38,4 +38,8 @@ char ** virshDomainInterfaceCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); +char ** virshStoragePoolNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + #endif diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 094874b64..56b6cfc73 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -34,8 +34,8 @@ #include "virstring.h" #include "virtime.h" -#define VIRSH_COMMON_OPT_POOL_FULL \ - VIRSH_COMMON_OPT_POOL(N_("pool name or uuid")) +#define VIRSH_COMMON_OPT_POOL_FULL(cflags) \ + VIRSH_COMMON_OPT_POOL(N_("pool name or uuid"), cflags) #define VIRSH_COMMON_OPT_POOL_BUILD \ {.name = "build", \ @@ -182,7 +182,7 @@ static const vshCmdInfo info_pool_autostart[] = { }; static const vshCmdOptDef opts_pool_autostart[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT), {.name = "disable", .type = VSH_OT_BOOL, @@ -575,7 +575,7 @@ static const vshCmdInfo info_pool_build[] = { }; static const vshCmdOptDef opts_pool_build[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), VIRSH_COMMON_OPT_POOL_NO_OVERWRITE, VIRSH_COMMON_OPT_POOL_OVERWRITE, @@ -625,7 +625,7 @@ static const vshCmdInfo info_pool_destroy[] = { }; static const vshCmdOptDef opts_pool_destroy[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE), {.name = NULL} }; @@ -665,7 +665,7 @@ static const vshCmdInfo info_pool_delete[] = { }; static const vshCmdOptDef opts_pool_delete[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE), {.name = NULL} }; @@ -705,7 +705,7 @@ static const vshCmdInfo info_pool_refresh[] = { }; static const vshCmdOptDef opts_pool_refresh[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = NULL} }; @@ -745,7 +745,7 @@ static const vshCmdInfo info_pool_dumpxml[] = { }; static const vshCmdOptDef opts_pool_dumpxml[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = "inactive", .type = VSH_OT_BOOL, @@ -1636,7 +1636,7 @@ static const vshCmdInfo info_pool_info[] = { }; static const vshCmdOptDef opts_pool_info[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = "bytes", .type = VSH_OT_BOOL, @@ -1726,7 +1726,7 @@ static const vshCmdInfo info_pool_name[] = { }; static const vshCmdOptDef opts_pool_name[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = NULL} }; @@ -1758,7 +1758,7 @@ static const vshCmdInfo info_pool_start[] = { }; static const vshCmdOptDef opts_pool_start[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE), VIRSH_COMMON_OPT_POOL_BUILD, VIRSH_COMMON_OPT_POOL_NO_OVERWRITE, VIRSH_COMMON_OPT_POOL_OVERWRITE, @@ -1819,7 +1819,7 @@ static const vshCmdInfo info_pool_undefine[] = { }; static const vshCmdOptDef opts_pool_undefine[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT), {.name = NULL} }; @@ -1859,7 +1859,7 @@ static const vshCmdInfo info_pool_uuid[] = { }; static const vshCmdOptDef opts_pool_uuid[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = NULL} }; @@ -1896,7 +1896,7 @@ static const vshCmdInfo info_pool_edit[] = { }; static const vshCmdOptDef opts_pool_edit[] = { - VIRSH_COMMON_OPT_POOL_FULL, + VIRSH_COMMON_OPT_POOL_FULL(0), {.name = NULL} }; diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 8265a3979..bfbb7c7d7 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -44,15 +44,19 @@ #include "virstring.h" #define VIRSH_COMMON_OPT_POOL_FULL \ - VIRSH_COMMON_OPT_POOL(N_("pool name or uuid")) + VIRSH_COMMON_OPT_POOL(N_("pool name or uuid"), \ + VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE) #define VIRSH_COMMON_OPT_POOL_NAME \ - VIRSH_COMMON_OPT_POOL(N_("pool name")) + VIRSH_COMMON_OPT_POOL(N_("pool name"), \ + VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE) #define VIRSH_COMMON_OPT_POOL_OPTIONAL \ {.name = "pool", \ .type = VSH_OT_STRING, \ - .help = N_("pool name or uuid") \ + .help = N_("pool name or uuid"), \ + .completer = virshStoragePoolNameCompleter, \ + .completer_flags = VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE, \ } #define VIRSH_COMMON_OPT_VOLUME_VOL \ diff --git a/tools/virsh.h b/tools/virsh.h index 528e04558..f2213ebb5 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -64,11 +64,13 @@ /* * Common command options */ -# define VIRSH_COMMON_OPT_POOL(_helpstr) \ +# define VIRSH_COMMON_OPT_POOL(_helpstr, cflags) \ {.name = "pool", \ .type = VSH_OT_DATA, \ .flags = VSH_OFLAG_REQ, \ - .help = _helpstr \ + .help = _helpstr, \ + .completer = virshStoragePoolNameCompleter, \ + .completer_flags = cflags, \ } # define VIRSH_COMMON_OPT_DOMAIN(_helpstr, cflags) \ -- 2.13.6
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- Diff to v1: - Adapted to new release docs/news.xml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index b4d980624..71305c203 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -37,6 +37,16 @@ <section title="New features"> </section> <section title="Improvements"> + <change> + <summary> + virsh: Enhance bash completion + </summary> + <description> + Implement more bash completions so that basic libvirt + objects can be autocompleted (e.g. networks, + interfaces, NWFilters, and so on). + </description> + </change> </section> <section title="Bug fixes"> </section> -- 2.13.6
On 01/24/2018 03:50 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
Diff to v1: - Adapted to new release
docs/news.xml | 10 ++++++++++ 1 file changed, 10 insertions(+)
Would you say that the auto completion being added in this release adds the lookup by name on various commands. Whether you want to be that precise is up to you... Reviewed-by: John Ferlan <jferlan@redhat.com> John
diff --git a/docs/news.xml b/docs/news.xml index b4d980624..71305c203 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -37,6 +37,16 @@ <section title="New features"> </section> <section title="Improvements"> + <change> + <summary> + virsh: Enhance bash completion + </summary> + <description> + Implement more bash completions so that basic libvirt + objects can be autocompleted (e.g. networks,
my email client spell checker says "auto completed" or "auto-completed" (IDC, but figured I'd note it).
+ interfaces, NWFilters, and so on). + </description> + </change> </section> <section title="Bug fixes"> </section>
On 01/24/2018 03:50 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
Diff to v1: - Pruned the list of flags accepted in the callback - Pools for vol_* are ACTIVE only - Other small nits raised in the review
tools/virsh-completer.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-pool.c | 28 ++++++++++++++-------------- tools/virsh-volume.c | 10 +++++++--- tools/virsh.h | 6 ++++-- 5 files changed, 77 insertions(+), 19 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
participants (2)
-
John Ferlan -
Michal Privoznik