[Qemu-devel] [PULL 00/11] Block patches

[Qemu-devel] [PULL 00/11] Block patches

[Stefan Hajnoczi]

The following changes since commit 0888a29caac6e1b668e498a0ad4d1fea15de012b:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2014-02-13 15:02:04 +0000)

are available in the git repository at:


  git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 0c5e94ee8339e1aa49020466eba232e6f7c31a0a:

  block: Open by reference will try device then node_name. (2014-02-14 18:05:39 +0100)

----------------------------------------------------------------
Block pull request

----------------------------------------------------------------
Benoît Canet (3):
      blockdev: Fix wrong usage of QDECREF causing snapshoted quorum to crash on close.
      block: Relax bdrv_lookup_bs constraints.
      block: Open by reference will try device then node_name.

Fam Zheng (1):
      qemu-iotests: Don't run 005 on vmdk split formats

Jeff Cody (5):
      block: Add notes to iSCSI's .bdrv_open and .bdrv_reopen_prepare
      block: Don't throw away errno via error_setg
      block: qemu-iotests - fix test 070 (vhdx)
      block: qemu-iotests - add vhdx log replay tests for qemu-img
      block: mirror - use local_err to avoid NULL errp

Kevin Wolf (2):
      sdhci: Drop unnecessary #include
      blockdev: Remove 'type' parameter from blockdev_init()

 block.c                    | 36 ++++++++++++++++--------------
 block/iscsi.c              | 10 +++++++--
 block/mirror.c             | 19 +++++++++++-----
 block/qcow2-snapshot.c     |  8 ++++---
 block/vmdk.c               |  6 ++---
 blockdev.c                 | 55 ++++++++++++++++++++++++++++++++--------------
 hw/sd/sdhci.c              |  1 -
 tests/qemu-iotests/005     |  2 ++
 tests/qemu-iotests/070     | 13 ++++++++++-
 tests/qemu-iotests/070.out | 15 ++++++++++++-
 10 files changed, 115 insertions(+), 50 deletions(-)

-- 
1.8.5.3

[Qemu-devel] [PULL 01/11] sdhci: Drop unnecessary #include

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/sd/sdhci.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 0906a1d..a0b90ba 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -26,7 +26,6 @@
 #include "sysemu/blockdev.h"
 #include "sysemu/dma.h"
 #include "qemu/timer.h"
-#include "block/block_int.h"
 #include "qemu/bitops.h"
 
 #include "sdhci.h"
-- 
1.8.5.3

[Qemu-devel] [PULL 02/11] blockdev: Remove 'type' parameter from blockdev_init()

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

blockdev-add doesn't know about the device that the backend will be
attached to, this is a legacy -drive concept. Move the remaining checks
that use it to drive_init().

[Fam Zheng <famz@redhat.com> suggested line-wrapping to 80 chars as
required by the coding standard.  I have fixed this.
--Stefan]

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 blockdev.c | 47 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 14 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 36ceece..d5f21f0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -308,7 +308,6 @@ typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType;
 
 /* Takes the ownership of bs_opts */
 static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
-                                BlockInterfaceType type,
                                 Error **errp)
 {
     const char *buf;
@@ -437,11 +436,6 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
 
     on_write_error = BLOCKDEV_ON_ERROR_ENOSPC;
     if ((buf = qemu_opt_get(opts, "werror")) != NULL) {
-        if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && type != IF_NONE) {
-            error_setg(errp, "werror is not supported by this bus type");
-            goto early_err;
-        }
-
         on_write_error = parse_block_error_action(buf, 0, &error);
         if (error_is_set(&error)) {
             error_propagate(errp, error);
@@ -451,11 +445,6 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
 
     on_read_error = BLOCKDEV_ON_ERROR_REPORT;
     if ((buf = qemu_opt_get(opts, "rerror")) != NULL) {
-        if (type != IF_IDE && type != IF_VIRTIO && type != IF_SCSI && type != IF_NONE) {
-            error_report("rerror is not supported by this bus type");
-            goto early_err;
-        }
-
         on_read_error = parse_block_error_action(buf, 1, &error);
         if (error_is_set(&error)) {
             error_propagate(errp, error);
@@ -469,7 +458,6 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
     dinfo->bdrv = bdrv_new(dinfo->id);
     dinfo->bdrv->open_flags = snapshot ? BDRV_O_SNAPSHOT : 0;
     dinfo->bdrv->read_only = ro;
-    dinfo->type = type;
     dinfo->refcount = 1;
     if (serial != NULL) {
         dinfo->serial = g_strdup(serial);
@@ -609,6 +597,14 @@ QemuOptsList qemu_legacy_drive_opts = {
             .type = QEMU_OPT_BOOL,
             .help = "open drive file as read-only",
         },{
+            .name = "rerror",
+            .type = QEMU_OPT_STRING,
+            .help = "read error action",
+        },{
+            .name = "werror",
+            .type = QEMU_OPT_STRING,
+            .help = "write error action",
+        },{
             .name = "copy-on-read",
             .type = QEMU_OPT_BOOL,
             .help = "copy read data from backing file into image file",
@@ -629,6 +625,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     int cyls, heads, secs, translation;
     int max_devs, bus_id, unit_id, index;
     const char *devaddr;
+    const char *werror, *rerror;
     bool read_only = false;
     bool copy_on_read;
     const char *filename;
@@ -872,8 +869,29 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
 
     filename = qemu_opt_get(legacy_opts, "file");
 
+    /* Check werror/rerror compatibility with if=... */
+    werror = qemu_opt_get(legacy_opts, "werror");
+    if (werror != NULL) {
+        if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO &&
+            type != IF_NONE) {
+            error_report("werror is not supported by this bus type");
+            goto fail;
+        }
+        qdict_put(bs_opts, "werror", qstring_from_str(werror));
+    }
+
+    rerror = qemu_opt_get(legacy_opts, "rerror");
+    if (rerror != NULL) {
+        if (type != IF_IDE && type != IF_VIRTIO && type != IF_SCSI &&
+            type != IF_NONE) {
+            error_report("rerror is not supported by this bus type");
+            goto fail;
+        }
+        qdict_put(bs_opts, "rerror", qstring_from_str(rerror));
+    }
+
     /* Actual block device init: Functionality shared with blockdev-add */
-    dinfo = blockdev_init(filename, bs_opts, type, &local_err);
+    dinfo = blockdev_init(filename, bs_opts, &local_err);
     if (dinfo == NULL) {
         if (error_is_set(&local_err)) {
             qerror_report_err(local_err);
@@ -893,6 +911,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     dinfo->secs = secs;
     dinfo->trans = translation;
 
+    dinfo->type = type;
     dinfo->bus = bus_id;
     dinfo->unit = unit_id;
     dinfo->devaddr = devaddr;
@@ -2276,7 +2295,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
 
     qdict_flatten(qdict);
 
-    blockdev_init(NULL, qdict, IF_NONE, &local_err);
+    blockdev_init(NULL, qdict, &local_err);
     if (error_is_set(&local_err)) {
         error_propagate(errp, local_err);
         goto fail;
-- 
1.8.5.3

[Qemu-devel] [PULL 03/11] block: Add notes to iSCSI's .bdrv_open and .bdrv_reopen_prepare

[Stefan Hajnoczi]

From: Jeff Cody <jcody@redhat.com>

iSCSI currently does not need to do any actions to support the
current usage of bdrv_reopen().  However, it is important to note
a couple of things: 1.) A connection will not be re-established to
an iSCSI target, and 2.) If iscsi_open() is changed to parse 'flags',
then iscsi_reopen_prepare() may need to be more than a stub.

In light of the above, this commit adds comments above both of the
functions to bring attention to these facts.

Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/iscsi.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 8d0f966..c97c040 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1099,6 +1099,10 @@ fail:
 /*
  * We support iscsi url's on the form
  * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
+ *
+ * Note: flags are currently not used by iscsi_open.  If this function
+ * is changed such that flags are used, please examine iscsi_reopen_prepare()
+ * to see if needs to be changed as well.
  */
 static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
                       Error **errp)
@@ -1336,11 +1340,13 @@ static int iscsi_refresh_limits(BlockDriverState *bs)
     return 0;
 }
 
-/* We have nothing to do for iSCSI reopen, stub just returns
- * success */
+/* Since iscsi_open() ignores bdrv_flags, there is nothing to do here in
+ * prepare.  Note that this will not re-establish a connection with an iSCSI
+ * target - it is effectively a NOP.  */
 static int iscsi_reopen_prepare(BDRVReopenState *state,
                                 BlockReopenQueue *queue, Error **errp)
 {
+    /* NOP */
     return 0;
 }
 
-- 
1.8.5.3

[Qemu-devel] [PULL 04/11] block: Don't throw away errno via error_setg

[Stefan Hajnoczi]

From: Jeff Cody <jcody@redhat.com>

There are a handful of places in the block layer where a failure path
has a valid -errno value, yet error_setg() is used.  Those instances
should instead use error_setg_errno(), to preserve as much error
information as possible.

This patch replaces those instances with error_setg_errno(), so that
errno is passed up the stack in the error message.

Reported-By: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/mirror.c         | 13 +++++++++----
 block/qcow2-snapshot.c |  8 +++++---
 block/vmdk.c           |  6 +++---
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 2a43334..26d18e4 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -633,6 +633,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
 {
     int64_t length, base_length;
     int orig_base_flags;
+    int ret;
 
     orig_base_flags = bdrv_get_flags(base);
 
@@ -642,19 +643,23 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
 
     length = bdrv_getlength(bs);
     if (length < 0) {
-        error_setg(errp, "Unable to determine length of %s", bs->filename);
+        error_setg_errno(errp, -length,
+                         "Unable to determine length of %s", bs->filename);
         goto error_restore_flags;
     }
 
     base_length = bdrv_getlength(base);
     if (base_length < 0) {
-        error_setg(errp, "Unable to determine length of %s", base->filename);
+        error_setg_errno(errp, -base_length,
+                         "Unable to determine length of %s", base->filename);
         goto error_restore_flags;
     }
 
     if (length > base_length) {
-        if (bdrv_truncate(base, length) < 0) {
-            error_setg(errp, "Top image %s is larger than base image %s, and "
+        ret = bdrv_truncate(base, length);
+        if (ret < 0) {
+            error_setg_errno(errp, -ret,
+                            "Top image %s is larger than base image %s, and "
                              "resize of base image failed",
                              bs->filename, base->filename);
             goto error_restore_flags;
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index ad8bf3d..2fc6320 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -606,7 +606,8 @@ int qcow2_snapshot_delete(BlockDriverState *bs,
     s->nb_snapshots--;
     ret = qcow2_write_snapshots(bs);
     if (ret < 0) {
-        error_setg(errp, "Failed to remove snapshot from snapshot list");
+        error_setg_errno(errp, -ret,
+                         "Failed to remove snapshot from snapshot list");
         return ret;
     }
 
@@ -624,7 +625,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs,
     ret = qcow2_update_snapshot_refcount(bs, sn.l1_table_offset,
                                          sn.l1_size, -1);
     if (ret < 0) {
-        error_setg(errp, "Failed to free the cluster and L1 table");
+        error_setg_errno(errp, -ret, "Failed to free the cluster and L1 table");
         return ret;
     }
     qcow2_free_clusters(bs, sn.l1_table_offset, sn.l1_size * sizeof(uint64_t),
@@ -633,7 +634,8 @@ int qcow2_snapshot_delete(BlockDriverState *bs,
     /* must update the copied flag on the current cluster offsets */
     ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 0);
     if (ret < 0) {
-        error_setg(errp, "Failed to update snapshot status in disk");
+        error_setg_errno(errp, -ret,
+                         "Failed to update snapshot status in disk");
         return ret;
     }
 
diff --git a/block/vmdk.c b/block/vmdk.c
index e809e2e..ff6f5ee 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1502,7 +1502,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
     if (flat) {
         ret = bdrv_truncate(bs, filesize);
         if (ret < 0) {
-            error_setg(errp, "Could not truncate file");
+            error_setg_errno(errp, -ret, "Could not truncate file");
         }
         goto exit;
     }
@@ -1562,7 +1562,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
 
     ret = bdrv_truncate(bs, le64_to_cpu(header.grain_offset) << 9);
     if (ret < 0) {
-        error_setg(errp, "Could not truncate file");
+        error_setg_errno(errp, -ret, "Could not truncate file");
         goto exit;
     }
 
@@ -1846,7 +1846,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
     if (desc_offset == 0) {
         ret = bdrv_truncate(new_bs, desc_len);
         if (ret < 0) {
-            error_setg(errp, "Could not truncate file");
+            error_setg_errno(errp, -ret, "Could not truncate file");
         }
     }
 exit:
-- 
1.8.5.3

[Qemu-devel] [PULL 05/11] block: qemu-iotests - fix test 070 (vhdx)

[Stefan Hajnoczi]

From: Jeff Cody <jcody@redhat.com>

VHDX test 070 failed, due to different output from qemu-io / qemu
when opening an image read-only that contains a log file.  Filter
the output, and update the expected results to match the correct
output.

Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/070     | 3 ++-
 tests/qemu-iotests/070.out | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/070 b/tests/qemu-iotests/070
index 41bf100..f84d2cb 100755
--- a/tests/qemu-iotests/070
+++ b/tests/qemu-iotests/070
@@ -56,7 +56,8 @@ _use_sample_img iotest-dirtylog-10G-4M.vhdx.bz2
 
 echo
 echo "=== Verify open image read-only fails, due to dirty log ==="
-$QEMU_IO -r -c "read -pP 0xa5 0 18M" "$TEST_IMG" 2>&1 | grep -o "Permission denied"
+$QEMU_IO -r -c "read -pP 0xa5 0 18M" "$TEST_IMG" 2>&1 | _filter_testdir \
+                                                      | _filter_qemu_io
 
 echo "=== Verify open image replays log  ==="
 $QEMU_IO  -c "read -pP 0xa5 0 18M" "$TEST_IMG" | _filter_qemu_io
diff --git a/tests/qemu-iotests/070.out b/tests/qemu-iotests/070.out
index 9db8ff2..0352adc 100644
--- a/tests/qemu-iotests/070.out
+++ b/tests/qemu-iotests/070.out
@@ -1,7 +1,9 @@
 QA output created by 070
 
 === Verify open image read-only fails, due to dirty log ===
-Permission denied
+qemu-io: can't open device TEST_DIR/iotest-dirtylog-10G-4M.vhdx: VHDX image file 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx' opened read-only, but contains a log that needs to be replayed.  To replay the log, execute:
+ qemu-img check -r all 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx': Operation not permitted
+ no file open, try 'help open'
 === Verify open image replays log  ===
 read 18874368/18874368 bytes at offset 0
 18 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-- 
1.8.5.3

[Qemu-devel] [PULL 06/11] block: qemu-iotests - add vhdx log replay tests for qemu-img

[Stefan Hajnoczi]

From: Jeff Cody <jcody@redhat.com>

VHDX logs can now be replayed via 'qemu-img check -r all'.  Add
tests to verify that the log replay is successful when using qemu-img.

Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/070     | 10 ++++++++++
 tests/qemu-iotests/070.out | 11 +++++++++++
 2 files changed, 21 insertions(+)

diff --git a/tests/qemu-iotests/070 b/tests/qemu-iotests/070
index f84d2cb..ce71fa4 100755
--- a/tests/qemu-iotests/070
+++ b/tests/qemu-iotests/070
@@ -62,6 +62,16 @@ $QEMU_IO -r -c "read -pP 0xa5 0 18M" "$TEST_IMG" 2>&1 | _filter_testdir \
 echo "=== Verify open image replays log  ==="
 $QEMU_IO  -c "read -pP 0xa5 0 18M" "$TEST_IMG" | _filter_qemu_io
 
+# extract fresh sample image again
+_use_sample_img iotest-dirtylog-10G-4M.vhdx.bz2
+
+echo "=== Verify qemu-img check -r all replays log  ==="
+$QEMU_IMG check -r all "$TEST_IMG" 2>&1 | _filter_testdir | _filter_qemu
+
+echo "=== Verify open image read-only succeeds after log replay ==="
+$QEMU_IO -r -c "read -pP 0xa5 0 18M" "$TEST_IMG" 2>&1 | _filter_testdir \
+                                                      | _filter_qemu_io
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/070.out b/tests/qemu-iotests/070.out
index 0352adc..922d62c 100644
--- a/tests/qemu-iotests/070.out
+++ b/tests/qemu-iotests/070.out
@@ -7,4 +7,15 @@ qemu-io: can't open device TEST_DIR/iotest-dirtylog-10G-4M.vhdx: VHDX image file
 === Verify open image replays log  ===
 read 18874368/18874368 bytes at offset 0
 18 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+=== Verify qemu-img check -r all replays log  ===
+The following inconsistencies were found and repaired:
+
+    0 leaked clusters
+    1 corruptions
+
+Double checking the fixed image now...
+No errors were found on the image.
+=== Verify open image read-only succeeds after log replay ===
+read 18874368/18874368 bytes at offset 0
+18 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 *** done
-- 
1.8.5.3

[Qemu-devel] [PULL 07/11] qemu-iotests: Don't run 005 on vmdk split formats

[Stefan Hajnoczi]

From: Fam Zheng <famz@redhat.com>

There would be too many extents that VMDK driver can't open all of them:

    005 0s ... - output mismatch (see 005.out.bad)
    --- 005.out     2013-12-24 09:27:27.608181030 +0800
    +++ 005.out.bad 2014-02-13 10:00:15.282184557 +0800
    @@ -4,10 +4,10 @@
     Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=5368709120000

     small read
    -read 4096/4096 bytes at offset 1024
    -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
    +qemu-io: can't open device /tmp/qemu-iotests/t.vmdk: Could not open '/tmp/qemu-iotests/t-s1016.vmdk': Too many open files
    +no file open, try 'help open'

     small write
    -wrote 4096/4096 bytes at offset 8192
    -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
    +qemu-io: can't open device /tmp/qemu-iotests/t.vmdk: Could not open '/tmp/qemu-iotests/t-s1016.vmdk': Too many open files
    +no file open, try 'help open'
     *** done

So disable the two subformats.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/005 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qemu-iotests/005 b/tests/qemu-iotests/005
index 9abcb84..ba1236d 100755
--- a/tests/qemu-iotests/005
+++ b/tests/qemu-iotests/005
@@ -44,6 +44,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt generic
 _supported_proto generic
 _supported_os Linux
+_unsupported_imgopts "subformat=twoGbMaxExtentFlat" \
+                     "subformat=twoGbMaxExtentSparse"
 
 # vpc is limited to 127GB, so we can't test it here
 if [ "$IMGFMT" = "vpc" ]; then
-- 
1.8.5.3

[Qemu-devel] [PULL 08/11] block: mirror - use local_err to avoid NULL errp

[Stefan Hajnoczi]

From: Jeff Cody <jcody@redhat.com>

When starting a block job, commit_active_start() relies on whether *errp
is set by mirror_start_job.  This allows it to determine if the mirror
job start failed, so that it can clean up any changes to open flags from
the bdrv_reopen().  If errp is NULL, then it will not be able to
determine if mirror_start_job failed or not.

To avoid this, use a local Error variable, and then propagate the error
(if any) to errp.

Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/mirror.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 26d18e4..e683959 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -634,6 +634,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
     int64_t length, base_length;
     int orig_base_flags;
     int ret;
+    Error *local_err = NULL;
 
     orig_base_flags = bdrv_get_flags(base);
 
@@ -668,9 +669,10 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
 
     bdrv_ref(base);
     mirror_start_job(bs, base, speed, 0, 0,
-                     on_error, on_error, cb, opaque, errp,
+                     on_error, on_error, cb, opaque, &local_err,
                      &commit_active_job_driver, false, base);
-    if (error_is_set(errp)) {
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
         goto error_restore_flags;
     }
 
-- 
1.8.5.3

[Qemu-devel] [PULL 09/11] blockdev: Fix wrong usage of QDECREF causing snapshoted quorum to crash on close.

[Stefan Hajnoczi]

From: Benoît Canet <benoit.canet@irqsave.net>

As bdrv_open() documentation states:
"The reference to the QDict belongs to the block layer
 * after the call (even on failure), so if the caller intends to reuse the
 * dictionary, it needs to use QINCREF() before calling bdrv_open."

the optional options dict will not be reused after bdrv_open() and should
belong to the block layer so remove the extra QDECREF(options).

Signed-off-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 blockdev.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index d5f21f0..ccd6a72 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1329,8 +1329,6 @@ static void external_snapshot_prepare(BlkTransactionState *common,
     if (ret != 0) {
         error_propagate(errp, local_err);
     }
-
-    QDECREF(options);
 }
 
 static void external_snapshot_commit(BlkTransactionState *common)
-- 
1.8.5.3

[Qemu-devel] [PULL 10/11] block: Relax bdrv_lookup_bs constraints.

[Stefan Hajnoczi]

From: Benoît Canet <benoit.canet@irqsave.net>

The following patch will reuse bdrv_lookup_bs in order to open images by
references so the rules of usage of bdrv_lookup_bs must be relaxed a bit.

Signed-off-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index 636aa11..ba2aecf 100644
--- a/block.c
+++ b/block.c
@@ -3574,30 +3574,26 @@ BlockDriverState *bdrv_lookup_bs(const char *device,
 {
     BlockDriverState *bs = NULL;
 
-    if ((!device && !node_name) || (device && node_name)) {
-        error_setg(errp, "Use either device or node-name but not both");
-        return NULL;
-    }
-
     if (device) {
         bs = bdrv_find(device);
 
-        if (!bs) {
-            error_set(errp, QERR_DEVICE_NOT_FOUND, device);
-            return NULL;
+        if (bs) {
+            return bs;
         }
-
-        return bs;
     }
 
-    bs = bdrv_find_node(node_name);
+    if (node_name) {
+        bs = bdrv_find_node(node_name);
 
-    if (!bs) {
-        error_set(errp, QERR_DEVICE_NOT_FOUND, node_name);
-        return NULL;
+        if (bs) {
+            return bs;
+        }
     }
 
-    return bs;
+    error_setg(errp, "Cannot find device=%s nor node_name=%s",
+                     device ? device : "",
+                     node_name ? node_name : "");
+    return NULL;
 }
 
 BlockDriverState *bdrv_next(BlockDriverState *bs)
-- 
1.8.5.3

[Qemu-devel] [PULL 11/11] block: Open by reference will try device then node_name.

[Stefan Hajnoczi]

From: Benoît Canet <benoit.canet@irqsave.net>

Since we introduced node_name for named bs of the graph modify the opening by
reference to use it as a fallback.

This patch also enforce the separation of the device id and graph node
namespaces.

Signed-off-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c    | 10 ++++++++--
 blockdev.c |  6 ++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index ba2aecf..8f718f9 100644
--- a/block.c
+++ b/block.c
@@ -796,6 +796,13 @@ static int bdrv_assign_node_name(BlockDriverState *bs,
         return -EINVAL;
     }
 
+    /* takes care of avoiding namespaces collisions */
+    if (bdrv_find(node_name)) {
+        error_setg(errp, "node-name=%s is conflicting with a device id",
+                   node_name);
+        return -EINVAL;
+    }
+
     /* takes care of avoiding duplicates node names */
     if (bdrv_find_node(node_name)) {
         error_setg(errp, "Duplicate node name");
@@ -977,9 +984,8 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename,
         }
         QDECREF(options);
 
-        bs = bdrv_find(reference);
+        bs = bdrv_lookup_bs(reference, reference, errp);
         if (!bs) {
-            error_setg(errp, "Cannot find block device '%s'", reference);
             return -ENODEV;
         }
         bdrv_ref(bs);
diff --git a/blockdev.c b/blockdev.c
index ccd6a72..dfb5ec7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -452,6 +452,12 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
         }
     }
 
+    if (bdrv_find_node(qemu_opts_id(opts))) {
+        error_setg(errp, "device id=%s is conflicting with a node-name",
+                   qemu_opts_id(opts));
+        goto early_err;
+    }
+
     /* init */
     dinfo = g_malloc0(sizeof(*dinfo));
     dinfo->id = g_strdup(qemu_opts_id(opts));
-- 
1.8.5.3

Re: [Qemu-devel] [PULL 00/11] Block patches

[Peter Maydell]

On 14 February 2014 17:29, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The following changes since commit 0888a29caac6e1b668e498a0ad4d1fea15de012b:
>
>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2014-02-13 15:02:04 +0000)
>
> are available in the git repository at:
>
>
>   git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 0c5e94ee8339e1aa49020466eba232e6f7c31a0a:
>
>   block: Open by reference will try device then node_name. (2014-02-14 18:05:39 +0100)

Applied, thanks.

-- PMM