All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/12] Block patches
@ 2015-01-23 18:20 Kevin Wolf
  2015-01-23 18:20 ` [Qemu-devel] [PULL 01/12] virtio-blk: Pass req to virtio_blk_handle_scsi_req Kevin Wolf
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Kevin Wolf @ 2015-01-23 18:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

The following changes since commit c6441452b50c44fdbb362b239ce623f77cf3cd51:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-audio-20150122-1' into staging (2015-01-22 18:57:36 +0000)

are available in the git repository at:


  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to bc63781ca350cad4c9eb142ce8f55bfaded4276e:

  Merge remote-tracking branch 'mreitz/block' into queue-block (2015-01-23 18:51:47 +0100)

----------------------------------------------------------------

Block patches for 2.3

----------------------------------------------------------------
Fam Zheng (2):
      virtio-blk: Pass req to virtio_blk_handle_scsi_req
      virtio-blk: Use blk_aio_ioctl

Jeff Cody (7):
      block: vmdk - make ret variable usage clear
      block: vmdk - move string allocations from stack to the heap
      block: qapi - move string allocation from stack to the heap
      block: remove unused variable in bdrv_commit
      block: mirror - change string allocation to 2-bytes
      block: update string sizes for filename,backing_file,exact_filename
      block: vhdx - force FileOffsetMB field to '0' for certain block states

Kevin Wolf (1):
      Merge remote-tracking branch 'mreitz/block' into queue-block

Max Reitz (3):
      qcow2: Add two more unalignment checks
      iotests: Add tests for more corruption cases
      iotests: Lower 064's memory usage

 block.c                        |   3 -
 block/mirror.c                 |   3 +-
 block/qapi.c                   |   7 ++-
 block/qcow.c                   |   2 +-
 block/qcow2-cluster.c          |  21 +++++++
 block/qcow2.c                  |   3 +-
 block/vhdx.c                   |  13 +++-
 block/vmdk.c                   |  51 +++++++++-------
 block/vvfat.c                  |   4 +-
 hw/block/virtio-blk.c          | 134 ++++++++++++++++++++++++++---------------
 include/block/block_int.h      |   8 +--
 include/hw/virtio/virtio-blk.h |   3 -
 qemu-img.c                     |   4 +-
 tests/qemu-iotests/060         |  15 +++++
 tests/qemu-iotests/060.out     |  13 ++++
 tests/qemu-iotests/064         |  19 +++++-
 tests/qemu-iotests/064.out     |  34 +++++++++--
 17 files changed, 239 insertions(+), 98 deletions(-)

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

* [Qemu-devel] [PULL 01/12] virtio-blk: Pass req to virtio_blk_handle_scsi_req
  2015-01-23 18:20 [Qemu-devel] [PULL 00/12] Block patches Kevin Wolf
@ 2015-01-23 18:20 ` Kevin Wolf
  2015-01-23 18:20 ` [Qemu-devel] [PULL 02/12] virtio-blk: Use blk_aio_ioctl Kevin Wolf
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2015-01-23 18:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

From: Fam Zheng <famz@redhat.com>

In preparation for calling blk_aio_ioctl. Also make the function static
as no other files need it.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/block/virtio-blk.c          | 9 +++++----
 include/hw/virtio/virtio-blk.h | 3 ---
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index b19b102..60cb1d8 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -127,12 +127,13 @@ static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock *s)
     return req;
 }
 
-int virtio_blk_handle_scsi_req(VirtIOBlock *blk,
-                               VirtQueueElement *elem)
+static int virtio_blk_handle_scsi_req(VirtIOBlockReq *req)
 {
     int status = VIRTIO_BLK_S_OK;
     struct virtio_scsi_inhdr *scsi = NULL;
-    VirtIODevice *vdev = VIRTIO_DEVICE(blk);
+    VirtIODevice *vdev = VIRTIO_DEVICE(req->dev);
+    VirtQueueElement *elem = &req->elem;
+    VirtIOBlock *blk = req->dev;
 
 #ifdef __linux__
     int i;
@@ -252,7 +253,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
 {
     int status;
 
-    status = virtio_blk_handle_scsi_req(req->dev, &req->elem);
+    status = virtio_blk_handle_scsi_req(req);
     virtio_blk_req_complete(req, status);
     virtio_blk_free_request(req);
 }
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 3979dc4..4652b70 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -153,9 +153,6 @@ VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s);
 
 void virtio_blk_free_request(VirtIOBlockReq *req);
 
-int virtio_blk_handle_scsi_req(VirtIOBlock *blk,
-                               VirtQueueElement *elem);
-
 void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb);
 
 void virtio_submit_multiwrite(BlockBackend *blk, MultiReqBuffer *mrb);
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 02/12] virtio-blk: Use blk_aio_ioctl
  2015-01-23 18:20 [Qemu-devel] [PULL 00/12] Block patches Kevin Wolf
  2015-01-23 18:20 ` [Qemu-devel] [PULL 01/12] virtio-blk: Pass req to virtio_blk_handle_scsi_req Kevin Wolf
@ 2015-01-23 18:20 ` Kevin Wolf
  2015-01-23 18:20 ` [Qemu-devel] [PULL 03/12] qcow2: Add two more unalignment checks Kevin Wolf
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2015-01-23 18:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

From: Fam Zheng <famz@redhat.com>

Use the asynchronous interface of ioctl. This will not make the VM
unresponsive if the ioctl takes a long time.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/block/virtio-blk.c | 125 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 79 insertions(+), 46 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 60cb1d8..4032fca 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -115,6 +115,56 @@ static void virtio_blk_flush_complete(void *opaque, int ret)
     virtio_blk_free_request(req);
 }
 
+#ifdef __linux__
+
+typedef struct {
+    VirtIOBlockReq *req;
+    struct sg_io_hdr hdr;
+} VirtIOBlockIoctlReq;
+
+static void virtio_blk_ioctl_complete(void *opaque, int status)
+{
+    VirtIOBlockIoctlReq *ioctl_req = opaque;
+    VirtIOBlockReq *req = ioctl_req->req;
+    VirtIODevice *vdev = VIRTIO_DEVICE(req->dev);
+    struct virtio_scsi_inhdr *scsi;
+    struct sg_io_hdr *hdr;
+
+    scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
+
+    if (status) {
+        status = VIRTIO_BLK_S_UNSUPP;
+        virtio_stl_p(vdev, &scsi->errors, 255);
+        goto out;
+    }
+
+    hdr = &ioctl_req->hdr;
+    /*
+     * From SCSI-Generic-HOWTO: "Some lower level drivers (e.g. ide-scsi)
+     * clear the masked_status field [hence status gets cleared too, see
+     * block/scsi_ioctl.c] even when a CHECK_CONDITION or COMMAND_TERMINATED
+     * status has occurred.  However they do set DRIVER_SENSE in driver_status
+     * field. Also a (sb_len_wr > 0) indicates there is a sense buffer.
+     */
+    if (hdr->status == 0 && hdr->sb_len_wr > 0) {
+        hdr->status = CHECK_CONDITION;
+    }
+
+    virtio_stl_p(vdev, &scsi->errors,
+                 hdr->status | (hdr->msg_status << 8) |
+                 (hdr->host_status << 16) | (hdr->driver_status << 24));
+    virtio_stl_p(vdev, &scsi->residual, hdr->resid);
+    virtio_stl_p(vdev, &scsi->sense_len, hdr->sb_len_wr);
+    virtio_stl_p(vdev, &scsi->data_len, hdr->dxfer_len);
+
+out:
+    virtio_blk_req_complete(req, status);
+    virtio_blk_free_request(req);
+    g_free(ioctl_req);
+}
+
+#endif
+
 static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock *s)
 {
     VirtIOBlockReq *req = virtio_blk_alloc_request(s);
@@ -137,7 +187,7 @@ static int virtio_blk_handle_scsi_req(VirtIOBlockReq *req)
 
 #ifdef __linux__
     int i;
-    struct sg_io_hdr hdr;
+    VirtIOBlockIoctlReq *ioctl_req;
 #endif
 
     /*
@@ -172,71 +222,52 @@ static int virtio_blk_handle_scsi_req(VirtIOBlockReq *req)
     }
 
 #ifdef __linux__
-    memset(&hdr, 0, sizeof(struct sg_io_hdr));
-    hdr.interface_id = 'S';
-    hdr.cmd_len = elem->out_sg[1].iov_len;
-    hdr.cmdp = elem->out_sg[1].iov_base;
-    hdr.dxfer_len = 0;
+    ioctl_req = g_new0(VirtIOBlockIoctlReq, 1);
+    ioctl_req->req = req;
+    ioctl_req->hdr.interface_id = 'S';
+    ioctl_req->hdr.cmd_len = elem->out_sg[1].iov_len;
+    ioctl_req->hdr.cmdp = elem->out_sg[1].iov_base;
+    ioctl_req->hdr.dxfer_len = 0;
 
     if (elem->out_num > 2) {
         /*
          * If there are more than the minimally required 2 output segments
          * there is write payload starting from the third iovec.
          */
-        hdr.dxfer_direction = SG_DXFER_TO_DEV;
-        hdr.iovec_count = elem->out_num - 2;
+        ioctl_req->hdr.dxfer_direction = SG_DXFER_TO_DEV;
+        ioctl_req->hdr.iovec_count = elem->out_num - 2;
 
-        for (i = 0; i < hdr.iovec_count; i++)
-            hdr.dxfer_len += elem->out_sg[i + 2].iov_len;
+        for (i = 0; i < ioctl_req->hdr.iovec_count; i++) {
+            ioctl_req->hdr.dxfer_len += elem->out_sg[i + 2].iov_len;
+        }
 
-        hdr.dxferp = elem->out_sg + 2;
+        ioctl_req->hdr.dxferp = elem->out_sg + 2;
 
     } else if (elem->in_num > 3) {
         /*
          * If we have more than 3 input segments the guest wants to actually
          * read data.
          */
-        hdr.dxfer_direction = SG_DXFER_FROM_DEV;
-        hdr.iovec_count = elem->in_num - 3;
-        for (i = 0; i < hdr.iovec_count; i++)
-            hdr.dxfer_len += elem->in_sg[i].iov_len;
+        ioctl_req->hdr.dxfer_direction = SG_DXFER_FROM_DEV;
+        ioctl_req->hdr.iovec_count = elem->in_num - 3;
+        for (i = 0; i < ioctl_req->hdr.iovec_count; i++) {
+            ioctl_req->hdr.dxfer_len += elem->in_sg[i].iov_len;
+        }
 
-        hdr.dxferp = elem->in_sg;
+        ioctl_req->hdr.dxferp = elem->in_sg;
     } else {
         /*
          * Some SCSI commands don't actually transfer any data.
          */
-        hdr.dxfer_direction = SG_DXFER_NONE;
+        ioctl_req->hdr.dxfer_direction = SG_DXFER_NONE;
     }
 
-    hdr.sbp = elem->in_sg[elem->in_num - 3].iov_base;
-    hdr.mx_sb_len = elem->in_sg[elem->in_num - 3].iov_len;
-
-    status = blk_ioctl(blk->blk, SG_IO, &hdr);
-    if (status) {
-        status = VIRTIO_BLK_S_UNSUPP;
-        goto fail;
-    }
+    ioctl_req->hdr.sbp = elem->in_sg[elem->in_num - 3].iov_base;
+    ioctl_req->hdr.mx_sb_len = elem->in_sg[elem->in_num - 3].iov_len;
 
-    /*
-     * From SCSI-Generic-HOWTO: "Some lower level drivers (e.g. ide-scsi)
-     * clear the masked_status field [hence status gets cleared too, see
-     * block/scsi_ioctl.c] even when a CHECK_CONDITION or COMMAND_TERMINATED
-     * status has occurred.  However they do set DRIVER_SENSE in driver_status
-     * field. Also a (sb_len_wr > 0) indicates there is a sense buffer.
-     */
-    if (hdr.status == 0 && hdr.sb_len_wr > 0) {
-        hdr.status = CHECK_CONDITION;
-    }
-
-    virtio_stl_p(vdev, &scsi->errors,
-                 hdr.status | (hdr.msg_status << 8) |
-                 (hdr.host_status << 16) | (hdr.driver_status << 24));
-    virtio_stl_p(vdev, &scsi->residual, hdr.resid);
-    virtio_stl_p(vdev, &scsi->sense_len, hdr.sb_len_wr);
-    virtio_stl_p(vdev, &scsi->data_len, hdr.dxfer_len);
-
-    return status;
+    blk_aio_ioctl(blk->blk, SG_IO, &ioctl_req->hdr,
+                  virtio_blk_ioctl_complete, ioctl_req);
+    return -EINPROGRESS;
 #else
     abort();
 #endif
@@ -254,8 +285,10 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
     int status;
 
     status = virtio_blk_handle_scsi_req(req);
-    virtio_blk_req_complete(req, status);
-    virtio_blk_free_request(req);
+    if (status != -EINPROGRESS) {
+        virtio_blk_req_complete(req, status);
+        virtio_blk_free_request(req);
+    }
 }
 
 void virtio_submit_multiwrite(BlockBackend *blk, MultiReqBuffer *mrb)
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 03/12] qcow2: Add two more unalignment checks
  2015-01-23 18:20 [Qemu-devel] [PULL 00/12] Block patches Kevin Wolf
  2015-01-23 18:20 ` [Qemu-devel] [PULL 01/12] virtio-blk: Pass req to virtio_blk_handle_scsi_req Kevin Wolf
  2015-01-23 18:20 ` [Qemu-devel] [PULL 02/12] virtio-blk: Use blk_aio_ioctl Kevin Wolf
@ 2015-01-23 18:20 ` Kevin Wolf
  2015-01-23 18:20 ` [Qemu-devel] [PULL 04/12] iotests: Add tests for more corruption cases Kevin Wolf
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2015-01-23 18:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

From: Max Reitz <mreitz@redhat.com>

This adds checks for unaligned L2 table offsets and unaligned data
cluster offsets (actually the preallocated offsets for zero clusters) to
the zero cluster expansion function.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cluster.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 1fea514..183177d 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1651,6 +1651,14 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
             continue;
         }
 
+        if (offset_into_cluster(s, l2_offset)) {
+            qcow2_signal_corruption(bs, true, -1, -1, "L2 table offset %#"
+                                    PRIx64 " unaligned (L1 index: %#x)",
+                                    l2_offset, i);
+            ret = -EIO;
+            goto fail;
+        }
+
         if (is_active_l1) {
             /* get active L2 tables from cache */
             ret = qcow2_cache_get(bs, s->l2_table_cache, l2_offset,
@@ -1709,6 +1717,19 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
                 }
             }
 
+            if (offset_into_cluster(s, offset)) {
+                qcow2_signal_corruption(bs, true, -1, -1, "Data cluster offset "
+                                        "%#" PRIx64 " unaligned (L2 offset: %#"
+                                        PRIx64 ", L2 index: %#x)", offset,
+                                        l2_offset, j);
+                if (!preallocated) {
+                    qcow2_free_clusters(bs, offset, s->cluster_size,
+                                        QCOW2_DISCARD_ALWAYS);
+                }
+                ret = -EIO;
+                goto fail;
+            }
+
             ret = qcow2_pre_write_overlap_check(bs, 0, offset, s->cluster_size);
             if (ret < 0) {
                 if (!preallocated) {
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 04/12] iotests: Add tests for more corruption cases
  2015-01-23 18:20 [Qemu-devel] [PULL 00/12] Block patches Kevin Wolf
                   ` (2 preceding siblings ...)
  2015-01-23 18:20 ` [Qemu-devel] [PULL 03/12] qcow2: Add two more unalignment checks Kevin Wolf
@ 2015-01-23 18:20 ` Kevin Wolf
  2015-01-23 18:20 ` [Qemu-devel] [PULL 05/12] block: vmdk - make ret variable usage clear Kevin Wolf
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2015-01-23 18:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

From: Max Reitz <mreitz@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/060     | 15 +++++++++++++++
 tests/qemu-iotests/060.out | 13 +++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index 73863bf..c81319c 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -186,6 +186,12 @@ $QEMU_IO -c "write 0 64k" "$TEST_IMG" | _filter_qemu_io
 poke_file "$TEST_IMG" "$l1_offset" "\x80\x00\x00\x00\x00\x04\x2a\x00"
 $QEMU_IO -c "read 0 64k" "$TEST_IMG" | _filter_qemu_io
 
+# Test how well zero cluster expansion can cope with this
+_make_test_img 64M
+$QEMU_IO -c "write 0 64k" "$TEST_IMG" | _filter_qemu_io
+poke_file "$TEST_IMG" "$l1_offset" "\x80\x00\x00\x00\x00\x04\x2a\x00"
+$QEMU_IMG amend -o compat=0.10 "$TEST_IMG"
+
 echo
 echo "=== Testing unaligned L2 entry ==="
 echo
@@ -195,6 +201,15 @@ poke_file "$TEST_IMG" "$l2_offset" "\x80\x00\x00\x00\x00\x05\x2a\x00"
 $QEMU_IO -c "read 0 64k" "$TEST_IMG" | _filter_qemu_io
 
 echo
+echo "=== Testing unaligned pre-allocated zero cluster ==="
+echo
+_make_test_img 64M
+$QEMU_IO -c "write 0 64k" "$TEST_IMG" | _filter_qemu_io
+poke_file "$TEST_IMG" "$l2_offset" "\x80\x00\x00\x00\x00\x05\x2a\x01"
+# zero cluster expansion
+$QEMU_IMG amend -o compat=0.10 "$TEST_IMG"
+
+echo
 echo "=== Testing unaligned reftable entry ==="
 echo
 _make_test_img 64M
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index 7d493bb..dc9f6b7 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -123,6 +123,11 @@ wrote 65536/65536 bytes at offset 0
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 qcow2: Marking image as corrupt: L2 table offset 0x42a00 unaligned (L1 index: 0); further corruption events will be suppressed
 read failed: Input/output error
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qcow2: Marking image as corrupt: L2 table offset 0x42a00 unaligned (L1 index: 0); further corruption events will be suppressed
+qemu-img: Error while amending options: Input/output error
 
 === Testing unaligned L2 entry ===
 
@@ -132,6 +137,14 @@ wrote 65536/65536 bytes at offset 0
 qcow2: Marking image as corrupt: Data cluster offset 0x52a00 unaligned (L2 offset: 0x40000, L2 index: 0); further corruption events will be suppressed
 read failed: Input/output error
 
+=== Testing unaligned pre-allocated zero cluster ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qcow2: Marking image as corrupt: Data cluster offset 0x52a00 unaligned (L2 offset: 0x40000, L2 index: 0); further corruption events will be suppressed
+qemu-img: Error while amending options: Input/output error
+
 === Testing unaligned reftable entry ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 05/12] block: vmdk - make ret variable usage clear
  2015-01-23 18:20 [Qemu-devel] [PULL 00/12] Block patches Kevin Wolf
                   ` (3 preceding siblings ...)
  2015-01-23 18:20 ` [Qemu-devel] [PULL 04/12] iotests: Add tests for more corruption cases Kevin Wolf
@ 2015-01-23 18:20 ` Kevin Wolf
  2015-01-23 18:20 ` [Qemu-devel] [PULL 06/12] block: vmdk - move string allocations from stack to the heap Kevin Wolf
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2015-01-23 18:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

From: Jeff Cody <jcody@redhat.com>

Keep the variable 'ret' something that is returned by the function it is
defined in.  For the return value of 'sscanf', use a more meaningful
variable name.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 52cb888..dc6459c 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -785,6 +785,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
                               const char *desc_file_path, Error **errp)
 {
     int ret;
+    int matches;
     char access[11];
     char type[11];
     char fname[512];
@@ -796,6 +797,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
     BDRVVmdkState *s = bs->opaque;
     VmdkExtent *extent;
 
+
     while (*p) {
         /* parse extent line in one of below formats:
          *
@@ -805,23 +807,23 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
          * RW [size in sectors] VMFSSPARSE "file-name.vmdk"
          */
         flat_offset = -1;
-        ret = sscanf(p, "%10s %" SCNd64 " %10s \"%511[^\n\r\"]\" %" SCNd64,
-                access, &sectors, type, fname, &flat_offset);
-        if (ret < 4 || strcmp(access, "RW")) {
+        matches = sscanf(p, "%10s %" SCNd64 " %10s \"%511[^\n\r\"]\" %" SCNd64,
+                         access, &sectors, type, fname, &flat_offset);
+        if (matches < 4 || strcmp(access, "RW")) {
             goto next_line;
         } else if (!strcmp(type, "FLAT")) {
-            if (ret != 5 || flat_offset < 0) {
+            if (matches != 5 || flat_offset < 0) {
                 error_setg(errp, "Invalid extent lines: \n%s", p);
                 return -EINVAL;
             }
         } else if (!strcmp(type, "VMFS")) {
-            if (ret == 4) {
+            if (matches == 4) {
                 flat_offset = 0;
             } else {
                 error_setg(errp, "Invalid extent lines:\n%s", p);
                 return -EINVAL;
             }
-        } else if (ret != 4) {
+        } else if (matches != 4) {
             error_setg(errp, "Invalid extent lines:\n%s", p);
             return -EINVAL;
         }
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 06/12] block: vmdk - move string allocations from stack to the heap
  2015-01-23 18:20 [Qemu-devel] [PULL 00/12] Block patches Kevin Wolf
                   ` (4 preceding siblings ...)
  2015-01-23 18:20 ` [Qemu-devel] [PULL 05/12] block: vmdk - make ret variable usage clear Kevin Wolf
@ 2015-01-23 18:20 ` Kevin Wolf
  2015-01-23 18:20 ` [Qemu-devel] [PULL 07/12] block: qapi - move string allocation " Kevin Wolf
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2015-01-23 18:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

From: Jeff Cody <jcody@redhat.com>

Functions 'vmdk_parse_extents' and 'vmdk_create' allocate several
PATH_MAX sized arrays on the stack.  Make these dynamically allocated.

Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c | 39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index dc6459c..7d079ad 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -792,12 +792,11 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
     const char *p = desc;
     int64_t sectors = 0;
     int64_t flat_offset;
-    char extent_path[PATH_MAX];
+    char *extent_path;
     BlockDriverState *extent_file;
     BDRVVmdkState *s = bs->opaque;
     VmdkExtent *extent;
 
-
     while (*p) {
         /* parse extent line in one of below formats:
          *
@@ -843,11 +842,13 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
             return -EINVAL;
         }
 
+        extent_path = g_malloc0(PATH_MAX);
         path_combine(extent_path, sizeof(extent_path),
                 desc_file_path, fname);
         extent_file = NULL;
         ret = bdrv_open(&extent_file, extent_path, NULL, NULL,
                         bs->open_flags | BDRV_O_PROTOCOL, NULL, errp);
+        g_free(extent_path);
         if (ret) {
             return ret;
         }
@@ -1797,10 +1798,15 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
     int ret = 0;
     bool flat, split, compress;
     GString *ext_desc_lines;
-    char path[PATH_MAX], prefix[PATH_MAX], postfix[PATH_MAX];
+    char *path = g_malloc0(PATH_MAX);
+    char *prefix = g_malloc0(PATH_MAX);
+    char *postfix = g_malloc0(PATH_MAX);
+    char *desc_line = g_malloc0(BUF_SIZE);
+    char *ext_filename = g_malloc0(PATH_MAX);
+    char *desc_filename = g_malloc0(PATH_MAX);
     const int64_t split_size = 0x80000000;  /* VMDK has constant split size */
     const char *desc_extent_line;
-    char parent_desc_line[BUF_SIZE] = "";
+    char *parent_desc_line = g_malloc0(BUF_SIZE);
     uint32_t parent_cid = 0xffffffff;
     uint32_t number_heads = 16;
     bool zeroed_grain = false;
@@ -1916,33 +1922,27 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
         }
         parent_cid = vmdk_read_cid(bs, 0);
         bdrv_unref(bs);
-        snprintf(parent_desc_line, sizeof(parent_desc_line),
+        snprintf(parent_desc_line, BUF_SIZE,
                 "parentFileNameHint=\"%s\"", backing_file);
     }
 
     /* Create extents */
     filesize = total_size;
     while (filesize > 0) {
-        char desc_line[BUF_SIZE];
-        char ext_filename[PATH_MAX];
-        char desc_filename[PATH_MAX];
         int64_t size = filesize;
 
         if (split && size > split_size) {
             size = split_size;
         }
         if (split) {
-            snprintf(desc_filename, sizeof(desc_filename), "%s-%c%03d%s",
+            snprintf(desc_filename, PATH_MAX, "%s-%c%03d%s",
                     prefix, flat ? 'f' : 's', ++idx, postfix);
         } else if (flat) {
-            snprintf(desc_filename, sizeof(desc_filename), "%s-flat%s",
-                    prefix, postfix);
+            snprintf(desc_filename, PATH_MAX, "%s-flat%s", prefix, postfix);
         } else {
-            snprintf(desc_filename, sizeof(desc_filename), "%s%s",
-                    prefix, postfix);
+            snprintf(desc_filename, PATH_MAX, "%s%s", prefix, postfix);
         }
-        snprintf(ext_filename, sizeof(ext_filename), "%s%s",
-                path, desc_filename);
+        snprintf(ext_filename, PATH_MAX, "%s%s", path, desc_filename);
 
         if (vmdk_create_extent(ext_filename, size,
                                flat, compress, zeroed_grain, opts, errp)) {
@@ -1952,7 +1952,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
         filesize -= size;
 
         /* Format description line */
-        snprintf(desc_line, sizeof(desc_line),
+        snprintf(desc_line, BUF_SIZE,
                     desc_extent_line, size / BDRV_SECTOR_SIZE, desc_filename);
         g_string_append(ext_desc_lines, desc_line);
     }
@@ -2007,6 +2007,13 @@ exit:
     g_free(backing_file);
     g_free(fmt);
     g_free(desc);
+    g_free(path);
+    g_free(prefix);
+    g_free(postfix);
+    g_free(desc_line);
+    g_free(ext_filename);
+    g_free(desc_filename);
+    g_free(parent_desc_line);
     g_string_free(ext_desc_lines, true);
     return ret;
 }
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 07/12] block: qapi - move string allocation from stack to the heap
  2015-01-23 18:20 [Qemu-devel] [PULL 00/12] Block patches Kevin Wolf
                   ` (5 preceding siblings ...)
  2015-01-23 18:20 ` [Qemu-devel] [PULL 06/12] block: vmdk - move string allocations from stack to the heap Kevin Wolf
@ 2015-01-23 18:20 ` Kevin Wolf
  2015-01-23 18:20 ` [Qemu-devel] [PULL 08/12] block: remove unused variable in bdrv_commit Kevin Wolf
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2015-01-23 18:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

From: Jeff Cody <jcody@redhat.com>

Rather than declaring 'backing_filename2' on the stack in
bdrv_query_image_info(), dynamically allocate it on the heap.

Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qapi.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index a6fd6f7..dec9f60 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -175,7 +175,6 @@ void bdrv_query_image_info(BlockDriverState *bs,
 {
     int64_t size;
     const char *backing_filename;
-    char backing_filename2[1024];
     BlockDriverInfo bdi;
     int ret;
     Error *err = NULL;
@@ -211,13 +210,14 @@ void bdrv_query_image_info(BlockDriverState *bs,
 
     backing_filename = bs->backing_file;
     if (backing_filename[0] != '\0') {
+        char *backing_filename2 = g_malloc0(1024);
         info->backing_filename = g_strdup(backing_filename);
         info->has_backing_filename = true;
-        bdrv_get_full_backing_filename(bs, backing_filename2,
-                                       sizeof(backing_filename2), &err);
+        bdrv_get_full_backing_filename(bs, backing_filename2, 1024, &err);
         if (err) {
             error_propagate(errp, err);
             qapi_free_ImageInfo(info);
+            g_free(backing_filename2);
             return;
         }
 
@@ -231,6 +231,7 @@ void bdrv_query_image_info(BlockDriverState *bs,
             info->backing_filename_format = g_strdup(bs->backing_format);
             info->has_backing_filename_format = true;
         }
+        g_free(backing_filename2);
     }
 
     ret = bdrv_query_snapshot_info_list(bs, &info->snapshots, &err);
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 08/12] block: remove unused variable in bdrv_commit
  2015-01-23 18:20 [Qemu-devel] [PULL 00/12] Block patches Kevin Wolf
                   ` (6 preceding siblings ...)
  2015-01-23 18:20 ` [Qemu-devel] [PULL 07/12] block: qapi - move string allocation " Kevin Wolf
@ 2015-01-23 18:20 ` Kevin Wolf
  2015-01-23 18:20 ` [Qemu-devel] [PULL 09/12] block: mirror - change string allocation to 2-bytes Kevin Wolf
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2015-01-23 18:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

From: Jeff Cody <jcody@redhat.com>

As Stefan pointed out, the variable 'filename' in bdrv_commit is unused,
despite being maintained in previous patches.

With this patch, get rid of the variable for good.

Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/block.c b/block.c
index cbe4a32..d45e4dd 100644
--- a/block.c
+++ b/block.c
@@ -2207,7 +2207,6 @@ int bdrv_commit(BlockDriverState *bs)
     int n, ro, open_flags;
     int ret = 0;
     uint8_t *buf = NULL;
-    char filename[PATH_MAX];
 
     if (!drv)
         return -ENOMEDIUM;
@@ -2222,8 +2221,6 @@ int bdrv_commit(BlockDriverState *bs)
     }
 
     ro = bs->backing_hd->read_only;
-    /* Use pstrcpy (not strncpy): filename must be NUL-terminated. */
-    pstrcpy(filename, sizeof(filename), bs->backing_hd->filename);
     open_flags =  bs->backing_hd->open_flags;
 
     if (ro) {
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 09/12] block: mirror - change string allocation to 2-bytes
  2015-01-23 18:20 [Qemu-devel] [PULL 00/12] Block patches Kevin Wolf
                   ` (7 preceding siblings ...)
  2015-01-23 18:20 ` [Qemu-devel] [PULL 08/12] block: remove unused variable in bdrv_commit Kevin Wolf
@ 2015-01-23 18:20 ` Kevin Wolf
  2015-01-23 18:20 ` [Qemu-devel] [PULL 10/12] block: update string sizes for filename, backing_file, exact_filename Kevin Wolf
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2015-01-23 18:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

From: Jeff Cody <jcody@redhat.com>

The backing_filename string in mirror_run() is only used to check
for a NULL string, so we don't need to allocate 1024 bytes (or, later,
PATH_MAX bytes), when we only need to copy the first 2 characters.

We technically only need 1 byte, as we are just checking for NULL, but
since backing_filename[] is populated by bdrv_get_backing_filename(), a
string size of 1 will always only return '\0';

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/mirror.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index 9019d1b..4056164 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -378,7 +378,8 @@ static void coroutine_fn mirror_run(void *opaque)
     int64_t sector_num, end, sectors_per_chunk, length;
     uint64_t last_pause_ns;
     BlockDriverInfo bdi;
-    char backing_filename[1024];
+    char backing_filename[2]; /* we only need 2 characters because we are only
+                                 checking for a NULL string */
     int ret = 0;
     int n;
 
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 10/12] block: update string sizes for filename, backing_file, exact_filename
  2015-01-23 18:20 [Qemu-devel] [PULL 00/12] Block patches Kevin Wolf
                   ` (8 preceding siblings ...)
  2015-01-23 18:20 ` [Qemu-devel] [PULL 09/12] block: mirror - change string allocation to 2-bytes Kevin Wolf
@ 2015-01-23 18:20 ` Kevin Wolf
  2015-01-23 18:20 ` [Qemu-devel] [PULL 11/12] block: vhdx - force FileOffsetMB field to '0' for certain block states Kevin Wolf
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2015-01-23 18:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

From: Jeff Cody <jcody@redhat.com>

The string field entries 'filename', 'backing_file', and
'exact_filename' in the BlockDriverState struct are defined as 1024
bytes.

However, many places that use these values accept a maximum of PATH_MAX
bytes, so we have a mixture of 1024 byte and PATH_MAX byte allocations.
This patch makes the BlockDriverStruct field string sizes match usage.

This patch also does a few fixes related to the size that needs to
happen now:

    * the block qapi driver is updated to use PATH_MAX bytes
    * the qcow and qcow2 drivers have an additional safety check
    * the block vvfat driver is updated to use PATH_MAX bytes
      for the size of backing_file, for systems where PATH_MAX is < 1024
      bytes.
    * qemu-img uses PATH_MAX rather than 1024.  These instances were not
      changed to be dynamically allocated, however, as the extra
      temporary 3K in stack usage for qemu-img does not seem worrisome.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qapi.c              | 4 ++--
 block/qcow.c              | 2 +-
 block/qcow2.c             | 3 ++-
 block/vvfat.c             | 4 ++--
 include/block/block_int.h | 8 ++++----
 qemu-img.c                | 4 ++--
 6 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index dec9f60..75c388e 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -210,10 +210,10 @@ void bdrv_query_image_info(BlockDriverState *bs,
 
     backing_filename = bs->backing_file;
     if (backing_filename[0] != '\0') {
-        char *backing_filename2 = g_malloc0(1024);
+        char *backing_filename2 = g_malloc0(PATH_MAX);
         info->backing_filename = g_strdup(backing_filename);
         info->has_backing_filename = true;
-        bdrv_get_full_backing_filename(bs, backing_filename2, 1024, &err);
+        bdrv_get_full_backing_filename(bs, backing_filename2, PATH_MAX, &err);
         if (err) {
             error_propagate(errp, err);
             qapi_free_ImageInfo(info);
diff --git a/block/qcow.c b/block/qcow.c
index ece2269..ccbe9e0 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -215,7 +215,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
     /* read the backing file name */
     if (header.backing_file_offset != 0) {
         len = header.backing_file_size;
-        if (len > 1023) {
+        if (len > 1023 || len > sizeof(bs->backing_file)) {
             error_setg(errp, "Backing file name too long");
             ret = -EINVAL;
             goto fail;
diff --git a/block/qcow2.c b/block/qcow2.c
index e4e690a..dbaf016 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -868,7 +868,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     /* read the backing file name */
     if (header.backing_file_offset != 0) {
         len = header.backing_file_size;
-        if (len > MIN(1023, s->cluster_size - header.backing_file_offset)) {
+        if (len > MIN(1023, s->cluster_size - header.backing_file_offset) ||
+            len > sizeof(bs->backing_file)) {
             error_setg(errp, "Backing file name too long");
             ret = -EINVAL;
             goto fail;
diff --git a/block/vvfat.c b/block/vvfat.c
index e34a789..a1a44f0 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2909,8 +2909,8 @@ static int enable_write_target(BDRVVVFATState *s, Error **errp)
 
     array_init(&(s->commits), sizeof(commit_t));
 
-    s->qcow_filename = g_malloc(1024);
-    ret = get_tmp_filename(s->qcow_filename, 1024);
+    s->qcow_filename = g_malloc(PATH_MAX);
+    ret = get_tmp_filename(s->qcow_filename, PATH_MAX);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "can't create temporary file");
         goto err;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 06a21dd..e264be9 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -339,13 +339,13 @@ struct BlockDriverState {
      * regarding this BDS's context */
     QLIST_HEAD(, BdrvAioNotifier) aio_notifiers;
 
-    char filename[1024];
-    char backing_file[1024]; /* if non zero, the image is a diff of
-                                this file image */
+    char filename[PATH_MAX];
+    char backing_file[PATH_MAX]; /* if non zero, the image is a diff of
+                                    this file image */
     char backing_format[16]; /* if non-zero and backing_file exists */
 
     QDict *full_open_options;
-    char exact_filename[1024];
+    char exact_filename[PATH_MAX];
 
     BlockDriverState *backing_hd;
     BlockDriverState *file;
diff --git a/qemu-img.c b/qemu-img.c
index 7876258..4e9a7f5 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2556,7 +2556,7 @@ static int img_rebase(int argc, char **argv)
 
     /* For safe rebasing we need to compare old and new backing file */
     if (!unsafe) {
-        char backing_name[1024];
+        char backing_name[PATH_MAX];
 
         blk_old_backing = blk_new_with_bs("old_backing", &error_abort);
         bs_old_backing = blk_bs(blk_old_backing);
@@ -2614,7 +2614,7 @@ static int img_rebase(int argc, char **argv)
         }
         old_backing_num_sectors = bdrv_nb_sectors(bs_old_backing);
         if (old_backing_num_sectors < 0) {
-            char backing_name[1024];
+            char backing_name[PATH_MAX];
 
             bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
             error_report("Could not get size of '%s': %s",
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 11/12] block: vhdx - force FileOffsetMB field to '0' for certain block states
  2015-01-23 18:20 [Qemu-devel] [PULL 00/12] Block patches Kevin Wolf
                   ` (9 preceding siblings ...)
  2015-01-23 18:20 ` [Qemu-devel] [PULL 10/12] block: update string sizes for filename, backing_file, exact_filename Kevin Wolf
@ 2015-01-23 18:20 ` Kevin Wolf
  2015-01-23 18:20 ` [Qemu-devel] [PULL 12/12] iotests: Lower 064's memory usage Kevin Wolf
  2015-01-26 10:16 ` [Qemu-devel] [PULL 00/12] Block patches Peter Maydell
  12 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2015-01-23 18:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

From: Jeff Cody <jcody@redhat.com>

The v1.0.0 spec calls out PAYLOAD_BLOCK_ZERO FileOffsetMB field as being
'reserved'.  In practice, this means that Hyper-V will fail to read a
disk image with PAYLOAD_BLOCK_ZERO block states with a FileOffsetMB
value other than 0.

The other states that indicate a block that is not there
(PAYLOAD_BLOCK_UNDEFINED, PAYLOAD_BLOCK_NOT_PRESENT,
 PAYLOAD_BLOCK_UNMAPPED) have multiple options for what FileOffsetMB may
be set to, and '0' is explicitly called out as an option.

For all the above states, we will also just set the FileOffsetMB value
to 0.

Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: a9fe92f53f07e6ab1693811e4312c0d1e958500b.1421787566.git.jcody@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/vhdx.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index 06f2b1a..bb3ed45 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1174,7 +1174,18 @@ static void vhdx_update_bat_table_entry(BlockDriverState *bs, BDRVVHDXState *s,
 {
     /* The BAT entry is a uint64, with 44 bits for the file offset in units of
      * 1MB, and 3 bits for the block state. */
-    s->bat[sinfo->bat_idx]  = sinfo->file_offset;
+    if ((state == PAYLOAD_BLOCK_ZERO)        ||
+        (state == PAYLOAD_BLOCK_UNDEFINED)   ||
+        (state == PAYLOAD_BLOCK_NOT_PRESENT) ||
+        (state == PAYLOAD_BLOCK_UNMAPPED)) {
+        s->bat[sinfo->bat_idx]  = 0;  /* For PAYLOAD_BLOCK_ZERO, the
+                                         FileOffsetMB field is denoted as
+                                         'reserved' in the v1.0 spec.  If it is
+                                         non-zero, MS Hyper-V will fail to read
+                                         the disk image */
+    } else {
+        s->bat[sinfo->bat_idx]  = sinfo->file_offset;
+    }
 
     s->bat[sinfo->bat_idx] |= state & VHDX_BAT_STATE_BIT_MASK;
 
-- 
1.8.3.1

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

* [Qemu-devel] [PULL 12/12] iotests: Lower 064's memory usage
  2015-01-23 18:20 [Qemu-devel] [PULL 00/12] Block patches Kevin Wolf
                   ` (10 preceding siblings ...)
  2015-01-23 18:20 ` [Qemu-devel] [PULL 11/12] block: vhdx - force FileOffsetMB field to '0' for certain block states Kevin Wolf
@ 2015-01-23 18:20 ` Kevin Wolf
  2015-01-26 10:16 ` [Qemu-devel] [PULL 00/12] Block patches Peter Maydell
  12 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2015-01-23 18:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf

From: Max Reitz <mreitz@redhat.com>

Test 064 reads a lot of data at once which currently results in qemu-io
having to allocate up to about 1 GB of memory (958 MB, to be exact).
This patch lowers that amount to 128 MB by making the test read smaller
chunks.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Message-id: 1422025185-25229-1-git-send-email-mreitz@redhat.com
---
 tests/qemu-iotests/064     | 19 +++++++++++++++++--
 tests/qemu-iotests/064.out | 34 ++++++++++++++++++++++++++++++----
 2 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/064 b/tests/qemu-iotests/064
index 1c74c31..7564563 100755
--- a/tests/qemu-iotests/064
+++ b/tests/qemu-iotests/064
@@ -54,7 +54,15 @@ $QEMU_IO -r -c "read -pP 0x96 33M 33M" "$TEST_IMG" | _filter_qemu_io
 
 echo
 echo "=== Verify pattern 0x00, 66M - 1024M ==="
-$QEMU_IO -r -c "read -pP 0x00 66M 958M" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -r -c "read -pP 0x00 66M 62M" \
+            -c "read -pP 0x00 128M 128M" \
+            -c "read -pP 0x00 256M 128M" \
+            -c "read -pP 0x00 384M 128M" \
+            -c "read -pP 0x00 512M 128M" \
+            -c "read -pP 0x00 640M 128M" \
+            -c "read -pP 0x00 768M 128M" \
+            -c "read -pP 0x00 896M 128M" \
+            "$TEST_IMG" | _filter_qemu_io
 
 echo
 echo "=== Verify pattern write, 0xc3 99M-157M ==="
@@ -63,7 +71,14 @@ $QEMU_IO -c "write -pP 0xc3 99M 58M" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IO -c "read -pP 0xa5 0 33M" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IO -c "read -pP 0x96 33M 33M" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IO -c "read -pP 0x00 66M 33M" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "read -pP 0x00 157MM 867MM" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -pP 0x00 157M 99M" \
+         -c "read -pP 0x00 256M 128M" \
+         -c "read -pP 0x00 384M 128M" \
+         -c "read -pP 0x00 512M 128M" \
+         -c "read -pP 0x00 640M 128M" \
+         -c "read -pP 0x00 768M 128M" \
+         -c "read -pP 0x00 896M 128M" \
+         "$TEST_IMG" | _filter_qemu_io
 # now verify what we should have actually written
 $QEMU_IO -c "read -pP 0xc3 99M 58M" "$TEST_IMG" | _filter_qemu_io
 
diff --git a/tests/qemu-iotests/064.out b/tests/qemu-iotests/064.out
index 5346a4e..1a5b9e2 100644
--- a/tests/qemu-iotests/064.out
+++ b/tests/qemu-iotests/064.out
@@ -9,8 +9,22 @@ read 34603008/34603008 bytes at offset 34603008
 33 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Verify pattern 0x00, 66M - 1024M ===
-read 1004535808/1004535808 bytes at offset 69206016
-958 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65011712/65011712 bytes at offset 69206016
+62 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 134217728/134217728 bytes at offset 134217728
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 134217728/134217728 bytes at offset 268435456
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 134217728/134217728 bytes at offset 402653184
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 134217728/134217728 bytes at offset 536870912
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 134217728/134217728 bytes at offset 671088640
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 134217728/134217728 bytes at offset 805306368
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 134217728/134217728 bytes at offset 939524096
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Verify pattern write, 0xc3 99M-157M ===
 wrote 60817408/60817408 bytes at offset 103809024
@@ -21,8 +35,20 @@ read 34603008/34603008 bytes at offset 34603008
 33 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 34603008/34603008 bytes at offset 69206016
 33 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 909115392/909115392 bytes at offset 164626432
-867 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 103809024/103809024 bytes at offset 164626432
+99 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 134217728/134217728 bytes at offset 268435456
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 134217728/134217728 bytes at offset 402653184
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 134217728/134217728 bytes at offset 536870912
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 134217728/134217728 bytes at offset 671088640
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 134217728/134217728 bytes at offset 805306368
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 134217728/134217728 bytes at offset 939524096
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 60817408/60817408 bytes at offset 103809024
 58 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 *** done
-- 
1.8.3.1

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

* Re: [Qemu-devel] [PULL 00/12] Block patches
  2015-01-23 18:20 [Qemu-devel] [PULL 00/12] Block patches Kevin Wolf
                   ` (11 preceding siblings ...)
  2015-01-23 18:20 ` [Qemu-devel] [PULL 12/12] iotests: Lower 064's memory usage Kevin Wolf
@ 2015-01-26 10:16 ` Peter Maydell
  12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2015-01-26 10:16 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: QEMU Developers

On 23 January 2015 at 18:20, Kevin Wolf <kwolf@redhat.com> wrote:
> The following changes since commit c6441452b50c44fdbb362b239ce623f77cf3cd51:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/pull-audio-20150122-1' into staging (2015-01-22 18:57:36 +0000)
>
> are available in the git repository at:
>
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to bc63781ca350cad4c9eb142ce8f55bfaded4276e:
>
>   Merge remote-tracking branch 'mreitz/block' into queue-block (2015-01-23 18:51:47 +0100)
>
> ----------------------------------------------------------------
>
> Block patches for 2.3
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM

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

end of thread, other threads:[~2015-01-26 10:16 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-23 18:20 [Qemu-devel] [PULL 00/12] Block patches Kevin Wolf
2015-01-23 18:20 ` [Qemu-devel] [PULL 01/12] virtio-blk: Pass req to virtio_blk_handle_scsi_req Kevin Wolf
2015-01-23 18:20 ` [Qemu-devel] [PULL 02/12] virtio-blk: Use blk_aio_ioctl Kevin Wolf
2015-01-23 18:20 ` [Qemu-devel] [PULL 03/12] qcow2: Add two more unalignment checks Kevin Wolf
2015-01-23 18:20 ` [Qemu-devel] [PULL 04/12] iotests: Add tests for more corruption cases Kevin Wolf
2015-01-23 18:20 ` [Qemu-devel] [PULL 05/12] block: vmdk - make ret variable usage clear Kevin Wolf
2015-01-23 18:20 ` [Qemu-devel] [PULL 06/12] block: vmdk - move string allocations from stack to the heap Kevin Wolf
2015-01-23 18:20 ` [Qemu-devel] [PULL 07/12] block: qapi - move string allocation " Kevin Wolf
2015-01-23 18:20 ` [Qemu-devel] [PULL 08/12] block: remove unused variable in bdrv_commit Kevin Wolf
2015-01-23 18:20 ` [Qemu-devel] [PULL 09/12] block: mirror - change string allocation to 2-bytes Kevin Wolf
2015-01-23 18:20 ` [Qemu-devel] [PULL 10/12] block: update string sizes for filename, backing_file, exact_filename Kevin Wolf
2015-01-23 18:20 ` [Qemu-devel] [PULL 11/12] block: vhdx - force FileOffsetMB field to '0' for certain block states Kevin Wolf
2015-01-23 18:20 ` [Qemu-devel] [PULL 12/12] iotests: Lower 064's memory usage Kevin Wolf
2015-01-26 10:16 ` [Qemu-devel] [PULL 00/12] Block patches Peter Maydell

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.