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@xxxxxxxxxx> > --- 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