[PATCH] discover/udev.c: Added warning in system status log

Lulu_Su at wistron.com Lulu_Su at wistron.com
Tue Jul 13 20:59:38 AEST 2021


Hi Jeremy,

> [Looks like I made a mess of the previous reply; sending again with 
> the same content, this time with proper formatting]

>> When the same iso files are installed at the same time, the petitboot 
>> menu will only display the installed device first, and will not 
>> notify the user that the same iso file has been installed, so an 
>> alert is added to the system status log to remind the user that they 
>> have mounted the same iso file.

> What do you mean by 'iso file has been installed'?

> Keep in mind that the system status log is not very obvious, so I'm not sure if this will help the user much. This will also create a status message on the main menu screen, but that may be replaced quickly if another event occurs in quick sucession. Could you add a little detail on what issue the user is seeing here?

Forward user description:
" Tester has found during the Virtual media Install when he mounted USB and VIrtual Media only 1 USB option is visible in the petitboot for install. It should have another label which user can select for install.
Same was showing earlier properly looks like there was some change which has caused this. "

But in my understanding, if two devices have the same UUID, the list will only show the first one, right?

If I make changes based on user requirements, this change will affect all platforms. I don't confirm the necessity of this feature, so I want to add a warning, at least to remind users when they mount the same file, instead of letting them misunderstand that the device is broken.

Could you give me some suggestions?


>> @@ -184,6 +185,11 @@ static int udev_handle_block_add(struct pb_udev 
>> *udev, struct udev_device *dev,
>> +		if (ddev) {
>> +			pb_log("SKIP: %s UUID [%s] already present (as %s)\n",
>> +					name, uuid, ddev->device->id);
>> +			if (strncmp(name, ddev->device->id, strlen(ddev->device->id))) {

> When would this check ever fail? We'd have multiple block devices with the same kernel name.

When a device using the UUID already exists, but the device name is different (e.g. Already exists device1: sda, New device2:sdc )

>> +				ddev = discover_device_create(udev->handler, NULL, name);

> Why create the device here? We're about to skip discovery.

> There are status logging functions that do not require a dev pointer.

This is my old warning: "[sdc] The list doesn’t support displaying the same mount file"
I use this function to display warnings ( https://github.com/open-power/petitboot/blob/f7f98227ae991497ffe361eec3b7a9d1b2d94db4/discover/device-handler.h#L124 ) 
Besides creating sdc, could you suggest me a way to make this function display sdc?

>> +				device_handler_status_dev_info(udev-> handler, ddev,
>> +				_("The list doesn't support displaying the same mount file"));

> This log message seems overly specific, and doesn't really describe the situation (we're seeing a duplicate filesystem appear); we'd hit this case on a few different situations, and the term 'mount file' doesn't apply to all of those.

> How about:

> "%s: A duplicate filesystem has been detected (same as %s); only listing the first"
  name, ddev->device->id

> - would that work for your case?

Thank you for your suggestion, I will use this suggestion to modify the log message.

>Regards,


>Jeremy

Regards,
-Lulu Su

---------------------------------------------------------------------------------------------------------------------------------------------------------------
This email contains confidential or legally privileged information and is for the sole use of its intended recipient. 
Any unauthorized review, use, copying or distribution of this email or the content of this email is strictly prohibited.
If you are not the intended recipient, you may reply to the sender and should delete this e-mail immediately.
---------------------------------------------------------------------------------------------------------------------------------------------------------------



More information about the Petitboot mailing list