| Message ID | 1413984560-11318-1-git-send-email-christian.gmeiner@gmail.com |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
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
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
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
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
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.
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[],
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