| Message ID | 1422284444-12529-11-git-send-email-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 01/26/2015 08:00 AM, Max Reitz wrote: > Remove "growable" option from the "open" command and from the qemu-io > command line. qemu-io is about to be converted to BlockBackend which > will make sure that no request exceeds the image size, so the only way > to keep "growable" would be to use BlockBackend if it is not given and > to directly access the BDS if it is. > > qemu-io is a debugging tool, therefore removing a rarely used option > will have only a very small impact, if any. There was only one > qemu-iotest which used the option; since it is not critical, this patch > just removes it. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- Reviewed-by: Eric Blake <eblake@redhat.com> Do we want to ever reuse the test number that you are deleting?
On 2015-01-27 at 11:59, Eric Blake wrote: > On 01/26/2015 08:00 AM, Max Reitz wrote: >> Remove "growable" option from the "open" command and from the qemu-io >> command line. qemu-io is about to be converted to BlockBackend which >> will make sure that no request exceeds the image size, so the only way >> to keep "growable" would be to use BlockBackend if it is not given and >> to directly access the BDS if it is. >> >> qemu-io is a debugging tool, therefore removing a rarely used option >> will have only a very small impact, if any. There was only one >> qemu-iotest which used the option; since it is not critical, this patch >> just removes it. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- > Reviewed-by: Eric Blake <eblake@redhat.com> > > Do we want to ever reuse the test number that you are deleting? Good question, I think I have talked about that with Kevin before. It would not hurt too much if we were to accidentally reuse the test case number, most certainly not here in upstream. However, for all downstream versions of qemu, this might make adding the new test 16 difficult; but certainly not impossible (if someone is affected by this issue, he/she can just use 999 or something). So we may want to keep in mind not to reuse number 16, but if someone does, so be it. Max
On 01/27/2015 10:04 AM, Max Reitz wrote: > On 2015-01-27 at 11:59, Eric Blake wrote: >> On 01/26/2015 08:00 AM, Max Reitz wrote: >>> Remove "growable" option from the "open" command and from the qemu-io >>> command line. qemu-io is about to be converted to BlockBackend which >>> will make sure that no request exceeds the image size, so the only way >>> to keep "growable" would be to use BlockBackend if it is not given and >>> to directly access the BDS if it is. >>> >>> qemu-io is a debugging tool, therefore removing a rarely used option >>> will have only a very small impact, if any. There was only one >>> qemu-iotest which used the option; since it is not critical, this patch >>> just removes it. >>> >>> Signed-off-by: Max Reitz <mreitz@redhat.com> >>> --- >> Reviewed-by: Eric Blake <eblake@redhat.com> >> >> Do we want to ever reuse the test number that you are deleting? > > Good question, I think I have talked about that with Kevin before. It > would not hurt too much if we were to accidentally reuse the test case > number, most certainly not here in upstream. > > However, for all downstream versions of qemu, this might make adding the > new test 16 difficult; but certainly not impossible (if someone is > affected by this issue, he/she can just use 999 or something). So we may > want to keep in mind not to reuse number 16, but if someone does, so be it. Is it worth a placeholder file that has a comment mentioning that the test number is intentionally reserved (and if someone attempts to run, always passes)?
On 2015-01-27 at 12:10, Eric Blake wrote: > On 01/27/2015 10:04 AM, Max Reitz wrote: >> On 2015-01-27 at 11:59, Eric Blake wrote: >>> On 01/26/2015 08:00 AM, Max Reitz wrote: >>>> Remove "growable" option from the "open" command and from the qemu-io >>>> command line. qemu-io is about to be converted to BlockBackend which >>>> will make sure that no request exceeds the image size, so the only way >>>> to keep "growable" would be to use BlockBackend if it is not given and >>>> to directly access the BDS if it is. >>>> >>>> qemu-io is a debugging tool, therefore removing a rarely used option >>>> will have only a very small impact, if any. There was only one >>>> qemu-iotest which used the option; since it is not critical, this patch >>>> just removes it. >>>> >>>> Signed-off-by: Max Reitz <mreitz@redhat.com> >>>> --- >>> Reviewed-by: Eric Blake <eblake@redhat.com> >>> >>> Do we want to ever reuse the test number that you are deleting? >> Good question, I think I have talked about that with Kevin before. It >> would not hurt too much if we were to accidentally reuse the test case >> number, most certainly not here in upstream. >> >> However, for all downstream versions of qemu, this might make adding the >> new test 16 difficult; but certainly not impossible (if someone is >> affected by this issue, he/she can just use 999 or something). So we may >> want to keep in mind not to reuse number 16, but if someone does, so be it. > Is it worth a placeholder file that has a comment mentioning that the > test number is intentionally reserved (and if someone attempts to run, > always passes)? Seems good to me. It's a minor effort now and may avert some hassle later. Max
Am 27.01.2015 um 18:11 hat Max Reitz geschrieben: > On 2015-01-27 at 12:10, Eric Blake wrote: > >On 01/27/2015 10:04 AM, Max Reitz wrote: > >>On 2015-01-27 at 11:59, Eric Blake wrote: > >>>On 01/26/2015 08:00 AM, Max Reitz wrote: > >>>>Remove "growable" option from the "open" command and from the qemu-io > >>>>command line. qemu-io is about to be converted to BlockBackend which > >>>>will make sure that no request exceeds the image size, so the only way > >>>>to keep "growable" would be to use BlockBackend if it is not given and > >>>>to directly access the BDS if it is. > >>>> > >>>>qemu-io is a debugging tool, therefore removing a rarely used option > >>>>will have only a very small impact, if any. There was only one > >>>>qemu-iotest which used the option; since it is not critical, this patch > >>>>just removes it. > >>>> > >>>>Signed-off-by: Max Reitz <mreitz@redhat.com> > >>>>--- > >>>Reviewed-by: Eric Blake <eblake@redhat.com> > >>> > >>>Do we want to ever reuse the test number that you are deleting? > >>Good question, I think I have talked about that with Kevin before. It > >>would not hurt too much if we were to accidentally reuse the test case > >>number, most certainly not here in upstream. > >> > >>However, for all downstream versions of qemu, this might make adding the > >>new test 16 difficult; but certainly not impossible (if someone is > >>affected by this issue, he/she can just use 999 or something). So we may > >>want to keep in mind not to reuse number 16, but if someone does, so be it. > >Is it worth a placeholder file that has a comment mentioning that the > >test number is intentionally reserved (and if someone attempts to run, > >always passes)? > > Seems good to me. It's a minor effort now and may avert some hassle later. How about just keeping a comment line in group? Kevin
On 2015-02-02 at 14:36, Kevin Wolf wrote: > Am 27.01.2015 um 18:11 hat Max Reitz geschrieben: >> On 2015-01-27 at 12:10, Eric Blake wrote: >>> On 01/27/2015 10:04 AM, Max Reitz wrote: >>>> On 2015-01-27 at 11:59, Eric Blake wrote: >>>>> On 01/26/2015 08:00 AM, Max Reitz wrote: >>>>>> Remove "growable" option from the "open" command and from the qemu-io >>>>>> command line. qemu-io is about to be converted to BlockBackend which >>>>>> will make sure that no request exceeds the image size, so the only way >>>>>> to keep "growable" would be to use BlockBackend if it is not given and >>>>>> to directly access the BDS if it is. >>>>>> >>>>>> qemu-io is a debugging tool, therefore removing a rarely used option >>>>>> will have only a very small impact, if any. There was only one >>>>>> qemu-iotest which used the option; since it is not critical, this patch >>>>>> just removes it. >>>>>> >>>>>> Signed-off-by: Max Reitz <mreitz@redhat.com> >>>>>> --- >>>>> Reviewed-by: Eric Blake <eblake@redhat.com> >>>>> >>>>> Do we want to ever reuse the test number that you are deleting? >>>> Good question, I think I have talked about that with Kevin before. It >>>> would not hurt too much if we were to accidentally reuse the test case >>>> number, most certainly not here in upstream. >>>> >>>> However, for all downstream versions of qemu, this might make adding the >>>> new test 16 difficult; but certainly not impossible (if someone is >>>> affected by this issue, he/she can just use 999 or something). So we may >>>> want to keep in mind not to reuse number 16, but if someone does, so be it. >>> Is it worth a placeholder file that has a comment mentioning that the >>> test number is intentionally reserved (and if someone attempts to run, >>> always passes)? >> Seems good to me. It's a minor effort now and may avert some hassle later. > How about just keeping a comment line in group? Oh, that would be too simple. Or maybe just simple enough for me; yes, that seems nicer indeed. Max
diff --git a/qemu-io.c b/qemu-io.c index 81f8f64..0237ecb 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -50,7 +50,7 @@ static const cmdinfo_t close_cmd = { .oneline = "close the current open file", }; -static int openfile(char *name, int flags, int growable, QDict *opts) +static int openfile(char *name, int flags, QDict *opts) { Error *local_err = NULL; @@ -60,10 +60,6 @@ static int openfile(char *name, int flags, int growable, QDict *opts) return 1; } - if (growable) { - flags |= BDRV_O_PROTOCOL; - } - qemuio_blk = blk_new_open("hda", name, NULL, opts, flags, &local_err); if (!qemuio_blk) { fprintf(stderr, "%s: can't open%s%s: %s\n", progname, @@ -90,7 +86,6 @@ static void open_help(void) " -r, -- open file read-only\n" " -s, -- use snapshot file\n" " -n, -- disable host cache\n" -" -g, -- allow file to grow (only applies to protocols)\n" " -o, -- options to be given to the block driver" "\n"); } @@ -123,7 +118,6 @@ static int open_f(BlockDriverState *bs, int argc, char **argv) { int flags = 0; int readonly = 0; - int growable = 0; int c; QemuOpts *qopts; QDict *opts; @@ -139,9 +133,6 @@ static int open_f(BlockDriverState *bs, int argc, char **argv) case 'r': readonly = 1; break; - case 'g': - growable = 1; - break; case 'o': if (!qemu_opts_parse(&empty_opts, optarg, 0)) { printf("could not parse option list -- %s\n", optarg); @@ -164,9 +155,9 @@ static int open_f(BlockDriverState *bs, int argc, char **argv) qemu_opts_reset(&empty_opts); if (optind == argc - 1) { - return openfile(argv[optind], flags, growable, opts); + return openfile(argv[optind], flags, opts); } else if (optind == argc) { - return openfile(NULL, flags, growable, opts); + return openfile(NULL, flags, opts); } else { QDECREF(opts); return qemuio_command_usage(&open_cmd); @@ -200,7 +191,6 @@ static void usage(const char *name) " -r, --read-only export read-only\n" " -s, --snapshot use snapshot file\n" " -n, --nocache disable host cache\n" -" -g, --growable allow file to grow (only applies to protocols)\n" " -m, --misalign misalign allocations for O_DIRECT\n" " -k, --native-aio use kernel AIO implementation (on Linux only)\n" " -t, --cache=MODE use the given cache mode for the image\n" @@ -359,7 +349,6 @@ static void reenable_tty_echo(void) int main(int argc, char **argv) { int readonly = 0; - int growable = 0; const char *sopt = "hVc:d:f:rsnmgkt:T:"; const struct option lopt[] = { { "help", 0, NULL, 'h' }, @@ -371,7 +360,6 @@ int main(int argc, char **argv) { "snapshot", 0, NULL, 's' }, { "nocache", 0, NULL, 'n' }, { "misalign", 0, NULL, 'm' }, - { "growable", 0, NULL, 'g' }, { "native-aio", 0, NULL, 'k' }, { "discard", 1, NULL, 'd' }, { "cache", 1, NULL, 't' }, @@ -422,9 +410,6 @@ int main(int argc, char **argv) case 'm': qemuio_misalign = true; break; - case 'g': - growable = 1; - break; case 'k': flags |= BDRV_O_NATIVE_AIO; break; @@ -482,7 +467,7 @@ int main(int argc, char **argv) } if ((argc - optind) == 1) { - openfile(argv[optind], flags, growable, opts); + openfile(argv[optind], flags, opts); } command_loop(); diff --git a/tests/qemu-iotests/016 b/tests/qemu-iotests/016 deleted file mode 100755 index 52397aa..0000000 --- a/tests/qemu-iotests/016 +++ /dev/null @@ -1,73 +0,0 @@ -#!/bin/bash -# -# Test I/O after EOF for growable images. -# -# Copyright (C) 2009 Red Hat, Inc. -# -# 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/>. -# - -# creator -owner=hch@lst.de - -seq=`basename $0` -echo "QA output created by $seq" - -here=`pwd` -tmp=/tmp/$$ -status=1 # failure is the default! - -_cleanup() -{ - _cleanup_test_img -} -trap "_cleanup; exit \$status" 0 1 2 3 15 - -# get standard environment, filters and checks -. ./common.rc -. ./common.filter - -_supported_fmt raw -_supported_proto file sheepdog nfs -_supported_os Linux - - -# No -f, use probing for the protocol driver -QEMU_IO_PROTO="$QEMU_IO_PROG -g --cache $CACHEMODE" - -size=128M -_make_test_img $size - -echo -echo "== reading at EOF ==" -$QEMU_IO_PROTO -c "read -P 0 $size 512" "$TEST_IMG" | _filter_qemu_io - -echo -echo "== reading far past EOF ==" -$QEMU_IO_PROTO -c "read -P 0 256M 512" "$TEST_IMG" | _filter_qemu_io - -echo -echo "== writing at EOF ==" -$QEMU_IO_PROTO -c "write -P 66 $size 512" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "read -P 66 $size 512" "$TEST_IMG" | _filter_qemu_io - -echo -echo "== writing far past EOF ==" -$QEMU_IO_PROTO -c "write -P 66 256M 512" "$TEST_IMG" | _filter_qemu_io -$QEMU_IO -c "read -P 66 256M 512" "$TEST_IMG" | _filter_qemu_io - -# success, all done -echo "*** done" -rm -f $seq.full -status=0 diff --git a/tests/qemu-iotests/016.out b/tests/qemu-iotests/016.out deleted file mode 100644 index acbd60b..0000000 --- a/tests/qemu-iotests/016.out +++ /dev/null @@ -1,23 +0,0 @@ -QA output created by 016 -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 - -== reading at EOF == -read 512/512 bytes at offset 134217728 -512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== reading far past EOF == -read 512/512 bytes at offset 268435456 -512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== writing at EOF == -wrote 512/512 bytes at offset 134217728 -512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 512/512 bytes at offset 134217728 -512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) - -== writing far past EOF == -wrote 512/512 bytes at offset 268435456 -512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 512/512 bytes at offset 268435456 -512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 4b2b93b..ddfedb5 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -22,7 +22,6 @@ 013 rw auto 014 rw auto 015 rw snapshot auto -016 rw auto quick 017 rw backing auto quick 018 rw backing auto quick 019 rw backing auto quick
Remove "growable" option from the "open" command and from the qemu-io command line. qemu-io is about to be converted to BlockBackend which will make sure that no request exceeds the image size, so the only way to keep "growable" would be to use BlockBackend if it is not given and to directly access the BDS if it is. qemu-io is a debugging tool, therefore removing a rarely used option will have only a very small impact, if any. There was only one qemu-iotest which used the option; since it is not critical, this patch just removes it. Signed-off-by: Max Reitz <mreitz@redhat.com> --- qemu-io.c | 23 +++------------ tests/qemu-iotests/016 | 73 ---------------------------------------------- tests/qemu-iotests/016.out | 23 --------------- tests/qemu-iotests/group | 1 - 4 files changed, 4 insertions(+), 116 deletions(-) delete mode 100755 tests/qemu-iotests/016 delete mode 100644 tests/qemu-iotests/016.out