On 2023/7/3 06:16, Norbert Lange wrote:
Am So., 2. Juli 2023 um 13:07 Uhr schrieb Gao Xiang
<[email protected]>:
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 <[email protected]>
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.
Okay.
---
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 <[email protected]>
+ */
+
+#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.
Looks good to me, yet erofs-utils has rare people to work on reviewing,
I'm fine with either way.
Thanks,
Gao Xiang
