qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v7 0/2] Add 'offset' and 'size' options
@ 2016-10-27 12:18 Tomáš Golembiovský
  2016-10-27 12:18 ` [Qemu-devel] [PATCH v7 1/2] raw_bsd: add offset and size options Tomáš Golembiovský
  2016-10-27 12:18 ` [Qemu-devel] [PATCH v7 2/2] qemu-iotests: test 'offset' and 'size' options in raw driver Tomáš Golembiovský
  0 siblings, 2 replies; 5+ messages in thread
From: Tomáš Golembiovský @ 2016-10-27 12:18 UTC (permalink / raw)
  To: qemu-devel
  Cc: Tomáš Golembiovský, Kevin Wolf, Max Reitz,
	Markus Armbruster, Eric Blake, Daniel P . Berrange, qemu-block

v6 -> v7:
- added tests

v5 -> v6:
- fix alignment check condition
- when size is not specified and device size is being used take offset
  into account

v4 -> v5:
- added two missing overflow checks
- comments from Eric Blake:
  - renamed 'fail' label to 'end'
  - fixed optional fields in JSON scheme
  - no punctuation at the end of error_setg() message
  - spaces around PRI* macros
  - using QEMU_IS_ALIGNED
  - typos

v3 -> v4:
- fix stupid compilation error and formatting issue

v2 -> v3:
- changed overflow check to make it clearer
- produce error instead of warning when size is not multiple of sector
  size

v1 -> v2:
- options were moved from 'file' driver into 'raw' driver as suggested
- added support for writing, reopen and truncate when possible


Tomáš Golembiovský (2):
  raw_bsd: add offset and size options
  qemu-iotests: test 'offset' and 'size' options in raw driver

 block/raw_bsd.c            | 176 ++++++++++++++++++++++++++++++++++++++++++++-
 qapi/block-core.json       |  16 ++++-
 tests/qemu-iotests/171     | 170 +++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/171.out | 173 ++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 5 files changed, 532 insertions(+), 4 deletions(-)
 create mode 100755 tests/qemu-iotests/171
 create mode 100644 tests/qemu-iotests/171.out

-- 
2.10.1

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

* [Qemu-devel] [PATCH v7 1/2] raw_bsd: add offset and size options
  2016-10-27 12:18 [Qemu-devel] [PATCH v7 0/2] Add 'offset' and 'size' options Tomáš Golembiovský
@ 2016-10-27 12:18 ` Tomáš Golembiovský
  2016-10-27 13:32   ` Kevin Wolf
  2016-10-27 12:18 ` [Qemu-devel] [PATCH v7 2/2] qemu-iotests: test 'offset' and 'size' options in raw driver Tomáš Golembiovský
  1 sibling, 1 reply; 5+ messages in thread
From: Tomáš Golembiovský @ 2016-10-27 12:18 UTC (permalink / raw)
  To: qemu-devel
  Cc: Tomáš Golembiovský, Kevin Wolf, Max Reitz,
	Markus Armbruster, Eric Blake, Daniel P . Berrange, qemu-block

Added two new options 'offset' and 'size'. This makes it possible to use
only part of the file as a device. This can be used e.g. to limit the
access only to single partition in a disk image or use a disk inside a
tar archive (like OVA).

When 'size' is specified we do our best to honour it.

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 block/raw_bsd.c      | 176 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 qapi/block-core.json |  16 ++++-
 2 files changed, 188 insertions(+), 4 deletions(-)

diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 588d408..9eb187a 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -31,6 +31,30 @@
 #include "qapi/error.h"
 #include "qemu/option.h"
 
+typedef struct BDRVRawState {
+    uint64_t offset;
+    uint64_t size;
+    bool has_size;
+} BDRVRawState;
+
+static QemuOptsList raw_runtime_opts = {
+    .name = "raw",
+    .head = QTAILQ_HEAD_INITIALIZER(raw_runtime_opts.head),
+    .desc = {
+        {
+            .name = "offset",
+            .type = QEMU_OPT_SIZE,
+            .help = "offset in the disk where the image starts",
+        },
+        {
+            .name = "size",
+            .type = QEMU_OPT_SIZE,
+            .help = "virtual disk size",
+        },
+        { /* end of list */ }
+    },
+};
+
 static QemuOptsList raw_create_opts = {
     .name = "raw-create-opts",
     .head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head),
@@ -44,16 +68,108 @@ static QemuOptsList raw_create_opts = {
     }
 };
 
+static int raw_read_options(QDict *options, BlockDriverState *bs,
+    BDRVRawState *s, Error **errp)
+{
+    Error *local_err = NULL;
+    QemuOpts *opts = NULL;
+    int64_t real_size = 0;
+    int ret;
+
+    real_size = bdrv_getlength(bs->file->bs);
+    if (real_size < 0) {
+        error_setg_errno(errp, -real_size, "Could not get image size");
+        return real_size;
+    }
+
+    opts = qemu_opts_create(&raw_runtime_opts, NULL, 0, &error_abort);
+    qemu_opts_absorb_qdict(opts, options, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto end;
+    }
+
+    s->offset = qemu_opt_get_size(opts, "offset", 0);
+    if (qemu_opt_find(opts, "size") != NULL) {
+        s->size = qemu_opt_get_size(opts, "size", 0);
+        s->has_size = true;
+    } else {
+        s->has_size = false;
+        s->size = real_size - s->offset;
+    }
+
+    /* Check size and offset */
+    if (real_size < s->offset || (real_size - s->offset) < s->size) {
+        error_setg(errp, "The sum of offset (%" PRIu64 ") and size "
+            "(%" PRIu64 ") has to be smaller or equal to the "
+            " actual size of the containing file (%" PRId64 ")",
+            s->offset, s->size, real_size);
+        ret = -EINVAL;
+        goto end;
+    }
+
+    /* Make sure size is multiple of BDRV_SECTOR_SIZE to prevent rounding
+     * up and leaking out of the specified area. */
+    if (!QEMU_IS_ALIGNED(s->size, BDRV_SECTOR_SIZE)) {
+        error_setg(errp, "Specified size is not multiple of %llu",
+            BDRV_SECTOR_SIZE);
+        ret = -EINVAL;
+        goto end;
+    }
+
+    ret = 0;
+
+end:
+
+    qemu_opts_del(opts);
+
+    return ret;
+}
+
 static int raw_reopen_prepare(BDRVReopenState *reopen_state,
                               BlockReopenQueue *queue, Error **errp)
 {
-    return 0;
+    assert(reopen_state != NULL);
+    assert(reopen_state->bs != NULL);
+
+    reopen_state->opaque = g_new0(BDRVRawState, 1);
+
+    return raw_read_options(
+        reopen_state->options,
+        reopen_state->bs,
+        reopen_state->opaque,
+        errp);
+}
+
+static void raw_reopen_commit(BDRVReopenState *state)
+{
+    BDRVRawState *new_s = state->opaque;
+    BDRVRawState *s = state->bs->opaque;
+
+    memcpy(s, new_s, sizeof(BDRVRawState));
+
+    g_free(state->opaque);
+    state->opaque = NULL;
+}
+
+static void raw_reopen_abort(BDRVReopenState *state)
+{
+    g_free(state->opaque);
+    state->opaque = NULL;
 }
 
 static int coroutine_fn raw_co_preadv(BlockDriverState *bs, uint64_t offset,
                                       uint64_t bytes, QEMUIOVector *qiov,
                                       int flags)
 {
+    BDRVRawState *s = bs->opaque;
+
+    if (offset > UINT64_MAX - s->offset) {
+        return -EINVAL;
+    }
+    offset += s->offset;
+
     BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
     return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
 }
@@ -62,11 +178,23 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, uint64_t offset,
                                        uint64_t bytes, QEMUIOVector *qiov,
                                        int flags)
 {
+    BDRVRawState *s = bs->opaque;
     void *buf = NULL;
     BlockDriver *drv;
     QEMUIOVector local_qiov;
     int ret;
 
+    if (s->has_size && (offset > s->size || bytes > (s->size - offset))) {
+        /* There's not enough space for the data. Don't write anything and just
+         * fail to prevent leaking out of the size specified in options. */
+        return -ENOSPC;
+    }
+
+    if (offset > UINT64_MAX - s->offset) {
+        ret = -EINVAL;
+        goto fail;
+    }
+
     if (bs->probed && offset < BLOCK_PROBE_BUF_SIZE && bytes) {
         /* Handling partial writes would be a pain - so we just
          * require that guests have 512-byte request alignment if
@@ -101,6 +229,8 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, uint64_t offset,
         qiov = &local_qiov;
     }
 
+    offset += s->offset;
+
     BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
     ret = bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
 
@@ -117,8 +247,10 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
                                             int nb_sectors, int *pnum,
                                             BlockDriverState **file)
 {
+    BDRVRawState *s = bs->opaque;
     *pnum = nb_sectors;
     *file = bs->file->bs;
+    sector_num += s->offset / BDRV_SECTOR_SIZE;
     return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
            (sector_num << BDRV_SECTOR_BITS);
 }
@@ -138,7 +270,28 @@ static int coroutine_fn raw_co_pdiscard(BlockDriverState *bs,
 
 static int64_t raw_getlength(BlockDriverState *bs)
 {
-    return bdrv_getlength(bs->file->bs);
+    int64_t len;
+    BDRVRawState *s = bs->opaque;
+
+    /* Update size. It should not change unless the file was externally
+     * modified. */
+    len = bdrv_getlength(bs->file->bs);
+    if (len < 0) {
+        return len;
+    }
+
+    if (len < s->offset) {
+        s->size = 0;
+    } else {
+        if (s->has_size) {
+            /* Try to honour the size */
+            s->size = MIN(s->size, len - s->offset);
+        } else {
+            s->size = len - s->offset;
+        }
+    }
+
+    return s->size;
 }
 
 static int raw_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
@@ -158,6 +311,18 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
 
 static int raw_truncate(BlockDriverState *bs, int64_t offset)
 {
+    BDRVRawState *s = bs->opaque;
+
+    if (s->has_size) {
+        return -ENOTSUP;
+    }
+
+    if (INT64_MAX - offset < s->offset) {
+        return -EINVAL;
+    }
+
+    s->size = offset;
+    offset += s->offset;
     return bdrv_truncate(bs->file->bs, offset);
 }
 
@@ -197,6 +362,8 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
 static int raw_open(BlockDriverState *bs, QDict *options, int flags,
                     Error **errp)
 {
+    BDRVRawState *s = bs->opaque;
+
     bs->sg = bs->file->bs->sg;
     bs->supported_write_flags = BDRV_REQ_FUA &
         bs->file->bs->supported_write_flags;
@@ -214,7 +381,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
                 bs->file->bs->filename);
     }
 
-    return 0;
+    return raw_read_options(options, bs, s, errp);
 }
 
 static void raw_close(BlockDriverState *bs)
@@ -241,8 +408,11 @@ static int raw_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
 
 BlockDriver bdrv_raw = {
     .format_name          = "raw",
+    .instance_size        = sizeof(BDRVRawState),
     .bdrv_probe           = &raw_probe,
     .bdrv_reopen_prepare  = &raw_reopen_prepare,
+    .bdrv_reopen_commit   = &raw_reopen_commit,
+    .bdrv_reopen_abort    = &raw_reopen_abort,
     .bdrv_open            = &raw_open,
     .bdrv_close           = &raw_close,
     .bdrv_create          = &raw_create,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9d797b8..186ae75 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2224,6 +2224,20 @@
   'data': { 'filename': 'str' } }
 
 ##
+# @BlockdevOptionsRaw
+#
+# Driver specific block device options for the raw driver.
+#
+# @offset:      #optional position where the block device starts
+# @size:        #optional the assumed size of the device
+#
+# Since: 2.8
+##
+{ 'struct': 'BlockdevOptionsRaw',
+  'base': 'BlockdevOptionsGenericFormat',
+  'data': { '*offset': 'int', '*size': 'int' } }
+
+##
 # @BlockdevOptions
 #
 # Options for creating a block device.  Many options are available for all
@@ -2277,7 +2291,7 @@
       'qcow':       'BlockdevOptionsGenericCOWFormat',
       'qed':        'BlockdevOptionsGenericCOWFormat',
       'quorum':     'BlockdevOptionsQuorum',
-      'raw':        'BlockdevOptionsGenericFormat',
+      'raw':        'BlockdevOptionsRaw',
 # TODO rbd: Wait for structured options
       'replication':'BlockdevOptionsReplication',
 # TODO sheepdog: Wait for structured options
-- 
2.10.1

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

* [Qemu-devel] [PATCH v7 2/2] qemu-iotests: test 'offset' and 'size' options in raw driver
  2016-10-27 12:18 [Qemu-devel] [PATCH v7 0/2] Add 'offset' and 'size' options Tomáš Golembiovský
  2016-10-27 12:18 ` [Qemu-devel] [PATCH v7 1/2] raw_bsd: add offset and size options Tomáš Golembiovský
@ 2016-10-27 12:18 ` Tomáš Golembiovský
  2016-10-27 13:47   ` Kevin Wolf
  1 sibling, 1 reply; 5+ messages in thread
From: Tomáš Golembiovský @ 2016-10-27 12:18 UTC (permalink / raw)
  To: qemu-devel
  Cc: Tomáš Golembiovský, Kevin Wolf, Max Reitz,
	Markus Armbruster, Eric Blake, Daniel P . Berrange, qemu-block

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 tests/qemu-iotests/171     | 170 ++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/171.out | 173 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 344 insertions(+)
 create mode 100755 tests/qemu-iotests/171
 create mode 100644 tests/qemu-iotests/171.out

diff --git a/tests/qemu-iotests/171 b/tests/qemu-iotests/171
new file mode 100755
index 0000000..3bdc680
--- /dev/null
+++ b/tests/qemu-iotests/171
@@ -0,0 +1,170 @@
+#!/bin/bash
+#
+# Test 'offset' and 'size' options of the raw driver. Make sure we can't
+# (or can) read and write outside of the image size.
+#
+# Copyright (C) 2016 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=tgolembi@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+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
+_supported_os Linux
+
+
+# Create options string
+img_json() {
+    echo -n 'json:{"driver":"raw", '
+    echo -n "\"offset\":\"$img_offset\", "
+    if [ "$img_size" -ne -1 ] ; then
+        echo -n "\"size\":\"$img_size\", "
+    fi
+    echo -n '"file": {'
+    echo -n    '"driver":"file", '
+    echo -n    "\"filename\":\"$TEST_IMG\" "
+    echo -n "} }"
+}
+
+do_general_test() {
+    echo
+    echo "write to image"
+    $QEMU_IO -c "write -P 0x0a 0 $test_size" "$(img_json)" | _filter_qemu_io
+
+    echo
+    echo "check that offset is respected"
+    $QEMU_IO -c "read -v $((img_offset-2)) 4" $TEST_IMG | _filter_qemu_io
+
+    echo
+    echo "write before image boundary"
+    $QEMU_IO -c "write $((test_size-1)) 1" "$(img_json)" | _filter_qemu_io
+
+    echo
+    echo "write across image boundary"
+    $QEMU_IO -c "write $((test_size-1)) 2" "$(img_json)" | _filter_qemu_io
+
+    echo
+    echo "write at image boundary"
+    $QEMU_IO -c "write $test_size 1" "$(img_json)" | _filter_qemu_io
+
+    echo
+    echo "write after image boundary"
+    $QEMU_IO -c "write $((test_size+512)) 1" "$(img_json)" | _filter_qemu_io
+
+    echo
+    echo "writev before/after image boundary"
+    $QEMU_IO -c "writev $((test_size-512)) 512 512" "$(img_json)" | _filter_qemu_io
+
+    echo
+    echo "read the image"
+    $QEMU_IO -c "read 0 $test_size" "$(img_json)" | _filter_qemu_io
+
+    echo
+    echo "read before image boundary"
+    $QEMU_IO -c "read $((test_size-1)) 1" "$(img_json)" | _filter_qemu_io
+
+    echo
+    echo "read across image boundary"
+    $QEMU_IO -c "read $((test_size-1)) 2" "$(img_json)" | _filter_qemu_io
+
+    echo
+    echo "read at image boundary"
+    $QEMU_IO -c "read $test_size 1" "$(img_json)" | _filter_qemu_io
+
+    echo
+    echo "read after image boundary"
+    $QEMU_IO -c "read $((test_size+512)) 1" "$(img_json)" | _filter_qemu_io
+
+    echo
+    echo "readv before/after image boundary"
+    $QEMU_IO -c "readv $((test_size-512)) 512 512" "$(img_json)" | _filter_qemu_io
+}
+
+echo
+echo "== test 'offset' option =="
+size=4096
+img_offset=512
+img_size=-1
+test_size=$((size-img_offset))
+_make_test_img $size
+do_general_test
+_cleanup_test_img
+
+echo
+echo "== test 'offset' and 'size' options =="
+size=4096
+img_offset=512
+img_size=2048
+test_size=$img_size
+_make_test_img $size
+do_general_test
+_cleanup_test_img
+
+echo
+echo "== test misaligned 'offset' =="
+size=4096
+img_offset=10
+img_size=2048
+test_size=$img_size
+_make_test_img $size
+do_general_test
+_cleanup_test_img
+
+echo
+echo "== test reopen =="
+img_offset=512
+img_size=512
+_make_test_img $size
+(
+$QEMU_IO "$(img_json)"  <<EOT
+write -P 0x0a 0 512
+write -P 0x0a 511 1
+write -P 0x0a 512 1
+reopen -o driver=raw,offset=1536,size=1024
+write -P 0x0a 0 1024
+write -P 0x0a 1023 1
+write -P 0x0a 1024 1
+EOT
+) | _filter_qemu_io
+echo "checking boundaries"
+$QEMU_IO -c "read -v 510 4" $TEST_IMG | _filter_qemu_io
+$QEMU_IO -c "read -v 1022 4" $TEST_IMG | _filter_qemu_io
+$QEMU_IO -c "read -v 1534 4" $TEST_IMG | _filter_qemu_io
+$QEMU_IO -c "read -v 2558 4" $TEST_IMG | _filter_qemu_io
+_cleanup_test_img
+
+# success, all done
+echo
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/171.out b/tests/qemu-iotests/171.out
new file mode 100644
index 0000000..b69329d
--- /dev/null
+++ b/tests/qemu-iotests/171.out
@@ -0,0 +1,173 @@
+QA output created by 171
+
+== test 'offset' option ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4096
+
+write to image
+wrote 3584/3584 bytes at offset 0
+3.500 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+check that offset is respected
+000001fe:  00 00 0a 0a  ....
+read 4/4 bytes at offset 510
+4 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+write before image boundary
+wrote 1/1 bytes at offset 3583
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+write across image boundary
+write failed: Input/output error
+
+write at image boundary
+write failed: Input/output error
+
+write after image boundary
+write failed: Input/output error
+
+writev before/after image boundary
+writev failed: Input/output error
+
+read the image
+read 3584/3584 bytes at offset 0
+3.500 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+read before image boundary
+read 1/1 bytes at offset 3583
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+read across image boundary
+read failed: Input/output error
+
+read at image boundary
+read failed: Input/output error
+
+read after image boundary
+read failed: Input/output error
+
+readv before/after image boundary
+readv failed: Input/output error
+
+== test 'offset' and 'size' options ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4096
+
+write to image
+wrote 2048/2048 bytes at offset 0
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+check that offset is respected
+000001fe:  00 00 0a 0a  ....
+read 4/4 bytes at offset 510
+4 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+write before image boundary
+wrote 1/1 bytes at offset 2047
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+write across image boundary
+write failed: Input/output error
+
+write at image boundary
+write failed: Input/output error
+
+write after image boundary
+write failed: Input/output error
+
+writev before/after image boundary
+writev failed: Input/output error
+
+read the image
+read 2048/2048 bytes at offset 0
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+read before image boundary
+read 1/1 bytes at offset 2047
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+read across image boundary
+read failed: Input/output error
+
+read at image boundary
+read failed: Input/output error
+
+read after image boundary
+read failed: Input/output error
+
+readv before/after image boundary
+readv failed: Input/output error
+
+== test misaligned 'offset' ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4096
+
+write to image
+wrote 2048/2048 bytes at offset 0
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+check that offset is respected
+00000008:  00 00 0a 0a  ....
+read 4/4 bytes at offset 8
+4 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+write before image boundary
+wrote 1/1 bytes at offset 2047
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+write across image boundary
+write failed: Input/output error
+
+write at image boundary
+write failed: Input/output error
+
+write after image boundary
+write failed: Input/output error
+
+writev before/after image boundary
+writev failed: Input/output error
+
+read the image
+read 2048/2048 bytes at offset 0
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+read before image boundary
+read 1/1 bytes at offset 2047
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+read across image boundary
+read failed: Input/output error
+
+read at image boundary
+read failed: Input/output error
+
+read after image boundary
+read failed: Input/output error
+
+readv before/after image boundary
+readv failed: Input/output error
+
+== test reopen ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4096
+wrote 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1/1 bytes at offset 511
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+write failed: Input/output error
+wrote 1024/1024 bytes at offset 0
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1/1 bytes at offset 1023
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+write failed: Input/output error
+checking boundaries
+000001fe:  00 00 0a 0a  ....
+read 4/4 bytes at offset 510
+4 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+000003fe:  0a 0a 00 00  ....
+read 4/4 bytes at offset 1022
+4 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+000005fe:  00 00 0a 0a  ....
+read 4/4 bytes at offset 1534
+4 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+000009fe:  0a 0a 00 00  ....
+read 4/4 bytes at offset 2558
+4 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 7eb1770..d574597 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -162,3 +162,4 @@
 160 rw auto quick
 162 auto quick
 170 rw auto quick
+171 rw auto quick
-- 
2.10.1

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

* Re: [Qemu-devel] [PATCH v7 1/2] raw_bsd: add offset and size options
  2016-10-27 12:18 ` [Qemu-devel] [PATCH v7 1/2] raw_bsd: add offset and size options Tomáš Golembiovský
@ 2016-10-27 13:32   ` Kevin Wolf
  0 siblings, 0 replies; 5+ messages in thread
From: Kevin Wolf @ 2016-10-27 13:32 UTC (permalink / raw)
  To: Tomáš Golembiovský
  Cc: qemu-devel, Max Reitz, Markus Armbruster, Eric Blake,
	Daniel P . Berrange, qemu-block

Am 27.10.2016 um 14:18 hat Tomáš Golembiovský geschrieben:
> Added two new options 'offset' and 'size'. This makes it possible to use
> only part of the file as a device. This can be used e.g. to limit the
> access only to single partition in a disk image or use a disk inside a
> tar archive (like OVA).
> 
> When 'size' is specified we do our best to honour it.
> 
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>

The changes that you made look correct to me, but I'm afraid they aren't
complete: .bdrv_co_pwrite_zeroes() and .bdrv_co_pdiscard() don't
consider the offset and size, so calling these when an offset is used
will result in data corruption.

It might also be necessary to return -ENOTSUP in .bdrv_probe_geometry()
because we may return geometry for the real image file size rather than
the part that is visible for the guest.

I didn't see any other callbacks that involve offsets, but please
double-check.

Kevin

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

* Re: [Qemu-devel] [PATCH v7 2/2] qemu-iotests: test 'offset' and 'size' options in raw driver
  2016-10-27 12:18 ` [Qemu-devel] [PATCH v7 2/2] qemu-iotests: test 'offset' and 'size' options in raw driver Tomáš Golembiovský
@ 2016-10-27 13:47   ` Kevin Wolf
  0 siblings, 0 replies; 5+ messages in thread
From: Kevin Wolf @ 2016-10-27 13:47 UTC (permalink / raw)
  To: Tomáš Golembiovský
  Cc: qemu-devel, Max Reitz, Markus Armbruster, Eric Blake,
	Daniel P . Berrange, qemu-block

Am 27.10.2016 um 14:18 hat Tomáš Golembiovský geschrieben:
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>

Like in the implementation, this could use tests for discard and zero
writes (write -z in qemu-io).

>  tests/qemu-iotests/171     | 170 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/171.out | 173 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 344 insertions(+)
>  create mode 100755 tests/qemu-iotests/171
>  create mode 100644 tests/qemu-iotests/171.out
> 
> diff --git a/tests/qemu-iotests/171 b/tests/qemu-iotests/171
> new file mode 100755
> index 0000000..3bdc680
> --- /dev/null
> +++ b/tests/qemu-iotests/171
> @@ -0,0 +1,170 @@
> +#!/bin/bash
> +#
> +# Test 'offset' and 'size' options of the raw driver. Make sure we can't
> +# (or can) read and write outside of the image size.
> +#
> +# Copyright (C) 2016 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=tgolembi@redhat.com
> +
> +seq=`basename $0`
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +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
> +_supported_os Linux
> +
> +
> +# Create options string
> +img_json() {
> +    echo -n 'json:{"driver":"raw", '
> +    echo -n "\"offset\":\"$img_offset\", "
> +    if [ "$img_size" -ne -1 ] ; then
> +        echo -n "\"size\":\"$img_size\", "
> +    fi
> +    echo -n '"file": {'
> +    echo -n    '"driver":"file", '
> +    echo -n    "\"filename\":\"$TEST_IMG\" "
> +    echo -n "} }"
> +}
> +
> +do_general_test() {
> +    echo
> +    echo "write to image"
> +    $QEMU_IO -c "write -P 0x0a 0 $test_size" "$(img_json)" | _filter_qemu_io
> +
> +    echo
> +    echo "check that offset is respected"
> +    $QEMU_IO -c "read -v $((img_offset-2)) 4" $TEST_IMG | _filter_qemu_io
> +
> +    echo
> +    echo "write before image boundary"
> +    $QEMU_IO -c "write $((test_size-1)) 1" "$(img_json)" | _filter_qemu_io
> +
> +    echo
> +    echo "write across image boundary"
> +    $QEMU_IO -c "write $((test_size-1)) 2" "$(img_json)" | _filter_qemu_io
> +
> +    echo
> +    echo "write at image boundary"
> +    $QEMU_IO -c "write $test_size 1" "$(img_json)" | _filter_qemu_io
> +
> +    echo
> +    echo "write after image boundary"
> +    $QEMU_IO -c "write $((test_size+512)) 1" "$(img_json)" | _filter_qemu_io
> +
> +    echo
> +    echo "writev before/after image boundary"
> +    $QEMU_IO -c "writev $((test_size-512)) 512 512" "$(img_json)" | _filter_qemu_io
> +
> +    echo
> +    echo "read the image"
> +    $QEMU_IO -c "read 0 $test_size" "$(img_json)" | _filter_qemu_io

If you move this up, you could use -P 0x0a to verify that you really
read at the right offset. Otherwise you don't have any read with offset
that actually checks what has been read.

The rest looks good at the first sight.

Kevin

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

end of thread, other threads:[~2016-10-27 13:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-27 12:18 [Qemu-devel] [PATCH v7 0/2] Add 'offset' and 'size' options Tomáš Golembiovský
2016-10-27 12:18 ` [Qemu-devel] [PATCH v7 1/2] raw_bsd: add offset and size options Tomáš Golembiovský
2016-10-27 13:32   ` Kevin Wolf
2016-10-27 12:18 ` [Qemu-devel] [PATCH v7 2/2] qemu-iotests: test 'offset' and 'size' options in raw driver Tomáš Golembiovský
2016-10-27 13:47   ` Kevin Wolf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).