[PATCH] erofs-utils: Provide identical functionality without libuuid
Norbert Lange
nolange79 at gmail.com
Mon Jul 3 08:16:02 AEST 2023
Am So., 2. Juli 2023 um 13:07 Uhr schrieb Gao Xiang
<hsiangkao at linux.alibaba.com>:
>
> Hi Norbert,
>
> On 2023/7/2 18:19, Norbert Lange wrote:
> > The motivation is to have standalone (statically linked) erofs binaries
> > for simple initrd images, that are nevertheless able to (re)create
> > erofs images with a given UUID.
> >
> > For this reason a few of libuuid functions have implementations added
> > directly in erofs-utils.
> > A header liberofs_uuid.h provides the new functions, which are
> > always available. A further sideeffect is that code can be simplified
> > which calls into this functionality.
> >
> > The uuid_unparse function replacement is always a private
> > implementation and split into its own file, this further restricts
> > the (optional) dependency on libuuid only to the erofs-mkfs tool.
> >
> > Signed-off-by: Norbert Lange <nolange79 at gmail.com>
>
> Yeah, overall it looks good to me, some minor nits as below:
>
> (Also currently UUID makes the image nonreproducable, I wonder if we
> could use some image hash to calculate the whole UUID instead...)
Not the usecase I am after, I need to use an already known UUID for
mounting, even if the content changed.
>
> > ---
> > dump/Makefile.am | 2 +-
> > dump/main.c | 8 +---
> > fsck/Makefile.am | 2 +-
> > lib/Makefile.am | 4 +-
> > lib/liberofs_uuid.h | 9 ++++
> > lib/uuid.c | 106 ++++++++++++++++++++++++++++++++++++++++++++
> > lib/uuid_unparse.c | 21 +++++++++
> > mkfs/Makefile.am | 6 +--
> > mkfs/main.c | 21 ++-------
> > 9 files changed, 149 insertions(+), 30 deletions(-)
> > create mode 100644 lib/liberofs_uuid.h
> > create mode 100644 lib/uuid.c
> > create mode 100644 lib/uuid_unparse.c
> >
> > diff --git a/dump/Makefile.am b/dump/Makefile.am
> > index c2bef6d..90227a5 100644
> > --- a/dump/Makefile.am
> > +++ b/dump/Makefile.am
> > @@ -7,4 +7,4 @@ AM_CPPFLAGS = ${libuuid_CFLAGS}
> > dump_erofs_SOURCES = main.c
> > dump_erofs_CFLAGS = -Wall -I$(top_srcdir)/include
> > dump_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libselinux_LIBS} \
> > - ${libuuid_LIBS} ${liblz4_LIBS} ${liblzma_LIBS}
> > + ${liblz4_LIBS} ${liblzma_LIBS}
> > diff --git a/dump/main.c b/dump/main.c
> > index bc4e028..40af09f 100644
> > --- a/dump/main.c
> > +++ b/dump/main.c
> > @@ -17,10 +17,8 @@
> > #include "erofs/compress.h"
> > #include "erofs/fragments.h"
> > #include "../lib/liberofs_private.h"
> > +#include "../lib/liberofs_uuid.h"
> >
> > -#ifdef HAVE_LIBUUID
> > -#include <uuid.h>
> > -#endif
> >
> > struct erofsdump_cfg {
> > unsigned int totalshow;
> > @@ -620,9 +618,7 @@ static void erofsdump_show_superblock(void)
> > if (feat & feature_lists[i].flag)
> > fprintf(stdout, "%s ", feature_lists[i].name);
> > }
> > -#ifdef HAVE_LIBUUID
> > - uuid_unparse_lower(sbi.uuid, uuid_str);
> > -#endif
> > + erofs_uuid_unparse_lower(sbi.uuid, uuid_str);
> > fprintf(stdout, "\nFilesystem UUID: %s\n",
> > uuid_str);
> > }
> > diff --git a/fsck/Makefile.am b/fsck/Makefile.am
> > index e6a1fb6..4176d86 100644
> > --- a/fsck/Makefile.am
> > +++ b/fsck/Makefile.am
> > @@ -7,4 +7,4 @@ AM_CPPFLAGS = ${libuuid_CFLAGS}
> > fsck_erofs_SOURCES = main.c
> > fsck_erofs_CFLAGS = -Wall -I$(top_srcdir)/include
> > fsck_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libselinux_LIBS} \
> > - ${libuuid_LIBS} ${liblz4_LIBS} ${liblzma_LIBS}
> > + ${liblz4_LIBS} ${liblzma_LIBS}
> > diff --git a/lib/Makefile.am b/lib/Makefile.am
> > index faa7311..e243c1c 100644
> > --- a/lib/Makefile.am
> > +++ b/lib/Makefile.am
> > @@ -29,9 +29,9 @@ noinst_HEADERS += compressor.h
> > liberofs_la_SOURCES = config.c io.c cache.c super.c inode.c xattr.c exclude.c \
> > namei.c data.c compress.c compressor.c zmap.c decompress.c \
> > compress_hints.c hashmap.c sha256.c blobchunk.c dir.c \
> > - fragments.c rb_tree.c dedupe.c
> > + fragments.c rb_tree.c dedupe.c uuid_unparse.c uuid.c
> >
> > -liberofs_la_CFLAGS = -Wall -I$(top_srcdir)/include
> > +liberofs_la_CFLAGS = -Wall ${libuuid_CFLAGS} -I$(top_srcdir)/include
> > if ENABLE_LZ4
> > liberofs_la_CFLAGS += ${LZ4_CFLAGS}
> > liberofs_la_SOURCES += compressor_lz4.c
> > diff --git a/lib/liberofs_uuid.h b/lib/liberofs_uuid.h
> > new file mode 100644
> > index 0000000..d156699
> > --- /dev/null
> > +++ b/lib/liberofs_uuid.h
> > @@ -0,0 +1,9 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ OR Apache-2.0 */
> > +#ifndef __EROFS_LIB_UUID_H
> > +#define __EROFS_LIB_UUID_H
> > +
> > +void erofs_uuid_generate(unsigned char *out);
> > +void erofs_uuid_unparse_lower(const unsigned char *buf, char *out);
> > +int erofs_uuid_parse(const char *in, unsigned char *uu);
> > +
> > +#endif
> > \ No newline at end of file
> > diff --git a/lib/uuid.c b/lib/uuid.c
> > new file mode 100644
> > index 0000000..acff81a
> > --- /dev/null
> > +++ b/lib/uuid.c
> > @@ -0,0 +1,106 @@
> > +// SPDX-License-Identifier: GPL-2.0+ OR Apache-2.0
> > +/*
> > + * Copyright (C) 2023 Norbert Lange <nolange79 at gmail.com>
> > + */
> > +
> > +#include <string.h>
> > +#include <errno.h>
> > +
> > +#include "erofs/config.h"
> > +#include "erofs/defs.h"
> > +#include "liberofs_uuid.h"
> > +
> > +#ifdef HAVE_LIBUUID
> > +#include <uuid.h>
> > +#else
> > +
> > +#include <stdlib.h>
> > +#include <sys/random.h>
> > +
> > +/* Flags to be used, will be modified if kernel does not support them */
> > +static unsigned erofs_grnd_flag =
>
> Could we switch to "unsigned int" for this?
Sure
>
> > +#ifdef GRND_INSECURE
> > + GRND_INSECURE;
> > +#else
> > + 0x0004;
> > +#endif
> > +
> > +static int s_getrandom(void *pUid, unsigned size, bool insecure)
> > +{
> > + unsigned kflags = erofs_grnd_flag;
> > + unsigned flags = insecure ? kflags : 0;
>
> same here.
>
> Otherwise it looks good to me.
Found few a formatting inconsistencies as well, already fixed this locally.
Guess I will wait a bit more for feedback before posting a v2? Not sure
whats the proper order.
>
> Thanks,
> Gao Xiang
regards, Norbert
More information about the Linux-erofs
mailing list