* [U-Boot] [RFC] fs: make it possible to read the filesystem UUID
@ 2014-10-22 13:29 Christian Gmeiner
2014-10-29 12:15 ` Christian Gmeiner
2014-10-29 19:39 ` Simon Glass
0 siblings, 2 replies; 6+ messages in thread
From: Christian Gmeiner @ 2014-10-22 13:29 UTC (permalink / raw)
To: u-boot
Some filesystems have a UUID stored in its superblock. To
allow using root=UUID=... for the kernel command line we
need a way to read-out the filesystem UUID. This is what this
patch tries to do.
Keep in mind that this patch is an RFC and I hope to get some
feedback on this patch.
This is what blkid under linux gives me:
/dev/sdh1: LABEL="data" UUID="b6995cec-97e2-48b8-94dc-8ba7afc92bac" TYPE="ext4" PARTUUID="43f21f0e-01"
/dev/sdh2: LABEL="rfs" UUID="8d020de7-c75e-4674-948e-f7838a931b02" TYPE="ext4" PARTUUID="43f21f0e-02"
And here is the output from u-boot:
=> sata init
AHCI 0001.0300 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
flags: ncq stag pm led clo only pmp pio slum part
SATA Device Info:
S/N: 000060059798A1000014
Product model number: SFCA4096H1BR4TO-I-MS-236-STD
Firmware version: 20111021
Capacity: 7793856 sectors
=> fs_uuid sata 0:1 ; print fs_uuid; fs_uuid sata 0:2 ; print fs_uuid
fs_uuid=b6995cec-97e2-48b8-94dc-8ba7afc92bac
fs_uuid=8d020de7-c75e-4674-948e-f7838a931b02
Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
---
common/Makefile | 1 +
common/cmd_fs_uuid.c | 23 +++++++++++++++++++++++
fs/ext4/ext4_common.c | 9 +++++++++
fs/fs.c | 28 ++++++++++++++++++++++++++++
include/ext4fs.h | 1 +
include/fs.h | 2 ++
6 files changed, 64 insertions(+)
create mode 100644 common/cmd_fs_uuid.c
diff --git a/common/Makefile b/common/Makefile
index b19d379..d5d3315 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -188,6 +188,7 @@ obj-y += usb.o usb_hub.o
obj-$(CONFIG_USB_STORAGE) += usb_storage.o
endif
obj-$(CONFIG_CMD_FASTBOOT) += cmd_fastboot.o
+obj-$(CONFIG_CMD_FS_UUID) += cmd_fs_uuid.o
obj-$(CONFIG_CMD_USB_MASS_STORAGE) += cmd_usb_mass_storage.o
obj-$(CONFIG_CMD_THOR_DOWNLOAD) += cmd_thordown.o
diff --git a/common/cmd_fs_uuid.c b/common/cmd_fs_uuid.c
new file mode 100644
index 0000000..3af9518
--- /dev/null
+++ b/common/cmd_fs_uuid.c
@@ -0,0 +1,23 @@
+/*
+ * cmd_fs_uuid.c -- fs_uuid command
+ *
+ * Copyright (C) 2014, Bachmann electronic GmbH
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <common.h>
+#include <command.h>
+#include <fs.h>
+
+static int do_fs_uuid_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+ return do_fs_uuid(cmdtp, flag, argc, argv, FS_TYPE_ANY);
+}
+
+U_BOOT_CMD(
+ fs_uuid, 3, 0, do_fs_uuid_wrapper,
+ "filesystem UUDI related commands",
+ "<interface> <dev>:<part>\n"
+ " - print filesystem UUID\n"
+);
diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 33d69c9..926b6c6 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -2246,3 +2246,12 @@ fail:
return 0;
}
+
+void ext4fs_uuid(char *uuid_str)
+{
+ if (ext4fs_root == NULL)
+ return;
+
+ uuid_bin_to_str((unsigned char *)ext4fs_root->sblock.unique_id, uuid_str,
+ UUID_STR_FORMAT_STD);
+}
diff --git a/fs/fs.c b/fs/fs.c
index dd680f3..f8c6d64 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -85,6 +85,7 @@ struct fstype_info {
int (*size)(const char *filename);
int (*read)(const char *filename, void *buf, int offset, int len);
int (*write)(const char *filename, void *buf, int offset, int len);
+ void (*uuid)(char *uuid_str);
void (*close)(void);
};
@@ -113,6 +114,7 @@ static struct fstype_info fstypes[] = {
.size = ext4fs_size,
.read = ext4_read_file,
.write = fs_write_unsupported,
+ .uuid = ext4fs_uuid,
},
#endif
#ifdef CONFIG_SANDBOX
@@ -206,6 +208,14 @@ static void fs_close(void)
fs_type = FS_TYPE_ANY;
}
+void fs_uuid(char *uuid_str)
+{
+ struct fstype_info *info = fs_get_info(fs_type);
+
+ if (info->uuid)
+ info->uuid(uuid_str);
+}
+
int fs_ls(const char *dirname)
{
int ret;
@@ -289,6 +299,24 @@ int fs_write(const char *filename, ulong addr, int offset, int len)
return ret;
}
+int do_fs_uuid(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
+ int fstype)
+{
+ char uuid[37];
+ memset(uuid, 0, sizeof(uuid));
+
+ if (argc != 3)
+ return CMD_RET_USAGE;
+
+ if (fs_set_blk_dev(argv[1], argv[2], fstype))
+ return 1;
+
+ fs_uuid(uuid);
+ setenv("fs_uuid", uuid);
+
+ return 0;
+}
+
int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
int fstype)
{
diff --git a/include/ext4fs.h b/include/ext4fs.h
index 6c419f3..4011370 100644
--- a/include/ext4fs.h
+++ b/include/ext4fs.h
@@ -132,6 +132,7 @@ struct ext_filesystem *get_fs(void);
int ext4fs_open(const char *filename);
int ext4fs_read(char *buf, unsigned len);
int ext4fs_mount(unsigned part_length);
+void ext4fs_uuid(char *uuid_str);
void ext4fs_close(void);
void ext4fs_reinit_global(void);
int ext4fs_ls(const char *dirname);
diff --git a/include/fs.h b/include/fs.h
index 06a45f2..1956b67 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -82,6 +82,8 @@ int fs_write(const char *filename, ulong addr, int offset, int len);
* Common implementation for various filesystem commands, optionally limited
* to a specific filesystem type via the fstype parameter.
*/
+int do_fs_uuid(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
+ int fstype);
int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
int fstype);
int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [RFC] fs: make it possible to read the filesystem UUID
2014-10-22 13:29 [U-Boot] [RFC] fs: make it possible to read the filesystem UUID Christian Gmeiner
@ 2014-10-29 12:15 ` Christian Gmeiner
2014-10-29 19:57 ` Pavel Machek
2014-10-30 3:42 ` Stephen Warren
2014-10-29 19:39 ` Simon Glass
1 sibling, 2 replies; 6+ messages in thread
From: Christian Gmeiner @ 2014-10-29 12:15 UTC (permalink / raw)
To: u-boot
Hi all.
Adding Stephen Warren, Simon Glass and Pavel Machek to CC list
(./scripts/get_maintainer.pl -f fs/fs.c).
2014-10-22 15:29 GMT+02:00 Christian Gmeiner <christian.gmeiner@gmail.com>:
> Some filesystems have a UUID stored in its superblock. To
> allow using root=UUID=... for the kernel command line we
> need a way to read-out the filesystem UUID. This is what this
> patch tries to do.
>
> Keep in mind that this patch is an RFC and I hope to get some
> feedback on this patch.
>
> This is what blkid under linux gives me:
> /dev/sdh1: LABEL="data" UUID="b6995cec-97e2-48b8-94dc-8ba7afc92bac" TYPE="ext4" PARTUUID="43f21f0e-01"
> /dev/sdh2: LABEL="rfs" UUID="8d020de7-c75e-4674-948e-f7838a931b02" TYPE="ext4" PARTUUID="43f21f0e-02"
>
> And here is the output from u-boot:
> => sata init
> AHCI 0001.0300 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
> flags: ncq stag pm led clo only pmp pio slum part
> SATA Device Info:
> S/N: 000060059798A1000014
> Product model number: SFCA4096H1BR4TO-I-MS-236-STD
> Firmware version: 20111021
> Capacity: 7793856 sectors
> => fs_uuid sata 0:1 ; print fs_uuid; fs_uuid sata 0:2 ; print fs_uuid
> fs_uuid=b6995cec-97e2-48b8-94dc-8ba7afc92bac
> fs_uuid=8d020de7-c75e-4674-948e-f7838a931b02
>
Really no feedback?
> Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> ---
> common/Makefile | 1 +
> common/cmd_fs_uuid.c | 23 +++++++++++++++++++++++
> fs/ext4/ext4_common.c | 9 +++++++++
> fs/fs.c | 28 ++++++++++++++++++++++++++++
> include/ext4fs.h | 1 +
> include/fs.h | 2 ++
> 6 files changed, 64 insertions(+)
> create mode 100644 common/cmd_fs_uuid.c
>
> diff --git a/common/Makefile b/common/Makefile
> index b19d379..d5d3315 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -188,6 +188,7 @@ obj-y += usb.o usb_hub.o
> obj-$(CONFIG_USB_STORAGE) += usb_storage.o
> endif
> obj-$(CONFIG_CMD_FASTBOOT) += cmd_fastboot.o
> +obj-$(CONFIG_CMD_FS_UUID) += cmd_fs_uuid.o
>
> obj-$(CONFIG_CMD_USB_MASS_STORAGE) += cmd_usb_mass_storage.o
> obj-$(CONFIG_CMD_THOR_DOWNLOAD) += cmd_thordown.o
> diff --git a/common/cmd_fs_uuid.c b/common/cmd_fs_uuid.c
> new file mode 100644
> index 0000000..3af9518
> --- /dev/null
> +++ b/common/cmd_fs_uuid.c
> @@ -0,0 +1,23 @@
> +/*
> + * cmd_fs_uuid.c -- fs_uuid command
> + *
> + * Copyright (C) 2014, Bachmann electronic GmbH
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <fs.h>
> +
> +static int do_fs_uuid_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + return do_fs_uuid(cmdtp, flag, argc, argv, FS_TYPE_ANY);
> +}
> +
> +U_BOOT_CMD(
> + fs_uuid, 3, 0, do_fs_uuid_wrapper,
> + "filesystem UUDI related commands",
> + "<interface> <dev>:<part>\n"
> + " - print filesystem UUID\n"
> +);
> diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
> index 33d69c9..926b6c6 100644
> --- a/fs/ext4/ext4_common.c
> +++ b/fs/ext4/ext4_common.c
> @@ -2246,3 +2246,12 @@ fail:
>
> return 0;
> }
> +
> +void ext4fs_uuid(char *uuid_str)
> +{
> + if (ext4fs_root == NULL)
> + return;
> +
> + uuid_bin_to_str((unsigned char *)ext4fs_root->sblock.unique_id, uuid_str,
> + UUID_STR_FORMAT_STD);
> +}
> diff --git a/fs/fs.c b/fs/fs.c
> index dd680f3..f8c6d64 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -85,6 +85,7 @@ struct fstype_info {
> int (*size)(const char *filename);
> int (*read)(const char *filename, void *buf, int offset, int len);
> int (*write)(const char *filename, void *buf, int offset, int len);
> + void (*uuid)(char *uuid_str);
> void (*close)(void);
> };
>
> @@ -113,6 +114,7 @@ static struct fstype_info fstypes[] = {
> .size = ext4fs_size,
> .read = ext4_read_file,
> .write = fs_write_unsupported,
> + .uuid = ext4fs_uuid,
> },
> #endif
> #ifdef CONFIG_SANDBOX
> @@ -206,6 +208,14 @@ static void fs_close(void)
> fs_type = FS_TYPE_ANY;
> }
>
Should I do add an #ifdef CONFIG_CMD_FS_UUID around .uuid = ext4fs_uuid?
Or speaking more general: At which places should I add this ifdef to
opt-in the FS_UUID
feature?
> +void fs_uuid(char *uuid_str)
> +{
> + struct fstype_info *info = fs_get_info(fs_type);
> +
> + if (info->uuid)
> + info->uuid(uuid_str);
> +}
> +
> int fs_ls(const char *dirname)
> {
> int ret;
> @@ -289,6 +299,24 @@ int fs_write(const char *filename, ulong addr, int offset, int len)
> return ret;
> }
>
> +int do_fs_uuid(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> + int fstype)
> +{
> + char uuid[37];
> + memset(uuid, 0, sizeof(uuid));
> +
> + if (argc != 3)
> + return CMD_RET_USAGE;
> +
> + if (fs_set_blk_dev(argv[1], argv[2], fstype))
> + return 1;
> +
> + fs_uuid(uuid);
> + setenv("fs_uuid", uuid);
> +
> + return 0;
> +}
> +
> int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> int fstype)
> {
> diff --git a/include/ext4fs.h b/include/ext4fs.h
> index 6c419f3..4011370 100644
> --- a/include/ext4fs.h
> +++ b/include/ext4fs.h
> @@ -132,6 +132,7 @@ struct ext_filesystem *get_fs(void);
> int ext4fs_open(const char *filename);
> int ext4fs_read(char *buf, unsigned len);
> int ext4fs_mount(unsigned part_length);
> +void ext4fs_uuid(char *uuid_str);
> void ext4fs_close(void);
> void ext4fs_reinit_global(void);
> int ext4fs_ls(const char *dirname);
> diff --git a/include/fs.h b/include/fs.h
> index 06a45f2..1956b67 100644
> --- a/include/fs.h
> +++ b/include/fs.h
> @@ -82,6 +82,8 @@ int fs_write(const char *filename, ulong addr, int offset, int len);
> * Common implementation for various filesystem commands, optionally limited
> * to a specific filesystem type via the fstype parameter.
> */
> +int do_fs_uuid(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> + int fstype);
> int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> int fstype);
> int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> --
> 1.9.3
>
Thanks
--
Christian Gmeiner, MSc
https://soundcloud.com/christian-gmeiner
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC] fs: make it possible to read the filesystem UUID
2014-10-22 13:29 [U-Boot] [RFC] fs: make it possible to read the filesystem UUID Christian Gmeiner
2014-10-29 12:15 ` Christian Gmeiner
@ 2014-10-29 19:39 ` Simon Glass
1 sibling, 0 replies; 6+ messages in thread
From: Simon Glass @ 2014-10-29 19:39 UTC (permalink / raw)
To: u-boot
Hi Christian,
On 22 October 2014 07:29, Christian Gmeiner <christian.gmeiner@gmail.com> wrote:
> Some filesystems have a UUID stored in its superblock. To
> allow using root=UUID=... for the kernel command line we
> need a way to read-out the filesystem UUID. This is what this
> patch tries to do.
>
> Keep in mind that this patch is an RFC and I hope to get some
> feedback on this patch.
>
> This is what blkid under linux gives me:
> /dev/sdh1: LABEL="data" UUID="b6995cec-97e2-48b8-94dc-8ba7afc92bac" TYPE="ext4" PARTUUID="43f21f0e-01"
> /dev/sdh2: LABEL="rfs" UUID="8d020de7-c75e-4674-948e-f7838a931b02" TYPE="ext4" PARTUUID="43f21f0e-02"
>
> And here is the output from u-boot:
> => sata init
> AHCI 0001.0300 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
> flags: ncq stag pm led clo only pmp pio slum part
> SATA Device Info:
> S/N: 000060059798A1000014
> Product model number: SFCA4096H1BR4TO-I-MS-236-STD
> Firmware version: 20111021
> Capacity: 7793856 sectors
> => fs_uuid sata 0:1 ; print fs_uuid; fs_uuid sata 0:2 ; print fs_uuid
> fs_uuid=b6995cec-97e2-48b8-94dc-8ba7afc92bac
> fs_uuid=8d020de7-c75e-4674-948e-f7838a931b02
>
> Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> ---
> common/Makefile | 1 +
> common/cmd_fs_uuid.c | 23 +++++++++++++++++++++++
> fs/ext4/ext4_common.c | 9 +++++++++
> fs/fs.c | 28 ++++++++++++++++++++++++++++
> include/ext4fs.h | 1 +
> include/fs.h | 2 ++
> 6 files changed, 64 insertions(+)
> create mode 100644 common/cmd_fs_uuid.c
>
> diff --git a/common/Makefile b/common/Makefile
> index b19d379..d5d3315 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -188,6 +188,7 @@ obj-y += usb.o usb_hub.o
> obj-$(CONFIG_USB_STORAGE) += usb_storage.o
> endif
> obj-$(CONFIG_CMD_FASTBOOT) += cmd_fastboot.o
> +obj-$(CONFIG_CMD_FS_UUID) += cmd_fs_uuid.o
Can this go in the cmd_fs file and be a subcommand? 'fs uuid' instead
of 'fs_uuid'.
>
> obj-$(CONFIG_CMD_USB_MASS_STORAGE) += cmd_usb_mass_storage.o
> obj-$(CONFIG_CMD_THOR_DOWNLOAD) += cmd_thordown.o
> diff --git a/common/cmd_fs_uuid.c b/common/cmd_fs_uuid.c
> new file mode 100644
> index 0000000..3af9518
> --- /dev/null
> +++ b/common/cmd_fs_uuid.c
> @@ -0,0 +1,23 @@
> +/*
> + * cmd_fs_uuid.c -- fs_uuid command
> + *
> + * Copyright (C) 2014, Bachmann electronic GmbH
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <fs.h>
> +
> +static int do_fs_uuid_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + return do_fs_uuid(cmdtp, flag, argc, argv, FS_TYPE_ANY);
> +}
> +
> +U_BOOT_CMD(
> + fs_uuid, 3, 0, do_fs_uuid_wrapper,
> + "filesystem UUDI related commands",
> + "<interface> <dev>:<part>\n"
> + " - print filesystem UUID\n"
> +);
> diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
> index 33d69c9..926b6c6 100644
> --- a/fs/ext4/ext4_common.c
> +++ b/fs/ext4/ext4_common.c
> @@ -2246,3 +2246,12 @@ fail:
>
> return 0;
> }
> +
> +void ext4fs_uuid(char *uuid_str)
> +{
> + if (ext4fs_root == NULL)
> + return;
> +
> + uuid_bin_to_str((unsigned char *)ext4fs_root->sblock.unique_id, uuid_str,
> + UUID_STR_FORMAT_STD);
> +}
I think this function should return 0 on success, and -ve on error (see errno.h)
> diff --git a/fs/fs.c b/fs/fs.c
> index dd680f3..f8c6d64 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -85,6 +85,7 @@ struct fstype_info {
> int (*size)(const char *filename);
> int (*read)(const char *filename, void *buf, int offset, int len);
> int (*write)(const char *filename, void *buf, int offset, int len);
> + void (*uuid)(char *uuid_str);
Return error code. Also document the argument and return value. How
long should the string be?
> void (*close)(void);
> };
>
> @@ -113,6 +114,7 @@ static struct fstype_info fstypes[] = {
> .size = ext4fs_size,
> .read = ext4_read_file,
> .write = fs_write_unsupported,
> + .uuid = ext4fs_uuid,
> },
> #endif
> #ifdef CONFIG_SANDBOX
> @@ -206,6 +208,14 @@ static void fs_close(void)
> fs_type = FS_TYPE_ANY;
> }
>
> +void fs_uuid(char *uuid_str)
> +{
> + struct fstype_info *info = fs_get_info(fs_type);
> +
> + if (info->uuid)
> + info->uuid(uuid_str);
Return error code
> +}
> +
> int fs_ls(const char *dirname)
> {
> int ret;
> @@ -289,6 +299,24 @@ int fs_write(const char *filename, ulong addr, int offset, int len)
> return ret;
> }
>
> +int do_fs_uuid(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> + int fstype)
> +{
> + char uuid[37];
> + memset(uuid, 0, sizeof(uuid));
> +
> + if (argc != 3)
> + return CMD_RET_USAGE;
> +
> + if (fs_set_blk_dev(argv[1], argv[2], fstype))
> + return 1;
> +
> + fs_uuid(uuid);
Check error code
> + setenv("fs_uuid", uuid);
How about making the environment variable an option parameter. If not
given, the UUID is printed out. If given, it is stored in the env
variable.
> +
> + return 0;
> +}
> +
> int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> int fstype)
> {
> diff --git a/include/ext4fs.h b/include/ext4fs.h
> index 6c419f3..4011370 100644
> --- a/include/ext4fs.h
> +++ b/include/ext4fs.h
> @@ -132,6 +132,7 @@ struct ext_filesystem *get_fs(void);
> int ext4fs_open(const char *filename);
> int ext4fs_read(char *buf, unsigned len);
> int ext4fs_mount(unsigned part_length);
> +void ext4fs_uuid(char *uuid_str);
> void ext4fs_close(void);
> void ext4fs_reinit_global(void);
> int ext4fs_ls(const char *dirname);
> diff --git a/include/fs.h b/include/fs.h
> index 06a45f2..1956b67 100644
> --- a/include/fs.h
> +++ b/include/fs.h
> @@ -82,6 +82,8 @@ int fs_write(const char *filename, ulong addr, int offset, int len);
> * Common implementation for various filesystem commands, optionally limited
> * to a specific filesystem type via the fstype parameter.
> */
> +int do_fs_uuid(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> + int fstype);
> int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> int fstype);
> int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
BTW you can use patman (tools/patman/patman) which will run
get_maintainer.pl for you and send your patch, and deal with change
list, etc.
Regards,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC] fs: make it possible to read the filesystem UUID
2014-10-29 12:15 ` Christian Gmeiner
@ 2014-10-29 19:57 ` Pavel Machek
2014-10-30 1:42 ` Simon Glass
2014-10-30 3:42 ` Stephen Warren
1 sibling, 1 reply; 6+ messages in thread
From: Pavel Machek @ 2014-10-29 19:57 UTC (permalink / raw)
To: u-boot
Hi!
> Adding Stephen Warren, Simon Glass and Pavel Machek to CC list
> (./scripts/get_maintainer.pl -f fs/fs.c).
I'm not really fs maintainer, I added people that might be interested.
>
> 2014-10-22 15:29 GMT+02:00 Christian Gmeiner <christian.gmeiner@gmail.com>:
> > Some filesystems have a UUID stored in its superblock. To
> > allow using root=UUID=... for the kernel command line we
> > need a way to read-out the filesystem UUID. This is what this
> > patch tries to do.
> >
> > Keep in mind that this patch is an RFC and I hope to get some
> > feedback on this patch.
> >
> > This is what blkid under linux gives me:
> > /dev/sdh1: LABEL="data" UUID="b6995cec-97e2-48b8-94dc-8ba7afc92bac" TYPE="ext4" PARTUUID="43f21f0e-01"
> > /dev/sdh2: LABEL="rfs" UUID="8d020de7-c75e-4674-948e-f7838a931b02" TYPE="ext4" PARTUUID="43f21f0e-02"
> >
> > And here is the output from u-boot:
> > => sata init
> > AHCI 0001.0300 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
> > flags: ncq stag pm led clo only pmp pio slum part
> > SATA Device Info:
> > S/N: 000060059798A1000014
> > Product model number: SFCA4096H1BR4TO-I-MS-236-STD
> > Firmware version: 20111021
> > Capacity: 7793856 sectors
> > => fs_uuid sata 0:1 ; print fs_uuid; fs_uuid sata 0:2 ; print fs_uuid
> > fs_uuid=b6995cec-97e2-48b8-94dc-8ba7afc92bac
> > fs_uuid=8d020de7-c75e-4674-948e-f7838a931b02
> >
>
> Really no feedback?
I see nothing obviously wrong... But I'm not a fs/ maintainer. We
don't have one, AFICT.
> > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> > ---
> > common/Makefile | 1 +
> > common/cmd_fs_uuid.c | 23 +++++++++++++++++++++++
> > fs/ext4/ext4_common.c | 9 +++++++++
> > fs/fs.c | 28 ++++++++++++++++++++++++++++
> > include/ext4fs.h | 1 +
> > include/fs.h | 2 ++
> > 6 files changed, 64 insertions(+)
> > create mode 100644 common/cmd_fs_uuid.c
> >
> > diff --git a/common/Makefile b/common/Makefile
> > index b19d379..d5d3315 100644
> > --- a/common/Makefile
> > +++ b/common/Makefile
> > @@ -188,6 +188,7 @@ obj-y += usb.o usb_hub.o
> > obj-$(CONFIG_USB_STORAGE) += usb_storage.o
> > endif
> > obj-$(CONFIG_CMD_FASTBOOT) += cmd_fastboot.o
> > +obj-$(CONFIG_CMD_FS_UUID) += cmd_fs_uuid.o
> >
> > obj-$(CONFIG_CMD_USB_MASS_STORAGE) += cmd_usb_mass_storage.o
> > obj-$(CONFIG_CMD_THOR_DOWNLOAD) += cmd_thordown.o
> > diff --git a/common/cmd_fs_uuid.c b/common/cmd_fs_uuid.c
> > new file mode 100644
> > index 0000000..3af9518
> > --- /dev/null
> > +++ b/common/cmd_fs_uuid.c
> > @@ -0,0 +1,23 @@
> > +/*
> > + * cmd_fs_uuid.c -- fs_uuid command
> > + *
> > + * Copyright (C) 2014, Bachmann electronic GmbH
> > + *
> > + * SPDX-License-Identifier: GPL-2.0+
> > + */
> > +
> > +#include <common.h>
> > +#include <command.h>
> > +#include <fs.h>
> > +
> > +static int do_fs_uuid_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> > +{
> > + return do_fs_uuid(cmdtp, flag, argc, argv, FS_TYPE_ANY);
> > +}
> > +
> > +U_BOOT_CMD(
> > + fs_uuid, 3, 0, do_fs_uuid_wrapper,
> > + "filesystem UUDI related commands",
> > + "<interface> <dev>:<part>\n"
> > + " - print filesystem UUID\n"
> > +);
> > diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
> > index 33d69c9..926b6c6 100644
> > --- a/fs/ext4/ext4_common.c
> > +++ b/fs/ext4/ext4_common.c
> > @@ -2246,3 +2246,12 @@ fail:
> >
> > return 0;
> > }
> > +
> > +void ext4fs_uuid(char *uuid_str)
> > +{
> > + if (ext4fs_root == NULL)
> > + return;
> > +
> > + uuid_bin_to_str((unsigned char *)ext4fs_root->sblock.unique_id, uuid_str,
> > + UUID_STR_FORMAT_STD);
> > +}
> > diff --git a/fs/fs.c b/fs/fs.c
> > index dd680f3..f8c6d64 100644
> > --- a/fs/fs.c
> > +++ b/fs/fs.c
> > @@ -85,6 +85,7 @@ struct fstype_info {
> > int (*size)(const char *filename);
> > int (*read)(const char *filename, void *buf, int offset, int len);
> > int (*write)(const char *filename, void *buf, int offset, int len);
> > + void (*uuid)(char *uuid_str);
> > void (*close)(void);
> > };
> >
> > @@ -113,6 +114,7 @@ static struct fstype_info fstypes[] = {
> > .size = ext4fs_size,
> > .read = ext4_read_file,
> > .write = fs_write_unsupported,
> > + .uuid = ext4fs_uuid,
> > },
> > #endif
> > #ifdef CONFIG_SANDBOX
> > @@ -206,6 +208,14 @@ static void fs_close(void)
> > fs_type = FS_TYPE_ANY;
> > }
> >
>
> Should I do add an #ifdef CONFIG_CMD_FS_UUID around .uuid = ext4fs_uuid?
> Or speaking more general: At which places should I add this ifdef to
> opt-in the FS_UUID
> feature?
I guess so. And I guess it is worth it to remove ext4fs_uuid function
when !defined(CONFIG_CMD_FS_UUID)...
> > +void fs_uuid(char *uuid_str)
> > +{
> > + struct fstype_info *info = fs_get_info(fs_type);
> > +
> > + if (info->uuid)
> > + info->uuid(uuid_str);
> > +}
> > +
> > int fs_ls(const char *dirname)
> > {
> > int ret;
> > @@ -289,6 +299,24 @@ int fs_write(const char *filename, ulong addr, int offset, int len)
> > return ret;
> > }
> >
> > +int do_fs_uuid(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> > + int fstype)
> > +{
> > + char uuid[37];
> > + memset(uuid, 0, sizeof(uuid));
> > +
> > + if (argc != 3)
> > + return CMD_RET_USAGE;
> > +
> > + if (fs_set_blk_dev(argv[1], argv[2], fstype))
> > + return 1;
> > +
> > + fs_uuid(uuid);
> > + setenv("fs_uuid", uuid);
> > +
> > + return 0;
> > +}
> > +
> > int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> > int fstype)
> > {
> > diff --git a/include/ext4fs.h b/include/ext4fs.h
> > index 6c419f3..4011370 100644
> > --- a/include/ext4fs.h
> > +++ b/include/ext4fs.h
> > @@ -132,6 +132,7 @@ struct ext_filesystem *get_fs(void);
> > int ext4fs_open(const char *filename);
> > int ext4fs_read(char *buf, unsigned len);
> > int ext4fs_mount(unsigned part_length);
> > +void ext4fs_uuid(char *uuid_str);
> > void ext4fs_close(void);
> > void ext4fs_reinit_global(void);
> > int ext4fs_ls(const char *dirname);
> > diff --git a/include/fs.h b/include/fs.h
> > index 06a45f2..1956b67 100644
> > --- a/include/fs.h
> > +++ b/include/fs.h
> > @@ -82,6 +82,8 @@ int fs_write(const char *filename, ulong addr, int offset, int len);
> > * Common implementation for various filesystem commands, optionally limited
> > * to a specific filesystem type via the fstype parameter.
> > */
> > +int do_fs_uuid(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> > + int fstype);
> > int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> > int fstype);
> > int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> > --
> > 1.9.3
> >
>
> Thanks
> --
> Christian Gmeiner, MSc
>
> https://soundcloud.com/christian-gmeiner
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC] fs: make it possible to read the filesystem UUID
2014-10-29 19:57 ` Pavel Machek
@ 2014-10-30 1:42 ` Simon Glass
0 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2014-10-30 1:42 UTC (permalink / raw)
To: u-boot
Hi,
On 29 October 2014 13:57, Pavel Machek <pavel@denx.de> wrote:
> Hi!
>
>> Adding Stephen Warren, Simon Glass and Pavel Machek to CC list
>> (./scripts/get_maintainer.pl -f fs/fs.c).
>
> I'm not really fs maintainer, I added people that might be interested.
>>
>> 2014-10-22 15:29 GMT+02:00 Christian Gmeiner <christian.gmeiner@gmail.com>:
>> > Some filesystems have a UUID stored in its superblock. To
>> > allow using root=UUID=... for the kernel command line we
>> > need a way to read-out the filesystem UUID. This is what this
>> > patch tries to do.
>> >
>> > Keep in mind that this patch is an RFC and I hope to get some
>> > feedback on this patch.
>> >
>> > This is what blkid under linux gives me:
>> > /dev/sdh1: LABEL="data" UUID="b6995cec-97e2-48b8-94dc-8ba7afc92bac" TYPE="ext4" PARTUUID="43f21f0e-01"
>> > /dev/sdh2: LABEL="rfs" UUID="8d020de7-c75e-4674-948e-f7838a931b02" TYPE="ext4" PARTUUID="43f21f0e-02"
>> >
>> > And here is the output from u-boot:
>> > => sata init
>> > AHCI 0001.0300 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
>> > flags: ncq stag pm led clo only pmp pio slum part
>> > SATA Device Info:
>> > S/N: 000060059798A1000014
>> > Product model number: SFCA4096H1BR4TO-I-MS-236-STD
>> > Firmware version: 20111021
>> > Capacity: 7793856 sectors
>> > => fs_uuid sata 0:1 ; print fs_uuid; fs_uuid sata 0:2 ; print fs_uuid
>> > fs_uuid=b6995cec-97e2-48b8-94dc-8ba7afc92bac
>> > fs_uuid=8d020de7-c75e-4674-948e-f7838a931b02
>> >
>>
>> Really no feedback?
>
> I see nothing obviously wrong... But I'm not a fs/ maintainer. We
> don't have one, AFICT.
>
>> > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
>> > ---
>> > common/Makefile | 1 +
>> > common/cmd_fs_uuid.c | 23 +++++++++++++++++++++++
>> > fs/ext4/ext4_common.c | 9 +++++++++
>> > fs/fs.c | 28 ++++++++++++++++++++++++++++
>> > include/ext4fs.h | 1 +
>> > include/fs.h | 2 ++
>> > 6 files changed, 64 insertions(+)
>> > create mode 100644 common/cmd_fs_uuid.c
>> >
>> > diff --git a/common/Makefile b/common/Makefile
>> > index b19d379..d5d3315 100644
>> > --- a/common/Makefile
>> > +++ b/common/Makefile
>> > @@ -188,6 +188,7 @@ obj-y += usb.o usb_hub.o
>> > obj-$(CONFIG_USB_STORAGE) += usb_storage.o
>> > endif
>> > obj-$(CONFIG_CMD_FASTBOOT) += cmd_fastboot.o
>> > +obj-$(CONFIG_CMD_FS_UUID) += cmd_fs_uuid.o
>> >
>> > obj-$(CONFIG_CMD_USB_MASS_STORAGE) += cmd_usb_mass_storage.o
>> > obj-$(CONFIG_CMD_THOR_DOWNLOAD) += cmd_thordown.o
>> > diff --git a/common/cmd_fs_uuid.c b/common/cmd_fs_uuid.c
>> > new file mode 100644
>> > index 0000000..3af9518
>> > --- /dev/null
>> > +++ b/common/cmd_fs_uuid.c
>> > @@ -0,0 +1,23 @@
>> > +/*
>> > + * cmd_fs_uuid.c -- fs_uuid command
>> > + *
>> > + * Copyright (C) 2014, Bachmann electronic GmbH
>> > + *
>> > + * SPDX-License-Identifier: GPL-2.0+
>> > + */
>> > +
>> > +#include <common.h>
>> > +#include <command.h>
>> > +#include <fs.h>
>> > +
>> > +static int do_fs_uuid_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>> > +{
>> > + return do_fs_uuid(cmdtp, flag, argc, argv, FS_TYPE_ANY);
>> > +}
>> > +
>> > +U_BOOT_CMD(
>> > + fs_uuid, 3, 0, do_fs_uuid_wrapper,
>> > + "filesystem UUDI related commands",
>> > + "<interface> <dev>:<part>\n"
>> > + " - print filesystem UUID\n"
>> > +);
>> > diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
>> > index 33d69c9..926b6c6 100644
>> > --- a/fs/ext4/ext4_common.c
>> > +++ b/fs/ext4/ext4_common.c
>> > @@ -2246,3 +2246,12 @@ fail:
>> >
>> > return 0;
>> > }
>> > +
>> > +void ext4fs_uuid(char *uuid_str)
>> > +{
>> > + if (ext4fs_root == NULL)
>> > + return;
>> > +
>> > + uuid_bin_to_str((unsigned char *)ext4fs_root->sblock.unique_id, uuid_str,
>> > + UUID_STR_FORMAT_STD);
>> > +}
>> > diff --git a/fs/fs.c b/fs/fs.c
>> > index dd680f3..f8c6d64 100644
>> > --- a/fs/fs.c
>> > +++ b/fs/fs.c
>> > @@ -85,6 +85,7 @@ struct fstype_info {
>> > int (*size)(const char *filename);
>> > int (*read)(const char *filename, void *buf, int offset, int len);
>> > int (*write)(const char *filename, void *buf, int offset, int len);
>> > + void (*uuid)(char *uuid_str);
>> > void (*close)(void);
>> > };
>> >
>> > @@ -113,6 +114,7 @@ static struct fstype_info fstypes[] = {
>> > .size = ext4fs_size,
>> > .read = ext4_read_file,
>> > .write = fs_write_unsupported,
>> > + .uuid = ext4fs_uuid,
>> > },
>> > #endif
>> > #ifdef CONFIG_SANDBOX
>> > @@ -206,6 +208,14 @@ static void fs_close(void)
>> > fs_type = FS_TYPE_ANY;
>> > }
>> >
>>
>> Should I do add an #ifdef CONFIG_CMD_FS_UUID around .uuid = ext4fs_uuid?
>> Or speaking more general: At which places should I add this ifdef to
>> opt-in the FS_UUID
>> feature?
>
> I guess so. And I guess it is worth it to remove ext4fs_uuid function
> when !defined(CONFIG_CMD_FS_UUID)...
You add very little new code so I doubt it is worth it?
Regards,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC] fs: make it possible to read the filesystem UUID
2014-10-29 12:15 ` Christian Gmeiner
2014-10-29 19:57 ` Pavel Machek
@ 2014-10-30 3:42 ` Stephen Warren
1 sibling, 0 replies; 6+ messages in thread
From: Stephen Warren @ 2014-10-30 3:42 UTC (permalink / raw)
To: u-boot
On 10/29/2014 06:15 AM, Christian Gmeiner wrote:
> Hi all.
>
> Adding Stephen Warren, Simon Glass and Pavel Machek to CC list
> (./scripts/get_maintainer.pl -f fs/fs.c).
>
> 2014-10-22 15:29 GMT+02:00 Christian Gmeiner <christian.gmeiner@gmail.com>:
>> Some filesystems have a UUID stored in its superblock. To
>> allow using root=UUID=... for the kernel command line we
>> need a way to read-out the filesystem UUID. This is what this
>> patch tries to do.
Well, you can always to root=PARTUUID= instead of root=UUID=. The
advantage is that the kernel handles that so you don't even need an initrd.
Still, having this feature seems useful for systems that expect to parse
root=UUID= in an initrd.
>> common/Makefile | 1 +
>> common/cmd_fs_uuid.c | 23 +++++++++++++++++++++++
>> fs/ext4/ext4_common.c | 9 +++++++++
>> fs/fs.c | 28 ++++++++++++++++++++++++++++
>> include/ext4fs.h | 1 +
>> include/fs.h | 2 ++
You should document the new CONFIG define in the README.
> diff --git a/common/cmd_fs_uuid.c b/common/cmd_fs_uuid.c
> +U_BOOT_CMD(
> + fs_uuid, 3, 0, do_fs_uuid_wrapper,
> + "filesystem UUDI related commands",
Typo ................ ^^^^
And not a great command description. How about:
Look up a filesystem UUID
> + "<interface> <dev>:<part>\n"
> + " - print filesystem UUID\n"
> +);
Other commands don't have _ in the name. How about just fsuuid instead?
>> diff --git a/fs/fs.c b/fs/fs.c
>> @@ -85,6 +85,7 @@ struct fstype_info {
>> int (*size)(const char *filename);
>> int (*read)(const char *filename, void *buf, int offset, int len);
>> int (*write)(const char *filename, void *buf, int offset, int len);
>> + void (*uuid)(char *uuid_str);
This new function should return an error code. The ext4 implementation
in this patch doesn't always look up the UUID...
>> +void fs_uuid(char *uuid_str)
>> +{
>> + struct fstype_info *info = fs_get_info(fs_type);
>> +
>> + if (info->uuid)
>> + info->uuid(uuid_str);
>> +}
This function should propagate the error code back to the caller, and
return an error itself if !info->uuid.
>> +int do_fs_uuid(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>> + int fstype)
>> +{
>> + char uuid[37];
>> + memset(uuid, 0, sizeof(uuid));
>> +
>> + if (argc != 3)
>> + return CMD_RET_USAGE;
>> +
>> + if (fs_set_blk_dev(argv[1], argv[2], fstype))
>> + return 1;
>> +
>> + fs_uuid(uuid);
>> + setenv("fs_uuid", uuid);
I'd prefer if the function didn't unconditionally set a hard-coded
environment variable name. Rather, take a look at how the "part uuid"
command is implemented:
part uuid mmc 0:1 - print out the UUID
part uuid mmc 0:1 uuidvar - set uuidvar environment variable to the UUID
->
fsuuid mmc 0:1
fsuuid mmc 0:1 uuidvar
This allows interactive users to easily print out the value, whereas
scripts can control which environment variable the data is stored in, in
case they need to look up multiple different UUIDs or just want to use a
meaningful non-hard-coded variable name.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-10-30 3:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-22 13:29 [U-Boot] [RFC] fs: make it possible to read the filesystem UUID Christian Gmeiner
2014-10-29 12:15 ` Christian Gmeiner
2014-10-29 19:57 ` Pavel Machek
2014-10-30 1:42 ` Simon Glass
2014-10-30 3:42 ` Stephen Warren
2014-10-29 19:39 ` Simon Glass
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox