[OSS-Tools] [PATCH] libdt: generalize of_find_device_by_uuid for scoped lookup of all UUIDs

[OSS-Tools] [PATCH] libdt: generalize of_find_device_by_uuid for scoped lookup of all UUIDs

[Ahmad Fatoum]

Despite the generic name, of_find_device_by_uuid only handled
ID_PART_TABLE_UUID (diskuuid) and ID_PART_ENTRY_UUID (partuuid),
but not ID_PART_ENTRY_TYPE (GPT Partition Type UUID).

In preparation for doing Type UUID lookups, adjust of_find_device_by_uuid
to support all three types of GPT UUIDs.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
This patch is the missing prerequisite for my GPT Type UUID series.
---
 src/libdt.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/src/libdt.c b/src/libdt.c
index e54d7fb5649d..2c994c647ac9 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2405,7 +2405,9 @@ out:
 	return dev;
 }
 
-static struct udev_device *of_find_device_by_uuid(const char *uuid)
+static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
+						  const char *uuid,
+						  bool type_uuid)
 {
 	struct udev *udev;
 	struct udev_enumerate *enumerate;
@@ -2418,12 +2420,15 @@ static struct udev_device *of_find_device_by_uuid(const char *uuid)
 	}
 
 	enumerate = udev_enumerate_new(udev);
+	if (parent)
+		udev_enumerate_add_match_parent(enumerate, parent);
 	udev_enumerate_add_match_subsystem(enumerate, "block");
 	udev_enumerate_scan_devices(enumerate);
 	devices = udev_enumerate_get_list_entry(enumerate);
 	udev_list_entry_foreach(dev_list_entry, devices) {
 		const char *path, *devtype, *dev_uuid;
 		struct udev_device *device;
+		const char *property;
 
 		path = udev_list_entry_get_name(dev_list_entry);
 		device = udev_device_new_from_syspath(udev, path);
@@ -2432,16 +2437,19 @@ static struct udev_device *of_find_device_by_uuid(const char *uuid)
 		devtype = udev_device_get_devtype(device);
 		if (!devtype)
 			continue;
-		if (!strcmp(devtype, "disk")) {
-			dev_uuid = udev_device_get_property_value(device, "ID_PART_TABLE_UUID");
-			if (dev_uuid && !strcasecmp(dev_uuid, uuid))
-				return device;
-		} else if (!strcmp(devtype, "partition")) {
-			dev_uuid = udev_device_get_property_value(device, "ID_PART_ENTRY_UUID");
-			if (dev_uuid && !strcasecmp(dev_uuid, uuid))
-				return device;
-		}
 
+		if (type_uuid)
+			property = "ID_PART_ENTRY_TYPE";
+		else if (!strcmp(devtype, "disk"))
+			property = "ID_PART_TABLE_UUID";
+		else if (!strcmp(devtype, "partition"))
+			property = "ID_PART_ENTRY_UUID";
+		else
+			continue;
+
+		dev_uuid = udev_device_get_property_value(device, property);
+		if (dev_uuid && !strcasecmp(dev_uuid, uuid))
+			return device;
 	}
 	return NULL;
 }
@@ -2540,7 +2548,7 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t
 					node->full_name);
 			return -ENODEV;
 		}
-		dev = of_find_device_by_uuid(uuid);
+		dev = of_find_device_by_uuid(NULL, uuid, false);
 		if (!dev) {
 			fprintf(stderr, "%s: cannot find device for uuid %s\n", __func__,
 				uuid);
-- 
2.39.2

Re: [OSS-Tools] [PATCH] libdt: generalize of_find_device_by_uuid for scoped lookup of all UUIDs

[Roland Hieber]

On Mon, Jun 05, 2023 at 04:32:36PM +0200, Ahmad Fatoum wrote:
> Despite the generic name, of_find_device_by_uuid only handled
> ID_PART_TABLE_UUID (diskuuid) and ID_PART_ENTRY_UUID (partuuid),
> but not ID_PART_ENTRY_TYPE (GPT Partition Type UUID).
> 
> In preparation for doing Type UUID lookups, adjust of_find_device_by_uuid
> to support all three types of GPT UUIDs.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> This patch is the missing prerequisite for my GPT Type UUID series.
> ---
>  src/libdt.c | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/src/libdt.c b/src/libdt.c
> index e54d7fb5649d..2c994c647ac9 100644
> --- a/src/libdt.c
> +++ b/src/libdt.c
> @@ -2405,7 +2405,9 @@ out:
>  	return dev;
>  }
>  
> -static struct udev_device *of_find_device_by_uuid(const char *uuid)
> +static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
> +						  const char *uuid,
> +						  bool type_uuid)
>  {
>  	struct udev *udev;
>  	struct udev_enumerate *enumerate;
> @@ -2418,12 +2420,15 @@ static struct udev_device *of_find_device_by_uuid(const char *uuid)
>  	}
>  
>  	enumerate = udev_enumerate_new(udev);
> +	if (parent)
> +		udev_enumerate_add_match_parent(enumerate, parent);
>  	udev_enumerate_add_match_subsystem(enumerate, "block");
>  	udev_enumerate_scan_devices(enumerate);
>  	devices = udev_enumerate_get_list_entry(enumerate);
>  	udev_list_entry_foreach(dev_list_entry, devices) {
>  		const char *path, *devtype, *dev_uuid;
>  		struct udev_device *device;
> +		const char *property;
>  
>  		path = udev_list_entry_get_name(dev_list_entry);
>  		device = udev_device_new_from_syspath(udev, path);
> @@ -2432,16 +2437,19 @@ static struct udev_device *of_find_device_by_uuid(const char *uuid)
>  		devtype = udev_device_get_devtype(device);
>  		if (!devtype)
>  			continue;
> -		if (!strcmp(devtype, "disk")) {
> -			dev_uuid = udev_device_get_property_value(device, "ID_PART_TABLE_UUID");
> -			if (dev_uuid && !strcasecmp(dev_uuid, uuid))
> -				return device;
> -		} else if (!strcmp(devtype, "partition")) {
> -			dev_uuid = udev_device_get_property_value(device, "ID_PART_ENTRY_UUID");
> -			if (dev_uuid && !strcasecmp(dev_uuid, uuid))
> -				return device;
> -		}
>  
> +		if (type_uuid)
> +			property = "ID_PART_ENTRY_TYPE";
> +		else if (!strcmp(devtype, "disk"))
> +			property = "ID_PART_TABLE_UUID";
> +		else if (!strcmp(devtype, "partition"))
> +			property = "ID_PART_ENTRY_UUID";
> +		else
> +			continue;
> +
> +		dev_uuid = udev_device_get_property_value(device, property);
> +		if (dev_uuid && !strcasecmp(dev_uuid, uuid))
> +			return device;

I wonder if we can't simply add a udev_enumerate_add_match_property
above to do the filtering for us instead of iterating over the match
entries ourselves.

But the patch is correct, so:
Reviewed-by: Roland Hieber <rhi@pengutronix.de>

>  	}
>  	return NULL;
>  }
> @@ -2540,7 +2548,7 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t
>  					node->full_name);
>  			return -ENODEV;
>  		}
> -		dev = of_find_device_by_uuid(uuid);
> +		dev = of_find_device_by_uuid(NULL, uuid, false);
>  		if (!dev) {
>  			fprintf(stderr, "%s: cannot find device for uuid %s\n", __func__,
>  				uuid);
> -- 
> 2.39.2
> 
> 

-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686         | Fax:   +49-5121-206917-5555 |

Re: [OSS-Tools] [PATCH] libdt: generalize of_find_device_by_uuid for scoped lookup of all UUIDs

[Ahmad Fatoum]

Hello Roland,

On 07.06.23 10:33, Roland Hieber wrote:
> On Mon, Jun 05, 2023 at 04:32:36PM +0200, Ahmad Fatoum wrote:
>> Despite the generic name, of_find_device_by_uuid only handled
>> ID_PART_TABLE_UUID (diskuuid) and ID_PART_ENTRY_UUID (partuuid),
>> but not ID_PART_ENTRY_TYPE (GPT Partition Type UUID).
>>
>> In preparation for doing Type UUID lookups, adjust of_find_device_by_uuid
>> to support all three types of GPT UUIDs.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> This patch is the missing prerequisite for my GPT Type UUID series.
>> ---
>>  src/libdt.c | 30 +++++++++++++++++++-----------
>>  1 file changed, 19 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/libdt.c b/src/libdt.c
>> index e54d7fb5649d..2c994c647ac9 100644
>> --- a/src/libdt.c
>> +++ b/src/libdt.c
>> @@ -2405,7 +2405,9 @@ out:
>>  	return dev;
>>  }
>>  
>> -static struct udev_device *of_find_device_by_uuid(const char *uuid)
>> +static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
>> +						  const char *uuid,
>> +						  bool type_uuid)
>>  {
>>  	struct udev *udev;
>>  	struct udev_enumerate *enumerate;
>> @@ -2418,12 +2420,15 @@ static struct udev_device *of_find_device_by_uuid(const char *uuid)
>>  	}
>>  
>>  	enumerate = udev_enumerate_new(udev);
>> +	if (parent)
>> +		udev_enumerate_add_match_parent(enumerate, parent);
>>  	udev_enumerate_add_match_subsystem(enumerate, "block");
>>  	udev_enumerate_scan_devices(enumerate);
>>  	devices = udev_enumerate_get_list_entry(enumerate);
>>  	udev_list_entry_foreach(dev_list_entry, devices) {
>>  		const char *path, *devtype, *dev_uuid;
>>  		struct udev_device *device;
>> +		const char *property;
>>  
>>  		path = udev_list_entry_get_name(dev_list_entry);
>>  		device = udev_device_new_from_syspath(udev, path);
>> @@ -2432,16 +2437,19 @@ static struct udev_device *of_find_device_by_uuid(const char *uuid)
>>  		devtype = udev_device_get_devtype(device);
>>  		if (!devtype)
>>  			continue;
>> -		if (!strcmp(devtype, "disk")) {
>> -			dev_uuid = udev_device_get_property_value(device, "ID_PART_TABLE_UUID");
>> -			if (dev_uuid && !strcasecmp(dev_uuid, uuid))
>> -				return device;
>> -		} else if (!strcmp(devtype, "partition")) {
>> -			dev_uuid = udev_device_get_property_value(device, "ID_PART_ENTRY_UUID");
>> -			if (dev_uuid && !strcasecmp(dev_uuid, uuid))
>> -				return device;
>> -		}
>>  
>> +		if (type_uuid)
>> +			property = "ID_PART_ENTRY_TYPE";
>> +		else if (!strcmp(devtype, "disk"))
>> +			property = "ID_PART_TABLE_UUID";
>> +		else if (!strcmp(devtype, "partition"))
>> +			property = "ID_PART_ENTRY_UUID";
>> +		else
>> +			continue;
>> +
>> +		dev_uuid = udev_device_get_property_value(device, property);
>> +		if (dev_uuid && !strcasecmp(dev_uuid, uuid))
>> +			return device;
> 
> I wonder if we can't simply add a udev_enumerate_add_match_property
> above to do the filtering for us instead of iterating over the match
> entries ourselves.

I didn't know about udev_enumerate_add_match_property. Are multiple filters
OR'd or AND'd? For the !type_uuid case we need to check one or the other
property.

> But the patch is correct, so:
> Reviewed-by: Roland Hieber <rhi@pengutronix.de>

Thanks. I have applied this patch and will keep udev_enumerate_add_match_property
in mind for future rework.

Cheers,
Ahmad

> 
>>  	}
>>  	return NULL;
>>  }
>> @@ -2540,7 +2548,7 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t
>>  					node->full_name);
>>  			return -ENODEV;
>>  		}
>> -		dev = of_find_device_by_uuid(uuid);
>> +		dev = of_find_device_by_uuid(NULL, uuid, false);
>>  		if (!dev) {
>>  			fprintf(stderr, "%s: cannot find device for uuid %s\n", __func__,
>>  				uuid);
>> -- 
>> 2.39.2
>>
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [OSS-Tools] [PATCH] libdt: generalize of_find_device_by_uuid for scoped lookup of all UUIDs

[Roland Hieber]

On Wed, Jun 07, 2023 at 01:00:29PM +0200, Ahmad Fatoum wrote:
> Hello Roland,
> 
> On 07.06.23 10:33, Roland Hieber wrote:
> > On Mon, Jun 05, 2023 at 04:32:36PM +0200, Ahmad Fatoum wrote:
> >> Despite the generic name, of_find_device_by_uuid only handled
> >> ID_PART_TABLE_UUID (diskuuid) and ID_PART_ENTRY_UUID (partuuid),
> >> but not ID_PART_ENTRY_TYPE (GPT Partition Type UUID).
> >>
> >> In preparation for doing Type UUID lookups, adjust of_find_device_by_uuid
> >> to support all three types of GPT UUIDs.
> >>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >> This patch is the missing prerequisite for my GPT Type UUID series.
> >> ---
> >>  src/libdt.c | 30 +++++++++++++++++++-----------
> >>  1 file changed, 19 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/src/libdt.c b/src/libdt.c
> >> index e54d7fb5649d..2c994c647ac9 100644
> >> --- a/src/libdt.c
> >> +++ b/src/libdt.c
> >> @@ -2405,7 +2405,9 @@ out:
> >>  	return dev;
> >>  }
> >>  
> >> -static struct udev_device *of_find_device_by_uuid(const char *uuid)
> >> +static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
> >> +						  const char *uuid,
> >> +						  bool type_uuid)
> >>  {
> >>  	struct udev *udev;
> >>  	struct udev_enumerate *enumerate;
> >> @@ -2418,12 +2420,15 @@ static struct udev_device *of_find_device_by_uuid(const char *uuid)
> >>  	}
> >>  
> >>  	enumerate = udev_enumerate_new(udev);
> >> +	if (parent)
> >> +		udev_enumerate_add_match_parent(enumerate, parent);
> >>  	udev_enumerate_add_match_subsystem(enumerate, "block");
> >>  	udev_enumerate_scan_devices(enumerate);
> >>  	devices = udev_enumerate_get_list_entry(enumerate);
> >>  	udev_list_entry_foreach(dev_list_entry, devices) {
> >>  		const char *path, *devtype, *dev_uuid;
> >>  		struct udev_device *device;
> >> +		const char *property;
> >>  
> >>  		path = udev_list_entry_get_name(dev_list_entry);
> >>  		device = udev_device_new_from_syspath(udev, path);
> >> @@ -2432,16 +2437,19 @@ static struct udev_device *of_find_device_by_uuid(const char *uuid)
> >>  		devtype = udev_device_get_devtype(device);
> >>  		if (!devtype)
> >>  			continue;
> >> -		if (!strcmp(devtype, "disk")) {
> >> -			dev_uuid = udev_device_get_property_value(device, "ID_PART_TABLE_UUID");
> >> -			if (dev_uuid && !strcasecmp(dev_uuid, uuid))
> >> -				return device;
> >> -		} else if (!strcmp(devtype, "partition")) {
> >> -			dev_uuid = udev_device_get_property_value(device, "ID_PART_ENTRY_UUID");
> >> -			if (dev_uuid && !strcasecmp(dev_uuid, uuid))
> >> -				return device;
> >> -		}
> >>  
> >> +		if (type_uuid)
> >> +			property = "ID_PART_ENTRY_TYPE";
> >> +		else if (!strcmp(devtype, "disk"))
> >> +			property = "ID_PART_TABLE_UUID";
> >> +		else if (!strcmp(devtype, "partition"))
> >> +			property = "ID_PART_ENTRY_UUID";
> >> +		else
> >> +			continue;
> >> +
> >> +		dev_uuid = udev_device_get_property_value(device, property);
> >> +		if (dev_uuid && !strcasecmp(dev_uuid, uuid))
> >> +			return device;
> > 
> > I wonder if we can't simply add a udev_enumerate_add_match_property
> > above to do the filtering for us instead of iterating over the match
> > entries ourselves.
> 
> I didn't know about udev_enumerate_add_match_property. Are multiple filters
> OR'd or AND'd? For the !type_uuid case we need to check one or the other
> property.

Sorry, I don't know either… it looks to me like they are ANDed, but the
documentation is a bit sparse on the udev side…

 - Roland

> 
> > But the patch is correct, so:
> > Reviewed-by: Roland Hieber <rhi@pengutronix.de>
> 
> Thanks. I have applied this patch and will keep udev_enumerate_add_match_property
> in mind for future rework.
> 
> Cheers,
> Ahmad
> 
> > 
> >>  	}
> >>  	return NULL;
> >>  }
> >> @@ -2540,7 +2548,7 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t
> >>  					node->full_name);
> >>  			return -ENODEV;
> >>  		}
> >> -		dev = of_find_device_by_uuid(uuid);
> >> +		dev = of_find_device_by_uuid(NULL, uuid, false);
> >>  		if (!dev) {
> >>  			fprintf(stderr, "%s: cannot find device for uuid %s\n", __func__,
> >>  				uuid);
> >> -- 
> >> 2.39.2
> >>
> >>
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 

-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686         | Fax:   +49-5121-206917-5555 |