docs/manpages/virsh.rst | 21 +++++++++--------- tools/virsh-domain-monitor.c | 42 +++++++++++++++++++++++++----------- 2 files changed, 39 insertions(+), 24 deletions(-)
Some completers for libvirt related tools might want to list
domain IDs only. Just like the one I've implemented for
virt-viewer [1]. I've worked around it using some awk magic,
but if it was possible to just 'virsh list --id' then I could
drop awk.
1: https://www.redhat.com/archives/virt-tools-list/2019-May/msg00014.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
Diff to v1:
- Documented the new switch in the manpage
- Allowed --id to be used with --all
docs/manpages/virsh.rst | 21 +++++++++---------
tools/virsh-domain-monitor.c | 42 +++++++++++++++++++++++++-----------
2 files changed, 39 insertions(+), 24 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index d34a1c8684..848e1a6458 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -621,7 +621,7 @@ list
list [--inactive | --all]
[--managed-save] [--title]
- { [--table] | --name | --uuid }
+ { [--table] | --name | --uuid | --id }
[--persistent] [--transient]
[--with-managed-save] [--without-managed-save]
[--autostart] [--no-autostart]
@@ -758,16 +758,15 @@ If *--managed-save* is specified, then domains that have managed save state
in the listing. This flag is usable only with the default *--table* output.
Note that this flag does not filter the list of domains.
-If *--name* is specified, domain names are printed instead of the table
-formatted one per line. If *--uuid* is specified domain's UUID's are printed
-instead of names. Flag *--table* specifies that the legacy table-formatted
-output should be used. This is the default.
-
-If both *--name* and *--uuid* are specified, domain UUID's and names
-are printed side by side without any header. Flag *--table* specifies
-that the legacy table-formatted output should be used. This is the
-default if neither *--name* nor *--uuid* are specified. Option
-*--table* is mutually exclusive with options *--uuid* and *--name*.
+If *--name* is specified, domain names are printed instead of the
+table formatted one per line. If *--uuid* is specified domain's UUID's
+are printed instead of names. If *--id* is specified then domain's ID's
+are printed indead of names. However, it is possible to combine
+*--name*, *--uuid* and *--id* to select only desired fields for
+printing. Flag *--table* specifies that the legacy table-formatted
+output should be used, but it is mutually exclusive with *--name*,
+*--uuid* and *--id*. This is the default and will be used if neither of
+*--name*, *--uuid* or *--id* is specified.
If *--title* is specified, then the short domain description (title) is
printed in an extra column. This flag is usable only with the default
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index e0491d48ac..5d0a03afa9 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1919,6 +1919,10 @@ static const vshCmdOptDef opts_list[] = {
.type = VSH_OT_BOOL,
.help = N_("list domain names only")
},
+ {.name = "id",
+ .type = VSH_OT_BOOL,
+ .help = N_("list domain IDs only")
+ },
{.name = "table",
.type = VSH_OT_BOOL,
.help = N_("list table (default)")
@@ -1945,6 +1949,7 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
bool optTable = vshCommandOptBool(cmd, "table");
bool optUUID = vshCommandOptBool(cmd, "uuid");
bool optName = vshCommandOptBool(cmd, "name");
+ bool optID = vshCommandOptBool(cmd, "id");
size_t i;
char *title;
char uuid[VIR_UUID_STRING_BUFLEN];
@@ -1988,8 +1993,9 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
VSH_EXCLUSIVE_OPTIONS("table", "name");
VSH_EXCLUSIVE_OPTIONS("table", "uuid");
+ VSH_EXCLUSIVE_OPTIONS("table", "id");
- if (!optUUID && !optName)
+ if (!optUUID && !optName && !optID)
optTable = true;
if (!(list = virshDomainListCollect(ctl, flags)))
@@ -2007,6 +2013,8 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
}
for (i = 0; i < list->ndomains; i++) {
+ const char *sep = "";
+
dom = list->domains[i];
id = virDomainGetID(dom);
if (id != (unsigned int) -1)
@@ -2044,20 +2052,28 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- } else if (optUUID && optName) {
- if (virDomainGetUUIDString(dom, uuid) < 0) {
- vshError(ctl, "%s", _("Failed to get domain's UUID"));
- goto cleanup;
+ } else {
+ if (optUUID) {
+ if (virDomainGetUUIDString(dom, uuid) < 0) {
+ vshError(ctl, "%s", _("Failed to get domain's UUID"));
+ goto cleanup;
+ }
+ vshPrint(ctl, "%s", uuid);
+ sep = " ";
}
- vshPrint(ctl, "%-36s %-30s\n", uuid, virDomainGetName(dom));
- } else if (optUUID) {
- if (virDomainGetUUIDString(dom, uuid) < 0) {
- vshError(ctl, "%s", _("Failed to get domain's UUID"));
- goto cleanup;
+ if (optID) {
+ /* If we are asked to print IDs only then do that
+ * only for live domains. */
+ if (id == (unsigned int) -1 && !optUUID && !optName)
+ continue;
+ vshPrint(ctl, "%s%s", sep, id_buf);
+ sep = " ";
}
- vshPrint(ctl, "%s\n", uuid);
- } else if (optName) {
- vshPrint(ctl, "%s\n", virDomainGetName(dom));
+ if (optName) {
+ vshPrint(ctl, "%s%s", sep, virDomainGetName(dom));
+ sep = " ";
+ }
+ vshPrint(ctl, "\n");
}
}
--
2.26.2
On Thu, Oct 22, 2020 at 04:41:18PM +0200, Michal Privoznik wrote:
>Some completers for libvirt related tools might want to list
>domain IDs only. Just like the one I've implemented for
>virt-viewer [1]. I've worked around it using some awk magic,
>but if it was possible to just 'virsh list --id' then I could
>drop awk.
>
>1: https://www.redhat.com/archives/virt-tools-list/2019-May/msg00014.html
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
>
>Diff to v1:
>- Documented the new switch in the manpage
>- Allowed --id to be used with --all
>
> docs/manpages/virsh.rst | 21 +++++++++---------
> tools/virsh-domain-monitor.c | 42 +++++++++++++++++++++++++-----------
> 2 files changed, 39 insertions(+), 24 deletions(-)
>
>diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
>index d34a1c8684..848e1a6458 100644
>--- a/docs/manpages/virsh.rst
>+++ b/docs/manpages/virsh.rst
>@@ -621,7 +621,7 @@ list
>
> list [--inactive | --all]
> [--managed-save] [--title]
>- { [--table] | --name | --uuid }
>+ { [--table] | --name | --uuid | --id }
> [--persistent] [--transient]
> [--with-managed-save] [--without-managed-save]
> [--autostart] [--no-autostart]
>@@ -758,16 +758,15 @@ If *--managed-save* is specified, then domains that have managed save state
> in the listing. This flag is usable only with the default *--table* output.
> Note that this flag does not filter the list of domains.
>
>-If *--name* is specified, domain names are printed instead of the table
>-formatted one per line. If *--uuid* is specified domain's UUID's are printed
>-instead of names. Flag *--table* specifies that the legacy table-formatted
>-output should be used. This is the default.
>-
>-If both *--name* and *--uuid* are specified, domain UUID's and names
>-are printed side by side without any header. Flag *--table* specifies
>-that the legacy table-formatted output should be used. This is the
>-default if neither *--name* nor *--uuid* are specified. Option
>-*--table* is mutually exclusive with options *--uuid* and *--name*.
>+If *--name* is specified, domain names are printed instead of the
>+table formatted one per line. If *--uuid* is specified domain's UUID's
>+are printed instead of names. If *--id* is specified then domain's ID's
>+are printed indead of names. However, it is possible to combine
>+*--name*, *--uuid* and *--id* to select only desired fields for
>+printing. Flag *--table* specifies that the legacy table-formatted
>+output should be used, but it is mutually exclusive with *--name*,
>+*--uuid* and *--id*. This is the default and will be used if neither of
>+*--name*, *--uuid* or *--id* is specified.
>
Didn't you want to add something like the following?
If neither *--name* nor *--uuid* is specified, but *--id* is, then only active
domains are listed, even with the *--all* parameter as otherwise the output
would just contain bunch of lines with just *-1*.
Otherwise the code is very unclear for no reason.
If that is the case, then
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
On 11/6/20 2:59 PM, Martin Kletzander wrote:
> On Thu, Oct 22, 2020 at 04:41:18PM +0200, Michal Privoznik wrote:
>> Some completers for libvirt related tools might want to list
>> domain IDs only. Just like the one I've implemented for
>> virt-viewer [1]. I've worked around it using some awk magic,
>> but if it was possible to just 'virsh list --id' then I could
>> drop awk.
>>
>> 1: https://www.redhat.com/archives/virt-tools-list/2019-May/msg00014.html
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>
>> Diff to v1:
>> - Documented the new switch in the manpage
>> - Allowed --id to be used with --all
>>
>> docs/manpages/virsh.rst     | 21 +++++++++---------
>> tools/virsh-domain-monitor.c | 42 +++++++++++++++++++++++++-----------
>> 2 files changed, 39 insertions(+), 24 deletions(-)
>>
>> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
>> index d34a1c8684..848e1a6458 100644
>> --- a/docs/manpages/virsh.rst
>> +++ b/docs/manpages/virsh.rst
>> @@ -621,7 +621,7 @@ list
>>
>> Â Â list [--inactive | --all]
>> Â Â Â Â Â Â Â [--managed-save] [--title]
>> -Â Â Â Â Â Â Â { [--table] | --name | --uuid }
>> +Â Â Â Â Â Â Â { [--table] | --name | --uuid | --id }
>> Â Â Â Â Â Â Â [--persistent] [--transient]
>> Â Â Â Â Â Â Â [--with-managed-save] [--without-managed-save]
>> Â Â Â Â Â Â Â [--autostart] [--no-autostart]
>> @@ -758,16 +758,15 @@ If *--managed-save* is specified, then domains
>> that have managed save state
>> in the listing. This flag is usable only with the default *--table*
>> output.
>> Note that this flag does not filter the list of domains.
>>
>> -If *--name* is specified, domain names are printed instead of the table
>> -formatted one per line. If *--uuid* is specified domain's UUID's are
>> printed
>> -instead of names. Flag *--table* specifies that the legacy
>> table-formatted
>> -output should be used. This is the default.
>> -
>> -If both *--name* and *--uuid* are specified, domain UUID's and names
>> -are printed side by side without any header. Flag *--table* specifies
>> -that the legacy table-formatted output should be used. This is the
>> -default if neither *--name* nor *--uuid* are specified. Option
>> -*--table* is mutually exclusive with options *--uuid* and *--name*.
>> +If *--name* is specified, domain names are printed instead of the
>> +table formatted one per line. If *--uuid* is specified domain's UUID's
>> +are printed instead of names. If *--id* is specified then domain's ID's
>> +are printed indead of names. However, it is possible to combine
>> +*--name*, *--uuid* and *--id* to select only desired fields for
>> +printing. Flag *--table* specifies that the legacy table-formatted
>> +output should be used, but it is mutually exclusive with *--name*,
>> +*--uuid* and *--id*. This is the default and will be used if neither of
>> +*--name*, *--uuid* or *--id* is specified.
>>
>
> Didn't you want to add something like the following?
>
> Â If neither *--name* nor *--uuid* is specified, but *--id* is, then
> only active
> Â domains are listed, even with the *--all* parameter as otherwise the
> output
> Â would just contain bunch of lines with just *-1*. >
> Otherwise the code is very unclear for no reason.
Yes, this behaviour is not documented. Mostly because I don't expect
regular users to use 'virsh list --id'. It was meant for completers,
e.g. virt-viewer has --id argument and its completer could list domain
IDs. Is it okay if I squash your suggestion in and push? Or do you want
me to send v3?
Michal
On Sat, Nov 07, 2020 at 11:43:05AM +0100, Michal Prívozník wrote:
>On 11/6/20 2:59 PM, Martin Kletzander wrote:
>> On Thu, Oct 22, 2020 at 04:41:18PM +0200, Michal Privoznik wrote:
>>> Some completers for libvirt related tools might want to list
>>> domain IDs only. Just like the one I've implemented for
>>> virt-viewer [1]. I've worked around it using some awk magic,
>>> but if it was possible to just 'virsh list --id' then I could
>>> drop awk.
>>>
>>> 1: https://www.redhat.com/archives/virt-tools-list/2019-May/msg00014.html
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>>> ---
>>>
>>> Diff to v1:
>>> - Documented the new switch in the manpage
>>> - Allowed --id to be used with --all
>>>
>>> docs/manpages/virsh.rst | 21 +++++++++---------
>>> tools/virsh-domain-monitor.c | 42 +++++++++++++++++++++++++-----------
>>> 2 files changed, 39 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
>>> index d34a1c8684..848e1a6458 100644
>>> --- a/docs/manpages/virsh.rst
>>> +++ b/docs/manpages/virsh.rst
>>> @@ -621,7 +621,7 @@ list
>>>
>>> list [--inactive | --all]
>>> [--managed-save] [--title]
>>> - { [--table] | --name | --uuid }
>>> + { [--table] | --name | --uuid | --id }
>>> [--persistent] [--transient]
>>> [--with-managed-save] [--without-managed-save]
>>> [--autostart] [--no-autostart]
>>> @@ -758,16 +758,15 @@ If *--managed-save* is specified, then domains
>>> that have managed save state
>>> in the listing. This flag is usable only with the default *--table*
>>> output.
>>> Note that this flag does not filter the list of domains.
>>>
>>> -If *--name* is specified, domain names are printed instead of the table
>>> -formatted one per line. If *--uuid* is specified domain's UUID's are
>>> printed
>>> -instead of names. Flag *--table* specifies that the legacy
>>> table-formatted
>>> -output should be used. This is the default.
>>> -
>>> -If both *--name* and *--uuid* are specified, domain UUID's and names
>>> -are printed side by side without any header. Flag *--table* specifies
>>> -that the legacy table-formatted output should be used. This is the
>>> -default if neither *--name* nor *--uuid* are specified. Option
>>> -*--table* is mutually exclusive with options *--uuid* and *--name*.
>>> +If *--name* is specified, domain names are printed instead of the
>>> +table formatted one per line. If *--uuid* is specified domain's UUID's
>>> +are printed instead of names. If *--id* is specified then domain's ID's
>>> +are printed indead of names. However, it is possible to combine
>>> +*--name*, *--uuid* and *--id* to select only desired fields for
>>> +printing. Flag *--table* specifies that the legacy table-formatted
>>> +output should be used, but it is mutually exclusive with *--name*,
>>> +*--uuid* and *--id*. This is the default and will be used if neither of
>>> +*--name*, *--uuid* or *--id* is specified.
>>>
>>
>> Didn't you want to add something like the following?
>>
>> If neither *--name* nor *--uuid* is specified, but *--id* is, then
>> only active
>> domains are listed, even with the *--all* parameter as otherwise the
>> output
>> would just contain bunch of lines with just *-1*. >
>> Otherwise the code is very unclear for no reason.
>
>Yes, this behaviour is not documented. Mostly because I don't expect
>regular users to use 'virsh list --id'. It was meant for completers,
>e.g. virt-viewer has --id argument and its completer could list domain
If this is the expectation when writing the code it should not be presented to
the user as just another option for them to use. Adding the intention to the
documentation could very nicely future-proof against user expectations.
>IDs. Is it okay if I squash your suggestion in and push? Or do you want
>me to send v3?
>
I was hoping someone will come up with a better wording, but it that sounds good
to you, then with that squashed in
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Feel free to decide yourself whether to also apply the suggestion from this
mail, that's not a biggie.
>Michal
© 2016 - 2024 Red Hat, Inc.