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

[Erik Skultety]

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