Util-Linux Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] blkzone: Add --force option
@ 2020-06-05  5:06 Shin'ichiro Kawasaki
  2020-06-05  7:45 ` Karel Zak
  0 siblings, 1 reply; 3+ messages in thread
From: Shin'ichiro Kawasaki @ 2020-06-05  5:06 UTC (permalink / raw)
  To: Karel Zak, util-linux; +Cc: Damien Le Moal, Shinichiro Kawasaki

Commit 7a2602f629fe ("blkzone: deny destructive ioctls on busy blockdev")
introduced exclusive mode to open block devices to submit zone management
ioctls. This avoids unintended status change of block devices used by the
system. However, it makes blkzone less usable for testing. For example,
the test case zbd/007 of blktests utilizes blkzone to reset zones of
block devices mapped to dm-linear devices. After the commit, the test
case fails with EBUSY error at blkzone reset, since the system uses the
reset target block device to map to the dm-linear device.

To allow blkzone to change status of zoned block devices used by the
system with intention, introduce --force option. With this option, block
devices are opened without exclusive mode.

Also fix missing initialization and too many periods in man page of
--verbose option.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 sys-utils/blkzone.8 |  5 ++++-
 sys-utils/blkzone.c | 17 ++++++++++++++---
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/sys-utils/blkzone.8 b/sys-utils/blkzone.8
index f50e3f5df..64ad23bb3 100644
--- a/sys-utils/blkzone.8
+++ b/sys-utils/blkzone.8
@@ -107,9 +107,12 @@ The maximum number of zones the command should operate on. The default value
 is the number of zones starting from \fIoffset\fR. This option cannot be
 used together with the option \fB\-\-length\fP.
 .TP
+.BR \-f , " \-\-force"
+Enforce commands to change zone status on block devices used by the system.
+.TP
 .BR \-v , " \-\-verbose"
 Display the number of zones returned in the report or the range of sectors
-reset..
+reset.
 .TP
 .BR \-V , " \-\-version"
 Display version information and exit.
diff --git a/sys-utils/blkzone.c b/sys-utils/blkzone.c
index 62430e634..5c170127b 100644
--- a/sys-utils/blkzone.c
+++ b/sys-utils/blkzone.c
@@ -81,6 +81,7 @@ struct blkzone_control {
 	uint64_t length;
 	uint32_t count;
 
+	unsigned int force : 1;
 	unsigned int verbose : 1;
 };
 
@@ -295,13 +296,16 @@ static int blkzone_action(struct blkzone_control *ctl)
 	struct blk_zone_range za = { .sector = 0 };
 	unsigned long zonesize;
 	uint64_t zlen;
-	int fd;
+	int fd, mode;
 
 	zonesize = blkdev_chunk_sectors(ctl->devname);
 	if (!zonesize)
 		errx(EXIT_FAILURE, _("%s: unable to determine zone size"), ctl->devname);
 
-	fd = init_device(ctl, O_WRONLY | O_EXCL);
+	mode = O_WRONLY;
+	if (!ctl->force)
+		mode |= O_EXCL;
+	fd = init_device(ctl, mode);
 
 	if (ctl->offset & (zonesize - 1))
 		errx(EXIT_FAILURE, _("%s: offset %" PRIu64 " is not aligned "
@@ -362,6 +366,7 @@ static void __attribute__((__noreturn__)) usage(void)
 	fputs(_(" -o, --offset <sector>  start sector of zone to act (in 512-byte sectors)\n"), out);
 	fputs(_(" -l, --length <sectors> maximum sectors to act (in 512-byte sectors)\n"), out);
 	fputs(_(" -c, --count <number>   maximum number of zones\n"), out);
+	fputs(_(" -f, --force            enforce on block devices used by the system\n"), out);
 	fputs(_(" -v, --verbose          display more details\n"), out);
 	fputs(USAGE_SEPARATOR, out);
 	printf(USAGE_HELP_OPTIONS(24));
@@ -382,12 +387,15 @@ int main(int argc, char **argv)
 		.count = 0,
 		.length = 0
 	};
+	ctl.force = 0;
+	ctl.verbose = 0;
 
 	static const struct option longopts[] = {
 	    { "help",    no_argument,       NULL, 'h' },
 	    { "count",   required_argument, NULL, 'c' }, /* max #of zones to operate on */
 	    { "length",  required_argument, NULL, 'l' }, /* max of sectors to operate on */
 	    { "offset",  required_argument, NULL, 'o' }, /* starting LBA */
+	    { "force", no_argument,         NULL, 'f' },
 	    { "verbose", no_argument,       NULL, 'v' },
 	    { "version", no_argument,       NULL, 'V' },
 	    { NULL, 0, NULL, 0 }
@@ -412,7 +420,7 @@ int main(int argc, char **argv)
 		argc--;
 	}
 
-	while ((c = getopt_long(argc, argv, "hc:l:o:vV", longopts, NULL)) != -1) {
+	while ((c = getopt_long(argc, argv, "hc:l:o:fvV", longopts, NULL)) != -1) {
 
 		err_exclusive_options(c, longopts, excl, excl_st);
 
@@ -429,6 +437,9 @@ int main(int argc, char **argv)
 			ctl.offset = strtosize_or_err(optarg,
 					_("failed to parse zone offset"));
 			break;
+		case 'f':
+			ctl.force = 1;
+			break;
 		case 'v':
 			ctl.verbose = 1;
 			break;
-- 
2.25.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] blkzone: Add --force option
  2020-06-05  5:06 [PATCH] blkzone: Add --force option Shin'ichiro Kawasaki
@ 2020-06-05  7:45 ` Karel Zak
  2020-06-05  8:16   ` Shinichiro Kawasaki
  0 siblings, 1 reply; 3+ messages in thread
From: Karel Zak @ 2020-06-05  7:45 UTC (permalink / raw)
  To: Shin'ichiro Kawasaki; +Cc: util-linux, Damien Le Moal

On Fri, Jun 05, 2020 at 02:06:18PM +0900, Shin'ichiro Kawasaki wrote:
> Also fix missing initialization and too many periods in man page of
> --verbose option.
 
The initialization is not missing :-)

> @@ -382,12 +387,15 @@ int main(int argc, char **argv)
>  		.count = 0,
>  		.length = 0
>  	};
> +	ctl.force = 0;
> +	ctl.verbose = 0;

This is unnecessary.

According to C standard all subobjects that are not initialized
explicitly shall be initialized implicitly the same as objects that
have static storage duration. (C11 standard, 6.7.8 Initialization).

For example: 

 struct A {
    int x, y, z;
 };

 struct A foo = { .x = 0 };

means that foo.y as well as foo.z are zero.

You can see this pattern pretty often our code.


Anyway, your patch has been merged (with tiny changes). Thanks!

    Karel

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] blkzone: Add --force option
  2020-06-05  7:45 ` Karel Zak
@ 2020-06-05  8:16   ` Shinichiro Kawasaki
  0 siblings, 0 replies; 3+ messages in thread
From: Shinichiro Kawasaki @ 2020-06-05  8:16 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux, Damien Le Moal

On Jun 05, 2020 / 09:45, Karel Zak wrote:
> On Fri, Jun 05, 2020 at 02:06:18PM +0900, Shin'ichiro Kawasaki wrote:
> > Also fix missing initialization and too many periods in man page of
> > --verbose option.
>  
> The initialization is not missing :-)
> 
> > @@ -382,12 +387,15 @@ int main(int argc, char **argv)
> >  		.count = 0,
> >  		.length = 0
> >  	};
> > +	ctl.force = 0;
> > +	ctl.verbose = 0;
> 
> This is unnecessary.
> 
> According to C standard all subobjects that are not initialized
> explicitly shall be initialized implicitly the same as objects that
> have static storage duration. (C11 standard, 6.7.8 Initialization).
> 
> For example: 
> 
>  struct A {
>     int x, y, z;
>  };
> 
>  struct A foo = { .x = 0 };
> 
> means that foo.y as well as foo.z are zero.
> 
> You can see this pattern pretty often our code.

Thanks. I misunderstood C language struct initialization. Also I noticed that
you made another commit to reduce confusion. This is helpful :)

> 
> 
> Anyway, your patch has been merged (with tiny changes). Thanks!

Thank you!

-- 
Best Regards,
Shin'ichiro Kawasaki

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-05  5:06 [PATCH] blkzone: Add --force option Shin'ichiro Kawasaki
2020-06-05  7:45 ` Karel Zak
2020-06-05  8:16   ` Shinichiro Kawasaki

Util-Linux Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/util-linux/0 util-linux/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 util-linux util-linux/ https://lore.kernel.org/util-linux \
		util-linux@vger.kernel.org
	public-inbox-index util-linux

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.util-linux


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git