[libvirt] [PATCH 0/4] Couple of patches about virsh completion

Lin Ma (4): virsh: Only return active domain names for detach-device-alias virsh: Add device name completion for target option of detach-disk command virsh-network: Introduce virshNetworkEventCallback to handle network events virsh: Add event name completion to 'network-event' command tools/virsh-completer.c | 27 +++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-domain.c | 3 ++- tools/virsh-network.c | 18 +++++++++++++++--- tools/virsh-network.h | 8 ++++++++ 5 files changed, 56 insertions(+), 4 deletions(-) -- 2.19.0

Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 5699018dcc..c8c4db1b2b 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11939,7 +11939,7 @@ static const vshCmdInfo info_detach_device_alias[] = { }; static const vshCmdOptDef opts_detach_device_alias[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL(0), + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "alias", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, -- 2.19.0

On 3/5/19 4:17 AM, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 5699018dcc..c8c4db1b2b 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11939,7 +11939,7 @@ static const vshCmdInfo info_detach_device_alias[] = { };
static const vshCmdOptDef opts_detach_device_alias[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL(0), + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "alias", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ,
Not quite. With user aliases one can detach a device from inactive XML: 1) virsh dumpxml --inactive fedora | grep -B4 -A2 ua- <disk type='volume' device='disk'> <driver name='qemu' type='raw'/> <source pool='iscsi-secret-pool' volume='unit:0:0:1' mode='host'/> <target dev='vdc' bus='virtio'/> <alias name='ua-disk1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </disk> 2) virsh detach-device-alias --config --domain fedora --alias ua-disk1 Device detach request sent successfully 3) virsh dumpxml --inactive fedora | grep -B4 -A2 ua- | wc -l 0 Another sign that this is not correct is that this command has --config option. Is there a bug perhaps that you're trying to fix? Michal

On 3/5/19 4:13 PM, Michal Privoznik wrote:
On 3/5/19 4:17 AM, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 5699018dcc..c8c4db1b2b 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11939,7 +11939,7 @@ static const vshCmdInfo info_detach_device_alias[] = { }; static const vshCmdOptDef opts_detach_device_alias[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL(0), + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "alias", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ,
Not quite. With user aliases one can detach a device from inactive XML:
1) virsh dumpxml --inactive fedora | grep -B4 -A2 ua- <disk type='volume' device='disk'> <driver name='qemu' type='raw'/> <source pool='iscsi-secret-pool' volume='unit:0:0:1' mode='host'/> <target dev='vdc' bus='virtio'/> <alias name='ua-disk1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </disk>
2) virsh detach-device-alias --config --domain fedora --alias ua-disk1 Device detach request sent successfully
3) virsh dumpxml --inactive fedora | grep -B4 -A2 ua- | wc -l 0
Another sign that this is not correct is that this command has --config option.
Is there a bug perhaps that you're trying to fix?
Oh, My bad. Thanks for your reviewing and the correction, Please drop this patch. Thanks, Lin

Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-domain.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c8c4db1b2b..911727cde6 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -12548,6 +12548,7 @@ static const vshCmdOptDef opts_detach_disk[] = { {.name = "target", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshDomainDiskTargetCompleter, .help = N_("target of disk device") }, VIRSH_COMMON_OPT_DOMAIN_PERSISTENT, -- 2.19.0

Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-network.c | 17 ++++++++++++++--- tools/virsh-network.h | 8 ++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 9b86ef8071..d5b3649050 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -1156,6 +1156,7 @@ struct virshNetEventData { bool loop; bool timestamp; int count; + virshNetworkEventCallback *cb; }; typedef struct virshNetEventData virshNetEventData; @@ -1195,6 +1196,12 @@ vshEventLifecyclePrint(virConnectPtr conn ATTRIBUTE_UNUSED, vshEventDone(data->ctl); } +virshNetworkEventCallback virshNetworkEventCallbacks[] = { + { "lifecycle", + VIR_NETWORK_EVENT_CALLBACK(vshEventLifecyclePrint), }, +}; +verify(VIR_NETWORK_EVENT_ID_LAST == ARRAY_CARDINALITY(virshNetworkEventCallbacks)); + static const vshCmdInfo info_network_event[] = { {.name = "help", .data = N_("Network Events") @@ -1246,7 +1253,7 @@ cmdNetworkEvent(vshControl *ctl, const vshCmd *cmd) size_t i; for (i = 0; i < VIR_NETWORK_EVENT_ID_LAST; i++) - vshPrint(ctl, "%s\n", virshNetworkEventIdTypeToString(i)); + vshPrint(ctl, "%s\n", virshNetworkEventCallbacks[i].name); return true; } @@ -1256,7 +1263,10 @@ cmdNetworkEvent(vshControl *ctl, const vshCmd *cmd) vshError(ctl, "%s", _("either --list or --event <type> is required")); return false; } - if ((event = virshNetworkEventIdTypeFromString(eventName)) < 0) { + for (event = 0; event < VIR_NETWORK_EVENT_ID_LAST; event++) + if (STREQ(eventName, virshNetworkEventCallbacks[event].name)) + break; + if (event == VIR_NETWORK_EVENT_ID_LAST) { vshError(ctl, _("unknown event type %s"), eventName); return false; } @@ -1265,6 +1275,7 @@ cmdNetworkEvent(vshControl *ctl, const vshCmd *cmd) data.loop = vshCommandOptBool(cmd, "loop"); data.timestamp = vshCommandOptBool(cmd, "timestamp"); data.count = 0; + data.cb = &virshNetworkEventCallbacks[event]; if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0) return false; @@ -1274,7 +1285,7 @@ cmdNetworkEvent(vshControl *ctl, const vshCmd *cmd) goto cleanup; if ((eventId = virConnectNetworkEventRegisterAny(priv->conn, net, event, - VIR_NETWORK_EVENT_CALLBACK(vshEventLifecyclePrint), + data.cb->cb, &data, NULL)) < 0) goto cleanup; switch (vshEventWait(ctl)) { diff --git a/tools/virsh-network.h b/tools/virsh-network.h index 0fff4b7748..9c86eb5bc9 100644 --- a/tools/virsh-network.h +++ b/tools/virsh-network.h @@ -32,6 +32,14 @@ virshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd, virshCommandOptNetworkBy(_ctl, _cmd, _name, \ VIRSH_BYUUID | VIRSH_BYNAME) +struct virshNetworkEventCallback { + const char *name; + virConnectNetworkEventGenericCallback cb; +}; +typedef struct virshNetworkEventCallback virshNetworkEventCallback; + +extern virshNetworkEventCallback virshNetworkEventCallbacks[]; + extern const vshCmdDef networkCmds[]; #endif /* LIBVIRT_VIRSH_NETWORK_H */ -- 2.19.0

On 3/5/19 4:17 AM, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-network.c | 17 ++++++++++++++--- tools/virsh-network.h | 8 ++++++++ 2 files changed, 22 insertions(+), 3 deletions(-)
ACK Michal

Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-completer.c | 27 +++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-network.c | 1 + 3 files changed, 32 insertions(+) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index 7c68e2e832..c4adbb70d0 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -27,6 +27,7 @@ #include "virsh-nodedev.h" #include "virsh-util.h" #include "virsh-secret.h" +#include "virsh-network.h" #include "internal.h" #include "virutil.h" #include "viralloc.h" @@ -415,6 +416,32 @@ virshNetworkNameCompleter(vshControl *ctl, } +char ** +virshNetworkEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED, + const vshCmd *cmd ATTRIBUTE_UNUSED, + unsigned int flags) +{ + size_t i = 0; + char **ret = NULL; + + virCheckFlags(0, NULL); + + if (VIR_ALLOC_N(ret, VIR_NETWORK_EVENT_ID_LAST + 1) < 0) + goto error; + + for (i = 0; i < VIR_NETWORK_EVENT_ID_LAST; i++) { + if (VIR_STRDUP(ret[i], virshNetworkEventCallbacks[i].name) < 0) + goto error; + } + + return ret; + + error: + virStringListFree(ret); + return NULL; +} + + char ** virshNodeDeviceNameCompleter(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED, diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 4563fd76ac..2e2e1edafb 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -55,6 +55,10 @@ char ** virshNetworkNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); +char ** virshNetworkEventNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + char ** virshNodeDeviceNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); diff --git a/tools/virsh-network.c b/tools/virsh-network.c index d5b3649050..9adc63a8fa 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -1216,6 +1216,7 @@ static const vshCmdOptDef opts_network_event[] = { VIRSH_COMMON_OPT_NETWORK_OT_STRING(N_("filter by network name or uuid"), 0), {.name = "event", .type = VSH_OT_STRING, + .completer = virshNetworkEventNameCompleter, .help = N_("which event type to wait for") }, {.name = "loop", -- 2.19.0

On 3/5/19 4:17 AM, Lin Ma wrote:
Signed-off-by: Lin Ma <lma@suse.com> --- tools/virsh-completer.c | 27 +++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-network.c | 1 + 3 files changed, 32 insertions(+)
ACK Michal

On 3/5/19 4:17 AM, Lin Ma wrote:
Lin Ma (4): virsh: Only return active domain names for detach-device-alias virsh: Add device name completion for target option of detach-disk command virsh-network: Introduce virshNetworkEventCallback to handle network events virsh: Add event name completion to 'network-event' command
tools/virsh-completer.c | 27 +++++++++++++++++++++++++++ tools/virsh-completer.h | 4 ++++ tools/virsh-domain.c | 3 ++- tools/virsh-network.c | 18 +++++++++++++++--- tools/virsh-network.h | 8 ++++++++ 5 files changed, 56 insertions(+), 4 deletions(-)
I've ACKed 2-4 and I'm pushing them now. Thanks, Michal
participants (2)
-
Lin Ma
-
Michal Privoznik