[Qemu-devel] [PATCH v2 0/2] block: driver should override flags in bdrv_open()

[Qemu-devel] [PATCH v2 0/2] block: driver should override flags in bdrv_open()

[Max Reitz]

BDRV_O_PROTOCOL is an internal qemu flag which a user should be able to
override by explicitly specifying a block driver. This series implements
this and adds two iotests (one for NBD, one for file) to test it.


v2 (rebase on current master):
- Patch 1: Conflict in iotest 051's reference output (in a line that is
  removed by this patch anyway)


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/2:[0002] [FC] 'block: driver should override flags in bdrv_open()'
002/2:[----] [-C] 'iotests: Add tests for overriding BDRV_O_PROTOCOL'


Max Reitz (2):
  block: driver should override flags in bdrv_open()
  iotests: Add tests for overriding BDRV_O_PROTOCOL

 block.c                    | 49 +++++++++++++++++++++-------------
 tests/qemu-iotests/051     |  1 -
 tests/qemu-iotests/051.out |  3 ---
 tests/qemu-iotests/119     | 60 ++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/119.out | 11 ++++++++
 tests/qemu-iotests/120     | 65 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/120.out | 15 +++++++++++
 tests/qemu-iotests/group   |  2 ++
 8 files changed, 184 insertions(+), 22 deletions(-)
 create mode 100755 tests/qemu-iotests/119
 create mode 100644 tests/qemu-iotests/119.out
 create mode 100755 tests/qemu-iotests/120
 create mode 100644 tests/qemu-iotests/120.out

-- 
2.1.0

[Qemu-devel] [PATCH v2 1/2] block: driver should override flags in bdrv_open()

[Max Reitz]

The BDRV_O_PROTOCOL flag should have an impact only if no driver is
specified explicitly. Therefore, if bdrv_open() is called with an
explicit block driver argument (either through the options QDict or
through the drv parameter) and that block driver is a protocol block
driver, BDRV_O_PROTOCOL should be set; if it is a format block driver,
BDRV_O_PROTOCOL should be unset.

While there was code to unset the flag in case a format block driver
has been selected, it only followed the bdrv_fill_options() function
call whereas the flag in fact needs to be adjusted before it is used
there.

With that change, BDRV_O_PROTOCOL will always be set if the BDS should
be a protocol driver; if the driver has been specified explicitly, the
new code will set it; and bdrv_fill_options() will only "probe" a
protocol driver if BDRV_O_PROTOCOL is set. The probing after
bdrv_fill_options() cannot select a protocol driver.

Thus, bdrv_open_image() to open BDS.file is never called if a protocol
BDS is about to be created. With that change in turn it is impossible to
call bdrv_open_common() with a protocol drv and file != NULL, which
allows us to remove the bdrv_swap() call.

This change breaks a test case in qemu-iotest 051:
"-drive file=t.qcow2,file.driver=qcow2" now works because the explicitly
specified "qcow2" overrides the BDRV_O_PROTOCOL which is automatically
set for the "file" BDS (and the filename is just passed down).
Therefore, this patch removes that test case.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c                    | 49 +++++++++++++++++++++++++++++-----------------
 tests/qemu-iotests/051     |  1 -
 tests/qemu-iotests/051.out |  3 ---
 3 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/block.c b/block.c
index 0fe97de..4c620b1 100644
--- a/block.c
+++ b/block.c
@@ -992,14 +992,6 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
     }
     qdict_del(options, "node-name");
 
-    /* bdrv_open() with directly using a protocol as drv. This layer is already
-     * opened, so assign it to bs (while file becomes a closed BlockDriverState)
-     * and return immediately. */
-    if (file != NULL && drv->bdrv_file_open) {
-        bdrv_swap(file, bs);
-        return 0;
-    }
-
     bs->open_flags = flags;
     bs->guest_block_size = 512;
     bs->request_alignment = 512;
@@ -1127,14 +1119,17 @@ static QDict *parse_json_filename(const char *filename, Error **errp)
 /*
  * Fills in default options for opening images and converts the legacy
  * filename/flags pair to option QDict entries.
+ * The BDRV_O_PROTOCOL flag in *flags will be set or cleared accordingly if a
+ * block driver has been specified explicitly.
  */
-static int bdrv_fill_options(QDict **options, const char **pfilename, int flags,
-                             BlockDriver *drv, Error **errp)
+static int bdrv_fill_options(QDict **options, const char **pfilename,
+                             int *flags, BlockDriver *drv, Error **errp)
 {
     const char *filename = *pfilename;
     const char *drvname;
-    bool protocol = flags & BDRV_O_PROTOCOL;
+    bool protocol = *flags & BDRV_O_PROTOCOL;
     bool parse_filename = false;
+    BlockDriver *tmp_drv;
     Error *local_err = NULL;
 
     /* Parse json: pseudo-protocol */
@@ -1152,6 +1147,24 @@ static int bdrv_fill_options(QDict **options, const char **pfilename, int flags,
         *pfilename = filename = NULL;
     }
 
+    drvname = qdict_get_try_str(*options, "driver");
+
+    /* If the user has explicitly specified the driver, this choice should
+     * override the BDRV_O_PROTOCOL flag */
+    tmp_drv = drv;
+    if (!tmp_drv && drvname) {
+        tmp_drv = bdrv_find_format(drvname);
+    }
+    if (tmp_drv) {
+        protocol = tmp_drv->bdrv_file_open;
+    }
+
+    if (protocol) {
+        *flags |= BDRV_O_PROTOCOL;
+    } else {
+        *flags &= ~BDRV_O_PROTOCOL;
+    }
+
     /* Fetch the file name from the options QDict if necessary */
     if (protocol && filename) {
         if (!qdict_haskey(*options, "filename")) {
@@ -1166,7 +1179,6 @@ static int bdrv_fill_options(QDict **options, const char **pfilename, int flags,
 
     /* Find the right block driver */
     filename = qdict_get_try_str(*options, "filename");
-    drvname = qdict_get_try_str(*options, "driver");
 
     if (drv) {
         if (drvname) {
@@ -1502,7 +1514,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
         options = qdict_new();
     }
 
-    ret = bdrv_fill_options(&options, &filename, flags, drv, &local_err);
+    ret = bdrv_fill_options(&options, &filename, &flags, drv, &local_err);
     if (local_err) {
         goto fail;
     }
@@ -1521,11 +1533,6 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
     }
 
     assert(drvname || !(flags & BDRV_O_PROTOCOL));
-    if (drv && !drv->bdrv_file_open) {
-        /* If the user explicitly wants a format driver here, we'll need to add
-         * another layer for the protocol in bs->file */
-        flags &= ~BDRV_O_PROTOCOL;
-    }
 
     bs->options = options;
     options = qdict_clone_shallow(options);
@@ -1562,6 +1569,12 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
         goto fail;
     }
 
+    /* BDRV_O_PROTOCOL must be set iff a protocol BDS is about to be created */
+    assert(!!(flags & BDRV_O_PROTOCOL) == !!drv->bdrv_file_open);
+    /* file must be NULL if a protocol BDS is about to be created
+     * (the inverse results in an error message from bdrv_open_common()) */
+    assert(!(flags & BDRV_O_PROTOCOL) || !file);
+
     /* Open the image */
     ret = bdrv_open_common(bs, file, options, flags, drv, &local_err);
     if (ret < 0) {
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 0360f37..4a8055b 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -194,7 +194,6 @@ echo === Specifying the protocol layer ===
 echo
 
 run_qemu -drive file="$TEST_IMG",file.driver=file
-run_qemu -drive file="$TEST_IMG",file.driver=qcow2
 
 echo
 echo === Leaving out required options ===
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 2890eac..652dd63 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -253,9 +253,6 @@ Testing: -drive file=TEST_DIR/t.qcow2,file.driver=file
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
 
-Testing: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2: Block format 'qcow2' used by device '' doesn't support the option 'filename'
-
 
 === Leaving out required options ===
 
-- 
2.1.0

[Qemu-devel] [PATCH v2 2/2] iotests: Add tests for overriding BDRV_O_PROTOCOL

[Max Reitz]

This adds tests for overriding the qemu-internal BDRV_O_PROTOCOL flag by
explicitly specifying a block driver. As one test must be run over the
NBD protocol while the other must not, this patch adds two separate
iotests.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/119     | 60 ++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/119.out | 11 ++++++++
 tests/qemu-iotests/120     | 65 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/120.out | 15 +++++++++++
 tests/qemu-iotests/group   |  2 ++
 5 files changed, 153 insertions(+)
 create mode 100755 tests/qemu-iotests/119
 create mode 100644 tests/qemu-iotests/119.out
 create mode 100755 tests/qemu-iotests/120
 create mode 100644 tests/qemu-iotests/120.out

diff --git a/tests/qemu-iotests/119 b/tests/qemu-iotests/119
new file mode 100755
index 0000000..9a11f1b
--- /dev/null
+++ b/tests/qemu-iotests/119
@@ -0,0 +1,60 @@
+#!/bin/bash
+#
+# NBD test case for overriding BDRV_O_PROTOCOL by explicitly specifying
+# a driver
+#
+# Copyright (C) 2015 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
+
+_supported_fmt raw
+_supported_proto nbd
+_supported_os Linux
+
+_make_test_img 64M
+# This should not crash
+echo "{'execute': 'qmp_capabilities'}
+      {'execute': 'human-monitor-command',
+       'arguments': {'command-line': 'qemu-io drv \"read -P 0 0 64k\"'}}
+      {'execute': 'quit'}" \
+    | $QEMU -drive id=drv,if=none,file="$TEST_IMG",driver=nbd \
+            -qmp stdio -nodefaults \
+    | _filter_qmp | _filter_qemu_io
+
+# success, all done
+echo
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/119.out b/tests/qemu-iotests/119.out
new file mode 100644
index 0000000..58e7114
--- /dev/null
+++ b/tests/qemu-iotests/119.out
@@ -0,0 +1,11 @@
+QA output created by 119
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+QMP_VERSION
+{"return": {}}
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": ""}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
+
+*** done
diff --git a/tests/qemu-iotests/120 b/tests/qemu-iotests/120
new file mode 100755
index 0000000..9f13078
--- /dev/null
+++ b/tests/qemu-iotests/120
@@ -0,0 +1,65 @@
+#!/bin/bash
+#
+# Non-NBD test cases for overriding BDRV_O_PROTOCOL by explicitly
+# specifying a driver
+#
+# Copyright (C) 2015 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
+
+_supported_fmt generic
+_supported_proto file
+_supported_os Linux
+
+_make_test_img 64M
+
+echo "{'execute': 'qmp_capabilities'}
+      {'execute': 'human-monitor-command',
+       'arguments': {'command-line': 'qemu-io drv \"write -P 42 0 64k\"'}}
+      {'execute': 'quit'}" \
+    | $QEMU -qmp stdio -nodefaults \
+            -drive id=drv,if=none,file="$TEST_IMG",driver=raw,file.driver=$IMGFMT \
+    | _filter_qmp | _filter_qemu_io
+$QEMU_IO -c 'read -P 42 0 64k' "$TEST_IMG" | _filter_qemu_io
+
+$QEMU_IO_PROG -c 'read -P 42 0 64k' \
+    "json:{'driver': 'raw', 'file': {'driver': '$IMGFMT', 'file': {'filename': '$TEST_IMG'}}}" \
+    | _filter_qemu_io
+
+# success, all done
+echo
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/120.out b/tests/qemu-iotests/120.out
new file mode 100644
index 0000000..9131b1b
--- /dev/null
+++ b/tests/qemu-iotests/120.out
@@ -0,0 +1,15 @@
+QA output created by 120
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+QMP_VERSION
+{"return": {}}
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": ""}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 6262127..d45f91c 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -121,6 +121,8 @@
 114 rw auto quick
 115 rw auto
 116 rw auto quick
+119 rw auto quick
+120 rw auto quick
 121 rw auto
 123 rw auto quick
 128 rw auto quick
-- 
2.1.0

Re: [Qemu-devel] [PATCH v2 0/2] block: driver should override flags in bdrv_open()

[Max Reitz]

On 19.03.2015 19:53, Max Reitz wrote:
> BDRV_O_PROTOCOL is an internal qemu flag which a user should be able to
> override by explicitly specifying a block driver. This series implements
> this and adds two iotests (one for NBD, one for file) to test it.
>
>
> v2 (rebase on current master):
> - Patch 1: Conflict in iotest 051's reference output (in a line that is
>    removed by this patch anyway)
>
>
> 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/2:[0002] [FC] 'block: driver should override flags in bdrv_open()'
> 002/2:[----] [-C] 'iotests: Add tests for overriding BDRV_O_PROTOCOL'
>
>
> Max Reitz (2):
>    block: driver should override flags in bdrv_open()
>    iotests: Add tests for overriding BDRV_O_PROTOCOL
>
>   block.c                    | 49 +++++++++++++++++++++-------------
>   tests/qemu-iotests/051     |  1 -
>   tests/qemu-iotests/051.out |  3 ---
>   tests/qemu-iotests/119     | 60 ++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/119.out | 11 ++++++++
>   tests/qemu-iotests/120     | 65 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/120.out | 15 +++++++++++
>   tests/qemu-iotests/group   |  2 ++
>   8 files changed, 184 insertions(+), 22 deletions(-)
>   create mode 100755 tests/qemu-iotests/119
>   create mode 100644 tests/qemu-iotests/119.out
>   create mode 100755 tests/qemu-iotests/120
>   create mode 100644 tests/qemu-iotests/120.out

Ping

Re: [Qemu-devel] [PATCH v2 0/2] block: driver should override flags in bdrv_open()

[Max Reitz]

Hello and welcome to my two-monthly ping. Here it is: Ping!

On 19.03.2015 19:53, Max Reitz wrote:
> BDRV_O_PROTOCOL is an internal qemu flag which a user should be able to
> override by explicitly specifying a block driver. This series implements
> this and adds two iotests (one for NBD, one for file) to test it.
>
>
> v2 (rebase on current master):
> - Patch 1: Conflict in iotest 051's reference output (in a line that is
>    removed by this patch anyway)
>
>
> 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/2:[0002] [FC] 'block: driver should override flags in bdrv_open()'
> 002/2:[----] [-C] 'iotests: Add tests for overriding BDRV_O_PROTOCOL'
>
>
> Max Reitz (2):
>    block: driver should override flags in bdrv_open()
>    iotests: Add tests for overriding BDRV_O_PROTOCOL
>
>   block.c                    | 49 +++++++++++++++++++++-------------
>   tests/qemu-iotests/051     |  1 -
>   tests/qemu-iotests/051.out |  3 ---
>   tests/qemu-iotests/119     | 60 ++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/119.out | 11 ++++++++
>   tests/qemu-iotests/120     | 65 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/120.out | 15 +++++++++++
>   tests/qemu-iotests/group   |  2 ++
>   8 files changed, 184 insertions(+), 22 deletions(-)
>   create mode 100755 tests/qemu-iotests/119
>   create mode 100644 tests/qemu-iotests/119.out
>   create mode 100755 tests/qemu-iotests/120
>   create mode 100644 tests/qemu-iotests/120.out

Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/2] block: driver should override flags in bdrv_open()

[Stefan Hajnoczi]

On Thu, Mar 19, 2015 at 6:53 PM, Max Reitz <mreitz@redhat.com> wrote:
> BDRV_O_PROTOCOL is an internal qemu flag which a user should be able to
> override by explicitly specifying a block driver. This series implements
> this and adds two iotests (one for NBD, one for file) to test it.
>
>
> v2 (rebase on current master):
> - Patch 1: Conflict in iotest 051's reference output (in a line that is
>   removed by this patch anyway)
>
>
> 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/2:[0002] [FC] 'block: driver should override flags in bdrv_open()'
> 002/2:[----] [-C] 'iotests: Add tests for overriding BDRV_O_PROTOCOL'
>
>
> Max Reitz (2):
>   block: driver should override flags in bdrv_open()
>   iotests: Add tests for overriding BDRV_O_PROTOCOL
>
>  block.c                    | 49 +++++++++++++++++++++-------------
>  tests/qemu-iotests/051     |  1 -
>  tests/qemu-iotests/051.out |  3 ---
>  tests/qemu-iotests/119     | 60 ++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/119.out | 11 ++++++++
>  tests/qemu-iotests/120     | 65 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/120.out | 15 +++++++++++
>  tests/qemu-iotests/group   |  2 ++
>  8 files changed, 184 insertions(+), 22 deletions(-)
>  create mode 100755 tests/qemu-iotests/119
>  create mode 100644 tests/qemu-iotests/119.out
>  create mode 100755 tests/qemu-iotests/120
>  create mode 100644 tests/qemu-iotests/120.out

Please resend and CC Kevin (for qemu-iotests and block.c).

Stefan

Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/2] block: driver should override flags in bdrv_open()

[Kevin Wolf]

Am 09.06.2015 um 10:41 hat Stefan Hajnoczi geschrieben:
> On Thu, Mar 19, 2015 at 6:53 PM, Max Reitz <mreitz@redhat.com> wrote:
> > BDRV_O_PROTOCOL is an internal qemu flag which a user should be able to
> > override by explicitly specifying a block driver. This series implements
> > this and adds two iotests (one for NBD, one for file) to test it.
> >
> >
> > v2 (rebase on current master):
> > - Patch 1: Conflict in iotest 051's reference output (in a line that is
> >   removed by this patch anyway)
> >
> >
> > 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/2:[0002] [FC] 'block: driver should override flags in bdrv_open()'
> > 002/2:[----] [-C] 'iotests: Add tests for overriding BDRV_O_PROTOCOL'
> >
> >
> > Max Reitz (2):
> >   block: driver should override flags in bdrv_open()
> >   iotests: Add tests for overriding BDRV_O_PROTOCOL
> >
> >  block.c                    | 49 +++++++++++++++++++++-------------
> >  tests/qemu-iotests/051     |  1 -
> >  tests/qemu-iotests/051.out |  3 ---
> >  tests/qemu-iotests/119     | 60 ++++++++++++++++++++++++++++++++++++++++++
> >  tests/qemu-iotests/119.out | 11 ++++++++
> >  tests/qemu-iotests/120     | 65 ++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/qemu-iotests/120.out | 15 +++++++++++
> >  tests/qemu-iotests/group   |  2 ++
> >  8 files changed, 184 insertions(+), 22 deletions(-)
> >  create mode 100755 tests/qemu-iotests/119
> >  create mode 100644 tests/qemu-iotests/119.out
> >  create mode 100755 tests/qemu-iotests/120
> >  create mode 100644 tests/qemu-iotests/120.out
> 
> Please resend and CC Kevin (for qemu-iotests and block.c).

It's only you who dropped me from CC. :-)

I know, I should finally make up my mind about this series...

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/2] block: driver should override flags in bdrv_open()

[Stefan Hajnoczi]

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

On Tue, Jun 09, 2015 at 10:59:42AM +0200, Kevin Wolf wrote:
> Am 09.06.2015 um 10:41 hat Stefan Hajnoczi geschrieben:
> > On Thu, Mar 19, 2015 at 6:53 PM, Max Reitz <mreitz@redhat.com> wrote:
> > > BDRV_O_PROTOCOL is an internal qemu flag which a user should be able to
> > > override by explicitly specifying a block driver. This series implements
> > > this and adds two iotests (one for NBD, one for file) to test it.
> > >
> > >
> > > v2 (rebase on current master):
> > > - Patch 1: Conflict in iotest 051's reference output (in a line that is
> > >   removed by this patch anyway)
> > >
> > >
> > > 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/2:[0002] [FC] 'block: driver should override flags in bdrv_open()'
> > > 002/2:[----] [-C] 'iotests: Add tests for overriding BDRV_O_PROTOCOL'
> > >
> > >
> > > Max Reitz (2):
> > >   block: driver should override flags in bdrv_open()
> > >   iotests: Add tests for overriding BDRV_O_PROTOCOL
> > >
> > >  block.c                    | 49 +++++++++++++++++++++-------------
> > >  tests/qemu-iotests/051     |  1 -
> > >  tests/qemu-iotests/051.out |  3 ---
> > >  tests/qemu-iotests/119     | 60 ++++++++++++++++++++++++++++++++++++++++++
> > >  tests/qemu-iotests/119.out | 11 ++++++++
> > >  tests/qemu-iotests/120     | 65 ++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/qemu-iotests/120.out | 15 +++++++++++
> > >  tests/qemu-iotests/group   |  2 ++
> > >  8 files changed, 184 insertions(+), 22 deletions(-)
> > >  create mode 100755 tests/qemu-iotests/119
> > >  create mode 100644 tests/qemu-iotests/119.out
> > >  create mode 100755 tests/qemu-iotests/120
> > >  create mode 100644 tests/qemu-iotests/120.out
> > 
> > Please resend and CC Kevin (for qemu-iotests and block.c).
> 
> It's only you who dropped me from CC. :-)

For sanity, can you check whether you are on the CC list in Max's second
ping message: 556F5A4D.50903@redhat.com?

I had another CC mix-up recently and want to check whether my mail tools
are dropping CCs or if it's just me being confused.

Thanks,
Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/2] block: driver should override flags in bdrv_open()

[Kevin Wolf]

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

Am 09.06.2015 um 12:19 hat Stefan Hajnoczi geschrieben:
> On Tue, Jun 09, 2015 at 10:59:42AM +0200, Kevin Wolf wrote:
> > Am 09.06.2015 um 10:41 hat Stefan Hajnoczi geschrieben:
> > > Please resend and CC Kevin (for qemu-iotests and block.c).
> > 
> > It's only you who dropped me from CC. :-)
> 
> For sanity, can you check whether you are on the CC list in Max's second
> ping message: 556F5A4D.50903@redhat.com?
> 
> I had another CC mix-up recently and want to check whether my mail tools
> are dropping CCs or if it's just me being confused.

Yes, I am on the CC list.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/2] block: driver should override flags in bdrv_open()

[Eric Blake]

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

On 06/09/2015 04:19 AM, Stefan Hajnoczi wrote:
> On Tue, Jun 09, 2015 at 10:59:42AM +0200, Kevin Wolf wrote:
>> Am 09.06.2015 um 10:41 hat Stefan Hajnoczi geschrieben:
>>> On Thu, Mar 19, 2015 at 6:53 PM, Max Reitz <mreitz@redhat.com> wrote:
>>>> BDRV_O_PROTOCOL is an internal qemu flag which a user should be able to
>>>> override by explicitly specifying a block driver. This series implements
>>>> this and adds two iotests (one for NBD, one for file) to test it.
>>>>

>>> Please resend and CC Kevin (for qemu-iotests and block.c).
>>
>> It's only you who dropped me from CC. :-)
> 
> For sanity, can you check whether you are on the CC list in Max's second
> ping message: 556F5A4D.50903@redhat.com?
> 
> I had another CC mix-up recently and want to check whether my mail tools
> are dropping CCs or if it's just me being confused.

mailman has an (extremely annoying, in my opinion) habit of munging cc:
lines to drop the name of any recipient who has set their list delivery
options to avoid duplicate messages where the recipient is in cc.
Ultimately, the end user still gets the messages (reply-to-all gets the
message back to the list, even though it drops the cc), but at the
expense of not directly going to the inbox as desired.  At least I still
only get one copy of the message, but yes, I'd rather have that one copy
come directly to me for all messages in the thread, rather than through
the list because I was munged out of cc.

I hope that mailman3/hyperkitty will have saner defaults and not munge
cc lists to exclude subscribers merely based on their preference on
duplicate mail receipt.

-- 
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: 604 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/2] block: driver should override flags in bdrv_open()

[Stefan Hajnoczi]

On Tue, Jun 9, 2015 at 12:49 PM, Eric Blake <eblake@redhat.com> wrote:
> On 06/09/2015 04:19 AM, Stefan Hajnoczi wrote:
>> On Tue, Jun 09, 2015 at 10:59:42AM +0200, Kevin Wolf wrote:
>>> Am 09.06.2015 um 10:41 hat Stefan Hajnoczi geschrieben:
>>>> On Thu, Mar 19, 2015 at 6:53 PM, Max Reitz <mreitz@redhat.com> wrote:
>>>>> BDRV_O_PROTOCOL is an internal qemu flag which a user should be able to
>>>>> override by explicitly specifying a block driver. This series implements
>>>>> this and adds two iotests (one for NBD, one for file) to test it.
>>>>>
>
>>>> Please resend and CC Kevin (for qemu-iotests and block.c).
>>>
>>> It's only you who dropped me from CC. :-)
>>
>> For sanity, can you check whether you are on the CC list in Max's second
>> ping message: 556F5A4D.50903@redhat.com?
>>
>> I had another CC mix-up recently and want to check whether my mail tools
>> are dropping CCs or if it's just me being confused.
>
> mailman has an (extremely annoying, in my opinion) habit of munging cc:
> lines to drop the name of any recipient who has set their list delivery
> options to avoid duplicate messages where the recipient is in cc.
> Ultimately, the end user still gets the messages (reply-to-all gets the
> message back to the list, even though it drops the cc), but at the
> expense of not directly going to the inbox as desired.  At least I still
> only get one copy of the message, but yes, I'd rather have that one copy
> come directly to me for all messages in the thread, rather than through
> the list because I was munged out of cc.
>
> I hope that mailman3/hyperkitty will have saner defaults and not munge
> cc lists to exclude subscribers merely based on their preference on
> duplicate mail receipt.

Wow!  You are right:

I checked my @redhat.com inbox and I see 556F5A4D.50903@redhat.com has
Kevin in CC.

I checked my @gmail.com inbox (subscribed to mailing list) and I see
556F5A4D.50903@redhat.com does not have Kevin in CC.

I guess my @redhat.com email was received directly from Max because I
was in CC list.  It didn't pass through Mailman.  I saw the original,
unmodified list of CCs with Kevin included.

The @gmail.com email was received from the mailing list.  Mailman did
what you described.

This means I cannot send "Please CC maintainer" emails anymore because
I cannot be sure whether the maintainer was CCed!

Stefan

Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/2] block: driver should override flags in bdrv_open()

[Eric Blake]

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

On 06/09/2015 08:48 AM, Stefan Hajnoczi wrote:
> I guess my @redhat.com email was received directly from Max because I
> was in CC list.  It didn't pass through Mailman.  I saw the original,
> unmodified list of CCs with Kevin included.
> 
> The @gmail.com email was received from the mailing list.  Mailman did
> what you described.

And that's what makes it so annoying.  Original recipients have the full
list, but later additions to the thread don't know who the original
recipients were, only that including the list in reply will also reach
the original recipients (albeit maybe not filtered into the right folder).

I have no idea if mailman has some tweakable setting that can change
this bad out-of-the-box default of munging cc's that so many lists use,
but even if it does, it requires a list admin to tweak the setting, for
each affected list.

> 
> This means I cannot send "Please CC maintainer" emails anymore because
> I cannot be sure whether the maintainer was CCed!

Well, unless you know the maintainer is one of the people who does NOT
use the mailman setting of "don't send me duplicate mails" (sadly, we
have deduced that Kevin does use it, making it harder to tell if he is
aware of a thread).

-- 
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: 604 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/2] block: driver should override flags in bdrv_open()

[Kevin Wolf]

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

Am 09.06.2015 um 17:21 hat Eric Blake geschrieben:
> On 06/09/2015 08:48 AM, Stefan Hajnoczi wrote:
> > I guess my @redhat.com email was received directly from Max because I
> > was in CC list.  It didn't pass through Mailman.  I saw the original,
> > unmodified list of CCs with Kevin included.
> > 
> > The @gmail.com email was received from the mailing list.  Mailman did
> > what you described.
> 
> And that's what makes it so annoying.  Original recipients have the full
> list, but later additions to the thread don't know who the original
> recipients were, only that including the list in reply will also reach
> the original recipients (albeit maybe not filtered into the right folder).
> 
> I have no idea if mailman has some tweakable setting that can change
> this bad out-of-the-box default of munging cc's that so many lists use,
> but even if it does, it requires a list admin to tweak the setting, for
> each affected list.
> 
> > 
> > This means I cannot send "Please CC maintainer" emails anymore because
> > I cannot be sure whether the maintainer was CCed!
> 
> Well, unless you know the maintainer is one of the people who does NOT
> use the mailman setting of "don't send me duplicate mails" (sadly, we
> have deduced that Kevin does use it, making it harder to tell if he is
> aware of a thread).

Sorry, I wasn't aware about this behaviour. I thought I had finally
found the one use case where this option actually makes sense, but it
turns out that even if you really want the advertised thing, the option
is useless crap.

I've fixed my mailman settings and replicated the functionality I really
want in additional email filter rules. Hope I got it right and emails
that I should see won't be disappearing...

Please let me know if you still see me removed from CC lists.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/2] block: driver should override flags in bdrv_open()

[Stefan Hajnoczi]

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

On Wed, Jun 10, 2015 at 10:37:10AM +0200, Kevin Wolf wrote:
> Am 09.06.2015 um 17:21 hat Eric Blake geschrieben:
> > On 06/09/2015 08:48 AM, Stefan Hajnoczi wrote:
> > > I guess my @redhat.com email was received directly from Max because I
> > > was in CC list.  It didn't pass through Mailman.  I saw the original,
> > > unmodified list of CCs with Kevin included.
> > > 
> > > The @gmail.com email was received from the mailing list.  Mailman did
> > > what you described.
> > 
> > And that's what makes it so annoying.  Original recipients have the full
> > list, but later additions to the thread don't know who the original
> > recipients were, only that including the list in reply will also reach
> > the original recipients (albeit maybe not filtered into the right folder).
> > 
> > I have no idea if mailman has some tweakable setting that can change
> > this bad out-of-the-box default of munging cc's that so many lists use,
> > but even if it does, it requires a list admin to tweak the setting, for
> > each affected list.
> > 
> > > 
> > > This means I cannot send "Please CC maintainer" emails anymore because
> > > I cannot be sure whether the maintainer was CCed!
> > 
> > Well, unless you know the maintainer is one of the people who does NOT
> > use the mailman setting of "don't send me duplicate mails" (sadly, we
> > have deduced that Kevin does use it, making it harder to tell if he is
> > aware of a thread).
> 
> Sorry, I wasn't aware about this behaviour. I thought I had finally
> found the one use case where this option actually makes sense, but it
> turns out that even if you really want the advertised thing, the option
> is useless crap.
> 
> I've fixed my mailman settings and replicated the functionality I really
> want in additional email filter rules. Hope I got it right and emails
> that I should see won't be disappearing...
> 
> Please let me know if you still see me removed from CC lists.

Thanks!

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v2 0/2] block: driver should override flags in bdrv_open()

[Kevin Wolf]

Am 19.03.2015 um 19:53 hat Max Reitz geschrieben:
> BDRV_O_PROTOCOL is an internal qemu flag which a user should be able to
> override by explicitly specifying a block driver. This series implements
> this and adds two iotests (one for NBD, one for file) to test it.
> 
> 
> v2 (rebase on current master):
> - Patch 1: Conflict in iotest 051's reference output (in a line that is
>   removed by this patch anyway)

Thanks, applied to the block branch.

Kevin