[libvirt PATCH v2 04/16] virsh: Add --active, --inactive, --all to nodedev-list

Erik Skultety eskultet at redhat.com
Fri Aug 21 10:34:58 UTC 2020


On Tue, Aug 18, 2020 at 09:47:54AM -0500, Jonathon Jongsma wrote:
> Now that we can filter active and inactive node devices in
> virConnectListAllNodeDevices(), add these switches to the virsh command.
>
> Eventual output (once everything is hooked up):
>
>     virsh # nodedev-list --inactive --cap mdev
>     mdev_07d8b8b0_7e04_4c0f_97ed_9214ce12723c
>     mdev_927c040f_ae7d_4a35_966e_286ba6ebbe1c
>
>     virsh # nodedev-list --active --cap mdev
>     mdev_bd2ea955_3402_4252_8c17_7468083a0f26
>
>     virsh # nodedev-list --all --cap mdev
>     mdev_07d8b8b0_7e04_4c0f_97ed_9214ce12723c
>     mdev_927c040f_ae7d_4a35_966e_286ba6ebbe1c
>     mdev_bd2ea955_3402_4252_8c17_7468083a0f26
>
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---

I think this patch would have been better placed later in the series like 10/16
maybe, but that's just a personal taste.

...

>
> +    if (inactive || all)
> +        flags |= VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE;
> +    if (active || all)

Do we need a bool for 'active' at all? I mean listing 'active' should be the
default unless '--all' was passed.

> +        flags |= VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE;
> +
>      if (!(list = virshNodeDeviceListCollect(ctl, caps, ncaps, flags))) {
>          ret = false;
>          goto cleanup;

The rest looks good.
Erik




More information about the libvir-list mailing list