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

[Lulu_Su at wistron.com]

Hi Jeremy,

> Thinking about this a little more, let me know if I have your requirements correct:
> 
>  - you have a system with two storage devices connected, both containing
>   the same installer image
> 
>  - petitboot is suppressing the duplicates
> 
>  - you want to display a message indicating that petitboot has skipped
>   *some* of the duplicates. Specifically, those duplicates that exist
>   on the separate devices, but *not* the duplicates that exist as part
>   of the partition layout that a single device is presenting.
> 
> Is that what you're trying to do here?

Yes, what you describe is exactly what I want to achieve.

> If that's the case, then the conditional that you've proposed makes sense, but using the device names (to establish whether the devices are on the same underlying media) doesn't seem like a reliable method of doing this - the names are fairly arbitrary.
> 
> Instead, if you do want to do that, I'd suggest looking at the udev properties of the device. Two options here:
> 
>  - the ID_PATH properties of the two partitions will be the same on
>   sda/sda1 in your examples
>
>  - the DEVPATH property of the sda device will be a substring of the
>   DEVPATH for the sda1 device.
> 
> This way, you're not relying on the "sda" -> "sda1" matching, which is specific to the drivers' naming scheme.
>
> [that's mainly why I'd suggested making the log message unconditional - the duplicate matching isn't as straightforward as you're  proposing here. If it's not a problem that you generate 3 log messages instead of 1, then that may be a simpler approach]

Thank you for your suggestion. According to your suggestion, I corrected the patch as follows: (Only list conditions)

@@ -179,11 +181,17 @@ static int udev_handle_block_add(struct pb_udev *udev, struct udev_device *dev,
 	/* We may see multipath devices; they'll have the same uuid as an
 	 * existing device, so only parse the first. */
	uuid = udev_device_get_property_value(dev, "ID_FS_UUID");
+	idpath = udev_device_get_property_value(dev, "ID_PATH");
 	if (uuid) {
 		ddev = device_lookup_by_uuid(udev->handler, uuid);
 		if (ddev) {
 			pb_log("SKIP: %s UUID [%s] already present (as %s)\n",
 					name, uuid, ddev->device->id);
+			if (strcmp(idpath, ddev->id_path)) {
+				device_handler_status_info(udev->handler, 
+				_("Duplicate filesystem (%s) has been detected; only listing the %s"), 
+				name, ddev->device->id);
+			} 
 			return 0;
 		}
 	}
@@ -211,6 +219,7 @@ static int udev_handle_block_add(struct pb_udev *udev, struct udev_device *dev,
 		}
 	}
 
+	ddev->id_path = talloc_strdup(ddev, idpath);
 	ddev->device_path = talloc_strdup(ddev, node);
 	talloc_free(devlinks);

As you suggested in the first part, I used ID_PATH instead of name as the condition.
But in the second part, after verifying several different .iso files, I found that not all files have a partition (sdc1), so it is not suitable as a condition.
Like you said, I didn't think thoroughly enough, so no longer set the display conditions, all the log messages will be displayed.

Using the modified patch, the messages displayed is as follows:
In Petitboot status log:
 Duplicate filesystem (sdc) has been detected; only listing the sda
 Duplicate filesystem (sdc1) has been detected; only listing the sda
 Duplicate filesystem (sdc) has been detected; only listing the sda
 Duplicate filesystem (sdc) has been detected; only listing the sda

In pb-discover.log:
[02:24:02] SKIP: sda1 UUID [2020-04-04-04-38-27-00] already present (as sda)
[02:28:45] SKIP: sdc UUID [2020-04-04-04-38-27-00] already present (as sda)
[02:28:45] SKIP: sdc1 UUID [2020-04-04-04-38-27-00] already present (as sda)
[02:28:45] SKIP: sdc UUID [2020-04-04-04-38-27-00] already present (as sda)
[02:28:45] SKIP: sdc UUID [2020-04-04-04-38-27-00] already present (as sda)

The reason for retaining the first part of the condition is that if the file is successfully mounted to the petitboot list (sda), then its partition (sda1) does not need to display a warning message.
This warning message is only displayed when trying to mount a file but a file with the same UUID is already mounted on petitboot.

Is the content of the warning message clearly stated?
After discussing and reaching a consensus, I will upload the third edition patch, 
thank you.

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.
---------------------------------------------------------------------------------------------------------------------------------------------------------------