[Qemu-devel] [PATCH v2 00/13] block: Various Coverity-spotted fixes

[Qemu-devel] [PATCH v2 00/13] block: Various Coverity-spotted fixes

[Max Reitz]

This series fixes various issues spotted by Coverity. None of these is
critical; most are just "If you do something crazy, qemu-img crashes" or
"But what if there is no qcow2 driver?". Therefore, while these are bug
fixes, it is a bit late to try to push them into 2.2.0. I am therefore
tempted to vote to target 2.3 instead.

Also, none is security-relevant. The only crashes which are fixed here
are sure to have resulted from dereferencing a NULL pointer.


v2:
- Patch 1: There are a couple of block drivers which have to always
  statically linked into qemu; those are file (because it's the standard
  protocol), raw (because currently it's the standard format) and qcow2
  (because it's the de-facto standard non-raw and backing format). This
  patch adds public symbols for their BlockDriver objects so they can be
  used from the block driver without having to resort to
  bdrv_find_format() (which may fail). [my interpretation of Markus and
  Kevin]
- Patch 2: Replaces patch 1 from v1; use the symbols introduced in patch
  1. [again, my interpretation of Markus and Kevin]
- Patch 3: No changes; albeit dropping the old patch 1 and introducing
  the new patch 2 instead, I decided not to rely on qcow at
  compile-time. qcow has been deprecated, so I can imagine some users
  not wanting to link qcow into qemu (whatever gain they expect from
  that). Therefore, although I think it's fine to rely on qcow2 at
  compile-time, I don't think it fine to rely on qcow as well.
- Patch 4: Added a note to the commit message on what concrete case is
  fixed, and why I don't add an iotest for that case [Kevin]
- Patch 11: Fixed the reference output of tests 026, 071 and 089


git-backport-diff against v1:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/13:[down] 'block: Make essential BlockDriver objects public'
002/13:[down] 'block: Omit bdrv_find_format for essential drivers'
003/13:[----] [--] 'block/vvfat: qcow driver may not be found'
004/13:[----] [--] 'block/nfs: Add create_opts'
005/13:[----] [--] 'block: Check create_opts before image creation'
006/13:[----] [--] 'qemu-img: Check create_opts before image creation'
007/13:[----] [--] 'qemu-img: Check create_opts before image amendment'
008/13:[----] [--] 'iotests: Only kill NBD server if it runs'
009/13:[----] [-C] 'iotests: Add test for unsupported image creation'
010/13:[----] [--] 'qcow2: Prevent numerical overflow'
011/13:[0130] [FC] 'qcow2: Flushing the caches in qcow2_close may fail'
012/13:[----] [--] 'qcow2: Respect bdrv_truncate() error'
013/13:[----] [--] 'block/raw-posix: Fix ret in raw_open_common()'


Max Reitz (13):
  block: Make essential BlockDriver objects public
  block: Omit bdrv_find_format for essential drivers
  block/vvfat: qcow driver may not be found
  block/nfs: Add create_opts
  block: Check create_opts before image creation
  qemu-img: Check create_opts before image creation
  qemu-img: Check create_opts before image amendment
  iotests: Only kill NBD server if it runs
  iotests: Add test for unsupported image creation
  qcow2: Prevent numerical overflow
  qcow2: Flushing the caches in qcow2_close may fail
  qcow2: Respect bdrv_truncate() error
  block/raw-posix: Fix ret in raw_open_common()

 block.c                      |  23 +++++----
 block/nfs.c                  |  15 ++++++
 block/qcow2-cluster.c        |   2 +-
 block/qcow2.c                |  26 +++++++---
 block/raw-posix.c            |   5 +-
 block/raw-win32.c            |   4 +-
 block/raw_bsd.c              |   4 +-
 block/vvfat.c                |   6 +++
 include/block/block_int.h    |   8 +++
 qemu-img.c                   |  21 ++++++++
 tests/qemu-iotests/026.out   | 120 +++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/071.out   |   8 +++
 tests/qemu-iotests/089.out   |   2 +
 tests/qemu-iotests/113       |  76 +++++++++++++++++++++++++++
 tests/qemu-iotests/113.out   |  15 ++++++
 tests/qemu-iotests/common.rc |   4 +-
 tests/qemu-iotests/group     |   1 +
 17 files changed, 316 insertions(+), 24 deletions(-)
 create mode 100755 tests/qemu-iotests/113
 create mode 100644 tests/qemu-iotests/113.out

-- 
1.9.3

[Qemu-devel] [PATCH v2 01/13] block: Make essential BlockDriver objects public

[Max Reitz]

There are some block drivers which are essential to QEMU and may not be
removed: These are raw, file and qcow2 (as the default non-raw format).
Make their BlockDriver objects public so they can be directly referenced
throughout the block layer without needing to call bdrv_find_format()
and having to deal with an error at runtime, while the real problem
occured during linking (where raw, file or qcow2 were not linked into
qemu).

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c             | 4 ++--
 block/raw-posix.c         | 4 ++--
 block/raw-win32.c         | 4 ++--
 block/raw_bsd.c           | 4 ++--
 include/block/block_int.h | 8 ++++++++
 5 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 8b9ffc4..0eeba36 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2847,7 +2847,7 @@ static QemuOptsList qcow2_create_opts = {
     }
 };
 
-static BlockDriver bdrv_qcow2 = {
+BlockDriver *bdrv_qcow2 = &(BlockDriver){
     .format_name        = "qcow2",
     .instance_size      = sizeof(BDRVQcowState),
     .bdrv_probe         = qcow2_probe,
@@ -2893,7 +2893,7 @@ static BlockDriver bdrv_qcow2 = {
 
 static void bdrv_qcow2_init(void)
 {
-    bdrv_register(&bdrv_qcow2);
+    bdrv_register(bdrv_qcow2);
 }
 
 block_init(bdrv_qcow2_init);
diff --git a/block/raw-posix.c b/block/raw-posix.c
index b1af77e..6766cec 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1684,7 +1684,7 @@ static QemuOptsList raw_create_opts = {
     }
 };
 
-static BlockDriver bdrv_file = {
+BlockDriver *bdrv_file = &(BlockDriver){
     .format_name = "file",
     .protocol_name = "file",
     .instance_size = sizeof(BDRVRawState),
@@ -2550,7 +2550,7 @@ static void bdrv_file_init(void)
      * Register all the drivers.  Note that order is important, the driver
      * registered last will get probed first.
      */
-    bdrv_register(&bdrv_file);
+    bdrv_register(bdrv_file);
     bdrv_register(&bdrv_host_device);
 #ifdef __linux__
     bdrv_register(&bdrv_host_floppy);
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 7b58881..96f097e 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -540,7 +540,7 @@ static QemuOptsList raw_create_opts = {
     }
 };
 
-static BlockDriver bdrv_file = {
+BlockDriver *bdrv_file = &(BlockDriver){
     .format_name	= "file",
     .protocol_name	= "file",
     .instance_size	= sizeof(BDRVRawState),
@@ -721,7 +721,7 @@ static BlockDriver bdrv_host_device = {
 
 static void bdrv_file_init(void)
 {
-    bdrv_register(&bdrv_file);
+    bdrv_register(bdrv_file);
     bdrv_register(&bdrv_host_device);
 }
 
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 401b967..cdb019e 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -173,7 +173,7 @@ static int raw_probe(const uint8_t *buf, int buf_size, const char *filename)
     return 1;
 }
 
-static BlockDriver bdrv_raw = {
+BlockDriver *bdrv_raw = &(BlockDriver){
     .format_name          = "raw",
     .bdrv_probe           = &raw_probe,
     .bdrv_reopen_prepare  = &raw_reopen_prepare,
@@ -202,7 +202,7 @@ static BlockDriver bdrv_raw = {
 
 static void bdrv_raw_init(void)
 {
-    bdrv_register(&bdrv_raw);
+    bdrv_register(bdrv_raw);
 }
 
 block_init(bdrv_raw_init);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index a1c17b9..2ff3fc4 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -411,6 +411,14 @@ struct BlockDriverState {
     Error *backing_blocker;
 };
 
+
+/* Essential block drivers which must always be statically linked into qemu, and
+ * which therefore can be accessed without using bdrv_find_format() */
+extern BlockDriver *bdrv_file;
+extern BlockDriver *bdrv_raw;
+extern BlockDriver *bdrv_qcow2;
+
+
 int get_tmp_filename(char *filename, int size);
 
 void bdrv_set_io_limits(BlockDriverState *bs,
-- 
1.9.3

[Qemu-devel] [PATCH v2 02/13] block: Omit bdrv_find_format for essential drivers

[Max Reitz]

We can always assume raw, file and qcow2 being available; so do not use
bdrv_find_format() to locate their BlockDriver objects but statically
reference the respective objects.

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/block.c b/block.c
index 9d003bc..fbd7cb2 100644
--- a/block.c
+++ b/block.c
@@ -629,7 +629,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
     }
 
     if (!path_has_protocol(filename) || !allow_protocol_prefix) {
-        return bdrv_find_format("file");
+        return bdrv_file;
     }
 
     p = strchr(filename, ':');
@@ -658,12 +658,7 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
 
     /* Return the raw BlockDriver * to scsi-generic devices or empty drives */
     if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) {
-        drv = bdrv_find_format("raw");
-        if (!drv) {
-            error_setg(errp, "Could not find raw image format");
-            ret = -ENOENT;
-        }
-        *pdrv = drv;
+        *pdrv = bdrv_raw;
         return ret;
     }
 
@@ -1293,7 +1288,6 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
     /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
     char *tmp_filename = g_malloc0(PATH_MAX + 1);
     int64_t total_size;
-    BlockDriver *bdrv_qcow2;
     QemuOpts *opts = NULL;
     QDict *snapshot_options;
     BlockDriverState *bs_snapshot;
@@ -1318,7 +1312,6 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
         goto out;
     }
 
-    bdrv_qcow2 = bdrv_find_format("qcow2");
     opts = qemu_opts_create(bdrv_qcow2->create_opts, NULL, 0,
                             &error_abort);
     qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size);
-- 
1.9.3

[Qemu-devel] [PATCH v2 03/13] block/vvfat: qcow driver may not be found

[Max Reitz]

Although virtually impossible right now, bdrv_find_format("qcow") may
fail. The vvfat block driver should heed that case.

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/vvfat.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/block/vvfat.c b/block/vvfat.c
index cefe3a4..e34a789 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2917,6 +2917,12 @@ static int enable_write_target(BDRVVVFATState *s, Error **errp)
     }
 
     bdrv_qcow = bdrv_find_format("qcow");
+    if (!bdrv_qcow) {
+        error_setg(errp, "Failed to locate qcow driver");
+        ret = -ENOENT;
+        goto err;
+    }
+
     opts = qemu_opts_create(bdrv_qcow->create_opts, NULL, 0, &error_abort);
     qemu_opt_set_number(opts, BLOCK_OPT_SIZE, s->sector_count * 512);
     qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, "fat:");
-- 
1.9.3

[Qemu-devel] [PATCH v2 04/13] block/nfs: Add create_opts

[Max Reitz]

The nfs protocol driver is capable of creating images, but did not
specify any creation options. Fix it.

A way to test this issue is the following:

$ qemu-img create -f nfs nfs://127.0.0.1/foo.qcow2 64M

Without this patch, it segfaults. With this patch, it does not. However,
this is not something that should really work; qemu-img should check
whether the parameter for the -f option (and -O for convert) is indeed a
format, and error out if it is not. Therefore, I am not making it an
iotest.

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/nfs.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/block/nfs.c b/block/nfs.c
index c76e368..ca9e24e 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -409,6 +409,19 @@ out:
     return ret;
 }
 
+static QemuOptsList nfs_create_opts = {
+    .name = "nfs-create-opts",
+    .head = QTAILQ_HEAD_INITIALIZER(nfs_create_opts.head),
+    .desc = {
+        {
+            .name = BLOCK_OPT_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Virtual disk size"
+        },
+        { /* end of list */ }
+    }
+};
+
 static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
 {
     int ret = 0;
@@ -470,6 +483,8 @@ static BlockDriver bdrv_nfs = {
 
     .instance_size                  = sizeof(NFSClient),
     .bdrv_needs_filename            = true,
+    .create_opts                    = &nfs_create_opts,
+
     .bdrv_has_zero_init             = nfs_has_zero_init,
     .bdrv_get_allocated_file_size   = nfs_get_allocated_file_size,
     .bdrv_truncate                  = nfs_file_truncate,
-- 
1.9.3

[Qemu-devel] [PATCH v2 05/13] block: Check create_opts before image creation

[Max Reitz]

If a driver supports image creation, it needs to set the .create_opts
field. We can use that to make sure .create_opts for both drivers
involved is not NULL in bdrv_img_create(), which is important so that
the create_opts pointer in that function is not NULL after the
qemu_opts_append() calls and when going into qemu_opts_create().

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/block.c b/block.c
index fbd7cb2..29cf4bb 100644
--- a/block.c
+++ b/block.c
@@ -5546,6 +5546,18 @@ void bdrv_img_create(const char *filename, const char *fmt,
         return;
     }
 
+    if (!drv->create_opts) {
+        error_setg(errp, "Format driver '%s' does not support image creation",
+                   drv->format_name);
+        return;
+    }
+
+    if (!proto_drv->create_opts) {
+        error_setg(errp, "Protocol driver '%s' does not support image creation",
+                   proto_drv->format_name);
+        return;
+    }
+
     create_opts = qemu_opts_append(create_opts, drv->create_opts);
     create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
 
-- 
1.9.3

[Qemu-devel] [PATCH v2 06/13] qemu-img: Check create_opts before image creation

[Max Reitz]

If a driver supports image creation, it needs to set the .create_opts
field. We can use that to make sure .create_opts for both drivers
involved is not NULL for the target image in qemu-img convert, which is
important so that the create_opts pointer in img_convert() is not NULL
after the qemu_opts_append() calls and when going into
qemu_opts_create().

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index a42335c..8c4edf3 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1531,6 +1531,20 @@ static int img_convert(int argc, char **argv)
         goto out;
     }
 
+    if (!drv->create_opts) {
+        error_report("Format driver '%s' does not support image creation",
+                     drv->format_name);
+        ret = -1;
+        goto out;
+    }
+
+    if (!proto_drv->create_opts) {
+        error_report("Protocol driver '%s' does not support image creation",
+                     proto_drv->format_name);
+        ret = -1;
+        goto out;
+    }
+
     create_opts = qemu_opts_append(create_opts, drv->create_opts);
     create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
 
-- 
1.9.3

[Qemu-devel] [PATCH v2 07/13] qemu-img: Check create_opts before image amendment

[Max Reitz]

The image options which can be amended are described by the .create_opts
field for every driver. This field must therefore be non-NULL so that
anything can be amended in the first place. Check that this holds true
before going into qemu_opts_create() (because if .create_opts is NULL,
the create_opts pointer in img_amend() will be NULL after
qemu_opts_append()).

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index 8c4edf3..7876258 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2986,6 +2986,13 @@ static int img_amend(int argc, char **argv)
         goto out;
     }
 
+    if (!bs->drv->create_opts) {
+        error_report("Format driver '%s' does not support any options to amend",
+                     fmt);
+        ret = -1;
+        goto out;
+    }
+
     create_opts = qemu_opts_append(create_opts, bs->drv->create_opts);
     opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
     if (options && qemu_opts_do_parse(opts, options, NULL)) {
-- 
1.9.3

[Qemu-devel] [PATCH v2 08/13] iotests: Only kill NBD server if it runs

[Max Reitz]

There may be NBD tests which do not create a sample image and simply
test whether wrong usage of the protocol is rejected as expected. In
this case, there will be no NBD server and trying to kill it during
clean-up will fail.

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/common.rc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 9c49deb..f2554ec 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -175,7 +175,9 @@ _cleanup_test_img()
     case "$IMGPROTO" in
 
         nbd)
-            kill $QEMU_NBD_PID
+            if [ -n "$QEMU_NBD_PID" ]; then
+                kill $QEMU_NBD_PID
+            fi
             rm -f "$TEST_IMG_FILE"
             ;;
         file)
-- 
1.9.3

[Qemu-devel] [PATCH v2 09/13] iotests: Add test for unsupported image creation

[Max Reitz]

Add a test for creating and amending images (amendment uses the creation
options) with formats not supporting creation over protocols not
supporting creation.

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/113     | 76 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/113.out | 15 +++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 92 insertions(+)
 create mode 100755 tests/qemu-iotests/113
 create mode 100644 tests/qemu-iotests/113.out

diff --git a/tests/qemu-iotests/113 b/tests/qemu-iotests/113
new file mode 100755
index 0000000..a2cd96b
--- /dev/null
+++ b/tests/qemu-iotests/113
@@ -0,0 +1,76 @@
+#!/bin/bash
+#
+# Test case for accessing creation options on image formats and
+# protocols not supporting image creation
+#
+# Copyright (C) 2014 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=mreitz@redhat.com
+
+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
+
+# We can only test one format here because we need its sample file
+_supported_fmt bochs
+_supported_proto nbd
+_supported_os Linux
+
+echo
+echo '=== Unsupported image creation in qemu-img create ==='
+echo
+
+$QEMU_IMG create -f $IMGFMT nbd://example.com 2>&1 64M | _filter_imgfmt
+
+echo
+echo '=== Unsupported image creation in qemu-img convert ==='
+echo
+
+# We could use any input image format here, but this is a bochs test, so just
+# use the bochs image
+_use_sample_img empty.bochs.bz2
+$QEMU_IMG convert -f $IMGFMT -O $IMGFMT "$TEST_IMG" nbd://example.com 2>&1 \
+    | _filter_imgfmt
+
+echo
+echo '=== Unsupported format in qemu-img amend ==='
+echo
+
+# The protocol does not matter here
+_use_sample_img empty.bochs.bz2
+$QEMU_IMG amend -f $IMGFMT -o foo=bar "$TEST_IMG" 2>&1 | _filter_imgfmt
+
+
+# success, all done
+echo
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/113.out b/tests/qemu-iotests/113.out
new file mode 100644
index 0000000..00bdfd6
--- /dev/null
+++ b/tests/qemu-iotests/113.out
@@ -0,0 +1,15 @@
+QA output created by 113
+
+=== Unsupported image creation in qemu-img create ===
+
+qemu-img: nbd://example.com: Format driver 'IMGFMT' does not support image creation
+
+=== Unsupported image creation in qemu-img convert ===
+
+qemu-img: Format driver 'IMGFMT' does not support image creation
+
+=== Unsupported format in qemu-img amend ===
+
+qemu-img: Format driver 'IMGFMT' does not support any options to amend
+
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 9188049..da59f57 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -112,4 +112,5 @@
 107 rw auto quick
 108 rw auto quick
 111 rw auto quick
+113 rw auto quick
 114 rw auto quick
-- 
1.9.3

[Qemu-devel] [PATCH v2 10/13] qcow2: Prevent numerical overflow

[Max Reitz]

In qcow2_alloc_cluster_offset(), *num is limited to
INT_MAX >> BDRV_SECTOR_BITS by all callers. However, since remaining is
of type uint64_t, we might as well cast *num to that type before
performing the shift.

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-cluster.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index df0b2c9..1fea514 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1263,7 +1263,7 @@ int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
 
 again:
     start = offset;
-    remaining = *num << BDRV_SECTOR_BITS;
+    remaining = (uint64_t)*num << BDRV_SECTOR_BITS;
     cluster_offset = 0;
     *host_offset = 0;
     cur_bytes = 0;
-- 
1.9.3

[Qemu-devel] [PATCH v2 11/13] qcow2: Flushing the caches in qcow2_close may fail

[Max Reitz]

qcow2_cache_flush() may fail; if one of the caches failed to be flushed
successfully to disk in qcow2_close() the image should not be marked
clean, and we should emit a warning.

This breaks the (qcow2-specific) iotests 026, 071 and 089; change their
output accordingly.

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c              |  19 +++++--
 tests/qemu-iotests/026.out | 120 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/071.out |   8 +++
 tests/qemu-iotests/089.out |   2 +
 4 files changed, 146 insertions(+), 3 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 0eeba36..41f896f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1428,10 +1428,23 @@ static void qcow2_close(BlockDriverState *bs)
     s->l1_table = NULL;
 
     if (!(bs->open_flags & BDRV_O_INCOMING)) {
-        qcow2_cache_flush(bs, s->l2_table_cache);
-        qcow2_cache_flush(bs, s->refcount_block_cache);
+        int ret1, ret2;
 
-        qcow2_mark_clean(bs);
+        ret1 = qcow2_cache_flush(bs, s->l2_table_cache);
+        ret2 = qcow2_cache_flush(bs, s->refcount_block_cache);
+
+        if (ret1) {
+            error_report("Failed to flush the L2 table cache: %s",
+                         strerror(-ret1));
+        }
+        if (ret2) {
+            error_report("Failed to flush the refcount block cache: %s",
+                         strerror(-ret2));
+        }
+
+        if (!ret1 && !ret2) {
+            qcow2_mark_clean(bs);
+        }
     }
 
     qcow2_cache_destroy(bs, s->l2_table_cache);
diff --git a/tests/qemu-iotests/026.out b/tests/qemu-iotests/026.out
index f7c78e7..ad84ac2 100644
--- a/tests/qemu-iotests/026.out
+++ b/tests/qemu-iotests/026.out
@@ -14,6 +14,8 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_update; errno: 5; imm: off; once: off; write 
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 
 1 leaked clusters were found on the image.
@@ -21,6 +23,8 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_update; errno: 5; imm: off; once: off; write -b
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 
 1 leaked clusters were found on the image.
@@ -38,6 +42,8 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_update; errno: 28; imm: off; once: off; write 
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 1 leaked clusters were found on the image.
@@ -45,6 +51,8 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_update; errno: 28; imm: off; once: off; write -b
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 1 leaked clusters were found on the image.
@@ -70,7 +78,11 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 Event: l2_load; errno: 5; imm: off; once: off; write 
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 read failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -78,7 +90,11 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 Event: l2_load; errno: 5; imm: off; once: off; write -b
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 read failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -102,7 +118,11 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 Event: l2_load; errno: 28; imm: off; once: off; write 
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 read failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -110,12 +130,17 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 Event: l2_load; errno: 28; imm: off; once: off; write -b
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 read failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_update; errno: 5; imm: off; once: on; write 
+Failed to flush the L2 table cache: Input/output error
 write failed: Input/output error
 
 127 leaked clusters were found on the image.
@@ -123,6 +148,7 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_update; errno: 5; imm: off; once: on; write -b
+Failed to flush the L2 table cache: Input/output error
 write failed: Input/output error
 
 127 leaked clusters were found on the image.
@@ -130,6 +156,8 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_update; errno: 5; imm: off; once: off; write 
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 
 127 leaked clusters were found on the image.
@@ -137,6 +165,8 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_update; errno: 5; imm: off; once: off; write -b
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 
 127 leaked clusters were found on the image.
@@ -144,6 +174,7 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_update; errno: 28; imm: off; once: on; write 
+Failed to flush the L2 table cache: No space left on device
 write failed: No space left on device
 
 127 leaked clusters were found on the image.
@@ -151,6 +182,7 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_update; errno: 28; imm: off; once: on; write -b
+Failed to flush the L2 table cache: No space left on device
 write failed: No space left on device
 
 127 leaked clusters were found on the image.
@@ -158,6 +190,8 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_update; errno: 28; imm: off; once: off; write 
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 127 leaked clusters were found on the image.
@@ -165,6 +199,8 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_update; errno: 28; imm: off; once: off; write -b
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 127 leaked clusters were found on the image.
@@ -182,11 +218,15 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_alloc.write; errno: 5; imm: off; once: off; write 
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_alloc.write; errno: 5; imm: off; once: off; write -b
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 
 1 leaked clusters were found on the image.
@@ -204,11 +244,15 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_alloc.write; errno: 28; imm: off; once: off; write 
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_alloc.write; errno: 28; imm: off; once: off; write -b
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 1 leaked clusters were found on the image.
@@ -226,11 +270,15 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: write_aio; errno: 5; imm: off; once: off; write 
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: write_aio; errno: 5; imm: off; once: off; write -b
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -246,11 +294,15 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: write_aio; errno: 28; imm: off; once: off; write 
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: write_aio; errno: 28; imm: off; once: off; write -b
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -266,11 +318,15 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_load; errno: 5; imm: off; once: off; write 
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_load; errno: 5; imm: off; once: off; write -b
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -286,51 +342,67 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_load; errno: 28; imm: off; once: off; write 
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_load; errno: 28; imm: off; once: off; write -b
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_update_part; errno: 5; imm: off; once: on; write 
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_update_part; errno: 5; imm: off; once: on; write -b
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_update_part; errno: 5; imm: off; once: off; write 
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_update_part; errno: 5; imm: off; once: off; write -b
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_update_part; errno: 28; imm: off; once: on; write 
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_update_part; errno: 28; imm: off; once: on; write -b
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_update_part; errno: 28; imm: off; once: off; write 
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_update_part; errno: 28; imm: off; once: off; write -b
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -346,11 +418,15 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc; errno: 5; imm: off; once: off; write 
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc; errno: 5; imm: off; once: off; write -b
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -366,11 +442,15 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc; errno: 28; imm: off; once: off; write 
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc; errno: 28; imm: off; once: off; write -b
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -386,11 +466,15 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: cluster_alloc; errno: 5; imm: off; once: off; write 
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: cluster_alloc; errno: 5; imm: off; once: off; write -b
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -406,11 +490,15 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: cluster_alloc; errno: 28; imm: off; once: off; write 
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: cluster_alloc; errno: 28; imm: off; once: off; write -b
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 
@@ -429,6 +517,8 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc.hookup; errno: 28; imm: off; once: off; write 
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 55 leaked clusters were found on the image.
@@ -436,6 +526,8 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc.hookup; errno: 28; imm: off; once: off; write -b
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 251 leaked clusters were found on the image.
@@ -453,11 +545,15 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc.write; errno: 28; imm: off; once: off; write 
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc.write; errno: 28; imm: off; once: off; write -b
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -473,6 +569,8 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc.write_blocks; errno: 28; imm: off; once: off; write 
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 11 leaked clusters were found on the image.
@@ -480,6 +578,8 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc.write_blocks; errno: 28; imm: off; once: off; write -b
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 23 leaked clusters were found on the image.
@@ -497,6 +597,8 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc.write_table; errno: 28; imm: off; once: off; write 
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 11 leaked clusters were found on the image.
@@ -504,6 +606,8 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc.write_table; errno: 28; imm: off; once: off; write -b
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 23 leaked clusters were found on the image.
@@ -521,6 +625,8 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc.switch_table; errno: 28; imm: off; once: off; write 
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 11 leaked clusters were found on the image.
@@ -528,6 +634,8 @@ This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc.switch_table; errno: 28; imm: off; once: off; write -b
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 23 leaked clusters were found on the image.
@@ -543,6 +651,8 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_grow.alloc_table; errno: 5; imm: off; once: off
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -553,6 +663,8 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_grow.alloc_table; errno: 28; imm: off; once: off
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -563,6 +675,8 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_grow.write_table; errno: 5; imm: off; once: off
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -573,6 +687,8 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_grow.write_table; errno: 28; imm: off; once: off
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
@@ -583,6 +699,8 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_grow.activate_table; errno: 5; imm: off; once: off
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
 
 96 leaked clusters were found on the image.
@@ -595,6 +713,8 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_grow.activate_table; errno: 28; imm: off; once: off
+Failed to flush the L2 table cache: No space left on device
+Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
 
 96 leaked clusters were found on the image.
diff --git a/tests/qemu-iotests/071.out b/tests/qemu-iotests/071.out
index 5f840a9..0624581 100644
--- a/tests/qemu-iotests/071.out
+++ b/tests/qemu-iotests/071.out
@@ -30,10 +30,14 @@ blkverify: read sector_num=0 nb_sectors=4 contents mismatch in sector 0
 
 === Testing blkdebug through filename ===
 
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 read failed: Input/output error
 
 === Testing blkdebug through file blockref ===
 
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 read failed: Input/output error
 
 === Testing blkdebug on existing block device ===
@@ -48,6 +52,8 @@ read failed: Input/output error
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
+qemu-system-x86_64: Failed to flush the L2 table cache: Input/output error
+qemu-system-x86_64: Failed to flush the refcount block cache: Input/output error
 
 
 === Testing blkverify on existing block device ===
@@ -86,5 +92,7 @@ read failed: Input/output error
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
+qemu-system-x86_64: Failed to flush the L2 table cache: Input/output error
+qemu-system-x86_64: Failed to flush the refcount block cache: Input/output error
 
 *** done
diff --git a/tests/qemu-iotests/089.out b/tests/qemu-iotests/089.out
index b2b0390..bf3b8a0 100644
--- a/tests/qemu-iotests/089.out
+++ b/tests/qemu-iotests/089.out
@@ -24,6 +24,8 @@ read 512/512 bytes at offset 0
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
 wrote 512/512 bytes at offset 229376
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Failed to flush the L2 table cache: Input/output error
+Failed to flush the refcount block cache: Input/output error
 read failed: Input/output error
 
 === Testing qemu-img info output ===
-- 
1.9.3

[Qemu-devel] [PATCH v2 12/13] qcow2: Respect bdrv_truncate() error

[Max Reitz]

bdrv_truncate() may fail and qcow2_write_compressed() should return the
error code in that case.

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 41f896f..7a28169 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2163,8 +2163,7 @@ static int qcow2_write_compressed(BlockDriverState *bs, int64_t sector_num,
         /* align end of file to a sector boundary to ease reading with
            sector based I/Os */
         cluster_offset = bdrv_getlength(bs->file);
-        bdrv_truncate(bs->file, cluster_offset);
-        return 0;
+        return bdrv_truncate(bs->file, cluster_offset);
     }
 
     if (nb_sectors != s->cluster_sectors) {
-- 
1.9.3

[Qemu-devel] [PATCH v2 13/13] block/raw-posix: Fix ret in raw_open_common()

[Max Reitz]

The return value must be negative on error; there is one place in
raw_open_common() where errp is set, but ret remains 0. Fix it.

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/raw-posix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 6766cec..49d05d5 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -446,6 +446,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
     }
 
     if (fstat(s->fd, &st) < 0) {
+        ret = -errno;
         error_setg_errno(errp, errno, "Could not stat file");
         goto fail;
     }
-- 
1.9.3

Re: [Qemu-devel] [PATCH v2 01/13] block: Make essential BlockDriver objects public

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1329 bytes --]

On 11/27/2014 07:48 AM, Max Reitz wrote:
> There are some block drivers which are essential to QEMU and may not be
> removed: These are raw, file and qcow2 (as the default non-raw format).
> Make their BlockDriver objects public so they can be directly referenced
> throughout the block layer without needing to call bdrv_find_format()
> and having to deal with an error at runtime, while the real problem
> occured during linking (where raw, file or qcow2 were not linked into

s/occured/occurred/

> qemu).
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2.c             | 4 ++--
>  block/raw-posix.c         | 4 ++--
>  block/raw-win32.c         | 4 ++--
>  block/raw_bsd.c           | 4 ++--
>  include/block/block_int.h | 8 ++++++++
>  5 files changed, 16 insertions(+), 8 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> +++ b/block/qcow2.c
> @@ -2847,7 +2847,7 @@ static QemuOptsList qcow2_create_opts = {
>      }
>  };
>  
> -static BlockDriver bdrv_qcow2 = {
> +BlockDriver *bdrv_qcow2 = &(BlockDriver){

Do we want any use of 'const', to avoid accidental manipulation of the
pointer and/or pointed-to contents?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH v2 02/13] block: Omit bdrv_find_format for essential drivers

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]

On 11/27/2014 07:48 AM, Max Reitz wrote:
> We can always assume raw, file and qcow2 being available; so do not use
> bdrv_find_format() to locate their BlockDriver objects but statically
> reference the respective objects.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> @@ -1293,7 +1288,6 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
>      /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
>      char *tmp_filename = g_malloc0(PATH_MAX + 1);
>      int64_t total_size;
> -    BlockDriver *bdrv_qcow2;

Hmm - how hard would it be to get qemu to be clean under -Wshadow?  This
is a case where you would have had to change this hunk during patch 1 if
-Wshadow were in effect.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH v2 01/13] block: Make essential BlockDriver objects public

[Max Reitz]

On 2014-12-01 at 16:59, Eric Blake wrote:
> On 11/27/2014 07:48 AM, Max Reitz wrote:
>> There are some block drivers which are essential to QEMU and may not be
>> removed: These are raw, file and qcow2 (as the default non-raw format).
>> Make their BlockDriver objects public so they can be directly referenced
>> throughout the block layer without needing to call bdrv_find_format()
>> and having to deal with an error at runtime, while the real problem
>> occured during linking (where raw, file or qcow2 were not linked into
> s/occured/occurred/
>
>> qemu).
>>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   block/qcow2.c             | 4 ++--
>>   block/raw-posix.c         | 4 ++--
>>   block/raw-win32.c         | 4 ++--
>>   block/raw_bsd.c           | 4 ++--
>>   include/block/block_int.h | 8 ++++++++
>>   5 files changed, 16 insertions(+), 8 deletions(-)
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
>> +++ b/block/qcow2.c
>> @@ -2847,7 +2847,7 @@ static QemuOptsList qcow2_create_opts = {
>>       }
>>   };
>>   
>> -static BlockDriver bdrv_qcow2 = {
>> +BlockDriver *bdrv_qcow2 = &(BlockDriver){
> Do we want any use of 'const', to avoid accidental manipulation of the
> pointer and/or pointed-to contents?

Sounds good at first, but for instance qemu_opts_create() (which is 
often called with bdrv_qcow2->create_opts and the like) don't take a 
const pointer. We could fix all those functions, but trying to fix the 
const-ness of the block layer sounds like really tedious work to me... 
Also, bdrv_find_format() returns a non-const pointer so it's at least 
not more broken than it was before.

Max

Re: [Qemu-devel] [PATCH v2 02/13] block: Omit bdrv_find_format for essential drivers

[Max Reitz]

On 2014-12-01 at 17:01, Eric Blake wrote:
> On 11/27/2014 07:48 AM, Max Reitz wrote:
>> We can always assume raw, file and qcow2 being available; so do not use
>> bdrv_find_format() to locate their BlockDriver objects but statically
>> reference the respective objects.
>>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   block.c | 11 ++---------
>>   1 file changed, 2 insertions(+), 9 deletions(-)
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
>> @@ -1293,7 +1288,6 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
>>       /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
>>       char *tmp_filename = g_malloc0(PATH_MAX + 1);
>>       int64_t total_size;
>> -    BlockDriver *bdrv_qcow2;
> Hmm - how hard would it be to get qemu to be clean under -Wshadow?  This
> is a case where you would have had to change this hunk during patch 1 if
> -Wshadow were in effect.

Well, I know I once found a bug which would have been spotted by 
-Wshadow; so in principle, I'm in favor of trying to enforce -Wshadow. 
On the other hand, I guess it may be really hard. We could try some 
time, but I don't want to have to run through all of qemu's code...

Max

Re: [Qemu-devel] [PATCH v2 01/13] block: Make essential BlockDriver objects public

[Kevin Wolf]

Am 27.11.2014 um 15:48 hat Max Reitz geschrieben:
> There are some block drivers which are essential to QEMU and may not be
> removed: These are raw, file and qcow2 (as the default non-raw format).
> Make their BlockDriver objects public so they can be directly referenced
> throughout the block layer without needing to call bdrv_find_format()
> and having to deal with an error at runtime, while the real problem
> occured during linking (where raw, file or qcow2 were not linked into
> qemu).
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2.c             | 4 ++--
>  block/raw-posix.c         | 4 ++--
>  block/raw-win32.c         | 4 ++--
>  block/raw_bsd.c           | 4 ++--
>  include/block/block_int.h | 8 ++++++++
>  5 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 8b9ffc4..0eeba36 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2847,7 +2847,7 @@ static QemuOptsList qcow2_create_opts = {
>      }
>  };
>  
> -static BlockDriver bdrv_qcow2 = {
> +BlockDriver *bdrv_qcow2 = &(BlockDriver){

As you correctly guessed, I like the looks of this syntax, but it leaves
us with some bdrv_<format> objects being pointers, and most others still
directly being the BlockDriver object.

I think it would be better to define a public BlockDriver here and use
&bdrv_qcow2 where necessary. Perhaps not a blocker, but in case you need
to send a v3.

Kevin

Re: [Qemu-devel] [PATCH v2 03/13] block/vvfat: qcow driver may not be found

[Kevin Wolf]

Am 27.11.2014 um 15:48 hat Max Reitz geschrieben:
> Although virtually impossible right now, bdrv_find_format("qcow") may
> fail. The vvfat block driver should heed that case.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Max Reitz <mreitz@redhat.com>

I wonder what the subtle breakage would be that we would get if we
changed this from qcow1 to qcow2...

Kevin

Re: [Qemu-devel] [PATCH v2 00/13] block: Various Coverity-spotted fixes

[Kevin Wolf]

Am 27.11.2014 um 15:48 hat Max Reitz geschrieben:
> This series fixes various issues spotted by Coverity. None of these is
> critical; most are just "If you do something crazy, qemu-img crashes" or
> "But what if there is no qcow2 driver?". Therefore, while these are bug
> fixes, it is a bit late to try to push them into 2.2.0. I am therefore
> tempted to vote to target 2.3 instead.
> 
> Also, none is security-relevant. The only crashes which are fixed here
> are sure to have resulted from dereferencing a NULL pointer.

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH v2 01/13] block: Make essential BlockDriver objects public

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 888 bytes --]

On 12/02/2014 02:11 AM, Max Reitz wrote:

>>>   -static BlockDriver bdrv_qcow2 = {
>>> +BlockDriver *bdrv_qcow2 = &(BlockDriver){
>> Do we want any use of 'const', to avoid accidental manipulation of the
>> pointer and/or pointed-to contents?
> 
> Sounds good at first, but for instance qemu_opts_create() (which is
> often called with bdrv_qcow2->create_opts and the like) don't take a
> const pointer. We could fix all those functions, but trying to fix the
> const-ness of the block layer sounds like really tedious work to me...
> Also, bdrv_find_format() returns a non-const pointer so it's at least
> not more broken than it was before.

Fair enough - no need to hold up this patch on what would turn into a
much larger task of chasing const-correctness.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]