[PATCH libvirt v2 06/11] nodedev: detect AP matrix device

[Shalini Chellathurai Saroja]

On 11/30/20 2:46 PM, Erik Skultety wrote:
> On Fri, Nov 13, 2020 at 03:45:33PM +0100, Shalini Chellathurai Saroja wrote:
>> On 11/12/20 9:29 PM, Jonathon Jongsma wrote:
>>>> diff --git a/src/node_device/node_device_udev.c
>>>> b/src/node_device/node_device_udev.c index 6bbff571..5f57000e 100644
>>>> --- a/src/node_device/node_device_udev.c
>>>> +++ b/src/node_device/node_device_udev.c
>>>> @@ -1241,6 +1241,25 @@ udevProcessAPQueue(struct udev_device *device,
>>>>    }
>>>> +static int
>>>> +udevProcessAPMatrix(struct udev_device *device,
>>>> +                    virNodeDeviceDefPtr def)
>>>> +{
>>>> +    size_t i;
>>>> +    virNodeDevCapDataPtr data = &def->caps->data;
>>>> +
>>>> +    data->ap_matrix.addr =
>>>> g_strdup(udev_device_get_sysname(device));
>>>> +    def->name = g_strdup_printf("ap_%s", data->ap_matrix.addr);
>>> So you're setting this 'addr' field, but it is not used anywhere else in
>>> this patch. Perhaps you'll use it in upcoming patches. But it is not
>>> formatted into the node device xml or anything like that. Is that
>>> intentional?
>>>
>> Yes, it is not formatted into the node device xml.
>>
>> I will include this change in the patch 10.
>>
>>>> +
>>>> +    for (i = 0; i < strlen(def->name); i++) {
>>>> +        if (!(g_ascii_isalnum(*(def->name + i))))
>>>> +            *(def->name + i) = '_';
>>>> +    }
>>>> +
>>>> +    return 0;
>>>> +}
>>> Out of curiosity, what's the reason that you're
>>> hard-coding an "ap_" prefix to the nodedev name rather than just using
>>> udevGenerateDeviceName() like all of the other device types?
>> The name generated with udevGenerateDeviceName() is matrix_matrix (as both
>> udev_device_get_subsystem and udev_device_get_sysname returns matrix), which
>> is not very helpful, so we decided to go with ap_matrix instead.
> But if that is the case, I'm wondering why are you trying to generate it
> dynamically in the first place, why not hardcoding it to "ap_matrix"? I must be
> missing something here.
>
> Regards,
> Erik
ok, I will hard code it. Thank you for the review.

-- 
Kind regards
Shalini Chellathurai Saroja
Linux on Z and Virtualization Development
Vorsitzende des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294