Re: [Qemu-block] [Qemu-devel] [PATCH 1/1] qemu-iotests/109: Fix lock race condition

[John Snow]

On 08/01/2017 05:31 PM, Cleber Rosa wrote:
> A race condition is currently present between the clean up attempt of
> the QEMU process and the execution of qemu-img.  The actual (bad)
> output is:
> 
>  -Warning: Image size mismatch!
>  -Images are identical.
>  +qemu-img: Could not open '/tests/qemu-iotests/scratch/t.raw': 
> Failed to get "consistent read" lock
>  +Is another process using the image?
> 
> A KILL signal is sent to the QEMU process, but qemu-img may begin to
> run before the QEMU process is really gone.  qemu-img will then
> attempt to open the TEST_IMG file before it can secure a lock on it.
> 
> This attempts a more graceful shutdown, and waits for the QEMU process
> to exit.
> 
> Signed-off-by: Cleber Rosa 

Reviewed-by: John Snow 

Thanks for this, it was driving me batty.

Re: [Qemu-block] [PATCH 1/1] qemu-iotests/109: Fix lock race condition

[Jeff Cody]

On Tue, Aug 01, 2017 at 05:31:27PM -0400, Cleber Rosa wrote:
> A race condition is currently present between the clean up attempt of
> the QEMU process and the execution of qemu-img.  The actual (bad)
> output is:
> 
>  -Warning: Image size mismatch!
>  -Images are identical.
>  +qemu-img: Could not open '/tests/qemu-iotests/scratch/t.raw': 
> Failed to get "consistent read" lock
>  +Is another process using the image?
> 
> A KILL signal is sent to the QEMU process, but qemu-img may begin to
> run before the QEMU process is really gone.  qemu-img will then
> attempt to open the TEST_IMG file before it can secure a lock on it.
> 
> This attempts a more graceful shutdown, and waits for the QEMU process
> to exit.
> 
> Signed-off-by: Cleber Rosa 


Reviewed-by: Jeff Cody 


> ---
>  tests/qemu-iotests/109 |  3 ++-
>  tests/qemu-iotests/109.out | 56 
> ++
>  2 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/109 b/tests/qemu-iotests/109
> index 3b496a3..d70b574 100755
> --- a/tests/qemu-iotests/109
> +++ b/tests/qemu-iotests/109
> @@ -67,7 +67,8 @@ function run_qemu()
>  _send_qemu_cmd $QEMU_HANDLE '' "BLOCK_JOB_COMPLETED"
>  fi
>  _send_qemu_cmd $QEMU_HANDLE '{"execute":"query-block-jobs"}' "return"
> -_cleanup_qemu
> +_send_qemu_cmd $QEMU_HANDLE '{"execute":"quit"}' "return"
> +wait=1 _cleanup_qemu
>  }
>  
>  for fmt in qcow qcow2 qed vdi vmdk vpc; do
> diff --git a/tests/qemu-iotests/109.out b/tests/qemu-iotests/109.out
> index dc02f9e..c189e28 100644
> --- a/tests/qemu-iotests/109.out
> +++ b/tests/qemu-iotests/109.out
> @@ -12,12 +12,17 @@ Specify the 'raw' format explicitly to remove the 
> restrictions.
>  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": 
> "report"}}
>  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": LEN, "offset": 0, 
> "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
>  {"return": []}
> +{"return": {}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "SHUTDOWN", "data": {"guest": false}}
>  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": 
> "BLOCK_JOB_READY", "data": {"device": "src", "len": 1024, "offset": 1024, 
> "speed": 0, "type": "mirror"}}
>  {"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 1024, 
> "offset": 1024, "paused": false, "speed": 0, "ready": true, "type": 
> "mirror"}]}
> +{"return": {}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "SHUTDOWN", "data": {"guest": false}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 1024, "offset": 1024, 
> "speed": 0, "type": "mirror"}}
>  Warning: Image size mismatch!
>  Images are identical.
>  
> @@ -33,12 +38,17 @@ Specify the 'raw' format explicitly to remove the 
> restrictions.
>  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": 
> "report"}}
>  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": LEN, "offset": 512, 
> "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
>  {"return": []}
> +{"return": {}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "SHUTDOWN", "data": {"guest": false}}
>  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": 
> "BLOCK_JOB_READY", "data": {"device": "src", "len": 197120, "offset": 197120, 
> "speed": 0, "type": "mirror"}}
>  {"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 
> 197120, "offset": 197120, "paused": false, "speed": 0, "ready": true, "type": 
> "mirror"}]}
> +{"return": {}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "SHUTDOWN", "data": {"guest": false}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 197120, "offset": 
> 197120, "speed": 0, "type": "mirror"}}
>  Warning: Image size mismatch!
>  Images are identical.
>  
> @@ -54,12 +64,17 @@ Specify the 'raw' format explicitly to remove the 
> restrictions.
>  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", 

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10 1/1] qemu-iotests/109: Fix lock race condition

[Eric Blake]

On 08/01/2017 04:31 PM, Cleber Rosa wrote:
> A race condition is currently present between the clean up attempt of
> the QEMU process and the execution of qemu-img.  The actual (bad)
> output is:
> 
>  -Warning: Image size mismatch!
>  -Images are identical.
>  +qemu-img: Could not open '/tests/qemu-iotests/scratch/t.raw': 
> Failed to get "consistent read" lock
>  +Is another process using the image?
> 
> A KILL signal is sent to the QEMU process, but qemu-img may begin to
> run before the QEMU process is really gone.  qemu-img will then
> attempt to open the TEST_IMG file before it can secure a lock on it.
> 
> This attempts a more graceful shutdown, and waits for the QEMU process
> to exit.
> 
> Signed-off-by: Cleber Rosa 
> ---
>  tests/qemu-iotests/109 |  3 ++-
>  tests/qemu-iotests/109.out | 56 
> ++
>  2 files changed, 58 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake 

Probably missed -rc1, but still okay for -rc2

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [Qemu-devel] [PATCH v2 for-2.11 3/3] qemu-iotests: add option to save temp files on error

[Jeff Cody]

On Tue, Aug 01, 2017 at 08:34:01AM +0200, Markus Armbruster wrote:
> Jeff Cody  writes:
> 
> > Now that ./check takes care of cleaning up after each tests, it
> > can also selectively not clean up.  Add option to leave all output from
> > tests intact if that test encountered an error.
> >
> > Note: this currently only works for bash tests, as the python tests
> > still clean up after themselves manually.
> 
> Should we add a TODO comment for that?
> 

Couldn't hurt!

> Much appreciated work, by the way.  You might want to mention in one of
> your commit messages that this is also a step towards running iotests in
> parallel.
> 

Thanks.  I'll go ahead and spin a v3 to clean up commit messages and add a
TODO, since 2.11 won't open for a while.


> Another step towards sanity would be making $TEST_DIR instead of
> $source_iotests the current working directory for running tests.

Yep!

[Qemu-block] [PATCH 1/1] qemu-iotests/109: Fix lock race condition

[Cleber Rosa]

A race condition is currently present between the clean up attempt of
the QEMU process and the execution of qemu-img.  The actual (bad)
output is:

 -Warning: Image size mismatch!
 -Images are identical.
 +qemu-img: Could not open '/tests/qemu-iotests/scratch/t.raw': 
Failed to get "consistent read" lock
 +Is another process using the image?

A KILL signal is sent to the QEMU process, but qemu-img may begin to
run before the QEMU process is really gone.  qemu-img will then
attempt to open the TEST_IMG file before it can secure a lock on it.

This attempts a more graceful shutdown, and waits for the QEMU process
to exit.

Signed-off-by: Cleber Rosa 
---
 tests/qemu-iotests/109 |  3 ++-
 tests/qemu-iotests/109.out | 56 ++
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/109 b/tests/qemu-iotests/109
index 3b496a3..d70b574 100755
--- a/tests/qemu-iotests/109
+++ b/tests/qemu-iotests/109
@@ -67,7 +67,8 @@ function run_qemu()
 _send_qemu_cmd $QEMU_HANDLE '' "BLOCK_JOB_COMPLETED"
 fi
 _send_qemu_cmd $QEMU_HANDLE '{"execute":"query-block-jobs"}' "return"
-_cleanup_qemu
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"quit"}' "return"
+wait=1 _cleanup_qemu
 }
 
 for fmt in qcow qcow2 qed vdi vmdk vpc; do
diff --git a/tests/qemu-iotests/109.out b/tests/qemu-iotests/109.out
index dc02f9e..c189e28 100644
--- a/tests/qemu-iotests/109.out
+++ b/tests/qemu-iotests/109.out
@@ -12,12 +12,17 @@ Specify the 'raw' format explicitly to remove the 
restrictions.
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": 
"report"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": LEN, "offset": 0, 
"speed": 0, "type": "mirror", "error": "Operation not permitted"}}
 {"return": []}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false}}
 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": 
"BLOCK_JOB_READY", "data": {"device": "src", "len": 1024, "offset": 1024, 
"speed": 0, "type": "mirror"}}
 {"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 1024, 
"offset": 1024, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 1024, "offset": 1024, 
"speed": 0, "type": "mirror"}}
 Warning: Image size mismatch!
 Images are identical.
 
@@ -33,12 +38,17 @@ Specify the 'raw' format explicitly to remove the 
restrictions.
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": 
"report"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": LEN, "offset": 512, 
"speed": 0, "type": "mirror", "error": "Operation not permitted"}}
 {"return": []}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false}}
 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": 
"BLOCK_JOB_READY", "data": {"device": "src", "len": 197120, "offset": 197120, 
"speed": 0, "type": "mirror"}}
 {"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 197120, 
"offset": 197120, "paused": false, "speed": 0, "ready": true, "type": 
"mirror"}]}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 197120, "offset": 
197120, "speed": 0, "type": "mirror"}}
 Warning: Image size mismatch!
 Images are identical.
 
@@ -54,12 +64,17 @@ Specify the 'raw' format explicitly to remove the 
restrictions.
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": 
"report"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": LEN, "offset": 262144, 
"speed": 0, "type": "mirror", "error": "Operation not permitted"}}
 {"return": []}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, 

[Qemu-block] [PATCH 0/1] qemu-iotests/109: Fix lock race condition

[Cleber Rosa]

A race condition is currently present between the clean up attempt of
the QEMU process and the execution of qemu-img.

Cleber Rosa(1):
   qemu-iotests/109: Fix lock/race condition

 tests/qemu-iotests/109 |  3 ++-
 tests/qemu-iotests/109.out | 56 ++
 2 files changed, 58 insertions(+), 1 deletion(-)

Re: [Qemu-block] [PULL v2 00/14] Block layer patches for 2.10.0-rc1

[Peter Maydell]

On 1 August 2017 at 17:12, Kevin Wolf  wrote:
> The following changes since commit 5619c179057e24195ff19c8fe6d6a6cbcb16ed28:
>
>   Merge remote-tracking branch 
> 'remotes/pmaydell/tags/pull-target-arm-20170731' into staging (2017-07-31 
> 14:45:42 +0100)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 8e8eb0a9035e5b6c6447c82138570e388282cfa2:
>
>   block/qapi: Remove redundant NULL check to silence Coverity (2017-08-01 
> 18:09:33 +0200)
>
> 
> Block layer patches for 2.10.0-rc1
>

Applied, thanks.

-- PMM

Re: [Qemu-block] [PATCH v3 4/7] block: convert ThrottleGroup to object with QOM

[Manos Pitsidianakis]

On Tue, Aug 01, 2017 at 04:47:03PM +0100, Stefan Hajnoczi wrote:

On Mon, Jul 31, 2017 at 12:54:40PM +0300, Manos Pitsidianakis wrote:

ThrottleGroup is converted to an object. This will allow the future
throttle block filter drive easy creation and configuration of throttle
groups in QMP and cli.

A new QAPI struct, ThrottleLimits, is introduced to provide a shared
struct for all throttle configuration needs in QMP.

ThrottleGroups can be created via CLI as
-object throttle-group,id=foo,x-iops-total=100,x-..
where x-* are individual limit properties. Since we can't add non-scalar
properties in -object this interface must be used instead. However,
setting these properties must be disabled after initialization because
certain combinations of limits are forbidden and thus configuration
changes should be done in one transaction. The individual properties
will go away when support for non-scalar values in CLI is implemented
and thus are marked as experimental.

ThrottleGroup also has a `limits` property that uses the ThrottleLimits
struct.  It can be used to create ThrottleGroups or set the
configuration in existing groups as follows:

{ "execute": "object-add",
  "arguments": {
"qom-type": "throttle-group",
"id": "foo",
"props" : {
  "limits": {
  "iops-total": 100
  }
}
  }
}
{ "execute" : "qom-set",
"arguments" : {
"path" : "foo",
"property" : "limits",
"value" : {
"iops-total" : 99
}
}
}

This also means a group's configuration can be fetched with qom-get.

ThrottleGroups can be anonymous which means they can't get accessed by
other users ie they will always be units instead of group (Because they
have one ThrottleGroupMember).


blockdev.c automatically assigns -drive id= to the group name if
throttling.group= wasn't given.  So who will use anonymous single-drive
mode?


Manual filter nodes. It's possible to not pass a group name value and 
the resulting group will be anonymous. Are you suggesting to move this 
change to the throttle filter patch?



Signed-off-by: Manos Pitsidianakis 
---
Notes:
Note: I tested Markus Armbruster's patch in 
<87wp7fghi9@dusky.pond.sub.org>
on master and I can use this syntax successfuly:
-object '{ "qom-type" : "throttle-group", "id" : "foo", "props" : { 
"limits" \
: { "iops-total" : 1000 } } }'
If this gets merged using -object will be a little more verbose but at 
least we
won't have seperate properties, which is a good thing, so the x-* should be
dropped.

 block/throttle-groups.c | 402 
 include/block/throttle-groups.h |   3 +
 include/qemu/throttle-options.h |  59 --
 include/qemu/throttle.h |   3 +
 qapi/block-core.json|  48 +
 tests/test-throttle.c   |   1 +
 util/throttle.c | 151 +++
 7 files changed, 617 insertions(+), 50 deletions(-)

diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index f711a3dc62..b9c5036e44 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -25,9 +25,17 @@
 #include "qemu/osdep.h"
 #include "sysemu/block-backend.h"
 #include "block/throttle-groups.h"
+#include "qemu/throttle-options.h"
 #include "qemu/queue.h"
 #include "qemu/thread.h"
 #include "sysemu/qtest.h"
+#include "qapi/error.h"
+#include "qapi-visit.h"
+#include "qom/object.h"
+#include "qom/object_interfaces.h"
+
+static void throttle_group_obj_init(Object *obj);
+static void throttle_group_obj_complete(UserCreatable *obj, Error **errp);

 /* The ThrottleGroup structure (with its ThrottleState) is shared
  * among different ThrottleGroupMembers and it's independent from
@@ -54,6 +62,10 @@
  * that ThrottleGroupMember has throttled requests in the queue.
  */
 typedef struct ThrottleGroup {
+Object parent_obj;
+
+/* refuse individual property change if initialization is complete */
+bool is_initialized;
 char *name; /* This is constant during the lifetime of the group */

 QemuMutex lock; /* This lock protects the following four fields */
@@ -63,8 +75,7 @@ typedef struct ThrottleGroup {
 bool any_timer_armed[2];
 QEMUClockType clock_type;

-/* These two are protected by the global throttle_groups_lock */
-unsigned refcount;
+/* This is protected by the global throttle_groups_lock */
 QTAILQ_ENTRY(ThrottleGroup) list;
 } ThrottleGroup;

@@ -77,7 +88,8 @@ static QTAILQ_HEAD(, ThrottleGroup) throttle_groups =
  * If no ThrottleGroup is found with the given name a new one is
  * created.
  *
- * @name: the name of the ThrottleGroup
+ * @name: the name of the ThrottleGroup, NULL means a new anonymous group will
+ *be created.
  * @ret:  the ThrottleState member of the ThrottleGroup
  */
 ThrottleState *throttle_group_incref(const char *name)
@@ -87,32 +99,30 @@ ThrottleState *throttle_group_incref(const char *name)

 qemu_mutex_lock(_groups_lock);

Re: [Qemu-block] [PATCH v3 5/7] block: add throttle block filter driver

[Stefan Hajnoczi]

On Mon, Jul 31, 2017 at 12:54:41PM +0300, Manos Pitsidianakis wrote:
> +static int throttle_configure_tgm(BlockDriverState *bs,
> +  ThrottleGroupMember *tgm,
> +  QDict *options, Error **errp)
> +{
> +int ret;
> +ThrottleConfig cfg;
> +const char *group_name = NULL;
> +Error *local_err = NULL;
> +QemuOpts *opts = qemu_opts_create(_opts, NULL, 0, _err);

If there is no scenario where this can fail please use error_abort.


signature.asc
Description: PGP signature

Re: [Qemu-block] [PATCH v3 4/7] block: convert ThrottleGroup to object with QOM

[Stefan Hajnoczi]

On Mon, Jul 31, 2017 at 12:54:40PM +0300, Manos Pitsidianakis wrote:
> ThrottleGroup is converted to an object. This will allow the future
> throttle block filter drive easy creation and configuration of throttle
> groups in QMP and cli.
> 
> A new QAPI struct, ThrottleLimits, is introduced to provide a shared
> struct for all throttle configuration needs in QMP.
> 
> ThrottleGroups can be created via CLI as
> -object throttle-group,id=foo,x-iops-total=100,x-..
> where x-* are individual limit properties. Since we can't add non-scalar
> properties in -object this interface must be used instead. However,
> setting these properties must be disabled after initialization because
> certain combinations of limits are forbidden and thus configuration
> changes should be done in one transaction. The individual properties
> will go away when support for non-scalar values in CLI is implemented
> and thus are marked as experimental.
> 
> ThrottleGroup also has a `limits` property that uses the ThrottleLimits
> struct.  It can be used to create ThrottleGroups or set the
> configuration in existing groups as follows:
> 
> { "execute": "object-add",
>   "arguments": {
> "qom-type": "throttle-group",
> "id": "foo",
> "props" : {
>   "limits": {
>   "iops-total": 100
>   }
> }
>   }
> }
> { "execute" : "qom-set",
> "arguments" : {
> "path" : "foo",
> "property" : "limits",
> "value" : {
> "iops-total" : 99
> }
> }
> }
> 
> This also means a group's configuration can be fetched with qom-get.
> 
> ThrottleGroups can be anonymous which means they can't get accessed by
> other users ie they will always be units instead of group (Because they
> have one ThrottleGroupMember).

blockdev.c automatically assigns -drive id= to the group name if
throttling.group= wasn't given.  So who will use anonymous single-drive
mode?

> Signed-off-by: Manos Pitsidianakis 
> ---
> Notes:
> Note: I tested Markus Armbruster's patch in 
> <87wp7fghi9@dusky.pond.sub.org>
> on master and I can use this syntax successfuly:
> -object '{ "qom-type" : "throttle-group", "id" : "foo", "props" : { 
> "limits" \
> : { "iops-total" : 1000 } } }'
> If this gets merged using -object will be a little more verbose but at 
> least we
> won't have seperate properties, which is a good thing, so the x-* should 
> be
> dropped.
> 
>  block/throttle-groups.c | 402 
> 
>  include/block/throttle-groups.h |   3 +
>  include/qemu/throttle-options.h |  59 --
>  include/qemu/throttle.h |   3 +
>  qapi/block-core.json|  48 +
>  tests/test-throttle.c   |   1 +
>  util/throttle.c | 151 +++
>  7 files changed, 617 insertions(+), 50 deletions(-)
> 
> diff --git a/block/throttle-groups.c b/block/throttle-groups.c
> index f711a3dc62..b9c5036e44 100644
> --- a/block/throttle-groups.c
> +++ b/block/throttle-groups.c
> @@ -25,9 +25,17 @@
>  #include "qemu/osdep.h"
>  #include "sysemu/block-backend.h"
>  #include "block/throttle-groups.h"
> +#include "qemu/throttle-options.h"
>  #include "qemu/queue.h"
>  #include "qemu/thread.h"
>  #include "sysemu/qtest.h"
> +#include "qapi/error.h"
> +#include "qapi-visit.h"
> +#include "qom/object.h"
> +#include "qom/object_interfaces.h"
> +
> +static void throttle_group_obj_init(Object *obj);
> +static void throttle_group_obj_complete(UserCreatable *obj, Error **errp);
>  
>  /* The ThrottleGroup structure (with its ThrottleState) is shared
>   * among different ThrottleGroupMembers and it's independent from
> @@ -54,6 +62,10 @@
>   * that ThrottleGroupMember has throttled requests in the queue.
>   */
>  typedef struct ThrottleGroup {
> +Object parent_obj;
> +
> +/* refuse individual property change if initialization is complete */
> +bool is_initialized;
>  char *name; /* This is constant during the lifetime of the group */
>  
>  QemuMutex lock; /* This lock protects the following four fields */
> @@ -63,8 +75,7 @@ typedef struct ThrottleGroup {
>  bool any_timer_armed[2];
>  QEMUClockType clock_type;
>  
> -/* These two are protected by the global throttle_groups_lock */
> -unsigned refcount;
> +/* This is protected by the global throttle_groups_lock */
>  QTAILQ_ENTRY(ThrottleGroup) list;
>  } ThrottleGroup;
>  
> @@ -77,7 +88,8 @@ static QTAILQ_HEAD(, ThrottleGroup) throttle_groups =
>   * If no ThrottleGroup is found with the given name a new one is
>   * created.
>   *
> - * @name: the name of the ThrottleGroup
> + * @name: the name of the ThrottleGroup, NULL means a new anonymous group 
> will
> + *be created.
>   * @ret:  the ThrottleState member of the ThrottleGroup
>   */
>  ThrottleState *throttle_group_incref(const char *name)
> @@ -87,32 +99,30 @@ ThrottleState *throttle_group_incref(const char *name)
>  
>  

Re: [Qemu-block] [PATCH v3 6/7] block: add BlockDevOptionsThrottle to QAPI

[Stefan Hajnoczi]

On Mon, Jul 31, 2017 at 12:54:42PM +0300, Manos Pitsidianakis wrote:
> @@ -3095,6 +3096,22 @@
>  '*tls-creds': 'str' } }
>  
>  ##
> +# @BlockdevOptionsThrottle:
> +#
> +# Driver specific block device options for the throttle driver
> +#
> +# @throttle-group:   the name of the throttle-group object to use. It will be
> +#created if it doesn't already exist
> +# @file: reference to or definition of the data source block 
> device
> +# @limits:   ThrottleLimits options
> +# Since: 2.11
> +##
> +{ 'struct': 'BlockdevOptionsThrottle',
> +  'data': { '*throttle-group': 'str',
> +'file' : 'BlockdevRef',
> +'*limits' : 'ThrottleLimits'
> + } }

What happens when throttle-group isn't given?  Please document it.


signature.asc
Description: PGP signature

[Qemu-block] [PULL v2 00/14] Block layer patches for 2.10.0-rc1

[Kevin Wolf]

The following changes since commit 5619c179057e24195ff19c8fe6d6a6cbcb16ed28:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20170731' 
into staging (2017-07-31 14:45:42 +0100)

are available in the git repository at:

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

for you to fetch changes up to 8e8eb0a9035e5b6c6447c82138570e388282cfa2:

  block/qapi: Remove redundant NULL check to silence Coverity (2017-08-01 
18:09:33 +0200)


Block layer patches for 2.10.0-rc1


Eric Blake (2):
  iotests: Check dirty bitmap statistics in 124
  iotests: Add test of recent fix to 'qemu-img measure'

Kevin Wolf (8):
  qemu-iotests/041: Fix leaked scratch images
  qemu-iotests: Remove blkdebug.conf after tests
  qemu-iotests/141: Fix image cleanup
  qemu-iotests/153: Fix leaked scratch images
  qemu-iotests/162: Fix leaked temporary files
  qemu-iotests/063: Fix leaked image
  qemu-iotests/059: Fix leaked image files
  block/qapi: Remove redundant NULL check to silence Coverity

Manos Pitsidianakis (2):
  block: fix dangling bs->explicit_options in block.c
  block: fix leaks in bdrv_open_driver()

Max Reitz (2):
  iotests: Fix test 156
  iotests: Redirect stderr to stdout in 186

 block.c  | 24 +-
 block/qapi.c |  3 ++-
 tests/qemu-iotests/041   |  4 ++-
 tests/qemu-iotests/059   | 11 -
 tests/qemu-iotests/059.out   | 22 -
 tests/qemu-iotests/063   |  4 +--
 tests/qemu-iotests/074   |  1 +
 tests/qemu-iotests/124   |  7 +-
 tests/qemu-iotests/141   |  2 +-
 tests/qemu-iotests/153   |  1 +
 tests/qemu-iotests/156   |  4 +--
 tests/qemu-iotests/162   |  7 ++
 tests/qemu-iotests/179   |  1 +
 tests/qemu-iotests/186   |  2 +-
 tests/qemu-iotests/186.out   | 12 -
 tests/qemu-iotests/190   | 59 
 tests/qemu-iotests/190.out   | 11 +
 tests/qemu-iotests/common.rc |  3 +++
 tests/qemu-iotests/group |  1 +
 19 files changed, 140 insertions(+), 39 deletions(-)
 create mode 100755 tests/qemu-iotests/190
 create mode 100644 tests/qemu-iotests/190.out

Re: [Qemu-block] [PULL 00/15] Block layer patches for 2.10.0-rc1

[Kevin Wolf]

Am 01.08.2017 um 18:01 hat Peter Maydell geschrieben:
> On 1 August 2017 at 15:46, Kevin Wolf  wrote:
> > The following changes since commit 5619c179057e24195ff19c8fe6d6a6cbcb16ed28:
> >
> >   Merge remote-tracking branch 
> > 'remotes/pmaydell/tags/pull-target-arm-20170731' into staging (2017-07-31 
> > 14:45:42 +0100)
> >
> > are available in the git repository at:
> >
> >   git://repo.or.cz/qemu/kevin.git tags/for-upstream
> >
> > for you to fetch changes up to 9ae03670701fea9124f4f1a6d4b6d1badbf9e485:
> >
> >   block/qapi: Remove redundant NULL check to silence Coverity (2017-08-01 
> > 15:26:04 +0200)
> >
> > 
> > Block layer patches for 2.10.0-rc1
> >
> > 
> 
> Hi; I'm afraid the docs changes in this pull upset some of
> my test machines:
> 
> NetBSD, OpenBSD, OSX:
> 
>   GEN docs/qemu-block-drivers.html
> /root/qemu/docs/qemu-block-drivers.texi:800: Unmatched `@end'.
> makeinfo: Removing output file `docs/qemu-block-drivers.html' due to
> errors; use --force to preserve.
> Makefile:692: recipe for target 'docs/qemu-block-drivers.html' failed
> 
> (and same error again trying to build the .txt file)
> 
> S390x, PPC64 Linux give a warning:
>   GEN docs/qemu-block-drivers.html
> /home/linux1/qemu/docs/qemu-block-drivers.texi: warning: must specify
> a title with a title command or @top
> 
> (Other hosts still running the build job.)

I'm dropping the patch and sending v2.

Kevin

Re: [Qemu-block] [PULL 00/15] Block layer patches for 2.10.0-rc1

[Peter Maydell]

On 1 August 2017 at 15:46, Kevin Wolf  wrote:
> The following changes since commit 5619c179057e24195ff19c8fe6d6a6cbcb16ed28:
>
>   Merge remote-tracking branch 
> 'remotes/pmaydell/tags/pull-target-arm-20170731' into staging (2017-07-31 
> 14:45:42 +0100)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 9ae03670701fea9124f4f1a6d4b6d1badbf9e485:
>
>   block/qapi: Remove redundant NULL check to silence Coverity (2017-08-01 
> 15:26:04 +0200)
>
> 
> Block layer patches for 2.10.0-rc1
>
> 

Hi; I'm afraid the docs changes in this pull upset some of
my test machines:

NetBSD, OpenBSD, OSX:

  GEN docs/qemu-block-drivers.html
/root/qemu/docs/qemu-block-drivers.texi:800: Unmatched `@end'.
makeinfo: Removing output file `docs/qemu-block-drivers.html' due to
errors; use --force to preserve.
Makefile:692: recipe for target 'docs/qemu-block-drivers.html' failed

(and same error again trying to build the .txt file)

S390x, PPC64 Linux give a warning:
  GEN docs/qemu-block-drivers.html
/home/linux1/qemu/docs/qemu-block-drivers.texi: warning: must specify
a title with a title command or @top

(Other hosts still running the build job.)

thanks
-- PMM

Re: [Qemu-block] [PATCH 07/18] nbd: Minimal structured read for client

[Vladimir Sementsov-Ogievskiy]

01.08.2017 18:41, Vladimir Sementsov-Ogievskiy wrote:

07.02.2017 23:14, Eric Blake wrote:

On 02/03/2017 09:47 AM, Vladimir Sementsov-Ogievskiy wrote:
Minimal implementation: always send DF flag, to not deal with 
fragmented

replies.

This works well with your minimal server implementation, but I worry
that it will cause us to fall over when talking to a fully-compliant
server that chooses to send EOVERFLOW errors for any request larger than
64k when DF is set; it also makes it impossible to benefit from sparse
reads.  I guess that means we need to start thinking about followup
patches to flush out our implementation.  But maybe I can live with this
patch as is, since the goal of your series was not so much the full
power of structured reads, but getting to a point where we could use
structured reply for block status, even if it means your client can only
communicate with qemu-nbd as server for now, as long as we do get to the
rest of the patches for a full-blown structured read.


Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  block/nbd-client.c  |  47 +++
  block/nbd-client.h  |   2 +
  include/block/nbd.h |  15 +++--
  nbd/client.c| 170 
++--

  qemu-nbd.c  |   2 +-
  5 files changed, 203 insertions(+), 33 deletions(-)

Hmm - no change to the testsuite. Structured reads seems like the sort
of thing that it would be nice to test with some canned server replies,
particularly with server behavior that is permitted by the NBD protocol
but does not happen by default in qemu-nbd.


diff --git a/block/nbd-client.c b/block/nbd-client.c
index 3779c6c999..ff96bd1635 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -180,13 +180,20 @@ static void 
nbd_co_receive_reply(NBDClientSession *s,

  *reply = s->reply;
  if (reply->handle != request->handle ||
  !s->ioc) {
+reply->simple = true;
  reply->error = EIO;

I don't think this is quite right - by setting reply->simple to true,
you are forcing the caller to treat this as the final packet related to
this request->handle, even though that might not be the case.

As it is, I wonder if this code is correct, even before your patch - the
server is allowed to give responses out-of-order (if we request multiple
reads without waiting for the first response) - I don't see how setting
reply->error to EIO if the request->handle indicates that we are
receiving an out-of-order response to some other packet, but that our
request is still awaiting traffic.


Hmm, looks like it should initiate disconnect instead of just 
reporting error to io operation caller.





Also, nbd_co_send_request errors are not handled but just returned to 
the caller. Shouldn't the first


error on socket io initiate disconnect? I think, only the io errors, 
transferred from nbd-export block device


should be just returned to bdrv_{io} caller. NBD-protocol related errors 
(invalid handles, etc.) should initiate


disconnect, so that current and all future bdrv_{io}'s from client 
return -EIO.



--
Best regards,
Vladimir

Re: [Qemu-block] [PATCH 07/18] nbd: Minimal structured read for client

[Vladimir Sementsov-Ogievskiy]

07.02.2017 23:14, Eric Blake wrote:

On 02/03/2017 09:47 AM, Vladimir Sementsov-Ogievskiy wrote:

Minimal implementation: always send DF flag, to not deal with fragmented
replies.

This works well with your minimal server implementation, but I worry
that it will cause us to fall over when talking to a fully-compliant
server that chooses to send EOVERFLOW errors for any request larger than
64k when DF is set; it also makes it impossible to benefit from sparse
reads.  I guess that means we need to start thinking about followup
patches to flush out our implementation.  But maybe I can live with this
patch as is, since the goal of your series was not so much the full
power of structured reads, but getting to a point where we could use
structured reply for block status, even if it means your client can only
communicate with qemu-nbd as server for now, as long as we do get to the
rest of the patches for a full-blown structured read.


Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  block/nbd-client.c  |  47 +++
  block/nbd-client.h  |   2 +
  include/block/nbd.h |  15 +++--
  nbd/client.c| 170 ++--
  qemu-nbd.c  |   2 +-
  5 files changed, 203 insertions(+), 33 deletions(-)

Hmm - no change to the testsuite. Structured reads seems like the sort
of thing that it would be nice to test with some canned server replies,
particularly with server behavior that is permitted by the NBD protocol
but does not happen by default in qemu-nbd.


diff --git a/block/nbd-client.c b/block/nbd-client.c
index 3779c6c999..ff96bd1635 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -180,13 +180,20 @@ static void nbd_co_receive_reply(NBDClientSession *s,
  *reply = s->reply;
  if (reply->handle != request->handle ||
  !s->ioc) {
+reply->simple = true;
  reply->error = EIO;

I don't think this is quite right - by setting reply->simple to true,
you are forcing the caller to treat this as the final packet related to
this request->handle, even though that might not be the case.

As it is, I wonder if this code is correct, even before your patch - the
server is allowed to give responses out-of-order (if we request multiple
reads without waiting for the first response) - I don't see how setting
reply->error to EIO if the request->handle indicates that we are
receiving an out-of-order response to some other packet, but that our
request is still awaiting traffic.


Hmm, looks like it should initiate disconnect instead of just reporting 
error to io operation caller.



--
Best regards,
Vladimir

[Qemu-block] [PULL 14/15] qemu-iotests/059: Fix leaked image files

[Kevin Wolf]

qemu-iotests 059 left a whole lot of image files behind in the scratch
directory because VMDK creates additional files for extents and cleaning
them up requires the original image intact (it parses qemu-img info
output to find all extent files), but the image overwrote it many times
like it works for all other image formats.

In addition, _use_sample_img overwrites the TEST_IMG variable, causing
new images created afterwards to reuse the name of the sample file
rather than the usual t.IMGFMT.

This patch adds an intermediate _cleanup_test_img after each subtest
that created an image file with additional extent files, and also after
each use of a sample image. _cleanup_test_img is also changed so that it
resets TEST_IMG after a sample image is cleaned up.

Note that this test was failing before this commit and continues to do
so after it. This failure was introduced in commit 9877860 ('block/vmdk:
Report failures in vmdk_read_cid()') and needs to be dealt with
separately.

Signed-off-by: Kevin Wolf 
Reviewed-by: Jeff Cody 
Reviewed-by: Eric Blake 
---
 tests/qemu-iotests/059   | 11 ++-
 tests/qemu-iotests/059.out   | 22 +++---
 tests/qemu-iotests/common.rc |  3 +++
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index 6655aaf384..a1c34eeb7c 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -29,7 +29,8 @@ status=1  # failure is the default!
 
 _cleanup()
 {
-   _cleanup_test_img
+_cleanup_test_img
+rm -f "$TEST_IMG.qcow2"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
@@ -72,15 +73,18 @@ echo
 echo "=== Testing monolithicFlat creation and opening ==="
 IMGOPTS="subformat=monolithicFlat" _make_test_img 2G
 _img_info
+_cleanup_test_img
 
 echo
 echo "=== Testing monolithicFlat with zeroed_grain ==="
 IMGOPTS="subformat=monolithicFlat,zeroed_grain=on" _make_test_img 2G
+_cleanup_test_img
 
 echo
 echo "=== Testing big twoGbMaxExtentFlat ==="
 IMGOPTS="subformat=twoGbMaxExtentFlat" _make_test_img 1000G
 $QEMU_IMG info $TEST_IMG | _filter_testdir | sed -e 's/cid: [0-9]*/cid: 
/'
+_cleanup_test_img
 
 echo
 echo "=== Testing malformed VMFS extent description line ==="
@@ -114,6 +118,7 @@ echo "=== Testing monolithicFlat with internally generated 
JSON file name ==="
 IMGOPTS="subformat=monolithicFlat" _make_test_img 64M
 $QEMU_IO -c "open -o 
driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio"
 2>&1 \
 | _filter_testdir | _filter_imgfmt
+_cleanup_test_img
 
 echo
 echo "=== Testing version 3 ==="
@@ -123,6 +128,7 @@ for i in {0..99}; do
 $QEMU_IO -r -c "read -P $(( i % 10 + 0x30 )) $(( i * 64 * 1024 * 10 + i * 
512 )) 512" $TEST_IMG \
 | _filter_qemu_io
 done
+_cleanup_test_img
 
 echo
 echo "=== Testing 4TB monolithicFlat creation and IO ==="
@@ -130,6 +136,7 @@ IMGOPTS="subformat=monolithicFlat" _make_test_img 4T
 _img_info
 $QEMU_IO -c "write -P 0xa 900G 512" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IO -c "read -v 900G 1024" "$TEST_IMG" | _filter_qemu_io
+_cleanup_test_img
 
 echo
 echo "=== Testing qemu-img map on extents ==="
@@ -139,12 +146,14 @@ for fmt in monolithicSparse twoGbMaxExtentSparse; do
 $QEMU_IO -c "write 2147483136 1k" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IO -c "write 5G 1k" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IMG map "$TEST_IMG" | _filter_testdir
+_cleanup_test_img
 done
 
 echo
 echo "=== Testing afl image with a very large capacity ==="
 _use_sample_img afl9.vmdk.bz2
 _img_info
+_cleanup_test_img
 
 # success, all done
 echo "*** done"
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 6154509bc3..f6dce7947c 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -2259,8 +2259,8 @@ read 512/512 bytes at offset 64931328
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing 4TB monolithicFlat creation and IO ===
-Formatting 'TEST_DIR/iotest-version3.IMGFMT', fmt=IMGFMT size=4398046511104 
subformat=monolithicFlat
-image: TEST_DIR/iotest-version3.IMGFMT
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4398046511104 
subformat=monolithicFlat
+image: TEST_DIR/t.IMGFMT
 file format: IMGFMT
 virtual size: 4.0T (4398046511104 bytes)
 wrote 512/512 bytes at offset 966367641600
@@ -2333,7 +2333,7 @@ read 1024/1024 bytes at offset 966367641600
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing qemu-img map on extents ===
-Formatting 'TEST_DIR/iotest-version3.IMGFMT', fmt=IMGFMT size=33285996544 
subformat=monolithicSparse
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33285996544 
subformat=monolithicSparse
 wrote 1024/1024 bytes at offset 65024
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 1024/1024 bytes at offset 2147483136
@@ -2341,10 +2341,10 @@ wrote 1024/1024 bytes at offset 2147483136
 wrote 1024/1024 bytes at offset 

[Qemu-block] [PULL 11/15] qemu-iotests/153: Fix leaked scratch images

[Kevin Wolf]

qemu-iotests 153 left t.qcow2.c behind in the scratch directory. Make
sure to clean it up after completing the tests.

Signed-off-by: Kevin Wolf 
Reviewed-by: Jeff Cody 
Reviewed-by: Eric Blake 
---
 tests/qemu-iotests/153 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153
index 0b45d78ea3..fa25eb24bd 100755
--- a/tests/qemu-iotests/153
+++ b/tests/qemu-iotests/153
@@ -35,6 +35,7 @@ _cleanup()
 rm -f "${TEST_IMG}.convert"
 rm -f "${TEST_IMG}.a"
 rm -f "${TEST_IMG}.b"
+rm -f "${TEST_IMG}.c"
 rm -f "${TEST_IMG}.lnk"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
-- 
2.13.3

[Qemu-block] [PULL 13/15] qemu-iotests/063: Fix leaked image

[Kevin Wolf]

qemu-iotests 063 left t.raw.raw1 behind in the scratch directory because
it used the wrong suffix. Make sure to clean it up after completing the
test.

Signed-off-by: Kevin Wolf 
Reviewed-by: Jeff Cody 
Reviewed-by: Eric Blake 
---
 tests/qemu-iotests/063 | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/063 b/tests/qemu-iotests/063
index 352e78c778..e4f6ea9385 100755
--- a/tests/qemu-iotests/063
+++ b/tests/qemu-iotests/063
@@ -31,7 +31,7 @@ status=1  # failure is the default!
 _cleanup()
 {
_cleanup_test_img
-   rm -f "$TEST_IMG.orig" "$TEST_IMG.raw" "$TEST_IMG.raw2"
+   rm -f "$TEST_IMG.orig" "$TEST_IMG.raw1" "$TEST_IMG.raw2"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
@@ -91,8 +91,6 @@ if $QEMU_IMG convert -f $IMGFMT -O $IMGFMT -n 
"$TEST_IMG.orig" "$TEST_IMG" >/dev
 exit 1
 fi
 
-rm -f "$TEST_IMG.orig" "$TEST_IMG.raw" "$TEST_IMG.raw2"
-
 echo "*** done"
 rm -f $seq.full
 status=0
-- 
2.13.3

[Qemu-block] [PULL 10/15] qemu-iotests/141: Fix image cleanup

[Kevin Wolf]

qemu-iotests 141 attempted to use brace expansion to remove all images
with a single command. However, for this to work, the braces shouldn't
be quoted.

With this fix, the tests correctly cleans up its scratch images.

Signed-off-by: Kevin Wolf 
Reviewed-by: Jeff Cody 
Reviewed-by: Eric Blake 
---
 tests/qemu-iotests/141 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/141 b/tests/qemu-iotests/141
index 40a3405968..2f9d7b9bc2 100755
--- a/tests/qemu-iotests/141
+++ b/tests/qemu-iotests/141
@@ -31,7 +31,7 @@ _cleanup()
 {
 _cleanup_qemu
 _cleanup_test_img
-rm -f "$TEST_DIR/{b,m,o}.$IMGFMT"
+rm -f "$TEST_DIR"/{b,m,o}.$IMGFMT
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
-- 
2.13.3

[Qemu-block] [PULL 12/15] qemu-iotests/162: Fix leaked temporary files

[Kevin Wolf]

qemu-iotests 162 left qemu-nbd.pid behind in the scratch directory, and
potentially a file called '42' in the current directory. Make sure to
clean it up after completing the tests.

Signed-off-by: Kevin Wolf 
Reviewed-by: Jeff Cody 
Reviewed-by: Eric Blake 
---
 tests/qemu-iotests/162 | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/tests/qemu-iotests/162 b/tests/qemu-iotests/162
index cad2bd70ab..477a806360 100755
--- a/tests/qemu-iotests/162
+++ b/tests/qemu-iotests/162
@@ -28,6 +28,13 @@ echo "QA output created by $seq"
 here="$PWD"
 status=1   # failure is the default!
 
+_cleanup()
+{
+rm -f "${TEST_DIR}/qemu-nbd.pid"
+rm -f 42
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
-- 
2.13.3

[Qemu-block] [PULL 15/15] block/qapi: Remove redundant NULL check to silence Coverity

[Kevin Wolf]

When skipping implicit nodes in bdrv_block_device_info(), we know that
bs0 is always non-NULL; initially, because it's taken from a BdrvChild
and a BdrvChild never has a NULL bs, and after the first iteration
because implicit nodes always have a backing file.

Remove the NULL check and add an assertion that the implicit node does
indeed have a backing file.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Reviewed-by: Jeff Cody 
---
 block/qapi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/qapi.c b/block/qapi.c
index d2b18ee9df..5f1a71f5d2 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -145,8 +145,9 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
 
 /* Skip automatically inserted nodes that the user isn't aware of for
  * query-block (blk != NULL), but not for query-named-block-nodes */
-while (blk && bs0 && bs0->drv && bs0->implicit) {
+while (blk && bs0->drv && bs0->implicit) {
 bs0 = backing_bs(bs0);
+assert(bs0);
 }
 }
 
-- 
2.13.3

[Qemu-block] [PULL 01/15] docs: add qemu-block-drivers(7) man page

[Kevin Wolf]

From: Stefan Hajnoczi 

Block driver documentation is available in qemu-doc.html.  It would be
convenient to have documentation for formats, protocols, and filter
drivers in a man page.

Extract the relevant part of qemu-doc.html into a new file called
docs/qemu-block-drivers.texi.  This file can also be built as a
stand-alone document (man, html, etc).

Signed-off-by: Stefan Hajnoczi 
Signed-off-by: Kevin Wolf 
---
 Makefile |  23 +-
 docs/qemu-block-drivers.texi | 799 +++
 qemu-doc.texi| 780 +-
 3 files changed, 818 insertions(+), 784 deletions(-)
 create mode 100644 docs/qemu-block-drivers.texi

diff --git a/Makefile b/Makefile
index 97a58a0f4e..aa40957967 100644
--- a/Makefile
+++ b/Makefile
@@ -209,6 +209,9 @@ ifdef BUILD_DOCS
 DOCS=qemu-doc.html qemu-doc.txt qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8
 DOCS+=docs/interop/qemu-qmp-ref.html docs/interop/qemu-qmp-ref.txt 
docs/interop/qemu-qmp-ref.7
 DOCS+=docs/interop/qemu-ga-ref.html docs/interop/qemu-ga-ref.txt 
docs/interop/qemu-ga-ref.7
+DOCS+=docs/qemu-block-drivers.html
+DOCS+=docs/qemu-block-drivers.txt
+DOCS+=docs/qemu-block-drivers.7
 ifdef CONFIG_VIRTFS
 DOCS+=fsdev/virtfs-proxy-helper.1
 endif
@@ -530,6 +533,8 @@ distclean: clean
rm -f docs/interop/qemu-qmp-ref.txt docs/interop/qemu-ga-ref.txt
rm -f docs/interop/qemu-qmp-ref.pdf docs/interop/qemu-ga-ref.pdf
rm -f docs/interop/qemu-qmp-ref.html docs/interop/qemu-ga-ref.html
+   rm -f docs/qemu-block-drivers.7 docs/qemu-block-drivers.txt
+   rm -f docs/qemu-block-drivers.pdf docs/qemu-block-drivers.html
for d in $(TARGET_DIRS); do \
rm -rf $$d || exit 1 ; \
 done
@@ -570,11 +575,14 @@ install-doc: $(DOCS)
$(INSTALL_DATA) qemu-doc.txt "$(DESTDIR)$(qemu_docdir)"
$(INSTALL_DATA) docs/interop/qemu-qmp-ref.html 
"$(DESTDIR)$(qemu_docdir)"
$(INSTALL_DATA) docs/interop/qemu-qmp-ref.txt "$(DESTDIR)$(qemu_docdir)"
+   $(INSTALL_DATA) docs/qemu-block-drivers.html "$(DESTDIR)$(qemu_docdir)"
+   $(INSTALL_DATA) docs/qemu-block-drivers.txt "$(DESTDIR)$(qemu_docdir)"
 ifdef CONFIG_POSIX
$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man1"
$(INSTALL_DATA) qemu.1 "$(DESTDIR)$(mandir)/man1"
$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man7"
$(INSTALL_DATA) docs/interop/qemu-qmp-ref.7 "$(DESTDIR)$(mandir)/man7"
+   $(INSTALL_DATA) docs/qemu-block-drivers.7 "$(DESTDIR)$(mandir)/man7"
 ifneq ($(TOOLS),)
$(INSTALL_DATA) qemu-img.1 "$(DESTDIR)$(mandir)/man1"
$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man8"
@@ -721,15 +729,15 @@ fsdev/virtfs-proxy-helper.1: 
fsdev/virtfs-proxy-helper.texi
 qemu-nbd.8: qemu-nbd.texi qemu-option-trace.texi
 qemu-ga.8: qemu-ga.texi
 
-html: qemu-doc.html docs/interop/qemu-qmp-ref.html 
docs/interop/qemu-ga-ref.html
-info: qemu-doc.info docs/interop/qemu-qmp-ref.info 
docs/interop/qemu-ga-ref.info
-pdf: qemu-doc.pdf docs/interop/qemu-qmp-ref.pdf docs/interop/qemu-ga-ref.pdf
-txt: qemu-doc.txt docs/interop/qemu-qmp-ref.txt docs/interop/qemu-ga-ref.txt
+html: qemu-doc.html docs/interop/qemu-qmp-ref.html 
docs/interop/qemu-ga-ref.html docs/qemu-block-drivers.html
+info: qemu-doc.info docs/interop/qemu-qmp-ref.info 
docs/interop/qemu-ga-ref.info docs/qemu-block-drivers.info
+pdf: qemu-doc.pdf docs/interop/qemu-qmp-ref.pdf docs/interop/qemu-ga-ref.pdf 
docs/qemu-block-drivers.pdf
+txt: qemu-doc.txt docs/interop/qemu-qmp-ref.txt docs/interop/qemu-ga-ref.txt 
docs/qemu-block-drivers.txt
 
 qemu-doc.html qemu-doc.info qemu-doc.pdf qemu-doc.txt: \
qemu-img.texi qemu-nbd.texi qemu-options.texi qemu-option-trace.texi \
qemu-monitor.texi qemu-img-cmds.texi qemu-ga.texi \
-   qemu-monitor-info.texi
+   qemu-monitor-info.texi docs/qemu-block-drivers.texi
 
 docs/interop/qemu-ga-ref.dvi docs/interop/qemu-ga-ref.html \
 docs/interop/qemu-ga-ref.info docs/interop/qemu-ga-ref.pdf \
@@ -741,6 +749,11 @@ docs/interop/qemu-qmp-ref.dvi 
docs/interop/qemu-qmp-ref.html \
 docs/interop/qemu-qmp-ref.txt docs/interop/qemu-qmp-ref.7: \
docs/interop/qemu-qmp-ref.texi docs/interop/qemu-qmp-qapi.texi
 
+docs/qemu-block-drivers.dvi docs/qemu-block-drivers.html \
+docs/qemu-block-drivers.info docs/qemu-block-drivers.pdf \
+docs/qemu-block-drivers.txt docs/qemu-block-drivers.7: \
+   docs/qemu-block-drivers.texi
+
 
 ifdef CONFIG_WIN32
 
diff --git a/docs/qemu-block-drivers.texi b/docs/qemu-block-drivers.texi
new file mode 100644
index 00..43ec3faf15
--- /dev/null
+++ b/docs/qemu-block-drivers.texi
@@ -0,0 +1,799 @@
+@c man begin SYNOPSIS
+QEMU block driver reference manual
+@c man end
+
+@c man begin DESCRIPTION
+
+@node disk_images_formats
+
+@subsection Disk image file formats
+
+QEMU supports many image file formats that can be used with VMs as well as with
+any of the tools 

[Qemu-block] [PULL 02/15] iotests: Fix test 156

[Kevin Wolf]

From: Max Reitz 

On one hand, the _make_test_img invocation for creating the target image
was missing a -u because its backing file is not supposed to exist at
that point.

On the other hand, nobody noticed probably because the backing file is
created later on and _cleanup failed to remove it: The quotation marks
were misplaced so bash tried to delete a file literally called
"$TEST_IMG{,.target}..." instead of performing brace expansion. Thus, the
files stayed around after the first run and qemu-img create did not
complain about a missing backing file on any run but the first.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Jeff Cody 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/156 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/156 b/tests/qemu-iotests/156
index 2c4a06e2d8..e75dc4d743 100755
--- a/tests/qemu-iotests/156
+++ b/tests/qemu-iotests/156
@@ -38,7 +38,7 @@ status=1  # failure is the default!
 _cleanup()
 {
 _cleanup_qemu
-rm -f "$TEST_IMG{,.target}{,.backing,.overlay}"
+rm -f "$TEST_IMG"{,.target}{,.backing,.overlay}
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
@@ -83,7 +83,7 @@ _send_qemu_cmd $QEMU_HANDLE \
 'return'
 
 # Create target image
-TEST_IMG="$TEST_IMG.target.overlay" _make_test_img -b "$TEST_IMG.target" 1M
+TEST_IMG="$TEST_IMG.target.overlay" _make_test_img -u -b "$TEST_IMG.target" 1M
 
 # Mirror snapshot
 _send_qemu_cmd $QEMU_HANDLE \
-- 
2.13.3

[Qemu-block] [PULL 06/15] block: fix dangling bs->explicit_options in block.c

[Kevin Wolf]

From: Manos Pitsidianakis 

In some error paths it is possible to QDECREF a freed dangling
explicit_options, resulting in a heap overflow crash.  For example
bdrv_open_inherit()'s fail unrefs it, then calls bdrv_unref which calls
bdrv_close which also unrefs it.

Signed-off-by: Manos Pitsidianakis 
Signed-off-by: Kevin Wolf 
---
 block.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block.c b/block.c
index 37e72b7a96..7a78bc647b 100644
--- a/block.c
+++ b/block.c
@@ -2608,6 +2608,7 @@ fail:
 QDECREF(bs->options);
 QDECREF(options);
 bs->options = NULL;
+bs->explicit_options = NULL;
 bdrv_unref(bs);
 error_propagate(errp, local_err);
 return NULL;
@@ -3087,6 +3088,7 @@ static void bdrv_close(BlockDriverState *bs)
 QDECREF(bs->options);
 QDECREF(bs->explicit_options);
 bs->options = NULL;
+bs->explicit_options = NULL;
 QDECREF(bs->full_open_options);
 bs->full_open_options = NULL;
 }
-- 
2.13.3

[Qemu-block] [PULL 03/15] iotests: Redirect stderr to stdout in 186

[Kevin Wolf]

From: Max Reitz 

Without redirecting qemu's stderr to stdout, _filter_qemu will not apply
to warnings.  This results in $QEMU_PROG not being replaced by QEMU_PROG
which is not great if your qemu executable is not called
qemu-system-x86_64 (e.g. qemu-system-i386).

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Jeff Cody 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/186 |  2 +-
 tests/qemu-iotests/186.out | 12 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/186 b/tests/qemu-iotests/186
index ab83ee402a..2b9f618f90 100755
--- a/tests/qemu-iotests/186
+++ b/tests/qemu-iotests/186
@@ -56,7 +56,7 @@ function do_run_qemu()
 done
 fi
 echo quit
-) | $QEMU -S -nodefaults -display none -device virtio-scsi-pci -monitor 
stdio "$@"
+) | $QEMU -S -nodefaults -display none -device virtio-scsi-pci -monitor 
stdio "$@" 2>&1
 echo
 }
 
diff --git a/tests/qemu-iotests/186.out b/tests/qemu-iotests/186.out
index b8bf9a2550..c8377fe146 100644
--- a/tests/qemu-iotests/186.out
+++ b/tests/qemu-iotests/186.out
@@ -442,28 +442,28 @@ ide0-cd0 (NODE_NAME): null-co:// (null-co, read-only)
 Cache mode:   writeback
 (qemu) quit
 
-qemu-system-x86_64: -drive if=scsi,driver=null-co: warning: bus=0,unit=0 is 
deprecated with this machine type
 Testing: -drive if=scsi,driver=null-co
 QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) info block
+(qemu) QEMU_PROG: -drive if=scsi,driver=null-co: warning: bus=0,unit=0 is 
deprecated with this machine type
+info block
 scsi0-hd0 (NODE_NAME): null-co:// (null-co)
 Attached to:  /machine/unattached/device[27]/scsi.0/legacy[0]
 Cache mode:   writeback
 (qemu) quit
 
-qemu-system-x86_64: -drive if=scsi,media=cdrom: warning: bus=0,unit=0 is 
deprecated with this machine type
 Testing: -drive if=scsi,media=cdrom
 QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) info block
+(qemu) QEMU_PROG: -drive if=scsi,media=cdrom: warning: bus=0,unit=0 is 
deprecated with this machine type
+info block
 scsi0-cd0: [not inserted]
 Attached to:  /machine/unattached/device[27]/scsi.0/legacy[0]
 Removable device: not locked, tray closed
 (qemu) quit
 
-qemu-system-x86_64: -drive if=scsi,driver=null-co,media=cdrom: warning: 
bus=0,unit=0 is deprecated with this machine type
 Testing: -drive if=scsi,driver=null-co,media=cdrom
 QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) info block
+(qemu) QEMU_PROG: -drive if=scsi,driver=null-co,media=cdrom: warning: 
bus=0,unit=0 is deprecated with this machine type
+info block
 scsi0-cd0 (NODE_NAME): null-co:// (null-co, read-only)
 Attached to:  /machine/unattached/device[27]/scsi.0/legacy[0]
 Removable device: not locked, tray closed
-- 
2.13.3

[Qemu-block] [PULL 04/15] iotests: Check dirty bitmap statistics in 124

[Kevin Wolf]

From: Eric Blake 

We had a bug for multiple releases where dirty-bitmap count was
documented in bytes but reported in sectors; enhance the testsuite
to add coverage of DirtyBitmapInfo to ensure we do not regress again.

Signed-off-by: Eric Blake 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: John Snow 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/124 | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index d0d2c2bfb0..8e76e62f93 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -336,7 +336,12 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
(('0xab', 0, 512),
 ('0xfe', '16M', '256k'),
 ('0x64', '32736k', '64k')))
-
+# Check the dirty bitmap stats
+result = self.vm.qmp('query-block')
+self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/name', 'bitmap0')
+self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/count', 458752)
+self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/granularity', 
65536)
+self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/status', 'active')
 
 # Prepare a cluster_size=128k backup target without a backing file.
 (target, _) = bitmap0.new_target()
-- 
2.13.3

[Qemu-block] [PULL 05/15] iotests: Add test of recent fix to 'qemu-img measure'

[Kevin Wolf]

From: Eric Blake 

The new test 190 ensures we don't regress back to an infinite loop when
measuring the size of a 2T+ qcow2 image.  I did not append to test 178,
because that test is also designed to run with format 'raw'; also, this
gives us some coverage of the measure command under the quick group.

Signed-off-by: Eric Blake 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/190 | 59 ++
 tests/qemu-iotests/190.out | 11 +
 tests/qemu-iotests/group   |  1 +
 3 files changed, 71 insertions(+)
 create mode 100755 tests/qemu-iotests/190
 create mode 100644 tests/qemu-iotests/190.out

diff --git a/tests/qemu-iotests/190 b/tests/qemu-iotests/190
new file mode 100755
index 00..8f808fef5d
--- /dev/null
+++ b/tests/qemu-iotests/190
@@ -0,0 +1,59 @@
+#!/bin/bash
+#
+# qemu-img measure sub-command tests on huge qcow2 files
+#
+# Copyright (C) 2017 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 .
+#
+
+# creator
+owner=ebl...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1# failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+rm -f "$TEST_IMG.converted"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+# See 178 for more extensive tests across more formats
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+echo "== Huge file =="
+echo
+
+IMGOPTS='cluster_size=2M' _make_test_img 2T
+
+$QEMU_IMG measure -O raw -f qcow2 "$TEST_IMG"
+$QEMU_IMG measure -O qcow2 -o cluster_size=64k -f qcow2 "$TEST_IMG"
+$QEMU_IMG measure -O qcow2 -o cluster_size=2M -f qcow2 "$TEST_IMG"
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/190.out b/tests/qemu-iotests/190.out
new file mode 100644
index 00..d001942002
--- /dev/null
+++ b/tests/qemu-iotests/190.out
@@ -0,0 +1,11 @@
+QA output created by 190
+== Huge file ==
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=219902322
+required size: 219902322
+fully allocated size: 219902322
+required size: 335806464
+fully allocated size: 2199359062016
+required size: 18874368
+fully allocated size: 2199042129920
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 287f0ea27d..823811076d 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -184,3 +184,4 @@
 186 rw auto
 188 rw auto quick
 189 rw auto
+190 rw auto quick
-- 
2.13.3

[Qemu-block] [PULL 00/15] Block layer patches for 2.10.0-rc1

[Kevin Wolf]

The following changes since commit 5619c179057e24195ff19c8fe6d6a6cbcb16ed28:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20170731' 
into staging (2017-07-31 14:45:42 +0100)

are available in the git repository at:

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

for you to fetch changes up to 9ae03670701fea9124f4f1a6d4b6d1badbf9e485:

  block/qapi: Remove redundant NULL check to silence Coverity (2017-08-01 
15:26:04 +0200)


Block layer patches for 2.10.0-rc1


Eric Blake (2):
  iotests: Check dirty bitmap statistics in 124
  iotests: Add test of recent fix to 'qemu-img measure'

Kevin Wolf (8):
  qemu-iotests/041: Fix leaked scratch images
  qemu-iotests: Remove blkdebug.conf after tests
  qemu-iotests/141: Fix image cleanup
  qemu-iotests/153: Fix leaked scratch images
  qemu-iotests/162: Fix leaked temporary files
  qemu-iotests/063: Fix leaked image
  qemu-iotests/059: Fix leaked image files
  block/qapi: Remove redundant NULL check to silence Coverity

Manos Pitsidianakis (2):
  block: fix dangling bs->explicit_options in block.c
  block: fix leaks in bdrv_open_driver()

Max Reitz (2):
  iotests: Fix test 156
  iotests: Redirect stderr to stdout in 186

Stefan Hajnoczi (1):
  docs: add qemu-block-drivers(7) man page

 block.c  |  24 +-
 block/qapi.c |   3 +-
 Makefile |  23 +-
 docs/qemu-block-drivers.texi | 799 +++
 qemu-doc.texi| 780 +-
 tests/qemu-iotests/041   |   4 +-
 tests/qemu-iotests/059   |  11 +-
 tests/qemu-iotests/059.out   |  22 +-
 tests/qemu-iotests/063   |   4 +-
 tests/qemu-iotests/074   |   1 +
 tests/qemu-iotests/124   |   7 +-
 tests/qemu-iotests/141   |   2 +-
 tests/qemu-iotests/153   |   1 +
 tests/qemu-iotests/156   |   4 +-
 tests/qemu-iotests/162   |   7 +
 tests/qemu-iotests/179   |   1 +
 tests/qemu-iotests/186   |   2 +-
 tests/qemu-iotests/186.out   |  12 +-
 tests/qemu-iotests/190   |  59 
 tests/qemu-iotests/190.out   |  11 +
 tests/qemu-iotests/common.rc |   3 +
 tests/qemu-iotests/group |   1 +
 22 files changed, 958 insertions(+), 823 deletions(-)
 create mode 100644 docs/qemu-block-drivers.texi
 create mode 100755 tests/qemu-iotests/190
 create mode 100644 tests/qemu-iotests/190.out

[Qemu-block] [PATCH v4 12/15] qcow2: skip writing zero buffers to empty COW areas

[Anton Nefedov]

It can be detected that
  1. COW alignment of a write request is zeroes
  2. Respective areas on the underlying BDS already read as zeroes
 after being preallocated previously

If both of these true, COW may be skipped

Signed-off-by: Anton Nefedov 
---
 block/qcow2.h | 12 +++
 block/qcow2-cluster.c |  5 -
 block/qcow2.c | 60 ---
 block/trace-events|  1 +
 4 files changed, 69 insertions(+), 9 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 595ed9c..db1c6f5 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -363,6 +363,12 @@ typedef struct QCowL2Meta
 bool keep_old_clusters;
 
 /**
+ * True if the area is allocated at the end of data area
+ * (i.e. >= BDRVQcow2State::data_end)
+ */
+bool clusters_are_trailing;
+
+/**
  * Requests that overlap with this allocation and wait to be restarted
  * when the allocating request has completed.
  */
@@ -381,6 +387,12 @@ typedef struct QCowL2Meta
 Qcow2COWRegion cow_end;
 
 /**
+ * Indicates that both COW areas are empty (nb_bytes == 0)
+ * or filled with zeroes and do not require any more copying
+ */
+bool zero_cow;
+
+/**
  * The I/O vector with the data from the actual guest write request.
  * If non-NULL, this is meant to be merged together with the data
  * from @cow_start and @cow_end into one single write operation.
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 75baaf4..d54b96a 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -735,7 +735,7 @@ static int perform_cow(BlockDriverState *bs, QCowL2Meta *m)
 assert(start->offset + start->nb_bytes <= end->offset);
 assert(!m->data_qiov || m->data_qiov->size == data_bytes);
 
-if (start->nb_bytes == 0 && end->nb_bytes == 0) {
+if ((start->nb_bytes == 0 && end->nb_bytes == 0) || m->zero_cow) {
 return 0;
 }
 
@@ -1203,6 +1203,7 @@ static int handle_alloc(BlockDriverState *bs, uint64_t 
guest_offset,
 uint64_t *host_offset, uint64_t *bytes, QCowL2Meta **m)
 {
 BDRVQcow2State *s = bs->opaque;
+const uint64_t old_data_end = s->data_end;
 int l2_index;
 uint64_t *l2_table;
 uint64_t entry;
@@ -1324,6 +1325,7 @@ static int handle_alloc(BlockDriverState *bs, uint64_t 
guest_offset,
 .alloc_offset   = alloc_cluster_offset,
 .offset = start_of_cluster(s, guest_offset),
 .nb_clusters= nb_clusters,
+.clusters_are_trailing = alloc_cluster_offset >= old_data_end,
 
 .keep_old_clusters  = keep_old_clusters,
 
@@ -1335,6 +1337,7 @@ static int handle_alloc(BlockDriverState *bs, uint64_t 
guest_offset,
 .offset = nb_bytes,
 .nb_bytes   = avail_bytes - nb_bytes,
 },
+.zero_cow = false,
 };
 qemu_co_queue_init(&(*m)->dependent_requests);
 QLIST_INSERT_HEAD(>cluster_allocs, *m, next_in_flight);
diff --git a/block/qcow2.c b/block/qcow2.c
index 2ec8b03..e49ad50 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1921,6 +1921,11 @@ static bool merge_cow(uint64_t offset, unsigned bytes,
 continue;
 }
 
+/* If both COW regions are zeroes already, skip this too */
+if (m->zero_cow) {
+continue;
+}
+
 /* The data (middle) region must be immediately after the
  * start region */
 if (l2meta_cow_start(m) + m->cow_start.nb_bytes != offset) {
@@ -1971,26 +1976,61 @@ static bool is_zero(BlockDriverState *bs, int64_t 
offset, int64_t bytes)
 /*
  * If the specified area is beyond EOF, allocates it + prealloc_size
  * bytes ahead.
+ *
+ * Returns
+ *   true if the space is allocated and contains zeroes
  */
-static void coroutine_fn handle_prealloc(BlockDriverState *bs,
+static bool coroutine_fn handle_prealloc(BlockDriverState *bs,
  const QCowL2Meta *m)
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t start = m->alloc_offset;
 uint64_t end = start + m->nb_clusters * s->cluster_size;
+int ret;
 int64_t flen = bdrv_getlength(bs->file->bs);
 
 if (flen < 0) {
-return;
+return false;
 }
 
 if (end > flen) {
 /* try to alloc host space in one chunk for better locality */
-bdrv_co_pwrite_zeroes(bs->file, flen,
-  QEMU_ALIGN_UP(end + s->prealloc_size - flen,
-s->cluster_size),
-  BDRV_REQ_ALLOCATE);
+ret = bdrv_co_pwrite_zeroes(bs->file, flen,
+QEMU_ALIGN_UP(end + s->prealloc_size - 
flen,
+  s->cluster_size),
+BDRV_REQ_ALLOCATE);
+if (ret < 0) {
+return false;
+}
 }
+
+/* We're safe to assume that the area is zeroes if the 

[Qemu-block] [PATCH v4 09/15] qcow2: truncate preallocated space

[Anton Nefedov]

From: "Denis V. Lunev" 

This could be done after calculation of the end of data and metadata in
the qcow2 image.

Signed-off-by: Denis V. Lunev 
Signed-off-by: Anton Nefedov 
---
 block/qcow2.h  | 3 +++
 block/qcow2-cluster.c  | 9 +
 block/qcow2-refcount.c | 7 +++
 block/qcow2.c  | 7 +++
 4 files changed, 26 insertions(+)

diff --git a/block/qcow2.h b/block/qcow2.h
index ebbb9cf..595ed9c 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -330,6 +330,7 @@ typedef struct BDRVQcow2State {
 char *image_backing_format;
 
 uint64_t prealloc_size;
+uint64_t data_end;
 } BDRVQcow2State;
 
 typedef struct Qcow2COWRegion {
@@ -669,4 +670,6 @@ void qcow2_remove_persistent_dirty_bitmap(BlockDriverState 
*bs,
   const char *name,
   Error **errp);
 
+void qcow2_update_data_end(BlockDriverState *bs, uint64_t off);
+
 #endif
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 0185986..75baaf4 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -2014,3 +2014,12 @@ fail:
 g_free(l1_table);
 return ret;
 }
+
+void qcow2_update_data_end(BlockDriverState *bs, uint64_t off)
+{
+BDRVQcow2State *s = bs->opaque;
+
+if (s->data_end < off) {
+s->data_end = off;
+}
+}
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index c9b0dcb..d741a92 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -833,6 +833,9 @@ static int QEMU_WARN_UNUSED_RESULT 
update_refcount(BlockDriverState *bs,
 ret = alloc_refcount_block(bs, cluster_index, _block);
 if (ret < 0) {
 goto fail;
+} else {
+qcow2_update_data_end(bs, s->refcount_table_offset +
+s->refcount_table_size * sizeof(uint64_t));
 }
 }
 old_table_index = table_index;
@@ -954,6 +957,8 @@ retry:
 s->free_cluster_index - 1 > (INT64_MAX >> s->cluster_bits))
 {
 return -EFBIG;
+} else {
+qcow2_update_data_end(bs, s->free_cluster_index << s->cluster_bits);
 }
 
 #ifdef DEBUG_ALLOC2
@@ -1018,6 +1023,8 @@ int64_t qcow2_alloc_clusters_at(BlockDriverState *bs, 
uint64_t offset,
 
 if (ret < 0) {
 return ret;
+} else {
+qcow2_update_data_end(bs, offset + (nb_clusters << s->cluster_bits));
 }
 
 return i;
diff --git a/block/qcow2.c b/block/qcow2.c
index 2a1d2f2..4696106 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1498,6 +1498,8 @@ static int qcow2_do_open(BlockDriverState *bs, QDict 
*options, int flags,
 }
 }
 
+s->data_end = bdrv_getlength(bs->file->bs);
+
 #ifdef DEBUG_ALLOC
 {
 BdrvCheckResult result = {0};
@@ -2138,6 +2140,11 @@ static int qcow2_inactivate(BlockDriverState *bs)
 
 if (result == 0) {
 qcow2_mark_clean(bs);
+
+/* truncate preallocated space */
+if (!bs->read_only && s->data_end < bdrv_getlength(bs->file->bs)) {
+bdrv_truncate(bs->file, s->data_end, PREALLOC_MODE_OFF, NULL);
+}
 s->flags |= BDRV_O_INACTIVE;
 }
 
-- 
2.7.4

Re: [Qemu-block] [Qemu-devel] [PATCH 4/6] hw/block: Fix the return type

[Markus Armbruster]

Stefan Hajnoczi  writes:

> On Wed, Jul 26, 2017 at 08:02:53PM +0800, Mao Zhongyi wrote:
>> When the function no success value to transmit, it usually make the
>> function return void. It has turned out not to be a success, because
>> it means that the extra local_err variable and error_propagate() will
>> be needed. It leads to cumbersome code, therefore, transmit success/
>> failure in the return value is worth.
>> 
>> So fix the return type of blkconf_apply_backend_options(),
>> blkconf_geometry() and virtio_blk_data_plane_create() to avoid it.
>> 
>> Cc: John Snow 
>> Cc: Kevin Wolf 
>> Cc: Max Reitz 
>> Cc: Stefan Hajnoczi 
>> 
>> Signed-off-by: Mao Zhongyi 
>> ---
>>  hw/block/block.c| 21 -
>>  hw/block/dataplane/virtio-blk.c | 16 +---
>>  hw/block/dataplane/virtio-blk.h |  6 +++---
>>  include/hw/block/block.h| 10 +-
>>  4 files changed, 29 insertions(+), 24 deletions(-)
>> 
>> diff --git a/hw/block/block.c b/hw/block/block.c
>> index 27878d0..717bd0e 100644
>> --- a/hw/block/block.c
>> +++ b/hw/block/block.c
>> @@ -51,8 +51,8 @@ void blkconf_blocksizes(BlockConf *conf)
>>  }
>>  }
>>  
>> -void blkconf_apply_backend_options(BlockConf *conf, bool readonly,
>> -   bool resizable, Error **errp)
>> +int blkconf_apply_backend_options(BlockConf *conf, bool readonly,
>> +  bool resizable, Error **errp)
>
> I'm not a fan of these changes because it makes inconsistencies between
> the return value and the errp argument possible (e.g. returning success
> but setting errp, or returning failure without setting errp).

Opinions and practice vary on this one, and we've discussed the
tradeoffs a few times.

Having both an Error parameter and an error return value poses the
question whether the two agree.  When there's no success value to
transmit, you avoid the problem by making the function return void.  The
problem remains for all the function that return a value on success, and
therefore must return some error value on failure.  A related problem
even remains for functions returning void: consistency between side
effects and the Error parameter.

Returning void leads to awkward boilerplate:

Error err = NULL;

foo(..., err);
if (err) {
error_propagate(errp, err);
... bail out ...
}

Compare:

if (!foo(..., errp)) {
... bail out ...
}

Much easier on the eyes.

For what it's worth, GLib wants you to transmit success / failure in the
return value, too:

https://developer.gnome.org/glib/unstable/glib-Error-Reporting.html#gerror-rules

Plenty of older code returns void, some newer code doesn't, because
returning void is just too awkward.  We willfully deviated from GLib's
convention, and we're now paying the price.

See also
Subject: Re: [RFC 00/15] Error API: Flag errors in *errp even if errors are 
being ignored
Message-ID: <87o9t8qy7d@dusky.pond.sub.org>
https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg06191.html

> If you really want to do this please use bool as the return type instead
> of int.  int can be abused to return error information that should
> really be in the Error object.

For what it's worth, GLib wants bool[*].  Let's stick to that unless we
have a compelling reason to differ.


[*] Except being GLib, it wants its very own homemade version of bool.
Which is inferior to stdbool.h's in pretty much every conceivable way.

[Qemu-block] [PATCH v4 07/15] qcow2: preallocation at image expand

[Anton Nefedov]

From: "Denis V. Lunev" 

This patch adds image preallocation at expand to provide better locality
of QCOW2 image file and optimize this procedure for some distributed
storage where this procedure is slow.

Preallocation is not issued upon writing metadata clusters.

Possible conflicts are resolved by the common block layer code since
ALLOCATE requests are serialising.

Signed-off-by: Denis V. Lunev 
Signed-off-by: Anton Nefedov 
---
 block/qcow2.h   |  3 +++
 block/qcow2.c   | 62 +++--
 qemu-options.hx |  4 
 3 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 96a8d43..ebbb9cf 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -102,6 +102,7 @@
 #define QCOW2_OPT_L2_CACHE_SIZE "l2-cache-size"
 #define QCOW2_OPT_REFCOUNT_CACHE_SIZE "refcount-cache-size"
 #define QCOW2_OPT_CACHE_CLEAN_INTERVAL "cache-clean-interval"
+#define QCOW2_OPT_PREALLOC_SIZE "prealloc-size"
 
 typedef struct QCowHeader {
 uint32_t magic;
@@ -327,6 +328,8 @@ typedef struct BDRVQcow2State {
  * override) */
 char *image_backing_file;
 char *image_backing_format;
+
+uint64_t prealloc_size;
 } BDRVQcow2State;
 
 typedef struct Qcow2COWRegion {
diff --git a/block/qcow2.c b/block/qcow2.c
index bcdd212..66aa8c2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -674,6 +674,11 @@ static QemuOptsList qcow2_runtime_opts = {
 },
 BLOCK_CRYPTO_OPT_DEF_KEY_SECRET("encrypt.",
 "ID of secret providing qcow2 AES key or LUKS passphrase"),
+{
+.name = QCOW2_OPT_PREALLOC_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = "Preallocation amount at image expand",
+},
 { /* end of list */ }
 },
 };
@@ -1016,6 +1021,15 @@ static int qcow2_update_options_prepare(BlockDriverState 
*bs,
 goto fail;
 }
 
+s->prealloc_size =
+ROUND_UP(qemu_opt_get_size_del(opts, QCOW2_OPT_PREALLOC_SIZE, 0),
+ s->cluster_size);
+if (s->prealloc_size &&
+!(bs->file->bs->supported_zero_flags & BDRV_REQ_ALLOCATE))
+{
+s->prealloc_size = 0;
+}
+
 ret = 0;
 fail:
 QDECREF(encryptopts);
@@ -1898,6 +1912,43 @@ static bool merge_cow(uint64_t offset, unsigned bytes,
 return false;
 }
 
+/*
+ * If the specified area is beyond EOF, allocates it + prealloc_size
+ * bytes ahead.
+ */
+static void coroutine_fn handle_prealloc(BlockDriverState *bs,
+ const QCowL2Meta *m)
+{
+BDRVQcow2State *s = bs->opaque;
+uint64_t start = m->alloc_offset;
+uint64_t end = start + m->nb_clusters * s->cluster_size;
+int64_t flen = bdrv_getlength(bs->file->bs);
+
+if (flen < 0) {
+return;
+}
+
+if (end > flen) {
+/* try to alloc host space in one chunk for better locality */
+bdrv_co_pwrite_zeroes(bs->file, flen,
+  QEMU_ALIGN_UP(end + s->prealloc_size - flen,
+s->cluster_size),
+  BDRV_REQ_ALLOCATE);
+}
+}
+
+static void handle_alloc_space(BlockDriverState *bs, QCowL2Meta *l2meta)
+{
+BDRVQcow2State *s = bs->opaque;
+QCowL2Meta *m;
+
+for (m = l2meta; m != NULL; m = m->next) {
+if (s->prealloc_size) {
+handle_prealloc(bs, m);
+}
+}
+}
+
 static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
  uint64_t bytes, QEMUIOVector *qiov,
  int flags)
@@ -1982,24 +2033,31 @@ static coroutine_fn int 
qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
 goto fail;
 }
 
+qemu_co_mutex_unlock(>lock);
+
+if (bs->file->bs->supported_zero_flags & BDRV_REQ_ALLOCATE) {
+handle_alloc_space(bs, l2meta);
+}
+
 /* If we need to do COW, check if it's possible to merge the
  * writing of the guest data together with that of the COW regions.
  * If it's not possible (or not necessary) then write the
  * guest data now. */
 if (!merge_cow(offset, cur_bytes, _qiov, l2meta)) {
-qemu_co_mutex_unlock(>lock);
 BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
 trace_qcow2_writev_data(qemu_coroutine_self(),
 cluster_offset + offset_in_cluster);
 ret = bdrv_co_pwritev(bs->file,
   cluster_offset + offset_in_cluster,
   cur_bytes, _qiov, 0);
-qemu_co_mutex_lock(>lock);
 if (ret < 0) {
+qemu_co_mutex_lock(>lock);
 goto fail;
 }
 }
 
+qemu_co_mutex_lock(>lock);
+
 while (l2meta != NULL) {
 QCowL2Meta *next;
 
diff --git 

[Qemu-block] [PATCH v4 00/15] qcow2: space preallocation and COW improvements

[Anton Nefedov]

Changes in v4:

  - rebased on top of Eric's series
[PATCH v2 00/20] add byte-based block_status driver callbacks
http://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg04370.html

  - patch 5 fixed to compile without CONFIG_FALLOCATE and support
BDRV_REQ_ALLOCATE with xfsctl enabled

  - add patches 1, 2 to support write/zero flags in mirror and blkverify
drivers



Here goes a revisited series on qcow2 preallocation. It's probably a bit better
integrated this time and the amount of code is reduced significantly.

Changes in v3:
  - requests intersection detection from the previous versions is removed
from qcow2 driver. Instead, tracked request infrastructure from the common
block layer is used.

  - that made possible to omit preallocation for metadata writes.
Those are rare and won't affect performance.

  - 'Simultaneous writes' feature (patches v2 12-15) dropped by now;
 Might worth a separate series.

  - various remarks to the previous version fixed

  - some iotests added



Changes in v2:
  - introduce new BDRV flag for write_zeroes()
  instead of using driver callback directly.
Skipped introducing new functions like bdrv_co_pallocate() for now:
  1. it seems ok to keep calling this write_zeroes() as zeroes
  are expected;
  2. most of the code can be reused now anyway, so changes to
  write_zeroes() path are not significant
  3. write_zeroes() alignment and max-request limits can also be reused

As a possible alternative we can have bdrv_co_pallocate() which can
switch to pwrite_zeroes(,flags|=BDRV_REQ_ALLOCATE) early.



This pull request is to address a few performance problems of qcow2 format:

  1. non cluster-aligned write requests (to unallocated clusters) explicitly
pad data with zeroes if there is no backing data. This can be avoided
and the whole clusters are preallocated and zeroed in a single
efficient write_zeroes() operation, also providing better host file
continuity

  2. moreover, efficient write_zeroes() operation can be used to preallocate
space megabytes ahead which gives noticeable improvement on some storage
types (e.g. distributed storages where space allocation operation is
expensive)

  /* omitted in v3: */
  3. preallocating/zeroing the clusters in advance makes possible to enable
simultaneous writes to the same unallocated cluster, which is beneficial
for parallel sequential write operations which are not cluster-aligned

Performance test results are added to commit messages (see patch 3, 12)

Anton Nefedov (12):
  mirror: inherit supported write/zero flags
  blkverify: set supported write/zero flags
  block: introduce BDRV_REQ_ALLOCATE flag
  block: treat BDRV_REQ_ALLOCATE as serialising
  file-posix: support BDRV_REQ_ALLOCATE
  block: support BDRV_REQ_ALLOCATE in passthrough drivers
  qcow2: set inactive flag
  qcow2: move is_zero() up
  qcow2: skip writing zero buffers to empty COW areas
  qcow2: allocate image space by-cluster
  iotest 190: test BDRV_REQ_ALLOCATE
  iotest 134: test cluster-misaligned encrypted write

Denis V. Lunev (2):
  qcow2: preallocation at image expand
  qcow2: truncate preallocated space

Pavel Butsykin (1):
  qcow2: check space leak at the end of the image

 include/block/block.h  |   6 +-
 include/block/block_int.h  |   2 +-
 block/qcow2.h  |  18 
 block/blkdebug.c   |   3 +-
 block/blkverify.c  |   9 ++
 block/file-posix.c |   8 ++
 block/io.c |  47 +++--
 block/mirror.c |   5 +
 block/qcow2-cluster.c  |  14 ++-
 block/qcow2-refcount.c |   7 ++
 block/qcow2.c  | 203 -
 block/raw-format.c |   3 +-
 block/trace-events |   1 +
 qemu-options.hx|   4 +
 tests/qemu-iotests/026.out | 104 ++-
 tests/qemu-iotests/026.out.nocache | 104 ++-
 tests/qemu-iotests/029.out |   5 +-
 tests/qemu-iotests/060 |   2 +-
 tests/qemu-iotests/060.out |  13 ++-
 tests/qemu-iotests/061.out |   5 +-
 tests/qemu-iotests/066 |   2 +-
 tests/qemu-iotests/066.out |   9 +-
 tests/qemu-iotests/098.out |   7 +-
 tests/qemu-iotests/108.out |   5 +-
 tests/qemu-iotests/112.out |   5 +-
 tests/qemu-iotests/134 |   9 ++
 tests/qemu-iotests/134.out |  10 ++
 tests/qemu-iotests/190 | 146 ++
 tests/qemu-iotests/190.out |  50 +
 tests/qemu-iotests/group   |   1 +
 30 files changed, 705 insertions(+), 102 deletions(-)
 create mode 100755 tests/qemu-iotests/190
 create mode 100644 tests/qemu-iotests/190.out

-- 
2.7.4

[Qemu-block] [PATCH v4 13/15] qcow2: allocate image space by-cluster

[Anton Nefedov]

If COW areas of the newly allocated clusters are zeroes on the backing image:
(even if preallocation feature is not used or it cannot detect if the image
already reads as zeroes, e.g. writing to a hole / preallocated zero cluster)
efficient bdrv_write_zeroes(flags=BDRV_REQ_ALLOCATE) can be used on the whole
cluster instead of writing explicit zero buffers later in perform_cow().

iotest 060:
write to the discarded cluster does not trigger COW anymore.
so, break on write_aio event instead, will work for the test
(but write won't fail anymore, so update reference output)

iotest 066:
cluster-alignment areas that were not really COWed are now detected
as zeroes, hence the initial write has to be exactly the same size for
the maps to match

Signed-off-by: Anton Nefedov 
---
 block/qcow2.c  | 22 +-
 tests/qemu-iotests/060 |  2 +-
 tests/qemu-iotests/060.out |  3 ++-
 tests/qemu-iotests/066 |  2 +-
 tests/qemu-iotests/066.out |  4 ++--
 5 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index e49ad50..9c49d40 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2039,13 +2039,25 @@ static void handle_alloc_space(BlockDriverState *bs, 
QCowL2Meta *l2meta)
 QCowL2Meta *m;
 
 for (m = l2meta; m != NULL; m = m->next) {
-if (s->prealloc_size && handle_prealloc(bs, m)) {
-if (check_zero_cow(bs, m)) {
-trace_qcow2_skip_cow(qemu_coroutine_self(), m->offset,
- m->nb_clusters);
-m->zero_cow = true;
+bool preallocated_zeroes = s->prealloc_size && handle_prealloc(bs, m);
+
+if (!check_zero_cow(bs, m)) {
+continue;
+}
+
+if (!preallocated_zeroes &&
+(m->cow_start.nb_bytes != 0 || m->cow_end.nb_bytes != 0))
+{
+if (bdrv_co_pwrite_zeroes(bs->file, m->alloc_offset,
+  m->nb_clusters * s->cluster_size,
+  BDRV_REQ_ALLOCATE) != 0)
+{
+continue;
 }
 }
+
+trace_qcow2_skip_cow(qemu_coroutine_self(), m->offset, m->nb_clusters);
+m->zero_cow = true;
 }
 }
 
diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index 8e95c45..3a0f096 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -160,7 +160,7 @@ poke_file "$TEST_IMG" '131084' "\x00\x00" # 0x2000c
 # any unallocated cluster, leading to an attempt to overwrite the second L2
 # table. Finally, resume the COW write and see it fail (but not crash).
 echo "open -o file.driver=blkdebug $TEST_IMG
-break cow_read 0
+break write_aio 0
 aio_write 0k 1k
 wait_break 0
 write 64k 64k
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index a20e267..290ccec 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -114,7 +114,8 @@ qcow2: Marking image as corrupt: Preventing invalid write 
on metadata (overlaps
 blkdebug: Suspended request '0'
 write failed: Input/output error
 blkdebug: Resuming request '0'
-aio_write failed: No medium found
+wrote 1024/1024 bytes at offset 0
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing unallocated image header ===
 
diff --git a/tests/qemu-iotests/066 b/tests/qemu-iotests/066
index 8638217..3c216a1 100755
--- a/tests/qemu-iotests/066
+++ b/tests/qemu-iotests/066
@@ -71,7 +71,7 @@ echo
 _make_test_img $IMG_SIZE
 
 # Create data clusters (not aligned to an L2 table)
-$QEMU_IO -c 'write -P 42 1M 256k' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "write -P 42 $(((1024 + 32) * 1024)) 192k" "$TEST_IMG" | 
_filter_qemu_io
 orig_map=$($QEMU_IMG map --output=json "$TEST_IMG")
 
 # Convert the data clusters to preallocated zero clusters
diff --git a/tests/qemu-iotests/066.out b/tests/qemu-iotests/066.out
index f94aa5c..81ef795 100644
--- a/tests/qemu-iotests/066.out
+++ b/tests/qemu-iotests/066.out
@@ -22,8 +22,8 @@ Offset  Length  Mapped to   File
 === Writing to preallocated zero clusters ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67109376
-wrote 262144/262144 bytes at offset 1048576
-256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 196608/196608 bytes at offset 1081344
+192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 262144/262144 bytes at offset 1048576
 256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 196608/196608 bytes at offset 1081344
-- 
2.7.4

[Qemu-block] [PATCH v4 02/15] blkverify: set supported write/zero flags

[Anton Nefedov]

Signed-off-by: Anton Nefedov 
---
 block/blkverify.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/block/blkverify.c b/block/blkverify.c
index 06369f9..9ba65d0 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -140,6 +140,15 @@ static int blkverify_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto fail;
 }
 
+bs->supported_write_flags = BDRV_REQ_FUA &
+bs->file->bs->supported_write_flags &
+s->test_file->bs->supported_write_flags;
+
+bs->supported_zero_flags =
+(BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) &
+bs->file->bs->supported_zero_flags &
+s->test_file->bs->supported_zero_flags;
+
 ret = 0;
 fail:
 qemu_opts_del(opts);
-- 
2.7.4

[Qemu-block] [PATCH v4 04/15] block: treat BDRV_REQ_ALLOCATE as serialising

[Anton Nefedov]

The idea is that ALLOCATE requests may overlap with other requests.
Reuse the existing block layer infrastructure for serialising requests.
Use the following approach:
  - mark ALLOCATE serialising, so subsequent requests to the area wait
  - ALLOCATE request itself must never wait if another request is in flight
already. Return EAGAIN, let the caller reconsider.

Signed-off-by: Anton Nefedov 
---
 block/io.c | 27 +++
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/block/io.c b/block/io.c
index 04d495e..0a7a372 100644
--- a/block/io.c
+++ b/block/io.c
@@ -488,7 +488,8 @@ void bdrv_dec_in_flight(BlockDriverState *bs)
 bdrv_wakeup(bs);
 }
 
-static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self)
+static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self,
+   bool nowait)
 {
 BlockDriverState *bs = self->bs;
 BdrvTrackedRequest *req;
@@ -519,11 +520,14 @@ static bool coroutine_fn 
wait_serialising_requests(BdrvTrackedRequest *self)
  * will wait for us as soon as it wakes up, then just go on
  * (instead of producing a deadlock in the former case). */
 if (!req->waiting_for) {
+waited = true;
+if (nowait) {
+break;
+}
 self->waiting_for = req;
 qemu_co_queue_wait(>wait_queue, >reqs_lock);
 self->waiting_for = NULL;
 retry = true;
-waited = true;
 break;
 }
 }
@@ -1027,7 +1031,7 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild 
*child,
 }
 
 if (!(flags & BDRV_REQ_NO_SERIALISING)) {
-wait_serialising_requests(req);
+wait_serialising_requests(req, false);
 }
 
 if (flags & BDRV_REQ_COPY_ON_READ) {
@@ -1321,7 +1325,10 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild 
*child,
 max_transfer = QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_transfer, INT_MAX),
align);
 
-waited = wait_serialising_requests(req);
+waited = wait_serialising_requests(req, flags & BDRV_REQ_ALLOCATE);
+if (waited && flags & BDRV_REQ_ALLOCATE) {
+return -EAGAIN;
+}
 assert(!waited || !req->serialising);
 assert(req->overlap_offset <= offset);
 assert(offset + bytes <= req->overlap_offset + req->overlap_bytes);
@@ -1425,7 +1432,7 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild 
*child,
 
 /* RMW the unaligned part before head. */
 mark_request_serialising(req, align);
-wait_serialising_requests(req);
+wait_serialising_requests(req, false);
 bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
 ret = bdrv_aligned_preadv(child, req, offset & ~(align - 1), align,
   align, _qiov, 0);
@@ -1445,6 +1452,10 @@ static int coroutine_fn 
bdrv_co_do_zero_pwritev(BdrvChild *child,
 bytes -= zero_bytes;
 }
 
+if (flags & BDRV_REQ_ALLOCATE) {
+mark_request_serialising(req, align);
+}
+
 assert(!bytes || (offset & (align - 1)) == 0);
 if (bytes >= align) {
 /* Write the aligned part in the middle. */
@@ -1463,7 +1474,7 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild 
*child,
 assert(align == tail_padding_bytes + bytes);
 /* RMW the unaligned part after tail. */
 mark_request_serialising(req, align);
-wait_serialising_requests(req);
+wait_serialising_requests(req, false);
 bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
 ret = bdrv_aligned_preadv(child, req, offset, align,
   align, _qiov, 0);
@@ -1532,7 +1543,7 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
 struct iovec head_iov;
 
 mark_request_serialising(, align);
-wait_serialising_requests();
+wait_serialising_requests(, false);
 
 head_buf = qemu_blockalign(bs, align);
 head_iov = (struct iovec) {
@@ -1573,7 +1584,7 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
 bool waited;
 
 mark_request_serialising(, align);
-waited = wait_serialising_requests();
+waited = wait_serialising_requests(, false);
 assert(!waited || !use_local_qiov);
 
 tail_buf = qemu_blockalign(bs, align);
-- 
2.7.4

[Qemu-block] [PATCH v4 08/15] qcow2: set inactive flag

[Anton Nefedov]

Qcow2State and BlockDriverState flags have to be in sync

Signed-off-by: Anton Nefedov 
---
 block/qcow2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 66aa8c2..2a1d2f2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2138,6 +2138,7 @@ static int qcow2_inactivate(BlockDriverState *bs)
 
 if (result == 0) {
 qcow2_mark_clean(bs);
+s->flags |= BDRV_O_INACTIVE;
 }
 
 return result;
-- 
2.7.4

[Qemu-block] [PATCH v4 06/15] block: support BDRV_REQ_ALLOCATE in passthrough drivers

[Anton Nefedov]

Support the flag if the underlying BDS supports it

Signed-off-by: Anton Nefedov 
---
 block/blkdebug.c   | 3 ++-
 block/blkverify.c  | 2 +-
 block/mirror.c | 2 +-
 block/raw-format.c | 3 ++-
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 234c8fb9..2ac3e12 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -414,7 +414,8 @@ static int blkdebug_open(BlockDriverState *bs, QDict 
*options, int flags,
 
 bs->supported_write_flags = BDRV_REQ_FUA &
 bs->file->bs->supported_write_flags;
-bs->supported_zero_flags = (BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) &
+bs->supported_zero_flags =
+(BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_ALLOCATE) &
 bs->file->bs->supported_zero_flags;
 ret = -EINVAL;
 
diff --git a/block/blkverify.c b/block/blkverify.c
index 9ba65d0..b249636 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -145,7 +145,7 @@ static int blkverify_open(BlockDriverState *bs, QDict 
*options, int flags,
 s->test_file->bs->supported_write_flags;
 
 bs->supported_zero_flags =
-(BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) &
+(BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_ALLOCATE) &
 bs->file->bs->supported_zero_flags &
 s->test_file->bs->supported_zero_flags;
 
diff --git a/block/mirror.c b/block/mirror.c
index 7e539f1..5510776 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1059,7 +1059,7 @@ static void 
bdrv_mirror_top_refresh_filename(BlockDriverState *bs, QDict *opts)
 bs->supported_write_flags = BDRV_REQ_FUA &
 bs->backing->bs->supported_write_flags;
 bs->supported_zero_flags =
-(BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) &
+(BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_ALLOCATE) &
 bs->backing->bs->supported_zero_flags;
 }
 
diff --git a/block/raw-format.c b/block/raw-format.c
index 22c7e98..434af74 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -417,7 +417,8 @@ static int raw_open(BlockDriverState *bs, QDict *options, 
int flags,
 bs->sg = bs->file->bs->sg;
 bs->supported_write_flags = BDRV_REQ_FUA &
 bs->file->bs->supported_write_flags;
-bs->supported_zero_flags = (BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) &
+bs->supported_zero_flags =
+(BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_ALLOCATE) &
 bs->file->bs->supported_zero_flags;
 
 if (bs->probed && !bdrv_is_read_only(bs)) {
-- 
2.7.4

[Qemu-block] [PATCH v4 05/15] file-posix: support BDRV_REQ_ALLOCATE

[Anton Nefedov]

Current write_zeroes implementation is good enough to satisfy this flag too

Signed-off-by: Anton Nefedov 
---
 block/file-posix.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index 765a440..4ef1b1d 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -539,7 +539,10 @@ static int raw_open_common(BlockDriverState *bs, QDict 
*options,
 }
 if (S_ISREG(st.st_mode)) {
 s->discard_zeroes = true;
+#ifdef CONFIG_FALLOCATE
 s->has_fallocate = true;
+bs->supported_zero_flags |= BDRV_REQ_ALLOCATE;
+#endif
 }
 if (S_ISBLK(st.st_mode)) {
 #ifdef BLKDISCARDZEROES
@@ -574,6 +577,7 @@ static int raw_open_common(BlockDriverState *bs, QDict 
*options,
 #ifdef CONFIG_XFS
 if (platform_test_xfs_fd(s->fd)) {
 s->is_xfs = true;
+bs->supported_zero_flags |= BDRV_REQ_ALLOCATE;
 }
 #endif
 
@@ -1388,6 +1392,10 @@ static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData 
*aiocb)
 }
 s->has_fallocate = false;
 }
+
+if (!s->has_fallocate) {
+aiocb->bs->supported_zero_flags &= ~BDRV_REQ_ALLOCATE;
+}
 #endif
 
 return -ENOTSUP;
-- 
2.7.4

[Qemu-block] [PATCH v4 01/15] mirror: inherit supported write/zero flags

[Anton Nefedov]

Signed-off-by: Anton Nefedov 
---
 block/mirror.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/block/mirror.c b/block/mirror.c
index d46dace..7e539f1 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1056,6 +1056,11 @@ static void 
bdrv_mirror_top_refresh_filename(BlockDriverState *bs, QDict *opts)
 bdrv_refresh_filename(bs->backing->bs);
 pstrcpy(bs->exact_filename, sizeof(bs->exact_filename),
 bs->backing->bs->filename);
+bs->supported_write_flags = BDRV_REQ_FUA &
+bs->backing->bs->supported_write_flags;
+bs->supported_zero_flags =
+(BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) &
+bs->backing->bs->supported_zero_flags;
 }
 
 static void bdrv_mirror_top_close(BlockDriverState *bs)
-- 
2.7.4

Re: [Qemu-block] [RFC] block-insert-node and block-job-delete

[Manos Pitsidianakis]

On Tue, Aug 01, 2017 at 03:50:36PM +0200, Kevin Wolf wrote:

Am 31.07.2017 um 19:30 hat Manos Pitsidianakis geschrieben:

On Fri, Jul 28, 2017 at 02:08:43PM +0200, Kevin Wolf wrote:
> Am 27.07.2017 um 12:07 hat Stefan Hajnoczi geschrieben:
> > On Wed, Jul 26, 2017 at 09:23:20PM +0300, Manos Pitsidianakis wrote:
> > > On Wed, Jul 26, 2017 at 04:12:21PM +0100, Stefan Hajnoczi wrote:
> > > > On Wed, Jul 26, 2017 at 05:19:24PM +0300, Manos Pitsidianakis wrote:
> > > > > This proposal follows a discussion we had with Kevin and Stefan on 
filter
> > > > > node management.
> > > > >
> > > > > With block filter drivers arises a need to configure filter nodes on 
runtime
> > > > > with QMP on live graphs. A problem with doing live graph 
modifications is
> > > > > that some block jobs modify the graph when they are done and don't 
operate
> > > > > under any synchronisation, resulting in a race condition if we try to 
insert
> > > > > a filter node in the place of an existing edge.
> > > >
> > > > But maybe you are thinking about higher level race conditions between
> > > > QMP commands.  Can you give an example of the race?
> > >
> > > Exactly, synchronisation between the QMP/user actions. Here's an example,
> > > correct me if I'm wrong:
> > >
> > > User issues a block-commit to this tree to commit A onto B:
> > >BB -> A -> B
> > >
> > > This inserts the dummy mirror node at the top since this is a mirror job
> > > (active commit)
> > >BB -> dummy -> A -> B
> > >
> > > User wants to add a filter node F below the BB. First we create the node:
> > >BB -> dummy -> A -> B
> > >   F /
> > >
> > > Commit finishes before we do block-insert-node, dummy and A is removed 
from
> > > the BB's chain, mirror_exit calls bdrv_replace_node()
> > >
> > >BB > B
> > >F -> /
> > >
> > > Now inserting F under BB will error since dummy does not exist any more.
> >
> > I see the diagrams but the flow isn't clear without the user's QMP
> > commands.
> >
> > F is created using blockdev-add?  What are the parameters and how does
> > it know about dummy?  I think this is an interesting question in itself
> > because dummy is a transient internal node that QMP users don't
> > necessarily know about.
>
> We can expect that users of block-insert-node also know about block job
> filter nodes, simply because the former is newer than the latter.
>
> (I also don't like the name "dummy", this makes it sound like it's not
> really a proper node. In reality, there is little reason to treat it
> specially.)
>
> > What is the full block-insert-node command?
> >
> > > The proposal doesn't guard against the following:
> > > User issues a block-commit to this tree to commit B onto C:
> > >BB -> A -> B -> C
> > >
> > > The dummy node (commit's top bs is A):
> > >BB -> A -> dummy -> B -> C
> > >
> > > blockdev-add of a filter we wish to eventually be between A and C:
> > >BB -> A -> dummy -> B -> C
> > > F/
> > >
> > > if commit finishes before we issue block-insert-node (commit_complete()
> > > calls bdrv_set_backing_hd() which only touches A)
> > >BB -> A --> C
> > >   F -> dummy -> B /
> > >resulting in error when issuing block-insert-node,
> > >
> > > else if we set the job to manual-delete and insert F:
> > >BB -> A -> F -> dummy -> B -> C
> > > deleting the job will result in F being lost since the job's top bs wasn't
> > > updated.
> >
> > manual-delete is a solution for block jobs.  The write threshold
> > feature is a plain QMP command that in the future will create a
> > transient internal node (before write notifier).
>
> Yes, that becomes a legacy QMP command then. The new way is blockdev-add
> and block-insert-node for write threshold, too.
>
> Apart from that, a write threshold node never disappears by itself, it
> is only inserted or removed in the context of a QMP command. That makes
> it a lot less dangerous than automatic block completion.
>
> > I'm not sure it makes sense to turn the write threshold feature into a
> > block job.  I guess it could work, but it seems a little unnatural and
> > maybe there will be other features that use transient internal nodes.
>
> Yeah, no reason to do so. Just a manually created filter node.
>

Can you explain what you have in mind? The current workflow is using
block-set-write-threshold on the targetted bs. If we want write threshold to
be on node level we would want a new filter driver so that it can take
options special to write-threshold.


Yes, I think that's what we want.


Unless we make the notify filter be a write threshold by default, and
when using it internally by the backup job, disable the threshold and
add our backup notifier to the node's list. Of course the current
write threshold code could be refactored into a new driver so that it
doesn't have to rely on notifiers.


As discussed on IRC, we shouldn't implement a bad interface (which
notifiers are, they bypass normal block device 

[Qemu-block] [PATCH 1/3] block: add options parameter to bdrv_new_open_driver()

[Manos Pitsidianakis]

Allow passing a QDict *options parameter to bdrv_new_open_driver() so
that it can be used if a driver needs it upon creation. The previous
behaviour (empty bs->options and bs->explicit_options) remains when
options is NULL.

Signed-off-by: Manos Pitsidianakis 
---
 block.c   | 16 +---
 block/commit.c|  4 ++--
 block/mirror.c|  2 +-
 block/vvfat.c |  2 +-
 include/block/block.h |  2 +-
 5 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index 37e72b7a96..886a457ab0 100644
--- a/block.c
+++ b/block.c
@@ -1149,16 +1149,26 @@ free_and_fail:
 return ret;
 }
 
+/*
+ * If options is not NULL it is cloned (which adds another reference to the
+ * option entries). If the call to bdrv_new_open_driver() is successful, the
+ * caller should unref options to pass ownership.
+ * */
 BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_name,
-   int flags, Error **errp)
+   int flags, QDict *options, Error **errp)
 {
 BlockDriverState *bs;
 int ret;
 
 bs = bdrv_new();
 bs->open_flags = flags;
-bs->explicit_options = qdict_new();
-bs->options = qdict_new();
+if (options) {
+bs->explicit_options = qdict_clone_shallow(options);
+bs->options = qdict_clone_shallow(options);
+} else {
+bs->explicit_options = qdict_new();
+bs->options = qdict_new();
+}
 bs->opaque = NULL;
 
 update_options_from_flags(bs->options, flags);
diff --git a/block/commit.c b/block/commit.c
index c7857c3321..539e23c3f8 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -342,7 +342,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
 /* Insert commit_top block node above top, so we can block consistent read
  * on the backing chain below it */
 commit_top_bs = bdrv_new_open_driver(_commit_top, filter_node_name, 0,
- errp);
+ NULL, errp);
 if (commit_top_bs == NULL) {
 goto fail;
 }
@@ -494,7 +494,7 @@ int bdrv_commit(BlockDriverState *bs)
 backing_file_bs = backing_bs(bs);
 
 commit_top_bs = bdrv_new_open_driver(_commit_top, NULL, BDRV_O_RDWR,
- _err);
+ NULL, _err);
 if (commit_top_bs == NULL) {
 error_report_err(local_err);
 goto ro_cleanup;
diff --git a/block/mirror.c b/block/mirror.c
index c9a6a3ca86..e1a160e6ea 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1164,7 +1164,7 @@ static void mirror_start_job(const char *job_id, 
BlockDriverState *bs,
  * reads on the top, while disabling it in the intermediate nodes, and make
  * the backing chain writable. */
 mirror_top_bs = bdrv_new_open_driver(_mirror_top, filter_node_name,
- BDRV_O_RDWR, errp);
+ BDRV_O_RDWR, NULL, errp);
 if (mirror_top_bs == NULL) {
 return;
 }
diff --git a/block/vvfat.c b/block/vvfat.c
index a9e207f7f0..6c59473baf 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3194,7 +3194,7 @@ static int enable_write_target(BlockDriverState *bs, 
Error **errp)
 #endif
 
 backing = bdrv_new_open_driver(_write_target, NULL, 
BDRV_O_ALLOW_RDWR,
-   _abort);
+   NULL, _abort);
 *(void**) backing->opaque = s;
 
 bdrv_set_backing_hd(s->bs, backing, _abort);
diff --git a/include/block/block.h b/include/block/block.h
index 34770bb33a..020289a37d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -263,7 +263,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
*parent_options,
 BlockDriverState *bdrv_open(const char *filename, const char *reference,
 QDict *options, int flags, Error **errp);
 BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_name,
-   int flags, Error **errp);
+   int flags, QDict *options, Error 
**errp);
 BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
 BlockDriverState *bs,
 QDict *options, int flags);
-- 
2.11.0

Re: [Qemu-block] [RFC] block-insert-node and block-job-delete

[Kevin Wolf]

Am 31.07.2017 um 19:30 hat Manos Pitsidianakis geschrieben:
> On Fri, Jul 28, 2017 at 02:08:43PM +0200, Kevin Wolf wrote:
> > Am 27.07.2017 um 12:07 hat Stefan Hajnoczi geschrieben:
> > > On Wed, Jul 26, 2017 at 09:23:20PM +0300, Manos Pitsidianakis wrote:
> > > > On Wed, Jul 26, 2017 at 04:12:21PM +0100, Stefan Hajnoczi wrote:
> > > > > On Wed, Jul 26, 2017 at 05:19:24PM +0300, Manos Pitsidianakis wrote:
> > > > > > This proposal follows a discussion we had with Kevin and Stefan on 
> > > > > > filter
> > > > > > node management.
> > > > > >
> > > > > > With block filter drivers arises a need to configure filter nodes 
> > > > > > on runtime
> > > > > > with QMP on live graphs. A problem with doing live graph 
> > > > > > modifications is
> > > > > > that some block jobs modify the graph when they are done and don't 
> > > > > > operate
> > > > > > under any synchronisation, resulting in a race condition if we try 
> > > > > > to insert
> > > > > > a filter node in the place of an existing edge.
> > > > >
> > > > > But maybe you are thinking about higher level race conditions between
> > > > > QMP commands.  Can you give an example of the race?
> > > >
> > > > Exactly, synchronisation between the QMP/user actions. Here's an 
> > > > example,
> > > > correct me if I'm wrong:
> > > >
> > > > User issues a block-commit to this tree to commit A onto B:
> > > >BB -> A -> B
> > > >
> > > > This inserts the dummy mirror node at the top since this is a mirror job
> > > > (active commit)
> > > >BB -> dummy -> A -> B
> > > >
> > > > User wants to add a filter node F below the BB. First we create the 
> > > > node:
> > > >BB -> dummy -> A -> B
> > > >   F /
> > > >
> > > > Commit finishes before we do block-insert-node, dummy and A is removed 
> > > > from
> > > > the BB's chain, mirror_exit calls bdrv_replace_node()
> > > >
> > > >BB > B
> > > >F -> /
> > > >
> > > > Now inserting F under BB will error since dummy does not exist any more.
> > > 
> > > I see the diagrams but the flow isn't clear without the user's QMP
> > > commands.
> > > 
> > > F is created using blockdev-add?  What are the parameters and how does
> > > it know about dummy?  I think this is an interesting question in itself
> > > because dummy is a transient internal node that QMP users don't
> > > necessarily know about.
> > 
> > We can expect that users of block-insert-node also know about block job
> > filter nodes, simply because the former is newer than the latter.
> > 
> > (I also don't like the name "dummy", this makes it sound like it's not
> > really a proper node. In reality, there is little reason to treat it
> > specially.)
> > 
> > > What is the full block-insert-node command?
> > > 
> > > > The proposal doesn't guard against the following:
> > > > User issues a block-commit to this tree to commit B onto C:
> > > >BB -> A -> B -> C
> > > >
> > > > The dummy node (commit's top bs is A):
> > > >BB -> A -> dummy -> B -> C
> > > >
> > > > blockdev-add of a filter we wish to eventually be between A and C:
> > > >BB -> A -> dummy -> B -> C
> > > > F/
> > > >
> > > > if commit finishes before we issue block-insert-node (commit_complete()
> > > > calls bdrv_set_backing_hd() which only touches A)
> > > >BB -> A --> C
> > > >   F -> dummy -> B /
> > > >resulting in error when issuing block-insert-node,
> > > >
> > > > else if we set the job to manual-delete and insert F:
> > > >BB -> A -> F -> dummy -> B -> C
> > > > deleting the job will result in F being lost since the job's top bs 
> > > > wasn't
> > > > updated.
> > > 
> > > manual-delete is a solution for block jobs.  The write threshold
> > > feature is a plain QMP command that in the future will create a
> > > transient internal node (before write notifier).
> > 
> > Yes, that becomes a legacy QMP command then. The new way is blockdev-add
> > and block-insert-node for write threshold, too.
> > 
> > Apart from that, a write threshold node never disappears by itself, it
> > is only inserted or removed in the context of a QMP command. That makes
> > it a lot less dangerous than automatic block completion.
> > 
> > > I'm not sure it makes sense to turn the write threshold feature into a
> > > block job.  I guess it could work, but it seems a little unnatural and
> > > maybe there will be other features that use transient internal nodes.
> > 
> > Yeah, no reason to do so. Just a manually created filter node.
> > 
> 
> Can you explain what you have in mind? The current workflow is using
> block-set-write-threshold on the targetted bs. If we want write threshold to
> be on node level we would want a new filter driver so that it can take
> options special to write-threshold.

Yes, I think that's what we want.

> Unless we make the notify filter be a write threshold by default, and
> when using it internally by the backup job, disable the threshold and
> add our backup notifier to the node's 

[Qemu-block] [PATCH 2/3] block: skip implicit nodes in snapshots, blockjobs

[Manos Pitsidianakis]

Implicit filter nodes added at the top of nodes can interfere with block
jobs. This is not a problem when they are added by other jobs since
adding another job will issue a QERR_DEVICE_IN_USE, but it can happen in
the next commit which introduces an implicitly created throttle filter
node below BlockBackend, which we want to be skipped during automatic
operations on the graph since the user does not necessarily know about
their existence.

Signed-off-by: Manos Pitsidianakis 
---
 block.c   | 17 +
 blockdev.c| 12 
 include/block/block_int.h |  2 ++
 3 files changed, 31 insertions(+)

diff --git a/block.c b/block.c
index 886a457ab0..9ebdba28b0 100644
--- a/block.c
+++ b/block.c
@@ -4947,3 +4947,20 @@ bool bdrv_can_store_new_dirty_bitmap(BlockDriverState 
*bs, const char *name,
 
 return drv->bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp);
 }
+
+/* Get first non-implicit node down a bs chain. */
+BlockDriverState *bdrv_get_first_non_implicit(BlockDriverState *bs)
+{
+if (!bs) {
+return NULL;
+}
+
+if (!bs->implicit) {
+return bs;
+}
+
+/* at this point bs is implicit and must have a child */
+assert(bs->file);
+
+return bdrv_get_first_non_implicit(bs->file->bs);
+}
diff --git a/blockdev.c b/blockdev.c
index 23475abb72..d903a23786 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1664,6 +1664,9 @@ static void external_snapshot_prepare(BlkActionState 
*common,
 return;
 }
 
+/* Skip implicit filter nodes */
+state->old_bs = bdrv_get_first_non_implicit(state->old_bs);
+
 /* Acquire AioContext now so any threads operating on old_bs stop */
 state->aio_context = bdrv_get_aio_context(state->old_bs);
 aio_context_acquire(state->aio_context);
@@ -3095,6 +3098,9 @@ void qmp_block_commit(bool has_job_id, const char 
*job_id, const char *device,
 return;
 }
 
+/* Skip implicit filter nodes */
+bs = bdrv_get_first_non_implicit(bs);
+
 aio_context = bdrv_get_aio_context(bs);
 aio_context_acquire(aio_context);
 
@@ -3209,6 +3215,9 @@ static BlockJob *do_drive_backup(DriveBackup *backup, 
BlockJobTxn *txn,
 return NULL;
 }
 
+/* Skip implicit filter nodes */
+bs = bdrv_get_first_non_implicit(bs);
+
 aio_context = bdrv_get_aio_context(bs);
 aio_context_acquire(aio_context);
 
@@ -3484,6 +3493,9 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
 return;
 }
 
+/* Skip implicit filter nodes */
+bs = bdrv_get_first_non_implicit(bs);
+
 aio_context = bdrv_get_aio_context(bs);
 aio_context_acquire(aio_context);
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index d4f4ea7584..9eeae490f0 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -981,4 +981,6 @@ void bdrv_dec_in_flight(BlockDriverState *bs);
 
 void blockdev_close_all_bdrv_states(void);
 
+BlockDriverState *bdrv_get_first_non_implicit(BlockDriverState *bs);
+
 #endif /* BLOCK_INT_H */
-- 
2.11.0

[Qemu-block] [PATCH 0/3] block: remove legacy I/O throttling

[Manos Pitsidianakis]

This series depends on my other series 'add throttle block driver filter'
currently on v3, which as the name suggests adds a throttle filter driver.

Replacing the current I/O interface means the user will use the same options as
before and QEMU will create a hidden throttle filter node beneath the device's
BlockBackend. 

Manos Pitsidianakis (3):
  block: add options parameter to bdrv_new_open_driver()
  block: skip implicit nodes in snapshots, blockjobs
  block: remove legacy I/O throttling

 block.c |  41 ++-
 block/block-backend.c   | 149 +---
 block/commit.c  |   4 +-
 block/mirror.c  |   2 +-
 block/qapi.c|  14 ++--
 block/throttle.c|   8 +++
 block/vvfat.c   |   2 +-
 blockdev.c  |  67 ++
 include/block/block.h   |   2 +-
 include/block/block_int.h   |  15 
 include/block/throttle-groups.h |   2 +
 include/sysemu/block-backend.h  |   8 +--
 tests/test-throttle.c   |  15 ++--
 13 files changed, 235 insertions(+), 94 deletions(-)

-- 
2.11.0

[Qemu-block] [PATCH 3/3] block: remove legacy I/O throttling

[Manos Pitsidianakis]

This commit removes all I/O throttling from block/block-backend.c. In
order to support the existing interface, it is changed to use the
block/throttle.c filter driver.

The throttle filter node that is created by the legacy interface is
stored in a 'throttle_node' field in the BlockBackendPublic of the
device. The legacy throttle node is managed by the legacy interface
completely. More advanced configurations with the filter drive are
possible using the QMP API, but these will be ignored by the legacy
interface.

Signed-off-by: Manos Pitsidianakis 
---
 block.c |   8 +++
 block/block-backend.c   | 149 +---
 block/qapi.c|  14 ++--
 block/throttle.c|   8 +++
 blockdev.c  |  55 +++
 include/block/block_int.h   |  13 
 include/block/throttle-groups.h |   2 +
 include/sysemu/block-backend.h  |   8 +--
 tests/test-throttle.c   |  15 ++--
 9 files changed, 186 insertions(+), 86 deletions(-)

diff --git a/block.c b/block.c
index 9ebdba28b0..c6aad25286 100644
--- a/block.c
+++ b/block.c
@@ -1975,6 +1975,7 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState 
*child_bs,
 child = g_new(BdrvChild, 1);
 *child = (BdrvChild) {
 .bs = NULL,
+.parent_bs  = NULL,
 .name   = g_strdup(child_name),
 .role   = child_role,
 .perm   = perm,
@@ -2009,6 +2010,7 @@ BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
 if (child == NULL) {
 return NULL;
 }
+child->parent_bs = parent_bs;
 
 QLIST_INSERT_HEAD(_bs->children, child, next);
 return child;
@@ -3729,6 +3731,12 @@ const char *bdrv_get_parent_name(const BlockDriverState 
*bs)
 return name;
 }
 }
+if (c->parent_bs && c->parent_bs->implicit) {
+name = bdrv_get_parent_name(c->parent_bs);
+if (name && *name) {
+return name;
+}
+}
 }
 
 return NULL;
diff --git a/block/block-backend.c b/block/block-backend.c
index df0200fc49..45f45efee9 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -15,6 +15,7 @@
 #include "block/block_int.h"
 #include "block/blockjob.h"
 #include "block/throttle-groups.h"
+#include "qemu/throttle-options.h"
 #include "sysemu/blockdev.h"
 #include "sysemu/sysemu.h"
 #include "qapi-event.h"
@@ -282,8 +283,8 @@ static void blk_delete(BlockBackend *blk)
 assert(!blk->refcnt);
 assert(!blk->name);
 assert(!blk->dev);
-if (blk->public.throttle_group_member.throttle_state) {
-blk_io_limits_disable(blk);
+if (blk->public.throttle_node) {
+blk_io_limits_disable(blk, _abort);
 }
 if (blk->root) {
 blk_remove_bs(blk);
@@ -593,13 +594,7 @@ BlockBackend *blk_by_public(BlockBackendPublic *public)
  */
 void blk_remove_bs(BlockBackend *blk)
 {
-ThrottleTimers *tt;
-
 notifier_list_notify(>remove_bs_notifiers, blk);
-if (blk->public.throttle_group_member.throttle_state) {
-tt = >public.throttle_group_member.throttle_timers;
-throttle_timers_detach_aio_context(tt);
-}
 
 blk_update_root_state(blk);
 
@@ -620,12 +615,6 @@ int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, 
Error **errp)
 bdrv_ref(bs);
 
 notifier_list_notify(>insert_bs_notifiers, blk);
-if (blk->public.throttle_group_member.throttle_state) {
-throttle_timers_attach_aio_context(
->public.throttle_group_member.throttle_timers,
-bdrv_get_aio_context(bs));
-}
-
 return 0;
 }
 
@@ -983,13 +972,6 @@ int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t 
offset,
 }
 
 bdrv_inc_in_flight(bs);
-
-/* throttling disk I/O */
-if (blk->public.throttle_group_member.throttle_state) {
-
throttle_group_co_io_limits_intercept(>public.throttle_group_member,
-bytes, false);
-}
-
 ret = bdrv_co_preadv(blk->root, offset, bytes, qiov, flags);
 bdrv_dec_in_flight(bs);
 return ret;
@@ -1010,11 +992,6 @@ int coroutine_fn blk_co_pwritev(BlockBackend *blk, 
int64_t offset,
 }
 
 bdrv_inc_in_flight(bs);
-/* throttling disk I/O */
-if (blk->public.throttle_group_member.throttle_state) {
-
throttle_group_co_io_limits_intercept(>public.throttle_group_member,
-bytes, true);
-}
 
 if (!blk->enable_write_cache) {
 flags |= BDRV_REQ_FUA;
@@ -1682,16 +1659,9 @@ static AioContext *blk_aiocb_get_aio_context(BlockAIOCB 
*acb)
 void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)
 {
 BlockDriverState *bs = blk_bs(blk);
-ThrottleGroupMember *tgm = >public.throttle_group_member;
 
 if (bs) {
-if (tgm->throttle_state) {
-throttle_group_detach_aio_context(tgm);
-}
 bdrv_set_aio_context(bs, new_context);
-if 

Re: [Qemu-block] [PATCH] block: check BlockDriverState object before dereference

[Kevin Wolf]

Am 01.08.2017 um 10:35 hat Paolo Bonzini geschrieben:
> On 01/08/2017 02:14, John Snow wrote:
> > I may need some nudging towards understanding what the right solution
> > here is, though. Should the blk_aio_flush assume that there always is a
> > root BDS? should it not assume that?
> 
> I think blk_aio_flush is not special.  If there is no root BDS, either
> you return -ENOMEDIUM, or you crash.  But all functions should be doing
> the same.

The intended semantics is that they return -ENOMEDIUM (or fail at
least). This is how things have always worked, and that it crashes now
because of the bdrv_inc_in_flight() was not an intentional change, but
simply a bug in the patch.

> The former makes sense, but right now blk_prwv for one are crashing if
> there is no root BDS so the minimum patch would fix the caller rather
> than blk_aio_flush.

The synchronous versions don't crash, and bdrv_aio_prwv() would fix all
cases if bdrv_inc_in_flight() were moved inside the coroutine; probably
right before blk_aio_complete(). This would be more consistent with how
the synchronous versions work, too, increasing the in-flight count only
by 1 rather than 2 for an AIO request.

Kevin

Re: [Qemu-block] [Qemu-devel] [PATCH 4/6] hw/block: Fix the return type

[Stefan Hajnoczi]

On Wed, Jul 26, 2017 at 08:02:53PM +0800, Mao Zhongyi wrote:
> When the function no success value to transmit, it usually make the
> function return void. It has turned out not to be a success, because
> it means that the extra local_err variable and error_propagate() will
> be needed. It leads to cumbersome code, therefore, transmit success/
> failure in the return value is worth.
> 
> So fix the return type of blkconf_apply_backend_options(),
> blkconf_geometry() and virtio_blk_data_plane_create() to avoid it.
> 
> Cc: John Snow 
> Cc: Kevin Wolf 
> Cc: Max Reitz 
> Cc: Stefan Hajnoczi 
> 
> Signed-off-by: Mao Zhongyi 
> ---
>  hw/block/block.c| 21 -
>  hw/block/dataplane/virtio-blk.c | 16 +---
>  hw/block/dataplane/virtio-blk.h |  6 +++---
>  include/hw/block/block.h| 10 +-
>  4 files changed, 29 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/block/block.c b/hw/block/block.c
> index 27878d0..717bd0e 100644
> --- a/hw/block/block.c
> +++ b/hw/block/block.c
> @@ -51,8 +51,8 @@ void blkconf_blocksizes(BlockConf *conf)
>  }
>  }
>  
> -void blkconf_apply_backend_options(BlockConf *conf, bool readonly,
> -   bool resizable, Error **errp)
> +int blkconf_apply_backend_options(BlockConf *conf, bool readonly,
> +  bool resizable, Error **errp)

I'm not a fan of these changes because it makes inconsistencies between
the return value and the errp argument possible (e.g. returning success
but setting errp, or returning failure without setting errp).

If you really want to do this please use bool as the return type instead
of int.  int can be abused to return error information that should
really be in the Error object.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-block] [Qemu-devel] [PATCH v2] iscsi: use scsi_create_task()

[Paolo Bonzini]

On 28/07/2017 19:52, Marc-André Lureau wrote:
> 
> Stupid question: what's the benefit?  
> 
> scsi_create/scsi_free is a library API. If they have their own
> allocator, we better use it, or it may easily break, no?

Well, that would be an API breakage, but I see the point.  I think I
would prefer something like

static inline struct scsi_task *iscsi_create_task(...)
{
#if ...
return scsi_create_task(...)
#else
/* Older versions of libiscsi have a bug, so include our
 * implementation.
 */
struct scsi_task *task = g_try_malloc0(sizeof(struct scsi_task));
if (!task) {
task;
}

memcpy(>cdb[0], cdb, cdb_size);
task->cdb_size   = cdb_size;
task->xfer_dir   = xfer_dir;
task->expxferlen = expxferlen;
return task;
#endif
}

> 
> Change malloc to g_new0 in the
> existing code, and we even make it shorter...
> 
> 
> replacing malloc with g_new is the subject of another upcoming series :)
> (https://github.com/elmarco/clang-tools-extra/blob/master/clang-tidy/qemu/UseGnewCheck.cpp)
>  

Re: [Qemu-block] [PATCH v3 09/13] qcow2: move is_zero_sectors() up

[Anton Nefedov]

On 07/31/2017 10:13 PM, Eric Blake wrote:

On 07/31/2017 11:22 AM, Anton Nefedov wrote:

To be used in the following commit without a forward declaration.

Signed-off-by: Anton Nefedov 
---
  block/qcow2.c | 39 +++
  1 file changed, 19 insertions(+), 20 deletions(-)


This conflicts with my byte-based block status work; do you want to
rebase on top of my posts?



sure! will send the v4 soon

/Anton

Re: [Qemu-block] [PATCH v3 04/13] block: support BDRV_REQ_ALLOCATE in passthrough drivers

[Anton Nefedov]

On 07/31/2017 10:11 PM, Eric Blake wrote:

On 07/31/2017 11:21 AM, Anton Nefedov wrote:

Support the flag if the underlying BDS supports it

Signed-off-by: Anton Nefedov 
---
  block/blkdebug.c   | 3 ++-
  block/raw-format.c | 3 ++-
  2 files changed, 4 insertions(+), 2 deletions(-)


What about blkverify, commit, and mirror?  Should they also pass through
various flags?



looks like commit is read-only, but blkverify and mirror probably
should, will try to fix that

/Anton

Re: [Qemu-block] [PATCH v3] qemu-iotests: add a "how to" to ./README

[Stefan Hajnoczi]

On Mon, Jul 31, 2017 at 12:28:44PM -0500, Eric Blake wrote:
> On 07/31/2017 11:26 AM, Stefan Hajnoczi wrote:
> > There is not much getting started documentation for qemu-iotests.  This
> > patch explains how to create a new test and covers the overall testing
> > approach.
> > 
> > Cc: Ishani Chugh 
> > Reviewed-by: Eric Blake 
> > Reviewed-by: Philippe Mathieu-Daudé 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> 
> >  Please send improvements to the test suite, general feedback or just
> >  reports of failing tests cases to qemu-de...@nongnu.org with a CC:
> >  to qemu-block@nongnu.org.
> > +
> > 
> 
> Is this adding a spurious blank line at EOF?

Yes.  Please remove when merging.

Thanks,
Stefan


signature.asc
Description: PGP signature

Re: [Qemu-block] [PATCH] block: check BlockDriverState object before dereference

[Paolo Bonzini]

On 01/08/2017 02:14, John Snow wrote:
> I may need some nudging towards understanding what the right solution
> here is, though. Should the blk_aio_flush assume that there always is a
> root BDS? should it not assume that?

I think blk_aio_flush is not special.  If there is no root BDS, either
you return -ENOMEDIUM, or you crash.  But all functions should be doing
the same.

The former makes sense, but right now blk_prwv for one are crashing if
there is no root BDS so the minimum patch would fix the caller rather
than blk_aio_flush.

Paolo

> It's difficult for me to understand right now if the bug is in the
> expectation for the blk_ functions and the caller should be amended, or
> if you need changes to the way the blk_ functions are trying to
> increment a counter that doesn't exist.
> 
> I can handle the former fairly easily; if it's the latter, I'm afraid
> it's stuck in the middle of some of your changes and I'd need a stronger
> hint.

Re: [Qemu-block] [Qemu-devel] [PATCH v2 for-2.11 3/3] qemu-iotests: add option to save temp files on error

[Markus Armbruster]

Jeff Cody  writes:

> Now that ./check takes care of cleaning up after each tests, it
> can also selectively not clean up.  Add option to leave all output from
> tests intact if that test encountered an error.
>
> Note: this currently only works for bash tests, as the python tests
> still clean up after themselves manually.

Should we add a TODO comment for that?

Much appreciated work, by the way.  You might want to mention in one of
your commit messages that this is also a step towards running iotests in
parallel.

Another step towards sanity would be making $TEST_DIR instead of
$source_iotests the current working directory for running tests.