* [PATCH 1/2] tests: add blkdiscard offsets test
@ 2014-10-23 15:34 Federico Simoncelli
2014-10-23 15:34 ` [PATCH 2/2] blkdiscard: fail on sector misalignment Federico Simoncelli
0 siblings, 1 reply; 5+ messages in thread
From: Federico Simoncelli @ 2014-10-23 15:34 UTC (permalink / raw)
To: util-linux; +Cc: kzak, lczerner, Federico Simoncelli
Signed-off-by: Federico Simoncelli <fsimonce@redhat.com>
---
tests/commands.sh | 1 +
tests/expected/blkdiscard/offsets | 16 ++++++++++
tests/ts/blkdiscard/offsets | 62 +++++++++++++++++++++++++++++++++++++++
3 files changed, 79 insertions(+)
create mode 100644 tests/expected/blkdiscard/offsets
create mode 100755 tests/ts/blkdiscard/offsets
diff --git a/tests/commands.sh b/tests/commands.sh
index 4c7455e..e11bd2a 100644
--- a/tests/commands.sh
+++ b/tests/commands.sh
@@ -29,6 +29,7 @@ TS_HELPER_SYSINFO="$top_builddir/test_sysinfo"
# paths to commands
TS_CMD_ADDPART=${TS_CMD_ADDPART:-"$top_builddir/addpart"}
TS_CMD_DELPART=${TS_CMD_DELPART:-"$top_builddir/delpart"}
+TS_CMD_BLKDISCARD=${TS_CMD_BLKID-"$top_builddir/blkdiscard"}
TS_CMD_BLKID=${TS_CMD_BLKID-"$top_builddir/blkid"}
TS_CMD_CAL=${TS_CMD_CAL-"$top_builddir/cal"}
TS_CMD_COLRM=${TS_CMD_COLRM:-"$top_builddir/colrm"}
diff --git a/tests/expected/blkdiscard/offsets b/tests/expected/blkdiscard/offsets
new file mode 100644
index 0000000..766c39d
--- /dev/null
+++ b/tests/expected/blkdiscard/offsets
@@ -0,0 +1,16 @@
+create loop device from image
+testing offsets with full block size
+Discarded 10485760 bytes from the offset 0
+Discarded 10485248 bytes from the offset 512
+Discarded 10485248 bytes from the offset 512
+Discarded 10485248 bytes from the offset 512
+Discarded 10484736 bytes from the offset 1024
+testing offsets with specific length
+Discarded 5242880 bytes from the offset 0
+Discarded 5242880 bytes from the offset 0
+Discarded 5242880 bytes from the offset 0
+Discarded 5242880 bytes from the offset 512
+Discarded 5242880 bytes from the offset 512
+Discarded 5242880 bytes from the offset 512
+Discarded 5242880 bytes from the offset 1024
+detach loop device from image
diff --git a/tests/ts/blkdiscard/offsets b/tests/ts/blkdiscard/offsets
new file mode 100755
index 0000000..04bd437
--- /dev/null
+++ b/tests/ts/blkdiscard/offsets
@@ -0,0 +1,62 @@
+#!/bin/bash
+
+#
+# Copyright (C) 2014 Federico Simoncelli <fsimonce@redhat.com>
+#
+# This file is part of util-linux.
+#
+# This file 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 file 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.
+#
+TS_TOPDIR="${0%/*}/../.."
+TS_DESC="offsets"
+
+. $TS_TOPDIR/functions.sh
+ts_init "$*"
+
+ts_check_test_command "$TS_CMD_BLKDISCARD"
+
+ts_skip_nonroot
+ts_check_losetup
+
+set -o pipefail
+
+ORIGPWD=$(pwd)
+IMAGE_NAME="${TS_TESTNAME}-loop.img"
+IMAGE_PATH="$TS_OUTDIR/$IMAGE_NAME"
+
+truncate -s 10M $IMAGE_PATH
+
+ts_log "create loop device from image"
+DEVICE=$($TS_CMD_LOSETUP --show -f $IMAGE_PATH)
+CMD_SED_DEVICE="sed s#$DEVICE:\s##"
+
+ts_log "testing offsets with full block size"
+$TS_CMD_BLKDISCARD -v $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+$TS_CMD_BLKDISCARD -v -o 1 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+$TS_CMD_BLKDISCARD -v -o 511 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+$TS_CMD_BLKDISCARD -v -o 512 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+$TS_CMD_BLKDISCARD -v -o 1024 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+
+ts_log "testing offsets with specific length"
+$TS_CMD_BLKDISCARD -v -l 5242880 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+$TS_CMD_BLKDISCARD -v -l 5242881 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+$TS_CMD_BLKDISCARD -v -l 5243391 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+$TS_CMD_BLKDISCARD -v -o 1 -l 5242880 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+$TS_CMD_BLKDISCARD -v -o 511 -l 5242880 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+$TS_CMD_BLKDISCARD -v -o 512 -l 5242880 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+$TS_CMD_BLKDISCARD -v -o 1024 -l 5242880 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+
+ts_log "detach loop device from image"
+$TS_CMD_LOSETUP -d $DEVICE 2>&1 >> $TS_OUTPUT
+
+ts_cd "$ORIGPWD"
+
+ts_finalize
--
1.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] blkdiscard: fail on sector misalignment
2014-10-23 15:34 [PATCH 1/2] tests: add blkdiscard offsets test Federico Simoncelli
@ 2014-10-23 15:34 ` Federico Simoncelli
2014-10-24 9:37 ` Lukáš Czerner
0 siblings, 1 reply; 5+ messages in thread
From: Federico Simoncelli @ 2014-10-23 15:34 UTC (permalink / raw)
To: util-linux; +Cc: kzak, lczerner, Federico Simoncelli
---
sys-utils/blkdiscard.c | 10 +++++++---
tests/expected/blkdiscard/offsets | 12 ++++++------
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/sys-utils/blkdiscard.c b/sys-utils/blkdiscard.c
index 1eb2b285..76c72b8 100644
--- a/sys-utils/blkdiscard.c
+++ b/sys-utils/blkdiscard.c
@@ -144,9 +144,9 @@ int main(int argc, char **argv)
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);
+ /* check offset alignment to the sector size */
+ if (range[0] % secsize)
+ errx(EXIT_FAILURE, _("%s: offset %" PRIu64 " is not aligned"), path, range[0]);
/* is the range end behind the end of the device ?*/
if (range[0] > blksize)
@@ -155,6 +155,10 @@ int main(int argc, char **argv)
if (end < range[0] || end > blksize)
range[1] = blksize - range[0];
+ /* check length alignment to the sector size */
+ if (range[1] % secsize)
+ errx(EXIT_FAILURE, _("%s: length %" PRIu64 " is not aligned"), path, range[1]);
+
if (secure) {
if (ioctl(fd, BLKSECDISCARD, &range))
err(EXIT_FAILURE, _("%s: BLKSECDISCARD ioctl failed"), path);
diff --git a/tests/expected/blkdiscard/offsets b/tests/expected/blkdiscard/offsets
index 766c39d..abedb08 100644
--- a/tests/expected/blkdiscard/offsets
+++ b/tests/expected/blkdiscard/offsets
@@ -1,16 +1,16 @@
create loop device from image
testing offsets with full block size
Discarded 10485760 bytes from the offset 0
-Discarded 10485248 bytes from the offset 512
-Discarded 10485248 bytes from the offset 512
+blkdiscard: offset 1 is not aligned
+blkdiscard: offset 511 is not aligned
Discarded 10485248 bytes from the offset 512
Discarded 10484736 bytes from the offset 1024
testing offsets with specific length
Discarded 5242880 bytes from the offset 0
-Discarded 5242880 bytes from the offset 0
-Discarded 5242880 bytes from the offset 0
-Discarded 5242880 bytes from the offset 512
-Discarded 5242880 bytes from the offset 512
+blkdiscard: length 5242881 is not aligned
+blkdiscard: length 5243391 is not aligned
+blkdiscard: offset 1 is not aligned
+blkdiscard: offset 511 is not aligned
Discarded 5242880 bytes from the offset 512
Discarded 5242880 bytes from the offset 1024
detach loop device from image
--
1.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] blkdiscard: fail on sector misalignment
2014-10-23 15:34 ` [PATCH 2/2] blkdiscard: fail on sector misalignment Federico Simoncelli
@ 2014-10-24 9:37 ` Lukáš Czerner
2014-10-24 9:47 ` Federico Simoncelli
0 siblings, 1 reply; 5+ messages in thread
From: Lukáš Czerner @ 2014-10-24 9:37 UTC (permalink / raw)
To: Federico Simoncelli; +Cc: util-linux, kzak
On Thu, 23 Oct 2014, Federico Simoncelli wrote:
> Date: Thu, 23 Oct 2014 15:34:36 +0000
> From: Federico Simoncelli <fsimonce@redhat.com>
> To: util-linux@vger.kernel.org
> Cc: kzak@redhat.com, lczerner@redhat.com,
> Federico Simoncelli <fsimonce@redhat.com>
> Subject: [PATCH 2/2] blkdiscard: fail on sector misalignment
>
> ---
> sys-utils/blkdiscard.c | 10 +++++++---
> tests/expected/blkdiscard/offsets | 12 ++++++------
> 2 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/sys-utils/blkdiscard.c b/sys-utils/blkdiscard.c
> index 1eb2b285..76c72b8 100644
> --- a/sys-utils/blkdiscard.c
> +++ b/sys-utils/blkdiscard.c
> @@ -144,9 +144,9 @@ int main(int argc, char **argv)
> 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);
> + /* check offset alignment to the sector size */
> + if (range[0] % secsize)
> + errx(EXIT_FAILURE, _("%s: offset %" PRIu64 " is not aligned"), path, range[0]);
>
> /* is the range end behind the end of the device ?*/
> if (range[0] > blksize)
> @@ -155,6 +155,10 @@ int main(int argc, char **argv)
> if (end < range[0] || end > blksize)
> range[1] = blksize - range[0];
>
> + /* check length alignment to the sector size */
> + if (range[1] % secsize)
> + errx(EXIT_FAILURE, _("%s: length %" PRIu64 " is not aligned"), path, range[1]);
> +
Looks good, however remember that the tool is intended to be used by
the end user, can we be a bit more helpful ? We already know the
secsize it should be aligned to, so we can print that as well.
However it just occurred to me that aligning it to secsize is not
enough. It should be aligned to discard_granularity.
Thanks!
-Lukas
> if (secure) {
> if (ioctl(fd, BLKSECDISCARD, &range))
> err(EXIT_FAILURE, _("%s: BLKSECDISCARD ioctl failed"), path);
> diff --git a/tests/expected/blkdiscard/offsets b/tests/expected/blkdiscard/offsets
> index 766c39d..abedb08 100644
> --- a/tests/expected/blkdiscard/offsets
> +++ b/tests/expected/blkdiscard/offsets
> @@ -1,16 +1,16 @@
> create loop device from image
> testing offsets with full block size
> Discarded 10485760 bytes from the offset 0
> -Discarded 10485248 bytes from the offset 512
> -Discarded 10485248 bytes from the offset 512
> +blkdiscard: offset 1 is not aligned
> +blkdiscard: offset 511 is not aligned
> Discarded 10485248 bytes from the offset 512
> Discarded 10484736 bytes from the offset 1024
> testing offsets with specific length
> Discarded 5242880 bytes from the offset 0
> -Discarded 5242880 bytes from the offset 0
> -Discarded 5242880 bytes from the offset 0
> -Discarded 5242880 bytes from the offset 512
> -Discarded 5242880 bytes from the offset 512
> +blkdiscard: length 5242881 is not aligned
> +blkdiscard: length 5243391 is not aligned
> +blkdiscard: offset 1 is not aligned
> +blkdiscard: offset 511 is not aligned
> Discarded 5242880 bytes from the offset 512
> Discarded 5242880 bytes from the offset 1024
> detach loop device from image
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] blkdiscard: fail on sector misalignment
2014-10-24 9:37 ` Lukáš Czerner
@ 2014-10-24 9:47 ` Federico Simoncelli
2014-10-24 9:50 ` Lukáš Czerner
0 siblings, 1 reply; 5+ messages in thread
From: Federico Simoncelli @ 2014-10-24 9:47 UTC (permalink / raw)
To: Lukáš Czerner; +Cc: util-linux, kzak
----- Original Message -----
> From: "Lukáš Czerner" <lczerner@redhat.com>
> To: "Federico Simoncelli" <fsimonce@redhat.com>
> Cc: util-linux@vger.kernel.org, kzak@redhat.com
> Sent: Friday, October 24, 2014 11:37:33 AM
> Subject: Re: [PATCH 2/2] blkdiscard: fail on sector misalignment
>
> On Thu, 23 Oct 2014, Federico Simoncelli wrote:
>
> > Date: Thu, 23 Oct 2014 15:34:36 +0000
> > From: Federico Simoncelli <fsimonce@redhat.com>
> > To: util-linux@vger.kernel.org
> > Cc: kzak@redhat.com, lczerner@redhat.com,
> > Federico Simoncelli <fsimonce@redhat.com>
> > Subject: [PATCH 2/2] blkdiscard: fail on sector misalignment
> >
> > ---
> > sys-utils/blkdiscard.c | 10 +++++++---
> > tests/expected/blkdiscard/offsets | 12 ++++++------
> > 2 files changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/sys-utils/blkdiscard.c b/sys-utils/blkdiscard.c
> > index 1eb2b285..76c72b8 100644
> > --- a/sys-utils/blkdiscard.c
> > +++ b/sys-utils/blkdiscard.c
> > @@ -144,9 +144,9 @@ int main(int argc, char **argv)
> > 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);
> > + /* check offset alignment to the sector size */
> > + if (range[0] % secsize)
> > + errx(EXIT_FAILURE, _("%s: offset %" PRIu64 " is not aligned"), path,
> > range[0]);
> >
> > /* is the range end behind the end of the device ?*/
> > if (range[0] > blksize)
> > @@ -155,6 +155,10 @@ int main(int argc, char **argv)
> > if (end < range[0] || end > blksize)
> > range[1] = blksize - range[0];
> >
> > + /* check length alignment to the sector size */
> > + if (range[1] % secsize)
> > + errx(EXIT_FAILURE, _("%s: length %" PRIu64 " is not aligned"), path,
> > range[1]);
> > +
>
> Looks good, however remember that the tool is intended to be used by
> the end user, can we be a bit more helpful ? We already know the
> secsize it should be aligned to, so we can print that as well.
I will update the error message.
> However it just occurred to me that aligning it to secsize is not
> enough. It should be aligned to discard_granularity.
Aligning to discard_granularity is an optimization, on the other hand
not aligning to secsize would result in the ioctl failing (EINVAL).
--
Federico
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] blkdiscard: fail on sector misalignment
2014-10-24 9:47 ` Federico Simoncelli
@ 2014-10-24 9:50 ` Lukáš Czerner
0 siblings, 0 replies; 5+ messages in thread
From: Lukáš Czerner @ 2014-10-24 9:50 UTC (permalink / raw)
To: Federico Simoncelli; +Cc: util-linux, kzak
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2951 bytes --]
On Fri, 24 Oct 2014, Federico Simoncelli wrote:
> Date: Fri, 24 Oct 2014 05:47:04 -0400 (EDT)
> From: Federico Simoncelli <fsimonce@redhat.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: util-linux@vger.kernel.org, kzak@redhat.com
> Subject: Re: [PATCH 2/2] blkdiscard: fail on sector misalignment
>
> ----- Original Message -----
> > From: "Lukáš Czerner" <lczerner@redhat.com>
> > To: "Federico Simoncelli" <fsimonce@redhat.com>
> > Cc: util-linux@vger.kernel.org, kzak@redhat.com
> > Sent: Friday, October 24, 2014 11:37:33 AM
> > Subject: Re: [PATCH 2/2] blkdiscard: fail on sector misalignment
> >
> > On Thu, 23 Oct 2014, Federico Simoncelli wrote:
> >
> > > Date: Thu, 23 Oct 2014 15:34:36 +0000
> > > From: Federico Simoncelli <fsimonce@redhat.com>
> > > To: util-linux@vger.kernel.org
> > > Cc: kzak@redhat.com, lczerner@redhat.com,
> > > Federico Simoncelli <fsimonce@redhat.com>
> > > Subject: [PATCH 2/2] blkdiscard: fail on sector misalignment
> > >
> > > ---
> > > sys-utils/blkdiscard.c | 10 +++++++---
> > > tests/expected/blkdiscard/offsets | 12 ++++++------
> > > 2 files changed, 13 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/sys-utils/blkdiscard.c b/sys-utils/blkdiscard.c
> > > index 1eb2b285..76c72b8 100644
> > > --- a/sys-utils/blkdiscard.c
> > > +++ b/sys-utils/blkdiscard.c
> > > @@ -144,9 +144,9 @@ int main(int argc, char **argv)
> > > 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);
> > > + /* check offset alignment to the sector size */
> > > + if (range[0] % secsize)
> > > + errx(EXIT_FAILURE, _("%s: offset %" PRIu64 " is not aligned"), path,
> > > range[0]);
> > >
> > > /* is the range end behind the end of the device ?*/
> > > if (range[0] > blksize)
> > > @@ -155,6 +155,10 @@ int main(int argc, char **argv)
> > > if (end < range[0] || end > blksize)
> > > range[1] = blksize - range[0];
> > >
> > > + /* check length alignment to the sector size */
> > > + if (range[1] % secsize)
> > > + errx(EXIT_FAILURE, _("%s: length %" PRIu64 " is not aligned"), path,
> > > range[1]);
> > > +
> >
> > Looks good, however remember that the tool is intended to be used by
> > the end user, can we be a bit more helpful ? We already know the
> > secsize it should be aligned to, so we can print that as well.
>
> I will update the error message.
>
> > However it just occurred to me that aligning it to secsize is not
> > enough. It should be aligned to discard_granularity.
>
> Aligning to discard_granularity is an optimization, on the other hand
> not aligning to secsize would result in the ioctl failing (EINVAL).
Fair enough, but the unaligned part will be ignored and not
'discarded'. But I guess that it really does not matter.
-Lukas
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-10-24 9:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-23 15:34 [PATCH 1/2] tests: add blkdiscard offsets test Federico Simoncelli
2014-10-23 15:34 ` [PATCH 2/2] blkdiscard: fail on sector misalignment Federico Simoncelli
2014-10-24 9:37 ` Lukáš Czerner
2014-10-24 9:47 ` Federico Simoncelli
2014-10-24 9:50 ` Lukáš Czerner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox