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