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
