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

[Lulu_Su at wistron.com]

Hi Jeremy,

> No, that's not suitable. It's pretty unusual for there to be a filesystem on the base device and a partition. The reason this is occurring is that the install image is reporting itself as both an
> iso9660 device, *and* a MBR partition table. That's why we're seeing the duplicates on the same device.
> 
> The regular scenario for discovered filesystem is just the partitions carrying filesystems, and none on the top-level device. I assume you still need warnings for that case too. If you > only generate the message when DEVTYPE == disk, then you'll miss those, and this code will only work for these specific installer-type images.
>
> My suggestion is to sort-of reverse the duplicate detection - and just record in the first device (that provided a specific UUID) whether we've warned about that UUID.
> 
> So, when we find the first duplicate (sda1, sdc, or sdc1 in your examples), you generate the log message, and set ->dup_warn on the discover_device for sda.
> 
> Then, subsequent duplicates do not generate a warning, because->dup_warn is already set on the original duplicate (ie, sda).
> 
> However, this may not be sufficient for your example - I assume you require the warning to be generated when the new device, sdc, is attached. With this flow, it would be generated when we detect sda1, so may not be obvious to indicate to the user what is happening when the new virtual media device appears.
> 
> So, to implement it the way you're wanting here, it sounds like you're going to have to implement a system for duplicate tracking - eg, by keeping a enough info about the devices & UUIDs that you've seen, separate from the main device set, so you can generate the warnings in the way that you require (ie, warn once for a complete new device).

Thank you for your information, it has benefited me a lot.

Based on the previous discussion, I amended the conditions as follows:
	1. When the device sda ​​is mounted, use ID_PATH to determine that sad1 and sda are the same device.
	2. When a new device (sdc) is detected, the warning will be displayed only once; the Boolean value will not be cleared until the device is removed.
Therefore, the warning will only be displayed once. When a new device (sdc) with the same UUID is detected, and when the user repeatedly mounts the same device (sdc), it will still be displayed again.
Hope this modification meets your suggestion, thank you very much~

Here is the patch: 

discover/device-handler.h |  2 ++
 discover/udev.c           | 22 ++++++++++++++++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/discover/device-handler.h b/discover/device-handler.h
index 6591120..216d17b 100644
--- a/discover/device-handler.h
+++ b/discover/device-handler.h
@@ -25,6 +25,7 @@ struct discover_device {
 
 	const char		*uuid;
 	const char		*label;
+	const char		*id_path;
 
 	char			*mount_path;
 	char			*root_path;
@@ -36,6 +37,7 @@ struct discover_device {
 	bool			crypt_device;
 
 	bool			notified;
+	bool			dup_warn;
 
 	struct list		boot_options;
 	struct list		params;
diff --git a/discover/udev.c b/discover/udev.c
index 0c3da66..a140008 100644
--- a/discover/udev.c
+++ b/discover/udev.c
@@ -20,6 +20,7 @@
 #include <waiter/waiter.h>
 #include <system/system.h>
 #include <process/process.h>
+#include <i18n/i18n.h>
 
 #include "event.h"
 #include "udev.h"
@@ -101,6 +102,7 @@ static int udev_handle_block_add(struct pb_udev *udev, struct udev_device *dev,
 	const char *prop;
 	const char *type;
 	const char *devname;
+	const char *idpath;
 	const char *ignored_types[] = {
 		"linux_raid_member",
 		"swap",
@@ -179,11 +181,19 @@ 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);
+			/* Only warn once in petitboot status log to remind users */
+			if (strcmp(idpath, ddev->id_path) && !ddev->dup_warn) {
+				device_handler_status_info(udev->handler, 
+				_("Duplicate filesystem as %s detected; skipping duplicates"), 
+				ddev->device->id);
+				ddev->dup_warn = true;
+			} 
 			return 0;
 		}
 	}
@@ -211,6 +221,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);
 
@@ -355,6 +366,7 @@ static int udev_handle_dev_remove(struct pb_udev *udev, struct udev_device *dev)
 {
 	struct discover_device *ddev;
 	const char *name;
+	const char *uuid;
 
 	name = udev_device_get_sysname(dev);
 	if (!name) {
@@ -363,9 +375,15 @@ static int udev_handle_dev_remove(struct pb_udev *udev, struct udev_device *dev)
 	}
 
 	ddev = device_lookup_by_id(udev->handler, name);
-	if (!ddev)
+	if (!ddev) {
+		uuid = udev_device_get_property_value(dev, "ID_FS_UUID");
+		if (uuid) {
+			ddev = device_lookup_by_uuid(udev->handler, uuid);
+			if (ddev)
+				ddev->dup_warn = false;
+		}
 		return 0;
-
+	}
 	device_handler_remove(udev->handler, ddev);
 
 	return 0;

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