* [PATCH] blkdiscard: add new command @ 2012-09-12 21:49 Lukas Czerner 2012-09-12 22:43 ` Kay Sievers ` (2 more replies) 0 siblings, 3 replies; 23+ messages in thread From: Lukas Czerner @ 2012-09-12 21:49 UTC (permalink / raw) To: util-linux, kzak; +Cc: Lukas Czerner blkdiscard is used to discard device sectors. This is useful for solid-state drivers (SSDs) and thinly-provisioned storage. Unlike fstrim this command is used directly on the block device. blkkdiscard uses BLKDISCARD ioctl or BLKSECDISCARD ioctl for the secure discard. All data in the discarded region on the device will be lost! Signed-off-by: Lukas Czerner <lczerner@redhat.com> --- .gitignore | 1 + sys-utils/Makemodule.am | 5 ++ sys-utils/blkdiscard.8 | 66 ++++++++++++++++++ sys-utils/blkdiscard.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 245 insertions(+), 0 deletions(-) create mode 100644 sys-utils/blkdiscard.8 create mode 100644 sys-utils/blkdiscard.c diff --git a/.gitignore b/.gitignore index 5be008f..a777d16 100644 --- a/.gitignore +++ b/.gitignore @@ -96,6 +96,7 @@ tests/run.sh.trs /fsck.minix /fsfreeze /fstrim +/blkdiscard /getopt /hexdump /hwclock diff --git a/sys-utils/Makemodule.am b/sys-utils/Makemodule.am index d376f04..a6e3c07 100644 --- a/sys-utils/Makemodule.am +++ b/sys-utils/Makemodule.am @@ -54,6 +54,11 @@ dist_man_MANS += sys-utils/fstrim.8 fstrim_SOURCES = sys-utils/fstrim.c fstrim_LDADD = $(LDADD) libcommon.la +sbin_PROGRAMS += blkdiscard +dist_man_MANS += sys-utils/blkdiscard.8 +blkdiscard_SOURCES = sys-utils/blkdiscard.c +blkdiscard_LDADD = $(LDADD) libcommon.la + usrbin_exec_PROGRAMS += cytune dist_man_MANS += sys-utils/cytune.8 cytune_SOURCES = sys-utils/cytune.c sys-utils/cyclades.h diff --git a/sys-utils/blkdiscard.8 b/sys-utils/blkdiscard.8 new file mode 100644 index 0000000..fcc38f6 --- /dev/null +++ b/sys-utils/blkdiscard.8 @@ -0,0 +1,66 @@ +.\" -*- nroff -*- +.TH BLKDISCARD 8 "September 2012" "util-linux" "System Administration" +.SH NAME +blkdiscard \- discard sectors on a device +.SH SYNOPSIS +.B blkdiscard +.RB [ \-o +.IR offset ] +.RB [ \-l +.IR length ] +.RB [ \-s ] +.RB [ \-v ] +.I device + +.SH DESCRIPTION +.B blkdiscard +is used to discard device sectors. This is useful for solid-state +drivers (SSDs) and thinly-provisioned storage. Unlike +.BR fstrim (8) +this command is used directly on the block device. +.PP +By default, +.B blkdiscard +will discard all blocks on the device. Options may be used to +modify this behavior based on range or size, as explained below. +.PP +The +.I device +argument is the pathname of the block device. + +.B WARNING: All data in the discarded region on the device will be lost! + +.SH OPTIONS +The \fIoffset\fR and \fIlength\fR arguments may be +followed by the multiplicative suffixes KiB=1024, MiB=1024*1024, and so on for +GiB, TiB, PiB, EiB, ZiB and YiB (the "iB" is optional, e.g. "K" has the same +meaning as "KiB") or the suffixes KB=1000, MB=1000*1000, and so on for GB, PB, +EB, ZB and YB. +.IP "\fB\-h, \-\-help\fP" +Print help and exit. +.IP "\fB\-o, \-\-offset\fP \fIoffset\fP" +Byte offset in the device from which to discard. Provided value will be +aligned to the device sector size. Default value is zero. +.IP "\fB\-l, \-\-length\fP \fIlength\fP" +Number of bytes after starting point to discard. Provided value will be +aligned to the device sector size. If the specified value extends past the +end of the device, +.B blkdiscard +will stop at the device size boundary. Default value extends to the end +of the device. +.IP "\fB\-s, \-\-secure\fP" +Perform secure discard. Secure discard is the same as regular discard except +all copies of the discarded blocks possibly created by garbage collection must +also be erased. It has to be supported by the device. +.IP "\fB\-v, \-\-verbose\fP" +Print aligned \fIoffset\fR and \fIlength\fR arguments. + +.SH AUTHOR +.nf +Lukas Czerner <lczerner@redhat.com> +.fi +.SH SEE ALSO +.BR fstrim (8) +.SH AVAILABILITY +The blkdiscard command is part of the util-linux package and is available +from ftp://ftp.kernel.org/pub/linux/utils/util-linux/. diff --git a/sys-utils/blkdiscard.c b/sys-utils/blkdiscard.c new file mode 100644 index 0000000..bdcd06e --- /dev/null +++ b/sys-utils/blkdiscard.c @@ -0,0 +1,173 @@ +/* + * blkdiscard.c -- discard the part (or whole) of the block device. + * + * Copyright (C) 2012 Red Hat, Inc. All rights reserved. + * Written by Lukas Czerner <lczerner@redhat.com> + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + * This program uses BLKDISCARD ioctl to discard part or the whole block + * device if the device supports it. You can specify range (start and + * length) to be discarded, or simply discard the whole device. + */ + + +#include <string.h> +#include <unistd.h> +#include <stdlib.h> +#include <stdio.h> +#include <stdint.h> +#include <fcntl.h> +#include <limits.h> +#include <getopt.h> + +#include <sys/ioctl.h> +#include <sys/stat.h> +#include <linux/fs.h> + +#include "nls.h" +#include "strutils.h" +#include "c.h" +#include "closestream.h" + +#ifndef BLKDISCARD +#define BLKDISCARD _IO(0x12,119) +#endif + +#ifndef BLKSECDISCARD +#define BLKSECDISCARD _IO(0x12,125) +#endif + +static void __attribute__((__noreturn__)) usage(FILE *out) +{ + fputs(USAGE_HEADER, out); + fprintf(out, + _(" %s [options] <device>\n"), program_invocation_short_name); + fputs(USAGE_OPTIONS, out); + fputs(_(" -o, --offset <num> offset in bytes to discard from\n" + " -l, --length <num> length of bytes to discard from the offset\n" + " -s, --secure perform secure discard\n" + " -v, --verbose print aligned length and offset\n"), + out); + fputs(USAGE_SEPARATOR, out); + fputs(USAGE_HELP, out); + fputs(USAGE_VERSION, out); + fprintf(out, USAGE_MAN_TAIL("blkdiscard(8)")); + exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS); +} + +int main(int argc, char **argv) +{ + char *path; + int c, fd, verbose = 0, secure = 0; + uint64_t end, blksize, secsize, range[2]; + struct stat sb; + + static const struct option longopts[] = { + { "help", 0, 0, 'h' }, + { "version", 0, 0, 'V' }, + { "offset", 1, 0, 'o' }, + { "length", 1, 0, 'l' }, + { "secure", 0, 0, 's' }, + { "verbose", 0, 0, 'v' }, + { NULL, 0, 0, 0 } + }; + + setlocale(LC_ALL, ""); + bindtextdomain(PACKAGE, LOCALEDIR); + textdomain(PACKAGE); + atexit(close_stdout); + + range[0] = 0; + range[1] = ULLONG_MAX; + + while ((c = getopt_long(argc, argv, "hVsvo:l:", longopts, NULL)) != -1) { + switch(c) { + case 'h': + usage(stdout); + break; + case 'V': + printf(UTIL_LINUX_VERSION); + return EXIT_SUCCESS; + case 'l': + range[1] = strtosize_or_err(optarg, + _("failed to parse length")); + break; + case 'o': + range[0] = strtosize_or_err(optarg, + _("failed to parse offset")); + break; + case 's': + secure = 1; + break; + case 'v': + verbose = 1; + break; + default: + usage(stderr); + break; + } + } + + if (optind == argc) + errx(EXIT_FAILURE, _("no device specified.")); + + path = argv[optind++]; + + if (optind != argc) { + warnx(_("unexpected number of arguments")); + usage(stderr); + } + + if (stat(path, &sb) == -1) + err(EXIT_FAILURE, _("stat failed %s"), path); + if (!S_ISBLK(sb.st_mode)) + errx(EXIT_FAILURE, _("%s: not a block device"), path); + + fd = open(path, O_WRONLY); + if (fd < 0) + err(EXIT_FAILURE, _("cannot open %s"), path); + + if (ioctl(fd, BLKGETSIZE64, &blksize)) + err(EXIT_FAILURE, _("%s: BLKGETSIZE64 ioctl failed"), path); + + if (ioctl(fd, BLKSSZGET, &secsize)) + err(EXIT_FAILURE, _("%s: BLKSSZGET ioctl failed"), path); + + /* align range to the sector size */ + range[0] = (range[0] + secsize - 1) & ~(secsize - 1); + range[1] &= ~(secsize - 1); + + /* is the range end behind the end of the device ?*/ + end = range[0] + range[1]; + if (end < range[0] || end > blksize) + range[1] = blksize - range[0]; + + if (secure) { + if (ioctl(fd, BLKSECDISCARD, &range)) + err(EXIT_FAILURE, _("%s: BLKSECDISCARD ioctl failed"), path); + } else { + if (ioctl(fd, BLKDISCARD, &range)) + err(EXIT_FAILURE, _("%s: BLKDISCARD ioctl failed"), path); + } + + if (verbose) + /* TRANSLATORS: The standard value here is a very large number. */ + printf(_("%s: Discarded %" PRIu64 " bytes from the " + "offset %" PRIu64"\n"), path, + (uint64_t) range[1], (uint64_t) range[0]); + + close(fd); + return EXIT_SUCCESS; +} -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-12 21:49 [PATCH] blkdiscard: add new command Lukas Czerner @ 2012-09-12 22:43 ` Kay Sievers 2012-09-12 22:47 ` Davidlohr Bueso 2012-09-13 13:37 ` Lukáš Czerner 2012-09-26 19:47 ` Lukáš Czerner 2012-09-27 23:13 ` Karel Zak 2 siblings, 2 replies; 23+ messages in thread From: Kay Sievers @ 2012-09-12 22:43 UTC (permalink / raw) To: Lukas Czerner; +Cc: util-linux, kzak On Wed, Sep 12, 2012 at 11:49 PM, Lukas Czerner <lczerner@redhat.com> wrote: > blkdiscard is used to discard device sectors. This is useful for > solid-state drivers (SSDs) and thinly-provisioned storage. Unlike > fstrim this command is used directly on the block device. Should this maybe live in blockdev(8)? It's kind of unfortunate to create standalone tools for every other rather exotic block device feature. Thanks, Kay ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-12 22:43 ` Kay Sievers @ 2012-09-12 22:47 ` Davidlohr Bueso 2012-09-13 13:37 ` Lukáš Czerner 1 sibling, 0 replies; 23+ messages in thread From: Davidlohr Bueso @ 2012-09-12 22:47 UTC (permalink / raw) To: Kay Sievers; +Cc: Lukas Czerner, util-linux, kzak On Thu, 2012-09-13 at 00:43 +0200, Kay Sievers wrote: > On Wed, Sep 12, 2012 at 11:49 PM, Lukas Czerner <lczerner@redhat.com> wrote: > > blkdiscard is used to discard device sectors. This is useful for > > solid-state drivers (SSDs) and thinly-provisioned storage. Unlike > > fstrim this command is used directly on the block device. > > Should this maybe live in blockdev(8)? +1 > > It's kind of unfortunate to create standalone tools for every other > rather exotic block device feature. > > Thanks, > Kay > -- > To unsubscribe from this list: send the line "unsubscribe util-linux" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-12 22:43 ` Kay Sievers 2012-09-12 22:47 ` Davidlohr Bueso @ 2012-09-13 13:37 ` Lukáš Czerner 2012-09-13 13:57 ` Karel Zak 2012-09-13 15:32 ` Kay Sievers 1 sibling, 2 replies; 23+ messages in thread From: Lukáš Czerner @ 2012-09-13 13:37 UTC (permalink / raw) To: Kay Sievers; +Cc: Lukas Czerner, util-linux, kzak On Thu, 13 Sep 2012, Kay Sievers wrote: > Date: Thu, 13 Sep 2012 00:43:01 +0200 > From: Kay Sievers <kay@vrfy.org> > To: Lukas Czerner <lczerner@redhat.com> > Cc: util-linux@vger.kernel.org, kzak@redhat.com > Subject: Re: [PATCH] blkdiscard: add new command > > On Wed, Sep 12, 2012 at 11:49 PM, Lukas Czerner <lczerner@redhat.com> wrote: > > blkdiscard is used to discard device sectors. This is useful for > > solid-state drivers (SSDs) and thinly-provisioned storage. Unlike > > fstrim this command is used directly on the block device. > > Should this maybe live in blockdev(8)? > > It's kind of unfortunate to create standalone tools for every other > rather exotic block device feature. Discard is hardly exotic feature nowadays :). But anyway, I was not aware of blockdev(8) and indeed it seems like a better place to put this functionality. I'll look into it. Thanks! -Lukas > > Thanks, > Kay > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-13 13:37 ` Lukáš Czerner @ 2012-09-13 13:57 ` Karel Zak 2012-09-13 14:09 ` Lukáš Czerner 2012-09-13 15:33 ` Kay Sievers 2012-09-13 15:32 ` Kay Sievers 1 sibling, 2 replies; 23+ messages in thread From: Karel Zak @ 2012-09-13 13:57 UTC (permalink / raw) To: Lukáš Czerner; +Cc: Kay Sievers, util-linux On Thu, Sep 13, 2012 at 09:37:30AM -0400, Lukáš Czerner wrote: > On Thu, 13 Sep 2012, Kay Sievers wrote: > > > Date: Thu, 13 Sep 2012 00:43:01 +0200 > > From: Kay Sievers <kay@vrfy.org> > > To: Lukas Czerner <lczerner@redhat.com> > > Cc: util-linux@vger.kernel.org, kzak@redhat.com > > Subject: Re: [PATCH] blkdiscard: add new command > > > > On Wed, Sep 12, 2012 at 11:49 PM, Lukas Czerner <lczerner@redhat.com> wrote: > > > blkdiscard is used to discard device sectors. This is useful for > > > solid-state drivers (SSDs) and thinly-provisioned storage. Unlike > > > fstrim this command is used directly on the block device. > > > > Should this maybe live in blockdev(8)? > > > > It's kind of unfortunate to create standalone tools for every other > > rather exotic block device feature. > > Discard is hardly exotic feature nowadays :). But anyway, I was not aware of > blockdev(8) and indeed it seems like a better place to put this > functionality. I'll look into it. That's question ... we already have fstrim which is also wrapper for one ioctl and it seems that your blkdiscard requires some logic (devsize, sectorsize, align ranges, ...). We usually use blockdev for pretty primitive ioctls, add something more complex will be hack (like --getsz or --report). It's UNIX, it's not so unusual that utils do one thing and do it well ;-) Karel -- Karel Zak <kzak@redhat.com> http://karelzak.blogspot.com ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-13 13:57 ` Karel Zak @ 2012-09-13 14:09 ` Lukáš Czerner 2012-09-13 15:33 ` Kay Sievers 1 sibling, 0 replies; 23+ messages in thread From: Lukáš Czerner @ 2012-09-13 14:09 UTC (permalink / raw) To: Karel Zak; +Cc: Lukáš Czerner, Kay Sievers, util-linux [-- Attachment #1: Type: TEXT/PLAIN, Size: 2190 bytes --] On Thu, 13 Sep 2012, Karel Zak wrote: > Date: Thu, 13 Sep 2012 15:57:27 +0200 > From: Karel Zak <kzak@redhat.com> > To: Lukáš Czerner <lczerner@redhat.com> > Cc: Kay Sievers <kay@vrfy.org>, util-linux@vger.kernel.org > Subject: Re: [PATCH] blkdiscard: add new command > > On Thu, Sep 13, 2012 at 09:37:30AM -0400, Lukáš Czerner wrote: > > On Thu, 13 Sep 2012, Kay Sievers wrote: > > > > > Date: Thu, 13 Sep 2012 00:43:01 +0200 > > > From: Kay Sievers <kay@vrfy.org> > > > To: Lukas Czerner <lczerner@redhat.com> > > > Cc: util-linux@vger.kernel.org, kzak@redhat.com > > > Subject: Re: [PATCH] blkdiscard: add new command > > > > > > On Wed, Sep 12, 2012 at 11:49 PM, Lukas Czerner <lczerner@redhat.com> wrote: > > > > blkdiscard is used to discard device sectors. This is useful for > > > > solid-state drivers (SSDs) and thinly-provisioned storage. Unlike > > > > fstrim this command is used directly on the block device. > > > > > > Should this maybe live in blockdev(8)? > > > > > > It's kind of unfortunate to create standalone tools for every other > > > rather exotic block device feature. > > > > Discard is hardly exotic feature nowadays :). But anyway, I was not aware of > > blockdev(8) and indeed it seems like a better place to put this > > functionality. I'll look into it. > > That's question ... we already have fstrim which is also wrapper for > one ioctl and it seems that your blkdiscard requires some logic > (devsize, sectorsize, align ranges, ...). We usually use blockdev > for pretty primitive ioctls, add something more complex will be hack > (like --getsz or --report). > > It's UNIX, it's not so unusual that utils do one thing and do it well ;-) > > Karel Righ, I've looked at the blockdev(8) and indeed it lacks the ability to pass more "complex" arguments to the command itself. It would have to be updated to support this. Most likely to something like this: blockdev <blockdev options> <command> <command options> but I am not sure it worth it. And your UNIX point is quite valid. I am on the fence about this (with slight inclination to separate blkdiscard), but you're the maintainer so you have the privilege to decide :) -Lukas ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-13 13:57 ` Karel Zak 2012-09-13 14:09 ` Lukáš Czerner @ 2012-09-13 15:33 ` Kay Sievers 2012-09-13 15:52 ` Lukáš Czerner 1 sibling, 1 reply; 23+ messages in thread From: Kay Sievers @ 2012-09-13 15:33 UTC (permalink / raw) To: Karel Zak; +Cc: Lukáš Czerner, util-linux On Thu, Sep 13, 2012 at 3:57 PM, Karel Zak <kzak@redhat.com> wrote: > It's UNIX, it's not so unusual that utils do one thing and do it well ;-) How about looking at the sensible part of UNIX? I can provide you an endless list of crazy ideas in UNIX if you need that. And one tool per ioctl is certainly in that list. :) Kay ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-13 15:33 ` Kay Sievers @ 2012-09-13 15:52 ` Lukáš Czerner 0 siblings, 0 replies; 23+ messages in thread From: Lukáš Czerner @ 2012-09-13 15:52 UTC (permalink / raw) To: Kay Sievers; +Cc: Karel Zak, Lukáš Czerner, util-linux [-- Attachment #1: Type: TEXT/PLAIN, Size: 1526 bytes --] On Thu, 13 Sep 2012, Kay Sievers wrote: > Date: Thu, 13 Sep 2012 17:33:49 +0200 > From: Kay Sievers <kay@vrfy.org> > To: Karel Zak <kzak@redhat.com> > Cc: Lukáš Czerner <lczerner@redhat.com>, util-linux@vger.kernel.org > Subject: Re: [PATCH] blkdiscard: add new command > > On Thu, Sep 13, 2012 at 3:57 PM, Karel Zak <kzak@redhat.com> wrote: > > > It's UNIX, it's not so unusual that utils do one thing and do it well ;-) > > How about looking at the sensible part of UNIX? I can provide you an > endless list of crazy ideas in UNIX if you need that. And one tool per > ioctl is certainly in that list. :) Not necessarily. blockdev(8) is meant to provide user with a tool to invoke simple block device ioctl which usually does not require any arguments (except few with one argument). Changing its format to be blockdev <options> <command> <options> and extend it would certainly take away some of the nifty simplicity of that tool. That said, there is nothing wrong with having small and simple tool doing one thing and doing it right, in fact quite the opposite - it _is_ a good thing. And BTH I am not really suggesting one tool per ioctl, but blkdiscard is sufficiently different then those found in blockdev anyway (mostly -get, few -set and one -flush). In fact thinking about it some more, I am really in favour of separate tool for this functionality. BTW since when it's wrong to have such small and simple tools instead of multiplexing everything into one blob (Core OS anyone ?:)) ? -Lukas > > Kay > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-13 13:37 ` Lukáš Czerner 2012-09-13 13:57 ` Karel Zak @ 2012-09-13 15:32 ` Kay Sievers 1 sibling, 0 replies; 23+ messages in thread From: Kay Sievers @ 2012-09-13 15:32 UTC (permalink / raw) To: Lukáš Czerner; +Cc: util-linux, kzak On Thu, Sep 13, 2012 at 3:37 PM, Lukáš Czerner <lczerner@redhat.com> wrote: > On Thu, 13 Sep 2012, Kay Sievers wrote: > >> Date: Thu, 13 Sep 2012 00:43:01 +0200 >> From: Kay Sievers <kay@vrfy.org> >> To: Lukas Czerner <lczerner@redhat.com> >> Cc: util-linux@vger.kernel.org, kzak@redhat.com >> Subject: Re: [PATCH] blkdiscard: add new command >> >> On Wed, Sep 12, 2012 at 11:49 PM, Lukas Czerner <lczerner@redhat.com> wrote: >> > blkdiscard is used to discard device sectors. This is useful for >> > solid-state drivers (SSDs) and thinly-provisioned storage. Unlike >> > fstrim this command is used directly on the block device. >> >> Should this maybe live in blockdev(8)? >> >> It's kind of unfortunate to create standalone tools for every other >> rather exotic block device feature. > > Discard is hardly exotic feature nowadays :) Nowadays, stuff is usually done by filesystems during runtime or filesysystem formatters. Or secure erase of the whole disk, which usually also has the benefit of resetting the garbage collector tables in the device. Therefore, discarding areas with this new command seems pretty exotic to me. :) And also how does it overlap with hdparm's --trim-sector-ranges ? Kay ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-12 21:49 [PATCH] blkdiscard: add new command Lukas Czerner 2012-09-12 22:43 ` Kay Sievers @ 2012-09-26 19:47 ` Lukáš Czerner 2012-09-26 20:04 ` Kay Sievers 2012-09-27 9:42 ` Karel Zak 2012-09-27 23:13 ` Karel Zak 2 siblings, 2 replies; 23+ messages in thread From: Lukáš Czerner @ 2012-09-26 19:47 UTC (permalink / raw) To: Lukas Czerner; +Cc: util-linux, kzak On Wed, 12 Sep 2012, Lukas Czerner wrote: > Date: Wed, 12 Sep 2012 17:49:15 -0400 > From: Lukas Czerner <lczerner@redhat.com> > To: util-linux@vger.kernel.org, kzak@redhat.com > Cc: Lukas Czerner <lczerner@redhat.com> > Subject: [PATCH] blkdiscard: add new command > > blkdiscard is used to discard device sectors. This is useful for > solid-state drivers (SSDs) and thinly-provisioned storage. Unlike > fstrim this command is used directly on the block device. > > blkkdiscard uses BLKDISCARD ioctl or BLKSECDISCARD ioctl for the secure > discard. > > All data in the discarded region on the device will be lost! Hi Karel, any progress here ? Thanks! -Lukas > > Signed-off-by: Lukas Czerner <lczerner@redhat.com> > --- > .gitignore | 1 + > sys-utils/Makemodule.am | 5 ++ > sys-utils/blkdiscard.8 | 66 ++++++++++++++++++ > sys-utils/blkdiscard.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 245 insertions(+), 0 deletions(-) > create mode 100644 sys-utils/blkdiscard.8 > create mode 100644 sys-utils/blkdiscard.c > > diff --git a/.gitignore b/.gitignore > index 5be008f..a777d16 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -96,6 +96,7 @@ tests/run.sh.trs > /fsck.minix > /fsfreeze > /fstrim > +/blkdiscard > /getopt > /hexdump > /hwclock > diff --git a/sys-utils/Makemodule.am b/sys-utils/Makemodule.am > index d376f04..a6e3c07 100644 > --- a/sys-utils/Makemodule.am > +++ b/sys-utils/Makemodule.am > @@ -54,6 +54,11 @@ dist_man_MANS += sys-utils/fstrim.8 > fstrim_SOURCES = sys-utils/fstrim.c > fstrim_LDADD = $(LDADD) libcommon.la > > +sbin_PROGRAMS += blkdiscard > +dist_man_MANS += sys-utils/blkdiscard.8 > +blkdiscard_SOURCES = sys-utils/blkdiscard.c > +blkdiscard_LDADD = $(LDADD) libcommon.la > + > usrbin_exec_PROGRAMS += cytune > dist_man_MANS += sys-utils/cytune.8 > cytune_SOURCES = sys-utils/cytune.c sys-utils/cyclades.h > diff --git a/sys-utils/blkdiscard.8 b/sys-utils/blkdiscard.8 > new file mode 100644 > index 0000000..fcc38f6 > --- /dev/null > +++ b/sys-utils/blkdiscard.8 > @@ -0,0 +1,66 @@ > +.\" -*- nroff -*- > +.TH BLKDISCARD 8 "September 2012" "util-linux" "System Administration" > +.SH NAME > +blkdiscard \- discard sectors on a device > +.SH SYNOPSIS > +.B blkdiscard > +.RB [ \-o > +.IR offset ] > +.RB [ \-l > +.IR length ] > +.RB [ \-s ] > +.RB [ \-v ] > +.I device > + > +.SH DESCRIPTION > +.B blkdiscard > +is used to discard device sectors. This is useful for solid-state > +drivers (SSDs) and thinly-provisioned storage. Unlike > +.BR fstrim (8) > +this command is used directly on the block device. > +.PP > +By default, > +.B blkdiscard > +will discard all blocks on the device. Options may be used to > +modify this behavior based on range or size, as explained below. > +.PP > +The > +.I device > +argument is the pathname of the block device. > + > +.B WARNING: All data in the discarded region on the device will be lost! > + > +.SH OPTIONS > +The \fIoffset\fR and \fIlength\fR arguments may be > +followed by the multiplicative suffixes KiB=1024, MiB=1024*1024, and so on for > +GiB, TiB, PiB, EiB, ZiB and YiB (the "iB" is optional, e.g. "K" has the same > +meaning as "KiB") or the suffixes KB=1000, MB=1000*1000, and so on for GB, PB, > +EB, ZB and YB. > +.IP "\fB\-h, \-\-help\fP" > +Print help and exit. > +.IP "\fB\-o, \-\-offset\fP \fIoffset\fP" > +Byte offset in the device from which to discard. Provided value will be > +aligned to the device sector size. Default value is zero. > +.IP "\fB\-l, \-\-length\fP \fIlength\fP" > +Number of bytes after starting point to discard. Provided value will be > +aligned to the device sector size. If the specified value extends past the > +end of the device, > +.B blkdiscard > +will stop at the device size boundary. Default value extends to the end > +of the device. > +.IP "\fB\-s, \-\-secure\fP" > +Perform secure discard. Secure discard is the same as regular discard except > +all copies of the discarded blocks possibly created by garbage collection must > +also be erased. It has to be supported by the device. > +.IP "\fB\-v, \-\-verbose\fP" > +Print aligned \fIoffset\fR and \fIlength\fR arguments. > + > +.SH AUTHOR > +.nf > +Lukas Czerner <lczerner@redhat.com> > +.fi > +.SH SEE ALSO > +.BR fstrim (8) > +.SH AVAILABILITY > +The blkdiscard command is part of the util-linux package and is available > +from ftp://ftp.kernel.org/pub/linux/utils/util-linux/. > diff --git a/sys-utils/blkdiscard.c b/sys-utils/blkdiscard.c > new file mode 100644 > index 0000000..bdcd06e > --- /dev/null > +++ b/sys-utils/blkdiscard.c > @@ -0,0 +1,173 @@ > +/* > + * blkdiscard.c -- discard the part (or whole) of the block device. > + * > + * Copyright (C) 2012 Red Hat, Inc. All rights reserved. > + * Written by Lukas Czerner <lczerner@redhat.com> > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + * > + * This program uses BLKDISCARD ioctl to discard part or the whole block > + * device if the device supports it. You can specify range (start and > + * length) to be discarded, or simply discard the whole device. > + */ > + > + > +#include <string.h> > +#include <unistd.h> > +#include <stdlib.h> > +#include <stdio.h> > +#include <stdint.h> > +#include <fcntl.h> > +#include <limits.h> > +#include <getopt.h> > + > +#include <sys/ioctl.h> > +#include <sys/stat.h> > +#include <linux/fs.h> > + > +#include "nls.h" > +#include "strutils.h" > +#include "c.h" > +#include "closestream.h" > + > +#ifndef BLKDISCARD > +#define BLKDISCARD _IO(0x12,119) > +#endif > + > +#ifndef BLKSECDISCARD > +#define BLKSECDISCARD _IO(0x12,125) > +#endif > + > +static void __attribute__((__noreturn__)) usage(FILE *out) > +{ > + fputs(USAGE_HEADER, out); > + fprintf(out, > + _(" %s [options] <device>\n"), program_invocation_short_name); > + fputs(USAGE_OPTIONS, out); > + fputs(_(" -o, --offset <num> offset in bytes to discard from\n" > + " -l, --length <num> length of bytes to discard from the offset\n" > + " -s, --secure perform secure discard\n" > + " -v, --verbose print aligned length and offset\n"), > + out); > + fputs(USAGE_SEPARATOR, out); > + fputs(USAGE_HELP, out); > + fputs(USAGE_VERSION, out); > + fprintf(out, USAGE_MAN_TAIL("blkdiscard(8)")); > + exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS); > +} > + > +int main(int argc, char **argv) > +{ > + char *path; > + int c, fd, verbose = 0, secure = 0; > + uint64_t end, blksize, secsize, range[2]; > + struct stat sb; > + > + static const struct option longopts[] = { > + { "help", 0, 0, 'h' }, > + { "version", 0, 0, 'V' }, > + { "offset", 1, 0, 'o' }, > + { "length", 1, 0, 'l' }, > + { "secure", 0, 0, 's' }, > + { "verbose", 0, 0, 'v' }, > + { NULL, 0, 0, 0 } > + }; > + > + setlocale(LC_ALL, ""); > + bindtextdomain(PACKAGE, LOCALEDIR); > + textdomain(PACKAGE); > + atexit(close_stdout); > + > + range[0] = 0; > + range[1] = ULLONG_MAX; > + > + while ((c = getopt_long(argc, argv, "hVsvo:l:", longopts, NULL)) != -1) { > + switch(c) { > + case 'h': > + usage(stdout); > + break; > + case 'V': > + printf(UTIL_LINUX_VERSION); > + return EXIT_SUCCESS; > + case 'l': > + range[1] = strtosize_or_err(optarg, > + _("failed to parse length")); > + break; > + case 'o': > + range[0] = strtosize_or_err(optarg, > + _("failed to parse offset")); > + break; > + case 's': > + secure = 1; > + break; > + case 'v': > + verbose = 1; > + break; > + default: > + usage(stderr); > + break; > + } > + } > + > + if (optind == argc) > + errx(EXIT_FAILURE, _("no device specified.")); > + > + path = argv[optind++]; > + > + if (optind != argc) { > + warnx(_("unexpected number of arguments")); > + usage(stderr); > + } > + > + if (stat(path, &sb) == -1) > + err(EXIT_FAILURE, _("stat failed %s"), path); > + if (!S_ISBLK(sb.st_mode)) > + errx(EXIT_FAILURE, _("%s: not a block device"), path); > + > + fd = open(path, O_WRONLY); > + if (fd < 0) > + err(EXIT_FAILURE, _("cannot open %s"), path); > + > + if (ioctl(fd, BLKGETSIZE64, &blksize)) > + err(EXIT_FAILURE, _("%s: BLKGETSIZE64 ioctl failed"), path); > + > + if (ioctl(fd, BLKSSZGET, &secsize)) > + err(EXIT_FAILURE, _("%s: BLKSSZGET ioctl failed"), path); > + > + /* align range to the sector size */ > + range[0] = (range[0] + secsize - 1) & ~(secsize - 1); > + range[1] &= ~(secsize - 1); > + > + /* is the range end behind the end of the device ?*/ > + end = range[0] + range[1]; > + if (end < range[0] || end > blksize) > + range[1] = blksize - range[0]; > + > + if (secure) { > + if (ioctl(fd, BLKSECDISCARD, &range)) > + err(EXIT_FAILURE, _("%s: BLKSECDISCARD ioctl failed"), path); > + } else { > + if (ioctl(fd, BLKDISCARD, &range)) > + err(EXIT_FAILURE, _("%s: BLKDISCARD ioctl failed"), path); > + } > + > + if (verbose) > + /* TRANSLATORS: The standard value here is a very large number. */ > + printf(_("%s: Discarded %" PRIu64 " bytes from the " > + "offset %" PRIu64"\n"), path, > + (uint64_t) range[1], (uint64_t) range[0]); > + > + close(fd); > + return EXIT_SUCCESS; > +} > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-26 19:47 ` Lukáš Czerner @ 2012-09-26 20:04 ` Kay Sievers 2012-09-26 20:14 ` Lukáš Czerner 2012-09-27 9:42 ` Karel Zak 1 sibling, 1 reply; 23+ messages in thread From: Kay Sievers @ 2012-09-26 20:04 UTC (permalink / raw) To: Lukáš Czerner; +Cc: util-linux, kzak On Wed, Sep 26, 2012 at 9:47 PM, Lukáš Czerner <lczerner@redhat.com> wrote: > On Wed, 12 Sep 2012, Lukas Czerner wrote: > >> Date: Wed, 12 Sep 2012 17:49:15 -0400 >> From: Lukas Czerner <lczerner@redhat.com> >> To: util-linux@vger.kernel.org, kzak@redhat.com >> Cc: Lukas Czerner <lczerner@redhat.com> >> Subject: [PATCH] blkdiscard: add new command >> >> blkdiscard is used to discard device sectors. This is useful for >> solid-state drivers (SSDs) and thinly-provisioned storage. Unlike >> fstrim this command is used directly on the block device. >> >> blkkdiscard uses BLKDISCARD ioctl or BLKSECDISCARD ioctl for the secure >> discard. >> >> All data in the discarded region on the device will be lost! > > Hi Karel, > > any progress here ? Where is the patch for blockdev? I'm still convinced randomly named tools per new kernel ioctl is not what we want. People should get their act together, and not add the tool of the week to util-linux. Thanks, Kay ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-26 20:04 ` Kay Sievers @ 2012-09-26 20:14 ` Lukáš Czerner 2012-09-26 20:27 ` Kay Sievers 0 siblings, 1 reply; 23+ messages in thread From: Lukáš Czerner @ 2012-09-26 20:14 UTC (permalink / raw) To: Kay Sievers; +Cc: Lukáš Czerner, util-linux, kzak [-- Attachment #1: Type: TEXT/PLAIN, Size: 1510 bytes --] On Wed, 26 Sep 2012, Kay Sievers wrote: > Date: Wed, 26 Sep 2012 22:04:37 +0200 > From: Kay Sievers <kay@vrfy.org> > To: Lukáš Czerner <lczerner@redhat.com> > Cc: util-linux@vger.kernel.org, kzak@redhat.com > Subject: Re: [PATCH] blkdiscard: add new command > > On Wed, Sep 26, 2012 at 9:47 PM, Lukáš Czerner <lczerner@redhat.com> wrote: > > On Wed, 12 Sep 2012, Lukas Czerner wrote: > > > >> Date: Wed, 12 Sep 2012 17:49:15 -0400 > >> From: Lukas Czerner <lczerner@redhat.com> > >> To: util-linux@vger.kernel.org, kzak@redhat.com > >> Cc: Lukas Czerner <lczerner@redhat.com> > >> Subject: [PATCH] blkdiscard: add new command > >> > >> blkdiscard is used to discard device sectors. This is useful for > >> solid-state drivers (SSDs) and thinly-provisioned storage. Unlike > >> fstrim this command is used directly on the block device. > >> > >> blkkdiscard uses BLKDISCARD ioctl or BLKSECDISCARD ioctl for the secure > >> discard. > >> > >> All data in the discarded region on the device will be lost! > > > > Hi Karel, > > > > any progress here ? > > Where is the patch for blockdev? There is not. > > I'm still convinced randomly named tools per new kernel ioctl is not > what we want. People should get their act together, and not add the > tool of the week to util-linux. And I am still convinced that multiplexing different functionalities together into a single tool is not always the best way to go, especially when they does not have nothing in common. Thanks! -Lukas > > Thanks, > Kay > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-26 20:14 ` Lukáš Czerner @ 2012-09-26 20:27 ` Kay Sievers 2012-09-26 21:01 ` Lukáš Czerner 0 siblings, 1 reply; 23+ messages in thread From: Kay Sievers @ 2012-09-26 20:27 UTC (permalink / raw) To: Lukáš Czerner; +Cc: util-linux, kzak On Wed, Sep 26, 2012 at 10:14 PM, Lukáš Czerner <lczerner@redhat.com> wrote: > On Wed, 26 Sep 2012, Kay Sievers wrote: > >> Date: Wed, 26 Sep 2012 22:04:37 +0200 >> From: Kay Sievers <kay@vrfy.org> >> To: Lukáš Czerner <lczerner@redhat.com> >> Cc: util-linux@vger.kernel.org, kzak@redhat.com >> Subject: Re: [PATCH] blkdiscard: add new command >> >> On Wed, Sep 26, 2012 at 9:47 PM, Lukáš Czerner <lczerner@redhat.com> wrote: >> > On Wed, 12 Sep 2012, Lukas Czerner wrote: >> > >> >> Date: Wed, 12 Sep 2012 17:49:15 -0400 >> >> From: Lukas Czerner <lczerner@redhat.com> >> >> To: util-linux@vger.kernel.org, kzak@redhat.com >> >> Cc: Lukas Czerner <lczerner@redhat.com> >> >> Subject: [PATCH] blkdiscard: add new command >> >> >> >> blkdiscard is used to discard device sectors. This is useful for >> >> solid-state drivers (SSDs) and thinly-provisioned storage. Unlike >> >> fstrim this command is used directly on the block device. >> >> >> >> blkkdiscard uses BLKDISCARD ioctl or BLKSECDISCARD ioctl for the secure >> >> discard. >> >> >> >> All data in the discarded region on the device will be lost! >> > >> > Hi Karel, >> > >> > any progress here ? >> >> Where is the patch for blockdev? > > There is not. > >> I'm still convinced randomly named tools per new kernel ioctl is not >> what we want. People should get their act together, and not add the >> tool of the week to util-linux. > > And I am still convinced that multiplexing different functionalities > together into a single tool is not always the best way to go, > especially when they does not have nothing in common. They have in common that they operate directly on the raw kernel block device. This: blockdev <verb> [options] <devices> sounds certainly better than than one tool per every new command. And it's not only the needlessly duplicated binary, it's primarily consistency, but also maintenance, documentation, ... which is all needlessly exploded here. It's not so much about multiplexing, it's about avoiding needless duplication for rather exotic functions applying to one and the same domain, a block device, which already has a command. util-linux is mandatory in every installation, and it is already one of the most chaotic and weird collections of random things. We should certainly be more careful here when coming up with new stuff than people have been in the past. Kay ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-26 20:27 ` Kay Sievers @ 2012-09-26 21:01 ` Lukáš Czerner 0 siblings, 0 replies; 23+ messages in thread From: Lukáš Czerner @ 2012-09-26 21:01 UTC (permalink / raw) To: Kay Sievers; +Cc: Lukáš Czerner, util-linux, kzak [-- Attachment #1: Type: TEXT/PLAIN, Size: 3529 bytes --] On Wed, 26 Sep 2012, Kay Sievers wrote: > Date: Wed, 26 Sep 2012 22:27:49 +0200 > From: Kay Sievers <kay@vrfy.org> > To: Lukáš Czerner <lczerner@redhat.com> > Cc: util-linux@vger.kernel.org, kzak@redhat.com > Subject: Re: [PATCH] blkdiscard: add new command > > On Wed, Sep 26, 2012 at 10:14 PM, Lukáš Czerner <lczerner@redhat.com> wrote: > > On Wed, 26 Sep 2012, Kay Sievers wrote: > > > >> Date: Wed, 26 Sep 2012 22:04:37 +0200 > >> From: Kay Sievers <kay@vrfy.org> > >> To: Lukáš Czerner <lczerner@redhat.com> > >> Cc: util-linux@vger.kernel.org, kzak@redhat.com > >> Subject: Re: [PATCH] blkdiscard: add new command > >> > >> On Wed, Sep 26, 2012 at 9:47 PM, Lukáš Czerner <lczerner@redhat.com> wrote: > >> > On Wed, 12 Sep 2012, Lukas Czerner wrote: > >> > > >> >> Date: Wed, 12 Sep 2012 17:49:15 -0400 > >> >> From: Lukas Czerner <lczerner@redhat.com> > >> >> To: util-linux@vger.kernel.org, kzak@redhat.com > >> >> Cc: Lukas Czerner <lczerner@redhat.com> > >> >> Subject: [PATCH] blkdiscard: add new command > >> >> > >> >> blkdiscard is used to discard device sectors. This is useful for > >> >> solid-state drivers (SSDs) and thinly-provisioned storage. Unlike > >> >> fstrim this command is used directly on the block device. > >> >> > >> >> blkkdiscard uses BLKDISCARD ioctl or BLKSECDISCARD ioctl for the secure > >> >> discard. > >> >> > >> >> All data in the discarded region on the device will be lost! > >> > > >> > Hi Karel, > >> > > >> > any progress here ? > >> > >> Where is the patch for blockdev? > > > > There is not. > > > >> I'm still convinced randomly named tools per new kernel ioctl is not > >> what we want. People should get their act together, and not add the > >> tool of the week to util-linux. > > > > And I am still convinced that multiplexing different functionalities > > together into a single tool is not always the best way to go, > > especially when they does not have nothing in common. > > They have in common that they operate directly on the raw kernel block device. And that's where the similarity ends. The BLKDISCARD ioctl is sufficiently different from the others ioctls and also has different syntax. Unlike the blockdev commands it also changes the content of the raw device and it can even take significant time to finish. Actually it might be even worth adding a warning and ask user to proceed if not "forced" as this operation will destroy data on the device. That said I think that this is different enough to have it's own binary and manual page. > > This: > blockdev <verb> [options] <devices> > sounds certainly better than than one tool per every new command. Sure, except we're not creating new tool per every ioctl, do we. > > And it's not only the needlessly duplicated binary, it's primarily > consistency, but also maintenance, documentation, ... which is all > needlessly exploded here. > It's not so much about multiplexing, it's about avoiding needless > duplication for rather exotic functions applying to one and the same > domain, a block device, which already has a command. Again, this is hardly exotic function. And for the sake of documentation this should have its own manual page instead of being hidden in the bunch of one-line descriptions as it is in blockdev. -Lukas > > util-linux is mandatory in every installation, and it is already one > of the most chaotic and weird collections of random things. We should > certainly be more careful here when coming up with new stuff than > people have been in the past. > > Kay > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-26 19:47 ` Lukáš Czerner 2012-09-26 20:04 ` Kay Sievers @ 2012-09-27 9:42 ` Karel Zak 2012-09-27 15:21 ` Lukáš Czerner 1 sibling, 1 reply; 23+ messages in thread From: Karel Zak @ 2012-09-27 9:42 UTC (permalink / raw) To: Lukáš Czerner; +Cc: util-linux On Wed, Sep 26, 2012 at 03:47:06PM -0400, Lukáš Czerner wrote: > any progress here ? ah, it seems that the bikeshed already looks like rainbow... :-) Anyway, we already have fstrim(8). This command is the same like blkdiscard(8) but for filesystems. It also expects offset and length, it also calls an ioctl. I think the best would be to merge both tools and maintain the same thing on only one place. The blkdiscard(8) command maybe only a symlink to fstrim(8). It's also less invasive change: 4 files changed, 195 insertions(+), 36 deletions(-) (including man page) See below. Karel >From d091935f5d269fc789d5ac1ce86b1bb062d1f2ce Mon Sep 17 00:00:00 2001 From: Karel Zak <kzak@redhat.com> Date: Thu, 27 Sep 2012 11:21:35 +0200 Subject: [PATCH] blkdiscard: new command Add to fstrim(8) code to support new discard BLKDISCARD and BLKSECDISCARD ioctls for block devices. The new command is only symlink to fstrim(8) as the both utils share some code and the basic ideas. Based on patch from Lukas Czerner <lczerner@redhat.com>. Signed-off-by: Karel Zak <kzak@redhat.com> --- .gitignore | 1 + sys-utils/Makemodule.am | 10 +++- sys-utils/blkdiscard.8 | 66 ++++++++++++++++++++ sys-utils/fstrim.c | 154 ++++++++++++++++++++++++++++++++++++----------- 4 files changed, 195 insertions(+), 36 deletions(-) create mode 100644 sys-utils/blkdiscard.8 diff --git a/.gitignore b/.gitignore index 5be008f..5ef1c68 100644 --- a/.gitignore +++ b/.gitignore @@ -65,6 +65,7 @@ tests/run.sh.trs /addpart /agetty /arch +/blkdiscard /blkid /blockdev /cal diff --git a/sys-utils/Makemodule.am b/sys-utils/Makemodule.am index d376f04..08226cf 100644 --- a/sys-utils/Makemodule.am +++ b/sys-utils/Makemodule.am @@ -51,9 +51,17 @@ fsfreeze_SOURCES = sys-utils/fsfreeze.c sbin_PROGRAMS += fstrim dist_man_MANS += sys-utils/fstrim.8 -fstrim_SOURCES = sys-utils/fstrim.c +fstrim_SOURCES = sys-utils/fstrim.c lib/blkdev.c fstrim_LDADD = $(LDADD) libcommon.la +install-exec-hook-blkdiscard: + cd $(DESTDIR)$(sbindir) && ln -sf fstrim blkdiscard +uninstall-hook-blkdiscard: + rm -f $(DESTDIR)$(sbindir)/blkdiscard + +INSTALL_EXEC_HOOKS += install-exec-hook-blkdiscard +UNINSTALL_HOOKS += uninstall-hook-blkdiscard + usrbin_exec_PROGRAMS += cytune dist_man_MANS += sys-utils/cytune.8 cytune_SOURCES = sys-utils/cytune.c sys-utils/cyclades.h diff --git a/sys-utils/blkdiscard.8 b/sys-utils/blkdiscard.8 new file mode 100644 index 0000000..fcc38f6 --- /dev/null +++ b/sys-utils/blkdiscard.8 @@ -0,0 +1,66 @@ +.\" -*- nroff -*- +.TH BLKDISCARD 8 "September 2012" "util-linux" "System Administration" +.SH NAME +blkdiscard \- discard sectors on a device +.SH SYNOPSIS +.B blkdiscard +.RB [ \-o +.IR offset ] +.RB [ \-l +.IR length ] +.RB [ \-s ] +.RB [ \-v ] +.I device + +.SH DESCRIPTION +.B blkdiscard +is used to discard device sectors. This is useful for solid-state +drivers (SSDs) and thinly-provisioned storage. Unlike +.BR fstrim (8) +this command is used directly on the block device. +.PP +By default, +.B blkdiscard +will discard all blocks on the device. Options may be used to +modify this behavior based on range or size, as explained below. +.PP +The +.I device +argument is the pathname of the block device. + +.B WARNING: All data in the discarded region on the device will be lost! + +.SH OPTIONS +The \fIoffset\fR and \fIlength\fR arguments may be +followed by the multiplicative suffixes KiB=1024, MiB=1024*1024, and so on for +GiB, TiB, PiB, EiB, ZiB and YiB (the "iB" is optional, e.g. "K" has the same +meaning as "KiB") or the suffixes KB=1000, MB=1000*1000, and so on for GB, PB, +EB, ZB and YB. +.IP "\fB\-h, \-\-help\fP" +Print help and exit. +.IP "\fB\-o, \-\-offset\fP \fIoffset\fP" +Byte offset in the device from which to discard. Provided value will be +aligned to the device sector size. Default value is zero. +.IP "\fB\-l, \-\-length\fP \fIlength\fP" +Number of bytes after starting point to discard. Provided value will be +aligned to the device sector size. If the specified value extends past the +end of the device, +.B blkdiscard +will stop at the device size boundary. Default value extends to the end +of the device. +.IP "\fB\-s, \-\-secure\fP" +Perform secure discard. Secure discard is the same as regular discard except +all copies of the discarded blocks possibly created by garbage collection must +also be erased. It has to be supported by the device. +.IP "\fB\-v, \-\-verbose\fP" +Print aligned \fIoffset\fR and \fIlength\fR arguments. + +.SH AUTHOR +.nf +Lukas Czerner <lczerner@redhat.com> +.fi +.SH SEE ALSO +.BR fstrim (8) +.SH AVAILABILITY +The blkdiscard command is part of the util-linux package and is available +from ftp://ftp.kernel.org/pub/linux/utils/util-linux/. diff --git a/sys-utils/fstrim.c b/sys-utils/fstrim.c index 332601d..cfe2770 100644 --- a/sys-utils/fstrim.c +++ b/sys-utils/fstrim.c @@ -1,7 +1,7 @@ /* * fstrim.c -- discard the part (or whole) of mounted filesystem. * - * Copyright (C) 2010 Red Hat, Inc. All rights reserved. + * Copyright (C) 2010,2012 Red Hat, Inc. All rights reserved. * Written by Lukas Czerner <lczerner@redhat.com> * Karel Zak <kzak@redhat.com> * @@ -23,7 +23,6 @@ * online (mounted). You can specify range (start and length) to be * discarded, or simply discard whole filesystem. */ - #include <string.h> #include <unistd.h> #include <stdlib.h> @@ -41,6 +40,7 @@ #include "strutils.h" #include "c.h" #include "closestream.h" +#include "blkdev.h" #ifndef FITRIM struct fstrim_range { @@ -48,34 +48,107 @@ struct fstrim_range { uint64_t len; uint64_t minlen; }; -#define FITRIM _IOWR('X', 121, struct fstrim_range) +# define FITRIM _IOWR('X', 121, struct fstrim_range) +#endif + +#ifndef BLKDISCARD +# define BLKDISCARD _IO(0x12,119) +#endif +#ifndef BLKSECDISCARD +# define BLKSECDISCARD _IO(0x12,125) #endif +static int is_blk = 0; + static void __attribute__((__noreturn__)) usage(FILE *out) { fputs(USAGE_HEADER, out); - fprintf(out, - _(" %s [options] <mount point>\n"), program_invocation_short_name); - fputs(USAGE_OPTIONS, out); + + if (is_blk) { + fprintf(out, _(" %s [options] <device>\n"), program_invocation_short_name); + fputs(USAGE_OPTIONS, out); + fputs(_(" -s, --secure perform secure discard\n"), out); + } else { + fprintf(out, _(" %s [options] <mountpoint>\n"), program_invocation_short_name); + fputs(USAGE_OPTIONS, out); + fputs(_(" -m, --minimum <num> minimum extent length to discard\n"), out); + } fputs(_(" -o, --offset <num> offset in bytes to discard from\n" " -l, --length <num> length of bytes to discard from the offset\n" - " -m, --minimum <num> minimum extent length to discard\n" " -v, --verbose print number of discarded bytes\n"), out); fputs(USAGE_SEPARATOR, out); fputs(USAGE_HELP, out); fputs(USAGE_VERSION, out); - fprintf(out, USAGE_MAN_TAIL("fstrim(8)")); + fprintf(out, USAGE_MAN_TAIL(is_blk ? "blkdiscard(8)" : "fstrim(8)")); exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS); } -int main(int argc, char **argv) +static void do_fstrim(const char *path, uint64_t off, uint64_t len, uint64_t minlen) { - char *path; - int c, fd, verbose = 0; + int fd; + struct stat sb; struct fstrim_range range; + + range.start = off; + range.len = len; + range.minlen = minlen; + + fd = open(path, O_RDONLY); + if (fd < 0) + err(EXIT_FAILURE, _("cannot open %s"), path); + if (fstat(fd, &sb) == -1) + err(EXIT_FAILURE, _("stat failed %s"), path); + if (!S_ISDIR(sb.st_mode)) + errx(EXIT_FAILURE, _("%s: not a directory"), path); + if (ioctl(fd, FITRIM, &range)) + err(EXIT_FAILURE, _("%s: FITRIM ioctl failed"), path); + close(fd); +} + +static void do_blkdiscard(const char *path, uint64_t off, uint64_t len, int sec) +{ + int fd, secsize; struct stat sb; + uint64_t blksize, range[2], end; - static const struct option longopts[] = { + fd = open(path, O_WRONLY); + if (fd < 0) + err(EXIT_FAILURE, _("cannot open %s"), path); + if (fstat(fd, &sb) == -1) + err(EXIT_FAILURE, _("stat failed %s"), path); + if (!S_ISBLK(sb.st_mode)) + errx(EXIT_FAILURE, _("%s: not a block device"), path); + if (blkdev_get_size(fd, (unsigned long long *) &blksize) != 0) + err(EXIT_FAILURE, _("%s: failed to get device size"), path); + if (blkdev_get_sector_size(fd, &secsize) != 0) + err(EXIT_FAILURE, _("%s: failed to get sector size"), path); + + /* align range to the sector size */ + range[0] = (off + secsize - 1) & ~(secsize - 1); + range[1] = len & ~(secsize - 1); + + /* is the range end behind the end of the device ?*/ + end = range[0] + range[1]; + if (end < range[0] || end > blksize) + range[1] = blksize - range[0]; + + if (sec) { + if (ioctl(fd, BLKSECDISCARD, &range)) + err(EXIT_FAILURE, _("%s: BLKSECDISCARD ioctl failed"), path); + } else { + if (ioctl(fd, BLKDISCARD, &range)) + err(EXIT_FAILURE, _("%s: BLKDISCARD ioctl failed"), path); + } + close(fd); +} + +int main(int argc, char **argv) +{ + char *path; + int c, verbose = 0, secure = 0; + uint64_t len = UINT64_MAX, off = 0, minlen = 0; + + static const struct option fs_longopts[] = { { "help", 0, 0, 'h' }, { "version", 0, 0, 'V' }, { "offset", 1, 0, 'o' }, @@ -84,16 +157,32 @@ int main(int argc, char **argv) { "verbose", 0, 0, 'v' }, { NULL, 0, 0, 0 } }; + static const struct option blk_longopts[] = { + { "help", 0, 0, 'h' }, + { "version", 0, 0, 'V' }, + { "offset", 1, 0, 'o' }, + { "length", 1, 0, 'l' }, + { "secure", 0, 0, 's' }, + { "verbose", 0, 0, 'v' }, + { NULL, 0, 0, 0 } + }; setlocale(LC_ALL, ""); bindtextdomain(PACKAGE, LOCALEDIR); textdomain(PACKAGE); atexit(close_stdout); - memset(&range, 0, sizeof(range)); - range.len = ULLONG_MAX; + if (strcmp(program_invocation_short_name, "blkdiscard") == 0) + is_blk = 1; + + do { + if (is_blk) + c = getopt_long(argc, argv, "hVo:l:sv", blk_longopts, NULL); + else + c = getopt_long(argc, argv, "hVo:l:m:v", fs_longopts, NULL); + if (c == -1) + break; - while ((c = getopt_long(argc, argv, "hVo:l:m:v", longopts, NULL)) != -1) { switch(c) { case 'h': usage(stdout); @@ -102,17 +191,20 @@ int main(int argc, char **argv) printf(UTIL_LINUX_VERSION); return EXIT_SUCCESS; case 'l': - range.len = strtosize_or_err(optarg, + len = strtosize_or_err(optarg, _("failed to parse length")); break; case 'o': - range.start = strtosize_or_err(optarg, + off = strtosize_or_err(optarg, _("failed to parse offset")); break; case 'm': - range.minlen = strtosize_or_err(optarg, + minlen = strtosize_or_err(optarg, _("failed to parse minimum extent length")); break; + case 's': + secure = 1; + break; case 'v': verbose = 1; break; @@ -120,11 +212,11 @@ int main(int argc, char **argv) usage(stderr); break; } - } + } while (1); if (optind == argc) - errx(EXIT_FAILURE, _("no mountpoint specified.")); - + errx(EXIT_FAILURE, is_blk ? _("no device specified.") : + _("no mountpoint specified")); path = argv[optind++]; if (optind != argc) { @@ -132,22 +224,14 @@ int main(int argc, char **argv) usage(stderr); } - if (stat(path, &sb) == -1) - err(EXIT_FAILURE, _("stat failed %s"), path); - if (!S_ISDIR(sb.st_mode)) - errx(EXIT_FAILURE, _("%s: not a directory"), path); - - fd = open(path, O_RDONLY); - if (fd < 0) - err(EXIT_FAILURE, _("cannot open %s"), path); - - if (ioctl(fd, FITRIM, &range)) - err(EXIT_FAILURE, _("%s: FITRIM ioctl failed"), path); + if (is_blk) + do_blkdiscard(path, off, len, secure); + else + do_fstrim(path, off, len, minlen); if (verbose) /* TRANSLATORS: The standard value here is a very large number. */ - printf(_("%s: %" PRIu64 " bytes were trimmed\n"), - path, (uint64_t) range.len); - close(fd); + printf(_("%s: %" PRIu64 " bytes were trimmed\n"), path, len); + return EXIT_SUCCESS; } -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-27 9:42 ` Karel Zak @ 2012-09-27 15:21 ` Lukáš Czerner 2012-09-27 15:49 ` Martin K. Petersen 0 siblings, 1 reply; 23+ messages in thread From: Lukáš Czerner @ 2012-09-27 15:21 UTC (permalink / raw) To: Karel Zak; +Cc: Lukáš Czerner, util-linux [-- Attachment #1: Type: TEXT/PLAIN, Size: 13721 bytes --] On Thu, 27 Sep 2012, Karel Zak wrote: > Date: Thu, 27 Sep 2012 11:42:56 +0200 > From: Karel Zak <kzak@redhat.com> > To: Lukáš Czerner <lczerner@redhat.com> > Cc: util-linux@vger.kernel.org > Subject: Re: [PATCH] blkdiscard: add new command > > On Wed, Sep 26, 2012 at 03:47:06PM -0400, Lukáš Czerner wrote: > > any progress here ? > > ah, it seems that the bikeshed already looks like rainbow... :-) > > Anyway, we already have fstrim(8). This command is the same like > blkdiscard(8) but for filesystems. It also expects offset and length, > it also calls an ioctl. > > I think the best would be to merge both tools and maintain the same > thing on only one place. The blkdiscard(8) command maybe only a > symlink to fstrim(8). > > It's also less invasive change: > 4 files changed, 195 insertions(+), 36 deletions(-) > (including man page) > > See below. Hi Karel, TBH this is fugly :). Also what actually is the advantage of doing this ? So we saved 50 lines of code for this ugliness and instead of two separate binaries we have this one hybrid and symlink. I am not sure it's worth it. Can't we just have two separate binaries ? What is the problem with that ? Nevertheless it you're willing to carry this weird thing I do not really care very much as long as the blkdiscard is separate thing and not multiplexed with some other unrelated stuff. Also we probably need to add a warning and question. Something like. This will destroy all data in specified range. Do you want to proceed ? and add --force/-f argument. Thanks! -Lukas > > Karel > > From d091935f5d269fc789d5ac1ce86b1bb062d1f2ce Mon Sep 17 00:00:00 2001 > From: Karel Zak <kzak@redhat.com> > Date: Thu, 27 Sep 2012 11:21:35 +0200 > Subject: [PATCH] blkdiscard: new command > > Add to fstrim(8) code to support new discard BLKDISCARD and > BLKSECDISCARD ioctls for block devices. The new command is only > symlink to fstrim(8) as the both utils share some code and the basic > ideas. > > Based on patch from Lukas Czerner <lczerner@redhat.com>. > > Signed-off-by: Karel Zak <kzak@redhat.com> > --- > .gitignore | 1 + > sys-utils/Makemodule.am | 10 +++- > sys-utils/blkdiscard.8 | 66 ++++++++++++++++++++ > sys-utils/fstrim.c | 154 ++++++++++++++++++++++++++++++++++++----------- > 4 files changed, 195 insertions(+), 36 deletions(-) > create mode 100644 sys-utils/blkdiscard.8 > > diff --git a/.gitignore b/.gitignore > index 5be008f..5ef1c68 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -65,6 +65,7 @@ tests/run.sh.trs > /addpart > /agetty > /arch > +/blkdiscard > /blkid > /blockdev > /cal > diff --git a/sys-utils/Makemodule.am b/sys-utils/Makemodule.am > index d376f04..08226cf 100644 > --- a/sys-utils/Makemodule.am > +++ b/sys-utils/Makemodule.am > @@ -51,9 +51,17 @@ fsfreeze_SOURCES = sys-utils/fsfreeze.c > > sbin_PROGRAMS += fstrim > dist_man_MANS += sys-utils/fstrim.8 > -fstrim_SOURCES = sys-utils/fstrim.c > +fstrim_SOURCES = sys-utils/fstrim.c lib/blkdev.c > fstrim_LDADD = $(LDADD) libcommon.la > > +install-exec-hook-blkdiscard: > + cd $(DESTDIR)$(sbindir) && ln -sf fstrim blkdiscard > +uninstall-hook-blkdiscard: > + rm -f $(DESTDIR)$(sbindir)/blkdiscard > + > +INSTALL_EXEC_HOOKS += install-exec-hook-blkdiscard > +UNINSTALL_HOOKS += uninstall-hook-blkdiscard > + > usrbin_exec_PROGRAMS += cytune > dist_man_MANS += sys-utils/cytune.8 > cytune_SOURCES = sys-utils/cytune.c sys-utils/cyclades.h > diff --git a/sys-utils/blkdiscard.8 b/sys-utils/blkdiscard.8 > new file mode 100644 > index 0000000..fcc38f6 > --- /dev/null > +++ b/sys-utils/blkdiscard.8 > @@ -0,0 +1,66 @@ > +.\" -*- nroff -*- > +.TH BLKDISCARD 8 "September 2012" "util-linux" "System Administration" > +.SH NAME > +blkdiscard \- discard sectors on a device > +.SH SYNOPSIS > +.B blkdiscard > +.RB [ \-o > +.IR offset ] > +.RB [ \-l > +.IR length ] > +.RB [ \-s ] > +.RB [ \-v ] > +.I device > + > +.SH DESCRIPTION > +.B blkdiscard > +is used to discard device sectors. This is useful for solid-state > +drivers (SSDs) and thinly-provisioned storage. Unlike > +.BR fstrim (8) > +this command is used directly on the block device. > +.PP > +By default, > +.B blkdiscard > +will discard all blocks on the device. Options may be used to > +modify this behavior based on range or size, as explained below. > +.PP > +The > +.I device > +argument is the pathname of the block device. > + > +.B WARNING: All data in the discarded region on the device will be lost! > + > +.SH OPTIONS > +The \fIoffset\fR and \fIlength\fR arguments may be > +followed by the multiplicative suffixes KiB=1024, MiB=1024*1024, and so on for > +GiB, TiB, PiB, EiB, ZiB and YiB (the "iB" is optional, e.g. "K" has the same > +meaning as "KiB") or the suffixes KB=1000, MB=1000*1000, and so on for GB, PB, > +EB, ZB and YB. > +.IP "\fB\-h, \-\-help\fP" > +Print help and exit. > +.IP "\fB\-o, \-\-offset\fP \fIoffset\fP" > +Byte offset in the device from which to discard. Provided value will be > +aligned to the device sector size. Default value is zero. > +.IP "\fB\-l, \-\-length\fP \fIlength\fP" > +Number of bytes after starting point to discard. Provided value will be > +aligned to the device sector size. If the specified value extends past the > +end of the device, > +.B blkdiscard > +will stop at the device size boundary. Default value extends to the end > +of the device. > +.IP "\fB\-s, \-\-secure\fP" > +Perform secure discard. Secure discard is the same as regular discard except > +all copies of the discarded blocks possibly created by garbage collection must > +also be erased. It has to be supported by the device. > +.IP "\fB\-v, \-\-verbose\fP" > +Print aligned \fIoffset\fR and \fIlength\fR arguments. > + > +.SH AUTHOR > +.nf > +Lukas Czerner <lczerner@redhat.com> > +.fi > +.SH SEE ALSO > +.BR fstrim (8) > +.SH AVAILABILITY > +The blkdiscard command is part of the util-linux package and is available > +from ftp://ftp.kernel.org/pub/linux/utils/util-linux/. > diff --git a/sys-utils/fstrim.c b/sys-utils/fstrim.c > index 332601d..cfe2770 100644 > --- a/sys-utils/fstrim.c > +++ b/sys-utils/fstrim.c > @@ -1,7 +1,7 @@ > /* > * fstrim.c -- discard the part (or whole) of mounted filesystem. > * > - * Copyright (C) 2010 Red Hat, Inc. All rights reserved. > + * Copyright (C) 2010,2012 Red Hat, Inc. All rights reserved. > * Written by Lukas Czerner <lczerner@redhat.com> > * Karel Zak <kzak@redhat.com> > * > @@ -23,7 +23,6 @@ > * online (mounted). You can specify range (start and length) to be > * discarded, or simply discard whole filesystem. > */ > - > #include <string.h> > #include <unistd.h> > #include <stdlib.h> > @@ -41,6 +40,7 @@ > #include "strutils.h" > #include "c.h" > #include "closestream.h" > +#include "blkdev.h" > > #ifndef FITRIM > struct fstrim_range { > @@ -48,34 +48,107 @@ struct fstrim_range { > uint64_t len; > uint64_t minlen; > }; > -#define FITRIM _IOWR('X', 121, struct fstrim_range) > +# define FITRIM _IOWR('X', 121, struct fstrim_range) > +#endif > + > +#ifndef BLKDISCARD > +# define BLKDISCARD _IO(0x12,119) > +#endif > +#ifndef BLKSECDISCARD > +# define BLKSECDISCARD _IO(0x12,125) > #endif > > +static int is_blk = 0; > + > static void __attribute__((__noreturn__)) usage(FILE *out) > { > fputs(USAGE_HEADER, out); > - fprintf(out, > - _(" %s [options] <mount point>\n"), program_invocation_short_name); > - fputs(USAGE_OPTIONS, out); > + > + if (is_blk) { > + fprintf(out, _(" %s [options] <device>\n"), program_invocation_short_name); > + fputs(USAGE_OPTIONS, out); > + fputs(_(" -s, --secure perform secure discard\n"), out); > + } else { > + fprintf(out, _(" %s [options] <mountpoint>\n"), program_invocation_short_name); > + fputs(USAGE_OPTIONS, out); > + fputs(_(" -m, --minimum <num> minimum extent length to discard\n"), out); > + } > fputs(_(" -o, --offset <num> offset in bytes to discard from\n" > " -l, --length <num> length of bytes to discard from the offset\n" > - " -m, --minimum <num> minimum extent length to discard\n" > " -v, --verbose print number of discarded bytes\n"), out); > fputs(USAGE_SEPARATOR, out); > fputs(USAGE_HELP, out); > fputs(USAGE_VERSION, out); > - fprintf(out, USAGE_MAN_TAIL("fstrim(8)")); > + fprintf(out, USAGE_MAN_TAIL(is_blk ? "blkdiscard(8)" : "fstrim(8)")); > exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS); > } > > -int main(int argc, char **argv) > +static void do_fstrim(const char *path, uint64_t off, uint64_t len, uint64_t minlen) > { > - char *path; > - int c, fd, verbose = 0; > + int fd; > + struct stat sb; > struct fstrim_range range; > + > + range.start = off; > + range.len = len; > + range.minlen = minlen; > + > + fd = open(path, O_RDONLY); > + if (fd < 0) > + err(EXIT_FAILURE, _("cannot open %s"), path); > + if (fstat(fd, &sb) == -1) > + err(EXIT_FAILURE, _("stat failed %s"), path); > + if (!S_ISDIR(sb.st_mode)) > + errx(EXIT_FAILURE, _("%s: not a directory"), path); > + if (ioctl(fd, FITRIM, &range)) > + err(EXIT_FAILURE, _("%s: FITRIM ioctl failed"), path); > + close(fd); > +} > + > +static void do_blkdiscard(const char *path, uint64_t off, uint64_t len, int sec) > +{ > + int fd, secsize; > struct stat sb; > + uint64_t blksize, range[2], end; > > - static const struct option longopts[] = { > + fd = open(path, O_WRONLY); > + if (fd < 0) > + err(EXIT_FAILURE, _("cannot open %s"), path); > + if (fstat(fd, &sb) == -1) > + err(EXIT_FAILURE, _("stat failed %s"), path); > + if (!S_ISBLK(sb.st_mode)) > + errx(EXIT_FAILURE, _("%s: not a block device"), path); > + if (blkdev_get_size(fd, (unsigned long long *) &blksize) != 0) > + err(EXIT_FAILURE, _("%s: failed to get device size"), path); > + if (blkdev_get_sector_size(fd, &secsize) != 0) > + err(EXIT_FAILURE, _("%s: failed to get sector size"), path); > + > + /* align range to the sector size */ > + range[0] = (off + secsize - 1) & ~(secsize - 1); > + range[1] = len & ~(secsize - 1); > + > + /* is the range end behind the end of the device ?*/ > + end = range[0] + range[1]; > + if (end < range[0] || end > blksize) > + range[1] = blksize - range[0]; > + > + if (sec) { > + if (ioctl(fd, BLKSECDISCARD, &range)) > + err(EXIT_FAILURE, _("%s: BLKSECDISCARD ioctl failed"), path); > + } else { > + if (ioctl(fd, BLKDISCARD, &range)) > + err(EXIT_FAILURE, _("%s: BLKDISCARD ioctl failed"), path); > + } > + close(fd); > +} > + > +int main(int argc, char **argv) > +{ > + char *path; > + int c, verbose = 0, secure = 0; > + uint64_t len = UINT64_MAX, off = 0, minlen = 0; > + > + static const struct option fs_longopts[] = { > { "help", 0, 0, 'h' }, > { "version", 0, 0, 'V' }, > { "offset", 1, 0, 'o' }, > @@ -84,16 +157,32 @@ int main(int argc, char **argv) > { "verbose", 0, 0, 'v' }, > { NULL, 0, 0, 0 } > }; > + static const struct option blk_longopts[] = { > + { "help", 0, 0, 'h' }, > + { "version", 0, 0, 'V' }, > + { "offset", 1, 0, 'o' }, > + { "length", 1, 0, 'l' }, > + { "secure", 0, 0, 's' }, > + { "verbose", 0, 0, 'v' }, > + { NULL, 0, 0, 0 } > + }; > > setlocale(LC_ALL, ""); > bindtextdomain(PACKAGE, LOCALEDIR); > textdomain(PACKAGE); > atexit(close_stdout); > > - memset(&range, 0, sizeof(range)); > - range.len = ULLONG_MAX; > + if (strcmp(program_invocation_short_name, "blkdiscard") == 0) > + is_blk = 1; > + > + do { > + if (is_blk) > + c = getopt_long(argc, argv, "hVo:l:sv", blk_longopts, NULL); > + else > + c = getopt_long(argc, argv, "hVo:l:m:v", fs_longopts, NULL); > + if (c == -1) > + break; > > - while ((c = getopt_long(argc, argv, "hVo:l:m:v", longopts, NULL)) != -1) { > switch(c) { > case 'h': > usage(stdout); > @@ -102,17 +191,20 @@ int main(int argc, char **argv) > printf(UTIL_LINUX_VERSION); > return EXIT_SUCCESS; > case 'l': > - range.len = strtosize_or_err(optarg, > + len = strtosize_or_err(optarg, > _("failed to parse length")); > break; > case 'o': > - range.start = strtosize_or_err(optarg, > + off = strtosize_or_err(optarg, > _("failed to parse offset")); > break; > case 'm': > - range.minlen = strtosize_or_err(optarg, > + minlen = strtosize_or_err(optarg, > _("failed to parse minimum extent length")); > break; > + case 's': > + secure = 1; > + break; > case 'v': > verbose = 1; > break; > @@ -120,11 +212,11 @@ int main(int argc, char **argv) > usage(stderr); > break; > } > - } > + } while (1); > > if (optind == argc) > - errx(EXIT_FAILURE, _("no mountpoint specified.")); > - > + errx(EXIT_FAILURE, is_blk ? _("no device specified.") : > + _("no mountpoint specified")); > path = argv[optind++]; > > if (optind != argc) { > @@ -132,22 +224,14 @@ int main(int argc, char **argv) > usage(stderr); > } > > - if (stat(path, &sb) == -1) > - err(EXIT_FAILURE, _("stat failed %s"), path); > - if (!S_ISDIR(sb.st_mode)) > - errx(EXIT_FAILURE, _("%s: not a directory"), path); > - > - fd = open(path, O_RDONLY); > - if (fd < 0) > - err(EXIT_FAILURE, _("cannot open %s"), path); > - > - if (ioctl(fd, FITRIM, &range)) > - err(EXIT_FAILURE, _("%s: FITRIM ioctl failed"), path); > + if (is_blk) > + do_blkdiscard(path, off, len, secure); > + else > + do_fstrim(path, off, len, minlen); > > if (verbose) > /* TRANSLATORS: The standard value here is a very large number. */ > - printf(_("%s: %" PRIu64 " bytes were trimmed\n"), > - path, (uint64_t) range.len); > - close(fd); > + printf(_("%s: %" PRIu64 " bytes were trimmed\n"), path, len); > + > return EXIT_SUCCESS; > } > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-27 15:21 ` Lukáš Czerner @ 2012-09-27 15:49 ` Martin K. Petersen 2012-09-27 17:03 ` Lukáš Czerner 2012-09-27 23:17 ` Karel Zak 0 siblings, 2 replies; 23+ messages in thread From: Martin K. Petersen @ 2012-09-27 15:49 UTC (permalink / raw) To: Lukáš Czerner; +Cc: Karel Zak, util-linux >>>>> "Luk=C3=A1=C5=A1" =3D=3D Luk=C3=A1=C5=A1 Czerner <lczerner@redhat.com= > writes: Luk=C3=A1=C5=A1> TBH this is fugly :). Also what actually is the advantage = of Luk=C3=A1=C5=A1> doing this ? So we saved 50 lines of code for this uglines= s and Luk=C3=A1=C5=A1> instead of two separate binaries we have this one hybrid a= nd Luk=C3=A1=C5=A1> symlink. I am not sure it's worth it. Can't we just have t= wo Luk=C3=A1=C5=A1> separate binaries ? What is the problem with that ? It's not just discard. We should also consider the zeroout and write same use cases. They have nothing to do with fstrim. Time to incorporate and extend Garzik's blktool, perhaps? --=20 Martin K. Petersen Oracle Linux Engineering ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command @ 2012-09-27 17:03 ` Lukáš Czerner 0 siblings, 0 replies; 23+ messages in thread From: Lukáš Czerner @ 2012-09-27 17:03 UTC (permalink / raw) To: Martin K. Petersen Cc: Lukáš Czerner, Karel Zak, util-linux, linux-fsdevel [-- Attachment #1: Type: TEXT/PLAIN, Size: 1559 bytes --] On Thu, 27 Sep 2012, Martin K. Petersen wrote: > Date: Thu, 27 Sep 2012 11:49:22 -0400 > From: Martin K. Petersen <martin.petersen@oracle.com> > To: Lukáš Czerner <lczerner@redhat.com> > Cc: Karel Zak <kzak@redhat.com>, util-linux@vger.kernel.org > Subject: Re: [PATCH] blkdiscard: add new command > > >>>>> "Lukáš" == Lukáš Czerner <lczerner@redhat.com> writes: > > Lukáš> TBH this is fugly :). Also what actually is the advantage of > Lukáš> doing this ? So we saved 50 lines of code for this ugliness and > Lukáš> instead of two separate binaries we have this one hybrid and > Lukáš> symlink. I am not sure it's worth it. Can't we just have two > Lukáš> separate binaries ? What is the problem with that ? > > It's not just discard. We should also consider the zeroout and write > same use cases. They have nothing to do with fstrim. > > Time to incorporate and extend Garzik's blktool, perhaps? > Hmm, I do not know. blktool seems to be very similar to blockdev in the sense that it is for querying and changing block device setting which is not what we do with, discard, write same or zeroout where we actually change the data on the device itself. In my mind it just somehow does not fit there. But you're right that it is not just about discard or secure discard, we have other quite similar functionalities and it might be worth having one separate tool for all those, thanks for pointing this out. (CCing linux-fsdevel ... for the whole discussion see http://www.spinics.net/lists/util-linux-ng/msg06832.html) Thanks! -Lukas ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command @ 2012-09-27 17:03 ` Lukáš Czerner 0 siblings, 0 replies; 23+ messages in thread From: Lukáš Czerner @ 2012-09-27 17:03 UTC (permalink / raw) To: Martin K. Petersen Cc: Lukáš Czerner, Karel Zak, util-linux-u79uwXL29TY76Z2rM5mHXA, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1: Type: TEXT/PLAIN, Size: 1699 bytes --] On Thu, 27 Sep 2012, Martin K. Petersen wrote: > Date: Thu, 27 Sep 2012 11:49:22 -0400 > From: Martin K. Petersen <martin.petersen-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> > To: Lukáš Czerner <lczerner-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > Cc: Karel Zak <kzak-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, util-linux-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > Subject: Re: [PATCH] blkdiscard: add new command > > >>>>> "Lukáš" == Lukáš Czerner <lczerner-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> writes: > > Lukáš> TBH this is fugly :). Also what actually is the advantage of > Lukáš> doing this ? So we saved 50 lines of code for this ugliness and > Lukáš> instead of two separate binaries we have this one hybrid and > Lukáš> symlink. I am not sure it's worth it. Can't we just have two > Lukáš> separate binaries ? What is the problem with that ? > > It's not just discard. We should also consider the zeroout and write > same use cases. They have nothing to do with fstrim. > > Time to incorporate and extend Garzik's blktool, perhaps? > Hmm, I do not know. blktool seems to be very similar to blockdev in the sense that it is for querying and changing block device setting which is not what we do with, discard, write same or zeroout where we actually change the data on the device itself. In my mind it just somehow does not fit there. But you're right that it is not just about discard or secure discard, we have other quite similar functionalities and it might be worth having one separate tool for all those, thanks for pointing this out. (CCing linux-fsdevel ... for the whole discussion see http://www.spinics.net/lists/util-linux-ng/msg06832.html) Thanks! -Lukas ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-27 17:03 ` Lukáš Czerner @ 2012-09-27 17:33 ` Martin K. Petersen -1 siblings, 0 replies; 23+ messages in thread From: Martin K. Petersen @ 2012-09-27 17:33 UTC (permalink / raw) To: Lukáš Czerner Cc: Martin K. Petersen, Karel Zak, util-linux, linux-fsdevel >>>>> "Luk=C3=A1=C5=A1" =3D=3D Luk=C3=A1=C5=A1 Czerner <lczerner@redhat.com= > writes: Luk=C3=A1=C5=A1> Hmm, I do not know. blktool seems to be very similar to bl= ockdev Luk=C3=A1=C5=A1> in the sense that it is for querying and changing block de= vice Luk=C3=A1=C5=A1> setting which is not what we do with, discard, write same = or Luk=C3=A1=C5=A1> zeroout where we actually change the data on the device Luk=C3=A1=C5=A1> itself. In my mind it just somehow does not fit there. I was merely picking up on the fact that blockdev's argument handling didn't seem particularly friendly to the discard/zero/ws case. Whereas blktool looked more flexible. Luk=C3=A1=C5=A1> But you're right that it is not just about discard or secu= re Luk=C3=A1=C5=A1> discard, we have other quite similar functionalities and i= t might Luk=C3=A1=C5=A1> be worth having one separate tool for all those, thanks for Luk=C3=A1=C5=A1> pointing this out. I'm perfectly ok with a separate blkswissarmyknife command. I'm also ok with separate blkdiscard/blkwritesame/blkzeroout commands as long as they look and feel the same. --=20 Martin K. Petersen Oracle Linux Engineering ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command @ 2012-09-27 17:33 ` Martin K. Petersen 0 siblings, 0 replies; 23+ messages in thread From: Martin K. Petersen @ 2012-09-27 17:33 UTC (permalink / raw) To: Lukáš Czerner Cc: Martin K. Petersen, Karel Zak, util-linux, linux-fsdevel >>>>> "Lukáš" == Lukáš Czerner <lczerner@redhat.com> writes: Lukáš> Hmm, I do not know. blktool seems to be very similar to blockdev Lukáš> in the sense that it is for querying and changing block device Lukáš> setting which is not what we do with, discard, write same or Lukáš> zeroout where we actually change the data on the device Lukáš> itself. In my mind it just somehow does not fit there. I was merely picking up on the fact that blockdev's argument handling didn't seem particularly friendly to the discard/zero/ws case. Whereas blktool looked more flexible. Lukáš> But you're right that it is not just about discard or secure Lukáš> discard, we have other quite similar functionalities and it might Lukáš> be worth having one separate tool for all those, thanks for Lukáš> pointing this out. I'm perfectly ok with a separate blkswissarmyknife command. I'm also ok with separate blkdiscard/blkwritesame/blkzeroout commands as long as they look and feel the same. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-27 15:49 ` Martin K. Petersen 2012-09-27 17:03 ` Lukáš Czerner @ 2012-09-27 23:17 ` Karel Zak 1 sibling, 0 replies; 23+ messages in thread From: Karel Zak @ 2012-09-27 23:17 UTC (permalink / raw) To: Martin K. Petersen; +Cc: Lukáš Czerner, util-linux On Thu, Sep 27, 2012 at 11:49:22AM -0400, Martin K. Petersen wrote: > >>>>> "Lukáš" == Lukáš Czerner <lczerner@redhat.com> writes: > > Lukáš> TBH this is fugly :). Also what actually is the advantage of Yes, I agree my implementation is not too readable... > Lukáš> doing this ? So we saved 50 lines of code for this ugliness and > Lukáš> instead of two separate binaries we have this one hybrid and > Lukáš> symlink. I am not sure it's worth it. Can't we just have two > Lukáš> separate binaries ? What is the problem with that ? > > It's not just discard. We should also consider the zeroout and write > same use cases. They have nothing to do with fstrim. > > Time to incorporate and extend Garzik's blktool, perhaps? I have applied Lukas' implementation. It seems better to have it on the same place like fstrim. Karel -- Karel Zak <kzak@redhat.com> http://karelzak.blogspot.com ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] blkdiscard: add new command 2012-09-12 21:49 [PATCH] blkdiscard: add new command Lukas Czerner 2012-09-12 22:43 ` Kay Sievers 2012-09-26 19:47 ` Lukáš Czerner @ 2012-09-27 23:13 ` Karel Zak 2 siblings, 0 replies; 23+ messages in thread From: Karel Zak @ 2012-09-27 23:13 UTC (permalink / raw) To: Lukas Czerner; +Cc: util-linux On Wed, Sep 12, 2012 at 05:49:15PM -0400, Lukas Czerner wrote: > sys-utils/Makemodule.am | 5 ++ > sys-utils/blkdiscard.8 | 66 ++++++++++++++++++ > sys-utils/blkdiscard.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 245 insertions(+), 0 deletions(-) > create mode 100644 sys-utils/blkdiscard.8 > create mode 100644 sys-utils/blkdiscard.c Applied, thanks. -- Karel Zak <kzak@redhat.com> http://karelzak.blogspot.com ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2012-09-27 23:17 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-09-12 21:49 [PATCH] blkdiscard: add new command Lukas Czerner 2012-09-12 22:43 ` Kay Sievers 2012-09-12 22:47 ` Davidlohr Bueso 2012-09-13 13:37 ` Lukáš Czerner 2012-09-13 13:57 ` Karel Zak 2012-09-13 14:09 ` Lukáš Czerner 2012-09-13 15:33 ` Kay Sievers 2012-09-13 15:52 ` Lukáš Czerner 2012-09-13 15:32 ` Kay Sievers 2012-09-26 19:47 ` Lukáš Czerner 2012-09-26 20:04 ` Kay Sievers 2012-09-26 20:14 ` Lukáš Czerner 2012-09-26 20:27 ` Kay Sievers 2012-09-26 21:01 ` Lukáš Czerner 2012-09-27 9:42 ` Karel Zak 2012-09-27 15:21 ` Lukáš Czerner 2012-09-27 15:49 ` Martin K. Petersen 2012-09-27 17:03 ` Lukáš Czerner 2012-09-27 17:03 ` Lukáš Czerner 2012-09-27 17:33 ` Martin K. Petersen 2012-09-27 17:33 ` Martin K. Petersen 2012-09-27 23:17 ` Karel Zak 2012-09-27 23:13 ` Karel Zak
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.