Re: [PATCH] erofs-utils: mkfs: Add volume-name setting support

[Gao Xiang <xiang@kernel.org>]

Hi Naoto,

On Wed, Oct 05, 2022 at 01:02:37AM +0900, Naoto Yamaguchi wrote:
> The erofs_super_block has volume_name field.  On the other hand,
> mkfs.erofs is not supporting to set volume name.
> This patch add volume-name setting support to mkfs.erofs.
> Option keyword is similar to mkfs.vfat.
> 
> usage:
>   mkfs.erofs -n volume-name image-fn dir

Thanks for your patch! The patch itself generally looks good to me.


Just two minor ideas:

1) How about following mke2fs by using "-L volume-label" ?
https://www.man7.org/linux/man-pages/man8/mke2fs.8.html

2) If possible, how about adding a kernel ioctl for this if you have
more interest?  I mean FS_IOC_GETFSLABEL.

> 
> Signed-off-by: Naoto Yamaguchi <naoto.yamaguchi@aisin.co.jp>
> ---
>  include/erofs/internal.h |  1 +
>  man/mkfs.erofs.1         |  4 ++++
>  mkfs/main.c              | 13 ++++++++++++-
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> index 2e0aae8..7dc42eb 100644
> --- a/include/erofs/internal.h
> +++ b/include/erofs/internal.h
> @@ -92,6 +92,7 @@ struct erofs_sb_info {
>  	u64 inos;
>  
>  	u8 uuid[16];
> +	char volume_name[16];
>  
>  	u16 available_compr_algs;
>  	u16 lz4_max_distance;
> diff --git a/man/mkfs.erofs.1 b/man/mkfs.erofs.1
> index 11e8323..fb98505 100644
> --- a/man/mkfs.erofs.1
> +++ b/man/mkfs.erofs.1
> @@ -32,6 +32,10 @@ big pcluster feature if needed (Linux v5.13+).
>  Specify the level of debugging messages. The default is 2, which shows basic
>  warning messages.
>  .TP
> +.BI "\-n " volume-name
> +Set the volume name for the filesystem to volume-name.  The maximum length of
> +the volume name is 16 bytes.
> +.TP
>  .BI "\-x " #
>  Specify the upper limit of an xattr which is still inlined. The default is 2.
>  Disable storing xattrs if < 0.
> diff --git a/mkfs/main.c b/mkfs/main.c
> index 594ecf9..613ee46 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -80,6 +80,7 @@ static void usage(void)
>  	fputs("usage: [options] FILE DIRECTORY\n\n"
>  	      "Generate erofs image from DIRECTORY to FILE, and [options] are:\n"
>  	      " -d#                   set output message level to # (maximum 9)\n"
> +	      " -n volume-name        set the volume name (max 16 bytes).\n"

...

>  	      " -x#                   set xattr tolerance to # (< 0, disable xattrs; default 2)\n"
>  	      " -zX[,Y]               X=compressor (Y=compression level, optional)\n"
>  	      " -C#                   specify the size of compress physical cluster in bytes\n"


	      " -L volume-label       set the volume label (maximum 12)\n"

Thanks,
Gao Xiang