Re: [libvirt] [PATCH 4/7] virsh: nodedev: ability to filter CCW capabilities
On 05/26/2017 03:00 AM, Bjoern Walk wrote:
John Ferlan <jferlan(a)redhat.com> [2017-05-25, 03:21PM -0400]:
>
>
> On 05/22/2017 02:38 AM, Bjoern Walk wrote:
>> Now that the node_device driver is aware of CCW devices, let's hook up
>> virsh so that we can filter them properly.
>>
>> Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
>> Reviewed-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
>> Signed-off-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
>> ---
>> docs/formatnode.html.in | 12 ++++++++++++
>> include/libvirt/libvirt-nodedev.h | 1 +
>> src/conf/node_device_conf.h | 3 ++-
>> src/conf/virnodedeviceobj.c | 3 ++-
>> src/libvirt-nodedev.c | 1 +
>> tools/virsh-nodedev.c | 3 ++-
>> tools/virsh.pod | 2 +-
>> 7 files changed, 21 insertions(+), 4 deletions(-)
>>
>
>
> ...
>
>> diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
>> index 02ac54437..d460f26ec 100644
>> --- a/src/conf/virnodedeviceobj.c
>> +++ b/src/conf/virnodedeviceobj.c
>> @@ -568,7 +568,8 @@ virNodeDeviceMatch(virNodeDeviceObjPtr devobj,
>> MATCH(SCSI_GENERIC) ||
>> MATCH(DRM) ||
>> MATCH(MDEV_TYPES) ||
>> - MATCH(MDEV)))
>> + MATCH(MDEV) ||
>> + MATCH(CCW_DEV)))
>> return false;
>> }
>>
>
> But you didn't modify virNodeDeviceCapMatch in order perform the match
> that the MATCH does.
>
Damn, there's a lot that you can miss with these capabilities.
I sent a patch which should cover this for the future:
https://www.redhat.com/archives/libvir-list/2017-May/msg01003.html
> Also, another well hidden gem that either Erik Skultety or myself
will
> fix "eventually" is virNodeDeviceObjHasCap. See commit id
'e8fcac8ec'
> for some context. Essentially, the virNodeDeviceMatch only matches for
> the virNodeDeviceObjListExport API.
>
And even more. Thanks for the pointers.
In rethinking this - it doesn't necessarily seem as though this device
would have the same issues as NPIV (a SCSI_HOST device parent that has
capabilities of either "fc_host" or "vports" to describe a capable
child) or MDEV (a PCI device that a parent mediated device with "mdev"
capabilities to describe the children).
As long as you're comfortable that the match can be done - I'm good. The
mdev changes were recently reviewed by me so it was fresh in my mind.
John
> If you want to send something to squash in or just a v2 of this
patch -
> I can handle either.
>
> Reviewed-by: John Ferlan <jferlan(a)redhat.com>
Thanks.
>
> John
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>