[PATCH RFC 0/1] libblkid: Initial support for Stratis

[PATCH RFC 0/1] libblkid: Initial support for Stratis

[Tony Asleson]

My initial pass at adding support to libblkid for Stratis
https://github.com/stratis-storage.

Documentation including on disk format (needs a couple of updates):
https://stratis-storage.github.io/StratisSoftwareDesign.pdf

I would like to add the appropriate sample super block img and associated 
test(s) to this patch series as well.  However, at the moment I'm not exactly 
sure how to do that.  Pointers to how that is accomplished would be greatly
appreciated, otherwise I will splunk around in the code base until I figure it
out.

Thanks!

Regards,
Tony Asleson

Tony Asleson (1):
  libblkid: Add support for stratis

 libblkid/src/Makemodule.am             |   1 +
 libblkid/src/superblocks/stratis.c     | 104 +++++++++++++++++++++++++++++++++
 libblkid/src/superblocks/superblocks.c |   1 +
 libblkid/src/superblocks/superblocks.h |   1 +
 4 files changed, 107 insertions(+)
 create mode 100644 libblkid/src/superblocks/stratis.c

-- 
2.13.6

[PATCH RFC 1/1] libblkid: Add support for stratis

[Tony Asleson]

Initial support for stratis, ref.
https://github.com/stratis-storage

Signed-off-by: Tony Asleson <tasleson@redhat.com>
---
 libblkid/src/Makemodule.am             |   1 +
 libblkid/src/superblocks/stratis.c     | 104 +++++++++++++++++++++++++++++++++
 libblkid/src/superblocks/superblocks.c |   1 +
 libblkid/src/superblocks/superblocks.h |   1 +
 4 files changed, 107 insertions(+)
 create mode 100644 libblkid/src/superblocks/stratis.c

diff --git a/libblkid/src/Makemodule.am b/libblkid/src/Makemodule.am
index 415c9f408..65d2e66ed 100644
--- a/libblkid/src/Makemodule.am
+++ b/libblkid/src/Makemodule.am
@@ -82,6 +82,7 @@ libblkid_la_SOURCES = \
 	libblkid/src/superblocks/romfs.c \
 	libblkid/src/superblocks/silicon_raid.c \
 	libblkid/src/superblocks/squashfs.c \
+	libblkid/src/superblocks/stratis.c \
 	libblkid/src/superblocks/superblocks.c \
 	libblkid/src/superblocks/superblocks.h \
 	libblkid/src/superblocks/swap.c \
diff --git a/libblkid/src/superblocks/stratis.c b/libblkid/src/superblocks/stratis.c
new file mode 100644
index 000000000..ae23b0fa1
--- /dev/null
+++ b/libblkid/src/superblocks/stratis.c
@@ -0,0 +1,104 @@
+/*
+ * Copyright (C) 2018 Tony Asleson <tasleson@redhat.com>
+ *
+ * This file may be redistributed under the terms of the
+ * GNU Lesser General Public License.
+ */
+
+/*
+ * Specification for on disk format
+ * https://stratis-storage.github.io/StratisSoftwareDesign.pdf
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <stdint.h>
+#include <inttypes.h>
+
+#include "superblocks.h"
+#include "crc32c.h"
+
+struct stratis_sb {
+	uint32_t crc32;
+	uint8_t magic[16];
+	uint64_t sectors;
+	uint8_t reserved[4];
+	uint8_t pool_uuid[32];
+	uint8_t dev_uuid[32];
+	uint64_t mda_size;
+	uint64_t reserved_size;
+	uint64_t flags;
+	uint64_t initialization_time;
+} __attribute__ ((__packed__));
+
+#define BS 512
+#define FIRST_COPY_OFFSET BS
+#define SECOND_COPY_OFFSET (BS * 9)
+#define SB_AREA_SIZE (BS * 16)
+
+const char STRATIS_MAGIC[] = "!Stra0tis\x86\xff\x02^\x41rh";
+#define MAGIC_LEN (sizeof(STRATIS_MAGIC) - 1)
+
+#define _MAGIC_OFFSET (offsetof(const struct stratis_sb, magic))
+#define MAGIC_OFFSET_COPY_1 (FIRST_COPY_OFFSET + _MAGIC_OFFSET)
+#define MAGIC_OFFSET_COPY_2 (SECOND_COPY_OFFSET + _MAGIC_OFFSET)
+
+static int stratis_valid_sb(blkid_probe pr, uint8_t *p)
+{
+	const struct stratis_sb *stratis = (const struct stratis_sb *)p;
+	uint32_t crc = 0;
+
+	// generate CRC from byte position 4 for length 508 == 512 byte sector
+	crc = crc32c(~0L, p + sizeof(stratis->crc32),
+			BS - sizeof(stratis->crc32));
+	crc ^= ~0L;
+
+	return blkid_probe_verify_csum(pr, crc, le32_to_cpu(stratis->crc32));
+}
+
+static int probe_stratis(blkid_probe pr,
+		const struct blkid_idmag *mag __attribute__((__unused__)))
+{
+	const struct stratis_sb *stratis = NULL;
+	uint8_t *buf = blkid_probe_get_buffer(pr, 0, SB_AREA_SIZE);
+
+	if (!buf)
+		return errno ? -errno : 1;
+
+	if (stratis_valid_sb(pr, buf + FIRST_COPY_OFFSET)) {
+		stratis = (const struct stratis_sb *)(buf + FIRST_COPY_OFFSET);
+	} else {
+		if (!stratis_valid_sb(pr, buf + SECOND_COPY_OFFSET))
+			return 1;
+
+		stratis = (const struct stratis_sb *)
+				(buf + SECOND_COPY_OFFSET);
+	}
+
+	blkid_probe_strncpy_uuid(pr, (unsigned char *)stratis->dev_uuid,
+					sizeof(stratis->dev_uuid));
+	blkid_probe_set_value(pr, "POOL_UUID",
+				(unsigned char *)stratis->pool_uuid,
+				sizeof(stratis->pool_uuid));
+
+	blkid_probe_sprintf_value(pr, "SECTORS", "%" PRIu64, stratis->sectors);
+	blkid_probe_sprintf_value(pr, "INITIALIZATION_TIME", "%" PRIu64,
+					stratis->initialization_time);
+
+	return 0;
+}
+
+const struct blkid_idinfo stratis_idinfo = {
+	.name		= "stratis",
+	.usage		= BLKID_USAGE_RAID,
+	.probefunc	= probe_stratis,
+	.minsz		= SB_AREA_SIZE,
+	.magics		= {
+		{ .magic = STRATIS_MAGIC, .len = MAGIC_LEN,
+			.sboff = MAGIC_OFFSET_COPY_1},
+		{ .magic = STRATIS_MAGIC, .len = MAGIC_LEN,
+			.sboff = MAGIC_OFFSET_COPY_2},
+		{ NULL }
+	}
+};
diff --git a/libblkid/src/superblocks/superblocks.c b/libblkid/src/superblocks/superblocks.c
index 4387ab9e5..076541d1a 100644
--- a/libblkid/src/superblocks/superblocks.c
+++ b/libblkid/src/superblocks/superblocks.c
@@ -114,6 +114,7 @@ static const struct blkid_idinfo *idinfos[] =
 	&vmfs_volume_idinfo,
 	&ubi_idinfo,
 	&vdo_idinfo,
+	&stratis_idinfo,
 
 	/* Filesystems */
 	&vfat_idinfo,
diff --git a/libblkid/src/superblocks/superblocks.h b/libblkid/src/superblocks/superblocks.h
index 38f6a8a65..2723fb1d5 100644
--- a/libblkid/src/superblocks/superblocks.h
+++ b/libblkid/src/superblocks/superblocks.h
@@ -80,6 +80,7 @@ extern const struct blkid_idinfo f2fs_idinfo;
 extern const struct blkid_idinfo bcache_idinfo;
 extern const struct blkid_idinfo mpool_idinfo;
 extern const struct blkid_idinfo vdo_idinfo;
+extern const struct blkid_idinfo stratis_idinfo;
 
 /*
  * superblock functions
-- 
2.13.6

Re: [PATCH RFC 1/1] libblkid: Add support for stratis

[Karel Zak]

On Fri, Jan 05, 2018 at 11:15:39AM -0600, Tony Asleson wrote:
> +static int stratis_valid_sb(blkid_probe pr, uint8_t *p)
> +{
> +	const struct stratis_sb *stratis = (const struct stratis_sb *)p;
> +	uint32_t crc = 0;
> +
> +	// generate CRC from byte position 4 for length 508 == 512 byte sector
> +	crc = crc32c(~0L, p + sizeof(stratis->crc32),
> +			BS - sizeof(stratis->crc32));
> +	crc ^= ~0L;
> +
> +	return blkid_probe_verify_csum(pr, crc, le32_to_cpu(stratis->crc32));

Not sure that blkid_probe_verify_csum() is a good idea for your
use-case. 

The function ignores (returns 1) bad checksums if BLKID_SUBLKS_BADCSUM
flag is enabled. I guess you want to use second (backup) superblock 
if the primary one is broken. It would be better to use

    return crc == le32_to_cpu(stratis->crc32);

and do not use blkid_probe_verify_csum() at all.

> +static int probe_stratis(blkid_probe pr,
> +		const struct blkid_idmag *mag __attribute__((__unused__)))
> +{
> +	const struct stratis_sb *stratis = NULL;
> +	uint8_t *buf = blkid_probe_get_buffer(pr, 0, SB_AREA_SIZE);
> +
> +	if (!buf)
> +		return errno ? -errno : 1;
> +
> +	if (stratis_valid_sb(pr, buf + FIRST_COPY_OFFSET)) {
> +		stratis = (const struct stratis_sb *)(buf + FIRST_COPY_OFFSET);
> +	} else {
> +		if (!stratis_valid_sb(pr, buf + SECOND_COPY_OFFSET))
> +			return 1;
> +
> +		stratis = (const struct stratis_sb *)
> +				(buf + SECOND_COPY_OFFSET);
> +	}
> +
> +	blkid_probe_strncpy_uuid(pr, (unsigned char *)stratis->dev_uuid,
> +					sizeof(stratis->dev_uuid));
> +	blkid_probe_set_value(pr, "POOL_UUID",
> +				(unsigned char *)stratis->pool_uuid,
> +				sizeof(stratis->pool_uuid));
> +
> +	blkid_probe_sprintf_value(pr, "SECTORS", "%" PRIu64, stratis->sectors);

Hmm... SECTORS sounds pretty generic. Don't forget it will be
stored in udevd db. Would be possible to use any prefix?

For example:

    POOL_SECTORS

> +	blkid_probe_sprintf_value(pr, "INITIALIZATION_TIME", "%" PRIu64,
> +					stratis->initialization_time);

    POOL_INITTIME

Karel



-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH RFC 1/1] libblkid: Add support for stratis

[Tony Asleson]

Thank you for the suggestions, v2 patch posted which incorporates them.

Regards,
Tony

On 01/08/2018 05:16 AM, Karel Zak wrote:
> On Fri, Jan 05, 2018 at 11:15:39AM -0600, Tony Asleson wrote:
>> +static int stratis_valid_sb(blkid_probe pr, uint8_t *p)
>> +{
>> +	const struct stratis_sb *stratis = (const struct stratis_sb *)p;
>> +	uint32_t crc = 0;
>> +
>> +	// generate CRC from byte position 4 for length 508 == 512 byte sector
>> +	crc = crc32c(~0L, p + sizeof(stratis->crc32),
>> +			BS - sizeof(stratis->crc32));
>> +	crc ^= ~0L;
>> +
>> +	return blkid_probe_verify_csum(pr, crc, le32_to_cpu(stratis->crc32));
> 
> Not sure that blkid_probe_verify_csum() is a good idea for your
> use-case. 
> 
> The function ignores (returns 1) bad checksums if BLKID_SUBLKS_BADCSUM
> flag is enabled. I guess you want to use second (backup) superblock 
> if the primary one is broken. It would be better to use
> 
>     return crc == le32_to_cpu(stratis->crc32);
> 
> and do not use blkid_probe_verify_csum() at all.
> 
>> +static int probe_stratis(blkid_probe pr,
>> +		const struct blkid_idmag *mag __attribute__((__unused__)))
>> +{
>> +	const struct stratis_sb *stratis = NULL;
>> +	uint8_t *buf = blkid_probe_get_buffer(pr, 0, SB_AREA_SIZE);
>> +
>> +	if (!buf)
>> +		return errno ? -errno : 1;
>> +
>> +	if (stratis_valid_sb(pr, buf + FIRST_COPY_OFFSET)) {
>> +		stratis = (const struct stratis_sb *)(buf + FIRST_COPY_OFFSET);
>> +	} else {
>> +		if (!stratis_valid_sb(pr, buf + SECOND_COPY_OFFSET))
>> +			return 1;
>> +
>> +		stratis = (const struct stratis_sb *)
>> +				(buf + SECOND_COPY_OFFSET);
>> +	}
>> +
>> +	blkid_probe_strncpy_uuid(pr, (unsigned char *)stratis->dev_uuid,
>> +					sizeof(stratis->dev_uuid));
>> +	blkid_probe_set_value(pr, "POOL_UUID",
>> +				(unsigned char *)stratis->pool_uuid,
>> +				sizeof(stratis->pool_uuid));
>> +
>> +	blkid_probe_sprintf_value(pr, "SECTORS", "%" PRIu64, stratis->sectors);
> 
> Hmm... SECTORS sounds pretty generic. Don't forget it will be
> stored in udevd db. Would be possible to use any prefix?
> 
> For example:
> 
>     POOL_SECTORS
> 
>> +	blkid_probe_sprintf_value(pr, "INITIALIZATION_TIME", "%" PRIu64,
>> +					stratis->initialization_time);
> 
>     POOL_INITTIME
> 
> Karel
> 
> 
>