[PATCH 0/4 v2] Zoned block device support improvments

[PATCH 0/4 v2] Zoned block device support improvments

[Damien Le Moal]

The first 2 patches of this series are not zoned block device specific
and fix compilation warnings and modify .gitignore file to handle
/tools/f2fs_io/f2fs_io.

The last 2 patches are improvements of zoned block device support,
with the last patch being the most important.

Damien Le Moal (4):
  f2fs-tools: Fix various compilation warnings
  f2fs-tools: Add f2fs_io to .gitignore
  f2fs-tools: Improve zoned model check
  f2fs-tools: Fix multi-device format with zoned devices

 .gitignore          |  1 +
 include/f2fs_fs.h   |  8 ++++----
 lib/libf2fs.c       | 10 +++++++---
 lib/libf2fs_zoned.c | 47 ++++++++++++++++++++++++++++++++-------------
 tools/f2fstat.c     |  2 +-
 5 files changed, 47 insertions(+), 21 deletions(-)

Changes from v1:
* Dropped patch 4 as further analysis shows no problems with HA drives
  handling
* Fixed patch 3 to correctly handle zoned_model == none cases
* Added comment in the last patch as suggested by Chao

-- 
2.20.1

[PATCH 1/4 v2] f2fs-tools: Fix various compilation warnings

[Damien Le Moal]

Avoid various compilation warnings due to strncpy:

libf2fs.c:590:33: warning: ‘snprintf’ output may be truncated before
the last format character [-Wformat-truncation=]
  snprintf(rootdev, ret, "/dev/%s", buf);

../include/f2fs_fs.h:1384:2: warning: ‘strncpy’ specified bound
depends on the length of the source argument [-Wstringop-overflow=]
  strncpy(buf, features, strlen(features) + 1);

f2fstat.c:243:3: warning: ‘strncpy’ output truncated before
terminating nul copying as many bytes from a string as its length
[-Wstringop-truncation]
   strncpy(ptr_buf, name[i], strlen(name[i]));

This patch does not change any functionality.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
---
 include/f2fs_fs.h | 6 +++---
 lib/libf2fs.c     | 2 +-
 tools/f2fstat.c   | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index a730688..e073723 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -1379,9 +1379,9 @@ static inline int parse_feature(struct feature *table, const char *features)
 {
 	char *buf, *sub, *next;
 
-	buf = calloc(strlen(features) + 1, sizeof(char));
-	ASSERT(buf);
-	strncpy(buf, features, strlen(features) + 1);
+	buf = strdup(features);
+	if (!buf)
+		return -1;
 
 	for (sub = buf; sub && *sub; sub = next ? next + 1 : NULL) {
 		/* Skip the beginning blanks */
diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index c692bf2..60b84e0 100644
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -587,7 +587,7 @@ char *get_rootdev()
 		return NULL;
 	rootdev[ret] = '\0';
 
-	snprintf(rootdev, ret, "/dev/%s", buf);
+	snprintf(rootdev, ret + 1, "/dev/%s", buf);
 	return rootdev;
 #endif
 }
diff --git a/tools/f2fstat.c b/tools/f2fstat.c
index 7f485b0..0618806 100644
--- a/tools/f2fstat.c
+++ b/tools/f2fstat.c
@@ -240,7 +240,7 @@ void print_head(char *res)
 
 	for (i = 0; i < 20; i++) {
 		ptr = (i == 0) ? strtok(res, " ") : strtok(NULL, " ");
-		strncpy(ptr_buf, name[i], strlen(name[i]));
+		strcpy(ptr_buf, name[i]);
 		if (i == 1) {
 			prev_index = ptr_buf - buf - 1;
 		} else if (i == 7) {
-- 
2.20.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[PATCH 2/4 v2] f2fs-tools: Add f2fs_io to .gitignore

[Damien Le Moal]

GIT ignore compiled executable tools/f2fs_io/f2fs_io.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 3f04e85..c1341da 100644
--- a/.gitignore
+++ b/.gitignore
@@ -50,6 +50,7 @@ stamp-h1
 /tools/fibmap.f2fs
 /tools/parse.f2fs
 /tools/f2fscrypt
+/tools/f2fs_io/f2fs_io
 /tools/sg_write_buffer/sg_write_buffer
 
 # cscope files
-- 
2.20.1

[PATCH 3/4 v2] f2fs-tools: Improve zoned model check

[Damien Le Moal]

Return an error if an unknown zoned model is reported for a device or
if parsing of the device zoned model fails. Also add comments to
briefly explain the zone models and what to do in the absence of a
kernel reported zoned model for a device.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 include/f2fs_fs.h   |  2 +-
 lib/libf2fs.c       |  8 ++++++--
 lib/libf2fs_zoned.c | 38 ++++++++++++++++++++++++++------------
 3 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index e073723..97ad774 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -1257,7 +1257,7 @@ blk_zone_cond_str(struct blk_zone *blkz)
 
 #endif
 
-extern void f2fs_get_zoned_model(int);
+extern int f2fs_get_zoned_model(int);
 extern int f2fs_get_zone_blocks(int);
 extern int f2fs_check_zones(int);
 extern int f2fs_reset_zones(int);
diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index 60b84e0..5ca1bb0 100644
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -925,8 +925,12 @@ int get_device_info(int i)
 	}
 
 #if !defined(WITH_ANDROID) && defined(__linux__)
-	if (S_ISBLK(stat_buf->st_mode))
-		f2fs_get_zoned_model(i);
+	if (S_ISBLK(stat_buf->st_mode)) {
+		if (f2fs_get_zoned_model(i) < 0) {
+			free(stat_buf);
+			return -1;
+                }
+	}
 
 	if (dev->zoned_model != F2FS_ZONED_NONE) {
 		if (dev->zoned_model == F2FS_ZONED_HM)
diff --git a/lib/libf2fs_zoned.c b/lib/libf2fs_zoned.c
index 6e32f32..e396a95 100644
--- a/lib/libf2fs_zoned.c
+++ b/lib/libf2fs_zoned.c
@@ -24,7 +24,7 @@
 
 #ifdef HAVE_LINUX_BLKZONED_H
 
-void f2fs_get_zoned_model(int i)
+int f2fs_get_zoned_model(int i)
 {
 	struct device_info *dev = c.devices + i;
 	char str[128];
@@ -36,27 +36,41 @@ void f2fs_get_zoned_model(int i)
 		 "/sys/block/%s/queue/zoned",
 		 basename(dev->path));
 	file = fopen(str, "r");
-	if (!file)
-		goto not_zoned;
+	if (!file) {
+		/*
+		 * The kernel does not support zoned block devices, but we have
+		 * a block device file. This means that the device is not zoned
+		 * or is zoned but can be randomly written (i.e. host-aware
+		 * zoned model). Treat the device as a regular block device.
+		 */
+		dev->zoned_model = F2FS_ZONED_NONE;
+		return 0;
+	}
 
 	memset(str, 0, sizeof(str));
 	res = fscanf(file, "%s", str);
 	fclose(file);
 
-	if (res != 1)
-		goto not_zoned;
+	if (res != 1) {
+		MSG(0, "\tError: Failed to parse the device zoned model\n");
+		return -1;
+	}
 
-	if (strcmp(str, "host-aware") == 0) {
+	if (strcmp(str, "none") == 0) {
+		/* Regular block device */
+		dev->zoned_model = F2FS_ZONED_NONE;
+	} else if (strcmp(str, "host-aware") == 0) {
+		/* Host-aware zoned block device: can be randomly written */
 		dev->zoned_model = F2FS_ZONED_HA;
-		return;
-	}
-	if (strcmp(str, "host-managed") == 0) {
+	} else if (strcmp(str, "host-managed") == 0) {
+		/* Host-managed zoned block device: sequential writes needed */
 		dev->zoned_model = F2FS_ZONED_HM;
-		return;
+	} else {
+		MSG(0, "\tError: Unsupported device zoned model\n");
+		return -1;
 	}
 
-not_zoned:
-	dev->zoned_model = F2FS_ZONED_NONE;
+	return 0;
 }
 
 int f2fs_get_zone_blocks(int i)
-- 
2.20.1

[PATCH 4/4 v2] f2fs-tools: Fix multi-device format with zoned devices

[Damien Le Moal]

There is no need to require conventional zones for a zoned block device
that is not the first device of a multi-device volume. As a result,
there is no need to check the number of conventional zones of the
device if the device index is not 0.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 lib/libf2fs_zoned.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/libf2fs_zoned.c b/lib/libf2fs_zoned.c
index e396a95..97afe0b 100644
--- a/lib/libf2fs_zoned.c
+++ b/lib/libf2fs_zoned.c
@@ -216,7 +216,14 @@ int f2fs_check_zones(int j)
 		goto out;
 	}
 
-	if (dev->zoned_model == F2FS_ZONED_HM &&
+	/*
+	 * For a multi-device volume, fixed position metadata blocks are
+	 * stored * only on the first device of the volume. Checking for the
+	 * presence of * conventional zones (randomly writeabl zones) for
+	 * storing these blocks * on a host-managed device is thus needed only
+	 * for the device index 0.
+	 */
+	if (j == 0 && dev->zoned_model == F2FS_ZONED_HM &&
 			!dev->nr_rnd_zones) {
 		ERR_MSG("No conventional zone for super block\n");
 		ret = -1;
-- 
2.20.1

Re: [PATCH 3/4 v2] f2fs-tools: Improve zoned model check

[Chao Yu]

On 2019/3/27 15:54, Damien Le Moal wrote:
> Return an error if an unknown zoned model is reported for a device or
> if parsing of the device zoned model fails. Also add comments to
> briefly explain the zone models and what to do in the absence of a
> kernel reported zoned model for a device.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  include/f2fs_fs.h   |  2 +-
>  lib/libf2fs.c       |  8 ++++++--
>  lib/libf2fs_zoned.c | 38 ++++++++++++++++++++++++++------------
>  3 files changed, 33 insertions(+), 15 deletions(-)
> 
> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> index e073723..97ad774 100644
> --- a/include/f2fs_fs.h
> +++ b/include/f2fs_fs.h
> @@ -1257,7 +1257,7 @@ blk_zone_cond_str(struct blk_zone *blkz)
>  
>  #endif
>  
> -extern void f2fs_get_zoned_model(int);
> +extern int f2fs_get_zoned_model(int);
>  extern int f2fs_get_zone_blocks(int);
>  extern int f2fs_check_zones(int);
>  extern int f2fs_reset_zones(int);
> diff --git a/lib/libf2fs.c b/lib/libf2fs.c
> index 60b84e0..5ca1bb0 100644
> --- a/lib/libf2fs.c
> +++ b/lib/libf2fs.c
> @@ -925,8 +925,12 @@ int get_device_info(int i)
>  	}
>  
>  #if !defined(WITH_ANDROID) && defined(__linux__)
> -	if (S_ISBLK(stat_buf->st_mode))
> -		f2fs_get_zoned_model(i);
> +	if (S_ISBLK(stat_buf->st_mode)) {
> +		if (f2fs_get_zoned_model(i) < 0) {
> +			free(stat_buf);
> +			return -1;
> +                }

Needs tabs here...

Otherwise it looks good to me.

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

> +	}
>  
>  	if (dev->zoned_model != F2FS_ZONED_NONE) {
>  		if (dev->zoned_model == F2FS_ZONED_HM)
> diff --git a/lib/libf2fs_zoned.c b/lib/libf2fs_zoned.c
> index 6e32f32..e396a95 100644
> --- a/lib/libf2fs_zoned.c
> +++ b/lib/libf2fs_zoned.c
> @@ -24,7 +24,7 @@
>  
>  #ifdef HAVE_LINUX_BLKZONED_H
>  
> -void f2fs_get_zoned_model(int i)
> +int f2fs_get_zoned_model(int i)
>  {
>  	struct device_info *dev = c.devices + i;
>  	char str[128];
> @@ -36,27 +36,41 @@ void f2fs_get_zoned_model(int i)
>  		 "/sys/block/%s/queue/zoned",
>  		 basename(dev->path));
>  	file = fopen(str, "r");
> -	if (!file)
> -		goto not_zoned;
> +	if (!file) {
> +		/*
> +		 * The kernel does not support zoned block devices, but we have
> +		 * a block device file. This means that the device is not zoned
> +		 * or is zoned but can be randomly written (i.e. host-aware
> +		 * zoned model). Treat the device as a regular block device.
> +		 */
> +		dev->zoned_model = F2FS_ZONED_NONE;
> +		return 0;
> +	}
>  
>  	memset(str, 0, sizeof(str));
>  	res = fscanf(file, "%s", str);
>  	fclose(file);
>  
> -	if (res != 1)
> -		goto not_zoned;
> +	if (res != 1) {
> +		MSG(0, "\tError: Failed to parse the device zoned model\n");
> +		return -1;
> +	}
>  
> -	if (strcmp(str, "host-aware") == 0) {
> +	if (strcmp(str, "none") == 0) {
> +		/* Regular block device */
> +		dev->zoned_model = F2FS_ZONED_NONE;
> +	} else if (strcmp(str, "host-aware") == 0) {
> +		/* Host-aware zoned block device: can be randomly written */
>  		dev->zoned_model = F2FS_ZONED_HA;
> -		return;
> -	}
> -	if (strcmp(str, "host-managed") == 0) {
> +	} else if (strcmp(str, "host-managed") == 0) {
> +		/* Host-managed zoned block device: sequential writes needed */
>  		dev->zoned_model = F2FS_ZONED_HM;
> -		return;
> +	} else {
> +		MSG(0, "\tError: Unsupported device zoned model\n");
> +		return -1;
>  	}
>  
> -not_zoned:
> -	dev->zoned_model = F2FS_ZONED_NONE;
> +	return 0;
>  }
>  
>  int f2fs_get_zone_blocks(int i)
> 

Re: [PATCH 4/4 v2] f2fs-tools: Fix multi-device format with zoned devices

[Chao Yu]

On 2019/3/27 15:54, Damien Le Moal wrote:
> There is no need to require conventional zones for a zoned block device
> that is not the first device of a multi-device volume. As a result,
> there is no need to check the number of conventional zones of the
> device if the device index is not 0.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

Re: [PATCH 3/4 v2] f2fs-tools: Improve zoned model check

[Jaegeuk Kim]

On 04/01, Chao Yu wrote:
> On 2019/3/27 15:54, Damien Le Moal wrote:
> > Return an error if an unknown zoned model is reported for a device or
> > if parsing of the device zoned model fails. Also add comments to
> > briefly explain the zone models and what to do in the absence of a
> > kernel reported zoned model for a device.
> > 
> > Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> > ---
> >  include/f2fs_fs.h   |  2 +-
> >  lib/libf2fs.c       |  8 ++++++--
> >  lib/libf2fs_zoned.c | 38 ++++++++++++++++++++++++++------------
> >  3 files changed, 33 insertions(+), 15 deletions(-)
> > 
> > diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> > index e073723..97ad774 100644
> > --- a/include/f2fs_fs.h
> > +++ b/include/f2fs_fs.h
> > @@ -1257,7 +1257,7 @@ blk_zone_cond_str(struct blk_zone *blkz)
> >  
> >  #endif
> >  
> > -extern void f2fs_get_zoned_model(int);
> > +extern int f2fs_get_zoned_model(int);
> >  extern int f2fs_get_zone_blocks(int);
> >  extern int f2fs_check_zones(int);
> >  extern int f2fs_reset_zones(int);
> > diff --git a/lib/libf2fs.c b/lib/libf2fs.c
> > index 60b84e0..5ca1bb0 100644
> > --- a/lib/libf2fs.c
> > +++ b/lib/libf2fs.c
> > @@ -925,8 +925,12 @@ int get_device_info(int i)
> >  	}
> >  
> >  #if !defined(WITH_ANDROID) && defined(__linux__)
> > -	if (S_ISBLK(stat_buf->st_mode))
> > -		f2fs_get_zoned_model(i);
> > +	if (S_ISBLK(stat_buf->st_mode)) {
> > +		if (f2fs_get_zoned_model(i) < 0) {
> > +			free(stat_buf);
> > +			return -1;
> > +                }
> 
> Needs tabs here...

Applied. :)

> 
> Otherwise it looks good to me.
> 
> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> 
> Thanks,
> 
> > +	}
> >  
> >  	if (dev->zoned_model != F2FS_ZONED_NONE) {
> >  		if (dev->zoned_model == F2FS_ZONED_HM)
> > diff --git a/lib/libf2fs_zoned.c b/lib/libf2fs_zoned.c
> > index 6e32f32..e396a95 100644
> > --- a/lib/libf2fs_zoned.c
> > +++ b/lib/libf2fs_zoned.c
> > @@ -24,7 +24,7 @@
> >  
> >  #ifdef HAVE_LINUX_BLKZONED_H
> >  
> > -void f2fs_get_zoned_model(int i)
> > +int f2fs_get_zoned_model(int i)
> >  {
> >  	struct device_info *dev = c.devices + i;
> >  	char str[128];
> > @@ -36,27 +36,41 @@ void f2fs_get_zoned_model(int i)
> >  		 "/sys/block/%s/queue/zoned",
> >  		 basename(dev->path));
> >  	file = fopen(str, "r");
> > -	if (!file)
> > -		goto not_zoned;
> > +	if (!file) {
> > +		/*
> > +		 * The kernel does not support zoned block devices, but we have
> > +		 * a block device file. This means that the device is not zoned
> > +		 * or is zoned but can be randomly written (i.e. host-aware
> > +		 * zoned model). Treat the device as a regular block device.
> > +		 */
> > +		dev->zoned_model = F2FS_ZONED_NONE;
> > +		return 0;
> > +	}
> >  
> >  	memset(str, 0, sizeof(str));
> >  	res = fscanf(file, "%s", str);
> >  	fclose(file);
> >  
> > -	if (res != 1)
> > -		goto not_zoned;
> > +	if (res != 1) {
> > +		MSG(0, "\tError: Failed to parse the device zoned model\n");
> > +		return -1;
> > +	}
> >  
> > -	if (strcmp(str, "host-aware") == 0) {
> > +	if (strcmp(str, "none") == 0) {
> > +		/* Regular block device */
> > +		dev->zoned_model = F2FS_ZONED_NONE;
> > +	} else if (strcmp(str, "host-aware") == 0) {
> > +		/* Host-aware zoned block device: can be randomly written */
> >  		dev->zoned_model = F2FS_ZONED_HA;
> > -		return;
> > -	}
> > -	if (strcmp(str, "host-managed") == 0) {
> > +	} else if (strcmp(str, "host-managed") == 0) {
> > +		/* Host-managed zoned block device: sequential writes needed */
> >  		dev->zoned_model = F2FS_ZONED_HM;
> > -		return;
> > +	} else {
> > +		MSG(0, "\tError: Unsupported device zoned model\n");
> > +		return -1;
> >  	}
> >  
> > -not_zoned:
> > -	dev->zoned_model = F2FS_ZONED_NONE;
> > +	return 0;
> >  }
> >  
> >  int f2fs_get_zone_blocks(int i)
> >