[Qemu-block] [PATCH] qemu-img: add the 'dd' subcommand

[Reda Sallahi]

This patch adds a basic dd subcommand analogous to dd(1) to qemu-img.

For the start, this implements the bs, if, of and count options and requires
both if and of to be specified (no stdin/stdout if not specified) and doesn't
support tty, pipes, etc.

The image format must be specified with -O for the output if the raw format
is not the intended one.

get_size() was needed for the size syntax dd(1) supports which is different
from qemu_strtosz_suffix().

Two tests are added to test qemu-img dd.

Signed-off-by: Reda Sallahi 
---
 qemu-img-cmds.hx   |   6 +
 qemu-img.c | 645 -
 tests/qemu-iotests/158 |  53 
 tests/qemu-iotests/158.out |  15 ++
 tests/qemu-iotests/159 |  56 
 tests/qemu-iotests/159.out |  87 ++
 tests/qemu-iotests/group   |   2 +
 7 files changed, 863 insertions(+), 1 deletion(-)
 create mode 100755 tests/qemu-iotests/158
 create mode 100644 tests/qemu-iotests/158.out
 create mode 100755 tests/qemu-iotests/159
 create mode 100644 tests/qemu-iotests/159.out

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 7e95b2d..03bdd7a 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -45,6 +45,12 @@ STEXI
 @item convert [--object @var{objectdef}] [--image-opts] [-c] [-p] [-q] [-n] 
[-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o 
@var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S 
@var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
+DEF("dd", img_dd,
+"dd [--image-opts] [-f fmt] [-O output_fmt] [bs=block_size] [count=blocks] 
if=input of=output")
+STEXI
+@item dd [--image-opts] [-f @var{fmt}] [-O @var{output_fmt}] 
[bs=@var{block_size}] [count=@var{blocks}] if=@var{input} of=@var{output}
+ETEXI
+
 DEF("info", img_info,
 "info [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] 
[--backing-chain] filename")
 STEXI
diff --git a/qemu-img.c b/qemu-img.c
index ea5970b..d7f134d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -166,7 +166,14 @@ static void QEMU_NORETURN help(void)
"Parameters to compare subcommand:\n"
"  '-f' first image format\n"
"  '-F' second image format\n"
-   "  '-s' run in Strict mode - fail on different image size or sector 
allocation\n";
+   "  '-s' run in Strict mode - fail on different image size or sector 
allocation\n"
+   "\n"
+   "Parameters to dd subcommand:\n"
+   "  'bs=BYTES' read and write up to BYTES bytes at a time "
+   "(default: 512)\n"
+   "  'count=N' copy only N input blocks\n"
+   "  'if=FILE' read from FILE instead of stdin\n"
+   "  'of=FILE' write to FILE instead of stdout\n";
 
 printf("%s\nSupported formats:", help_msg);
 bdrv_iterate_format(format_print, NULL);
@@ -3794,6 +3801,642 @@ out:
 return 0;
 }
 
+#define C_BS  01
+#define C_CBS 02
+#define C_CONV04
+#define C_COUNT   010
+#define C_IBS 020
+#define C_IF  040
+#define C_IFLAG   0100
+#define C_OBS 0200
+#define C_OF  0400
+#define C_OFLAG   01000
+#define C_SEEK02000
+#define C_SKIP04000
+#define C_STATUS  01
+
+struct DdEss {
+unsigned int flags;
+unsigned int status;
+unsigned int conv;
+size_t count;
+size_t cbsz; /* Conversion block size */
+};
+
+struct DdIo {
+size_t bsz;/* Block size */
+off_t offset;
+const char *filename;
+unsigned int flags;
+uint8_t *buf;
+};
+
+struct DdOpts {
+const char *name;
+int (*f)(const char *, struct DdIo *, struct DdIo *, struct DdEss *);
+unsigned int flag;
+};
+
+static size_t get_size(const char *str)
+{
+/* XXX: handle {k,m,g}B notations */
+unsigned long num;
+size_t res = 0;
+const char *buf;
+int ret;
+
+errno = 0;
+if (strchr(str, '-')) {
+error_report("invalid number: '%s'", str);
+errno = EINVAL;
+return res;
+}
+ret = qemu_strtoul(str, , 0, );
+
+if (ret < 0) {
+error_report("invalid number: '%s'", str);
+return res;
+}
+
+switch (*buf) {
+case '\0':
+case 'c':
+res = num;
+break;
+case 'w':
+res = num * 2;
+break;
+case 'b':
+res = num * 512;
+break;
+case 'K':
+res = num * 1024;
+break;
+case 'M':
+res = num * 1024 * 1024;
+break;
+case 'G':
+res = num * 1024 * 1024 * 1024;
+break;
+case 'T':
+res = num * 1024 * 1024 * 1024 * 1024;
+break;
+case 'P':
+res = num * 1024 * 1024 * 1024 * 1024 * 1024;
+break;
+case 'E':
+res = num * 1024 * 1024 * 1024 * 1024 * 1024 * 1024;
+break;
+case 'Z':
+res = num * 1024 * 1024 * 1024 * 1024 * 1024 * 1024 * 1024;
+break;
+case 'Y':
+res = num * 1024 * 1024 * 1024 * 1024 * 1024 * 1024 

[Qemu-block] [PATCH v9 11/17] block: Simplify drive-mirror

[Eric Blake]

Now that we can support boxed commands, use it to greatly
reduce the number of parameters (and likelihood of getting
out of sync) when adjusting drive-mirror parameters.

Signed-off-by: Eric Blake 
Reviewed-by: John Snow 

---
v9: s/box/boxed/, trivial enough to keep R-b
v8: rebase, drop stale sentence in docs, don't rearrange initialiation
v7: new patch
---
 qapi/block-core.json | 20 +++---
 blockdev.c   | 76 +++-
 hmp.c| 25 -
 3 files changed, 60 insertions(+), 61 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index cbf4410..a085c85 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1108,6 +1108,21 @@
 #
 # Start mirroring a block device's writes to a new destination.
 #
+# See DriveMirror for parameter descriptions
+#
+# Returns: nothing on success
+#  If @device is not a valid block device, DeviceNotFound
+#
+# Since 1.3
+##
+{ 'command': 'drive-mirror', 'boxed': true,
+  'data': 'DriveMirror' }
+
+##
+# DriveMirror
+#
+# A set of parameters describing drive mirror setup.
+#
 # @device:  the name of the device whose writes should be mirrored.
 #
 # @target: the target of the new image. If the file exists, or if it
@@ -1154,12 +1169,9 @@
 # written. Both will result in identical contents.
 # Default is true. (Since 2.4)
 #
-# Returns: nothing on success
-#  If @device is not a valid block device, DeviceNotFound
-#
 # Since 1.3
 ##
-{ 'command': 'drive-mirror',
+{ 'struct': 'DriveMirror',
   'data': { 'device': 'str', 'target': 'str', '*format': 'str',
 '*node-name': 'str', '*replaces': 'str',
 'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
diff --git a/blockdev.c b/blockdev.c
index ddf30e1..f23bf99 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3458,19 +3458,7 @@ static void blockdev_mirror_common(BlockDriverState *bs,
  block_job_cb, bs, errp);
 }

-void qmp_drive_mirror(const char *device, const char *target,
-  bool has_format, const char *format,
-  bool has_node_name, const char *node_name,
-  bool has_replaces, const char *replaces,
-  enum MirrorSyncMode sync,
-  bool has_mode, enum NewImageMode mode,
-  bool has_speed, int64_t speed,
-  bool has_granularity, uint32_t granularity,
-  bool has_buf_size, int64_t buf_size,
-  bool has_on_source_error, BlockdevOnError 
on_source_error,
-  bool has_on_target_error, BlockdevOnError 
on_target_error,
-  bool has_unmap, bool unmap,
-  Error **errp)
+void qmp_drive_mirror(DriveMirror *arg, Error **errp)
 {
 BlockDriverState *bs;
 BlockBackend *blk;
@@ -3481,11 +3469,12 @@ void qmp_drive_mirror(const char *device, const char 
*target,
 QDict *options = NULL;
 int flags;
 int64_t size;
+const char *format = arg->format;

-blk = blk_by_name(device);
+blk = blk_by_name(arg->device);
 if (!blk) {
 error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-  "Device '%s' not found", device);
+  "Device '%s' not found", arg->device);
 return;
 }

@@ -3493,24 +3482,25 @@ void qmp_drive_mirror(const char *device, const char 
*target,
 aio_context_acquire(aio_context);

 if (!blk_is_available(blk)) {
-error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, arg->device);
 goto out;
 }
 bs = blk_bs(blk);
-if (!has_mode) {
-mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+if (!arg->has_mode) {
+arg->mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
 }

-if (!has_format) {
-format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
+if (!arg->has_format) {
+format = (arg->mode == NEW_IMAGE_MODE_EXISTING
+  ? NULL : bs->drv->format_name);
 }

 flags = bs->open_flags | BDRV_O_RDWR;
 source = backing_bs(bs);
-if (!source && sync == MIRROR_SYNC_MODE_TOP) {
-sync = MIRROR_SYNC_MODE_FULL;
+if (!source && arg->sync == MIRROR_SYNC_MODE_TOP) {
+arg->sync = MIRROR_SYNC_MODE_FULL;
 }
-if (sync == MIRROR_SYNC_MODE_NONE) {
+if (arg->sync == MIRROR_SYNC_MODE_NONE) {
 source = bs;
 }

@@ -3520,18 +3510,18 @@ void qmp_drive_mirror(const char *device, const char 
*target,
 goto out;
 }

-if (has_replaces) {
+if (arg->has_replaces) {
 BlockDriverState *to_replace_bs;
 AioContext *replace_aio_context;
 int64_t replace_size;

-if (!has_node_name) {
+if (!arg->has_node_name) {
 error_setg(errp, "a node-name must be provided when replacing a"
  

[Qemu-block] [PATCH v9 10/17] block: Simplify block_set_io_throttle

[Eric Blake]

Now that we can support boxed commands, use it to greatly
reduce the number of parameters (and likelihood of getting
out of sync) when adjusting throttle parameters.

Signed-off-by: Eric Blake 
Reviewed-by: Alberto Garcia 

---
v9: s/box/boxed/, trivial enough to keep R-b
v8: tweak doc wording
v7: new patch
---
 qapi/block-core.json |  20 --
 blockdev.c   | 111 +++
 hmp.c|  45 +
 3 files changed, 66 insertions(+), 110 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index ac8f5f6..cbf4410 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1312,6 +1312,21 @@
 # the device will be removed from its group and the rest of its
 # members will not be affected. The 'group' parameter is ignored.
 #
+# See BlockIOThrottle for parameter descriptions.
+#
+# Returns: Nothing on success
+#  If @device is not a valid block device, DeviceNotFound
+#
+# Since: 1.1
+##
+{ 'command': 'block_set_io_throttle', 'boxed': true,
+  'data': 'BlockIOThrottle' }
+
+##
+# BlockIOThrottle
+#
+# A set of parameters describing block throttling.
+#
 # @device: The name of the device
 #
 # @bps: total throughput limit in bytes per second
@@ -1378,12 +1393,9 @@
 #
 # @group: #optional throttle group name (Since 2.4)
 #
-# Returns: Nothing on success
-#  If @device is not a valid block device, DeviceNotFound
-#
 # Since: 1.1
 ##
-{ 'command': 'block_set_io_throttle',
+{ 'struct': 'BlockIOThrottle',
   'data': { 'device': 'str', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
 '*bps_max': 'int', '*bps_rd_max': 'int',
diff --git a/blockdev.c b/blockdev.c
index 0f8065c..ddf30e1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2631,49 +2631,17 @@ fail:
 }

 /* throttling disk I/O limits */
-void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
-   int64_t bps_wr,
-   int64_t iops,
-   int64_t iops_rd,
-   int64_t iops_wr,
-   bool has_bps_max,
-   int64_t bps_max,
-   bool has_bps_rd_max,
-   int64_t bps_rd_max,
-   bool has_bps_wr_max,
-   int64_t bps_wr_max,
-   bool has_iops_max,
-   int64_t iops_max,
-   bool has_iops_rd_max,
-   int64_t iops_rd_max,
-   bool has_iops_wr_max,
-   int64_t iops_wr_max,
-   bool has_bps_max_length,
-   int64_t bps_max_length,
-   bool has_bps_rd_max_length,
-   int64_t bps_rd_max_length,
-   bool has_bps_wr_max_length,
-   int64_t bps_wr_max_length,
-   bool has_iops_max_length,
-   int64_t iops_max_length,
-   bool has_iops_rd_max_length,
-   int64_t iops_rd_max_length,
-   bool has_iops_wr_max_length,
-   int64_t iops_wr_max_length,
-   bool has_iops_size,
-   int64_t iops_size,
-   bool has_group,
-   const char *group, Error **errp)
+void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
 {
 ThrottleConfig cfg;
 BlockDriverState *bs;
 BlockBackend *blk;
 AioContext *aio_context;

-blk = blk_by_name(device);
+blk = blk_by_name(arg->device);
 if (!blk) {
 error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-  "Device '%s' not found", device);
+  "Device '%s' not found", arg->device);
 return;
 }

@@ -2682,59 +2650,59 @@ void qmp_block_set_io_throttle(const char *device, 
int64_t bps, int64_t bps_rd,

 bs = blk_bs(blk);
 if (!bs) {
-error_setg(errp, "Device '%s' has no medium", device);
+error_setg(errp, "Device '%s' has no medium", arg->device);
 goto out;
 }

 throttle_config_init();
-cfg.buckets[THROTTLE_BPS_TOTAL].avg = bps;
-cfg.buckets[THROTTLE_BPS_READ].avg  = bps_rd;
-cfg.buckets[THROTTLE_BPS_WRITE].avg = bps_wr;
+cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps;
+cfg.buckets[THROTTLE_BPS_READ].avg  = arg->bps_rd;
+cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr;

-cfg.buckets[THROTTLE_OPS_TOTAL].avg = iops;
-cfg.buckets[THROTTLE_OPS_READ].avg  = iops_rd;
-

Re: [Qemu-block] [PATCH v5 02/10] HBitmap: Introduce "meta" bitmap to track bit changes

[John Snow]

On 06/22/2016 11:22 AM, Max Reitz wrote:
> On 03.06.2016 06:32, Fam Zheng wrote:
>> Upon each bit toggle, the corresponding bit in the meta bitmap will be
>> set.
>>
>> Signed-off-by: Fam Zheng 
>> Reviewed-by: John Snow 
>> ---
>>  block/dirty-bitmap.c   |  2 +-
>>  include/qemu/hbitmap.h | 17 +
>>  util/hbitmap.c | 69 
>> +++---
>>  3 files changed, 72 insertions(+), 16 deletions(-)
>>
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index ec073ee..628b77c 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -231,7 +231,7 @@ static void 
>> bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
>>  BdrvDirtyBitmap *bm, *next;
>>  QLIST_FOREACH_SAFE(bm, >dirty_bitmaps, list, next) {
>>  if ((!bitmap || bm == bitmap) && (!only_named || bm->name)) {
>> -assert(!bitmap->active_iterators);
>> +assert(!bm->active_iterators);
> 
> Same comment as for v4, this should be squashed into the previous patch.
> 
> Max
> 

Ah, replied too soon on #1. Thanks :)

Re: [Qemu-block] [PATCH 3/3] iotests: Test format probes

[Colin Lord]

On 07/12/2016 05:17 PM, John Snow wrote:
> 
> 
> On 07/11/2016 03:50 PM, Colin Lord wrote:
>> Adds a new iotest for testing that the format probing functions work as
>> expected. This is done by booting up a vm with a disk image without
>> specifying the image format. Then the format is checked using a call to
>> query-block.
>>
>> For any format specified, at least one check is done with an image of
>> the given format, and one check is done against a raw image. This is to
>> ensure that a probe correctly returns a high score when the format is
>> matched, and does not return a high score when the format should not be
>> matched, as would be the case with raw images.
>>
>> Signed-off-by: Colin Lord 
>> ---
>>  tests/qemu-iotests/158| 53 
>> +++
>>  tests/qemu-iotests/158.out|  5 
>>  tests/qemu-iotests/group  |  1 +
>>  tests/qemu-iotests/iotests.py |  5 
>>  4 files changed, 64 insertions(+)
>>  create mode 100644 tests/qemu-iotests/158
>>  create mode 100644 tests/qemu-iotests/158.out
>>
>> diff --git a/tests/qemu-iotests/158 b/tests/qemu-iotests/158
>> new file mode 100644
>> index 000..70ad298
>> --- /dev/null
>> +++ b/tests/qemu-iotests/158
>> @@ -0,0 +1,53 @@
>> +#!/usr/bin/env python
>> +
> 
>  No license? It will default to GPL2, but it may be better to
> explicitly state the license for a new file. 
> 
>> +import iotests
>> +
>> +imgs = {'raw': ['grub_mbr.raw'], 'bochs': ['empty.bochs'],
>> +'cloop': ['simple-pattern.cloop'],
>> +'parallels': ['parallels-v1', 'parallels-v2'],
>> +'qcow': ['empty.qcow'], 'qcow2': ['empty.qcow2'],
>> +'qed': ['empty.qed'], 'vdi': ['empty.vdi'],
>> +'vpc': ['virtualpc-dynamic.vhd'], 'vhdx': 
>> ['iotest-dynamic-1G.vhdx'],
>> +'vmdk': ['iotest-version3.vmdk'], 'luks': ['empty.luks'],
>> +'dmg': ['empty.dmg']}
>> +
> 
> Might be nicer to do one format->file mapping per line.
> 
>> +class TestProbingFormats(iotests.QMPTestCase):
>> +def setUp(self):
>> +self.vm = iotests.VM()
>> +self.img_paths = []
>> +luks_opts = ''
>> +if iotests.imgfmt == 'luks':
>> +luks_opts = (',key-secret=sec0')
>> +for img_name in imgs[iotests.imgfmt]:
>> +self.img_paths.append(iotests.use_sample_image(img_name))
>> +self.vm.add_drive_raw('file=%s,id=drive%s,media=cdrom%s' %
>> +  (self.img_paths[-1], len(self.img_paths) 
>> - 1,
>> +  luks_opts))
> 
> Perhaps id=drive%s could be id=drive%d, but I'm no pythonista.
> 
>> +if iotests.imgfmt == 'luks':
>> +self.vm.use_luks()
>> +if iotests.imgfmt != 'raw':
>> +self.img_paths.append(iotests.use_sample_image(imgs['raw'][0]))
>> +self.vm.add_drive_raw('file=%s,id=drive%s,media=cdrom' %
>> +  (self.img_paths[-1], len(self.img_paths) 
>> - 1))
>> +self.vm.launch()
>> +
> 
> Oh, everything as a CDROM? I guess there's no reason that doesn't work,
> but it strikes me as a bit unconventional. I guess you'll be testing
> some quite esoteric configurations this way.
> 
Yeah it looks a little odd. I actually did it this way because some of
the image formats are read-only and so they throw errors if you try to
mount them as writable. I wasn't finding an option anywhere to mount
drives as read only so I just took the quick way out and went with
cdrom. Is there a better way to mount drives as read only?

>> +def tearDown(self):
>> +self.vm.shutdown()
>> +for img in self.img_paths:
>> +iotests.rm_test_image(img)
>> +
>> +def test_probe_detects_format(self):
>> +result = self.vm.qmp('query-block')
>> +for i in range(len(self.img_paths) - 1):
>> +self.assert_qmp(result, 'return[%s]/inserted/image/format' %
>> +i, iotests.imgfmt)
>> +
> 
> Seems sane.
> 
>> +def test_probe_detects_raw(self):
>> +result = self.vm.qmp('query-block')
>> +self.assert_qmp(result, 'return[%s]/inserted/image/format' %
>> +(len(self.img_paths) - 1), 'raw')
>> +
> 
> Why do we need to test raw against every format, exactly? The absence or
> presence of other drives shouldn't affect the probing, so allowing us to
> test the raw probe with
> 
> ./check -v -raw 158
> 
> should be enough.
> 
> Unless I'm misunderstanding the point?
> 
I guess it might be unnecessary. I mostly had it in there to prevent a
hypothetical probe which always returns 100 from passing the tests. For
example, if I'm testing the bochs probe on a bochs image, if it returned
100 it would pass (which is correct). But it should also be tested
against another format to make sure the other format doesn't incorrectly
get detected as bochs. I guess this situation gets fixed naturally if
you test more than one format, it's 

Re: [Qemu-block] [PATCH for 2.7 resend] linux-aio: share one LinuxAioState within an AioContext

[Paolo Bonzini]

Ping.

On 04/07/2016 18:33, Paolo Bonzini wrote:
> This has better performance because it executes fewer system calls
> and does not use a bottom half per disk.
> 
> Originally proposed by Ming Lei.
> 
> Acked-by: Stefan Hajnoczi 
> Signed-off-by: Paolo Bonzini 
> ---
>  async.c|  23 +++
>  block/linux-aio.c  |  10 ++--
>  block/raw-posix.c  | 119 
> +
>  block/raw-win32.c  |   2 +-
>  include/block/aio.h|  13 
>  {block => include/block}/raw-aio.h |   0
>  6 files changed, 57 insertions(+), 110 deletions(-)
>  rename {block => include/block}/raw-aio.h (100%)
> 
> diff --git a/async.c b/async.c
> index b4bf205..6caa98c 100644
> --- a/async.c
> +++ b/async.c
> @@ -29,6 +29,7 @@
>  #include "block/thread-pool.h"
>  #include "qemu/main-loop.h"
>  #include "qemu/atomic.h"
> +#include "block/raw-aio.h"
>  
>  /***/
>  /* bottom halves (can be seen as timers which expire ASAP) */
> @@ -242,6 +243,14 @@ aio_ctx_finalize(GSource *source)
>  qemu_bh_delete(ctx->notify_dummy_bh);
>  thread_pool_free(ctx->thread_pool);
>  
> +#ifdef CONFIG_LINUX_AIO
> +if (ctx->linux_aio) {
> +laio_detach_aio_context(ctx->linux_aio, ctx);
> +laio_cleanup(ctx->linux_aio);
> +ctx->linux_aio = NULL;
> +}
> +#endif
> +
>  qemu_mutex_lock(>bh_lock);
>  while (ctx->first_bh) {
>  QEMUBH *next = ctx->first_bh->next;
> @@ -282,6 +291,17 @@ ThreadPool *aio_get_thread_pool(AioContext *ctx)
>  return ctx->thread_pool;
>  }
>  
> +#ifdef CONFIG_LINUX_AIO
> +LinuxAioState *aio_get_linux_aio(AioContext *ctx)
> +{
> +if (!ctx->linux_aio) {
> +ctx->linux_aio = laio_init();
> +laio_attach_aio_context(ctx->linux_aio, ctx);
> +}
> +return ctx->linux_aio;
> +}
> +#endif
> +
>  void aio_notify(AioContext *ctx)
>  {
>  /* Write e.g. bh->scheduled before reading ctx->notify_me.  Pairs
> @@ -345,6 +365,9 @@ AioContext *aio_context_new(Error **errp)
> false,
> (EventNotifierHandler *)
> event_notifier_dummy_cb);
> +#ifdef CONFIG_LINUX_AIO
> +ctx->linux_aio = NULL;
> +#endif
>  ctx->thread_pool = NULL;
>  qemu_mutex_init(>bh_lock);
>  rfifolock_init(>lock, aio_rfifolock_cb, ctx);
> diff --git a/block/linux-aio.c b/block/linux-aio.c
> index e468960..3eb0a0e 100644
> --- a/block/linux-aio.c
> +++ b/block/linux-aio.c
> @@ -50,6 +50,8 @@ typedef struct {
>  } LaioQueue;
>  
>  struct LinuxAioState {
> +AioContext *aio_context;
> +
>  io_context_t ctx;
>  EventNotifier e;
>  
> @@ -227,15 +229,14 @@ static void ioq_submit(LinuxAioState *s)
>  
>  void laio_io_plug(BlockDriverState *bs, LinuxAioState *s)
>  {
> -assert(!s->io_q.plugged);
> -s->io_q.plugged = 1;
> +s->io_q.plugged++;
>  }
>  
>  void laio_io_unplug(BlockDriverState *bs, LinuxAioState *s)
>  {
>  assert(s->io_q.plugged);
> -s->io_q.plugged = 0;
> -if (!s->io_q.blocked && !QSIMPLEQ_EMPTY(>io_q.pending)) {
> +if (--s->io_q.plugged == 0 &&
> +!s->io_q.blocked && !QSIMPLEQ_EMPTY(>io_q.pending)) {
>  ioq_submit(s);
>  }
>  }
> @@ -325,6 +326,7 @@ void laio_detach_aio_context(LinuxAioState *s, AioContext 
> *old_context)
>  
>  void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context)
>  {
> +s->aio_context = new_context;
>  s->completion_bh = aio_bh_new(new_context, qemu_laio_completion_bh, s);
>  aio_set_event_notifier(new_context, >e, false,
> qemu_laio_completion_cb);
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index bef7a67..aedf575 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -32,7 +32,7 @@
>  #include "trace.h"
>  #include "block/thread-pool.h"
>  #include "qemu/iov.h"
> -#include "raw-aio.h"
> +#include "block/raw-aio.h"
>  #include "qapi/util.h"
>  #include "qapi/qmp/qstring.h"
>  
> @@ -137,10 +137,6 @@ typedef struct BDRVRawState {
>  int open_flags;
>  size_t buf_align;
>  
> -#ifdef CONFIG_LINUX_AIO
> -int use_aio;
> -LinuxAioState *aio_ctx;
> -#endif
>  #ifdef CONFIG_XFS
>  bool is_xfs:1;
>  #endif
> @@ -154,9 +150,6 @@ typedef struct BDRVRawState {
>  typedef struct BDRVRawReopenState {
>  int fd;
>  int open_flags;
> -#ifdef CONFIG_LINUX_AIO
> -int use_aio;
> -#endif
>  } BDRVRawReopenState;
>  
>  static int fd_open(BlockDriverState *bs);
> @@ -374,58 +367,15 @@ static void raw_parse_flags(int bdrv_flags, int 
> *open_flags)
>  }
>  }
>  
> -static void raw_detach_aio_context(BlockDriverState *bs)
> -{
>  #ifdef CONFIG_LINUX_AIO
> -BDRVRawState *s = bs->opaque;
> -
> -if (s->use_aio) {
> -laio_detach_aio_context(s->aio_ctx, bdrv_get_aio_context(bs));
> -}
> -#endif
> -}

[Qemu-block] [PULL v2 33/34] vvfat: Fix qcow write target driver specification

[Kevin Wolf]

From: Max Reitz 

First, bdrv_open_child() expects all options for the child to be
prefixed by the child's name (and a separating dot). Second,
bdrv_open_child() does not take ownership of the QDict passed to it but
only extracts all options for the child, so if a QDict is created for
the sole purpose of passing it to bdrv_open_child(), it needs to be
freed afterwards.

This patch makes vvfat adhere to both of these rules.

Signed-off-by: Max Reitz 
Message-id: 20160711135452.11304-1-mre...@redhat.com
Reviewed-by: Eric Blake 
Reviewed-by: Kevin Wolf 
Signed-off-by: Max Reitz 
---
 block/vvfat.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index c3f24c6..ba2620f 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3018,9 +3018,10 @@ static int enable_write_target(BlockDriverState *bs, 
Error **errp)
 }
 
 options = qdict_new();
-qdict_put(options, "driver", qstring_from_str("qcow"));
+qdict_put(options, "write-target.driver", qstring_from_str("qcow"));
 s->qcow = bdrv_open_child(s->qcow_filename, options, "write-target", bs,
   _vvfat_qcow, false, errp);
+QDECREF(options);
 if (!s->qcow) {
 ret = -EINVAL;
 goto err;
-- 
1.8.3.1

[Qemu-block] [PULL v2 30/34] qemu-iotests: Test naming of throttling groups

[Kevin Wolf]

From: Alberto Garcia 

Throttling groups are named using the 'group' parameter of the
block_set_io_throttle command and the throttling.group command-line
option. If that parameter is unspecified the groups get the name of
the block device.

This patch adds a new test to check the naming of throttling groups.

Signed-off-by: Alberto Garcia 
Message-id: 
d87d02823a6b91609509d8bb18e2f5dbd9a6102c.1467986342.git.be...@igalia.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/093 | 98 ++
 tests/qemu-iotests/093.out |  4 +-
 2 files changed, 100 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
index ce8e13c..ffcb271 100755
--- a/tests/qemu-iotests/093
+++ b/tests/qemu-iotests/093
@@ -184,5 +184,103 @@ class ThrottleTestCase(iotests.QMPTestCase):
 class ThrottleTestCoroutine(ThrottleTestCase):
 test_img = "null-co://"
 
+class ThrottleTestGroupNames(iotests.QMPTestCase):
+test_img = "null-aio://"
+max_drives = 3
+
+def setUp(self):
+self.vm = iotests.VM()
+for i in range(0, self.max_drives):
+self.vm.add_drive(self.test_img, "throttling.iops-total=100")
+self.vm.launch()
+
+def tearDown(self):
+self.vm.shutdown()
+
+def set_io_throttle(self, device, params):
+params["device"] = device
+result = self.vm.qmp("block_set_io_throttle", conv_keys=False, 
**params)
+self.assert_qmp(result, 'return', {})
+
+def verify_name(self, device, name):
+result = self.vm.qmp("query-block")
+for r in result["return"]:
+if r["device"] == device:
+info = r["inserted"]
+if name:
+self.assertEqual(info["group"], name)
+else:
+self.assertFalse(info.has_key('group'))
+return
+
+raise Exception("No group information found for '%s'" % device)
+
+def test_group_naming(self):
+params = {"bps": 0,
+  "bps_rd": 0,
+  "bps_wr": 0,
+  "iops": 0,
+  "iops_rd": 0,
+  "iops_wr": 0}
+
+# Check the drives added using the command line.
+# The default throttling group name is the device name.
+for i in range(self.max_drives):
+devname = "drive%d" % i
+self.verify_name(devname, devname)
+
+# Clear throttling settings => the group name is gone.
+for i in range(self.max_drives):
+devname = "drive%d" % i
+self.set_io_throttle(devname, params)
+self.verify_name(devname, None)
+
+# Set throttling settings using block_set_io_throttle and
+# check the default group names.
+params["iops"] = 10
+for i in range(self.max_drives):
+devname = "drive%d" % i
+self.set_io_throttle(devname, params)
+self.verify_name(devname, devname)
+
+# Set a custom group name for each device
+for i in range(3):
+devname = "drive%d" % i
+groupname = "group%d" % i
+params['group'] = groupname
+self.set_io_throttle(devname, params)
+self.verify_name(devname, groupname)
+
+# Put drive0 in group1 and check that all other devices remain
+# unchanged
+params['group'] = 'group1'
+self.set_io_throttle('drive0', params)
+self.verify_name('drive0', 'group1')
+for i in range(1, self.max_drives):
+devname = "drive%d" % i
+groupname = "group%d" % i
+self.verify_name(devname, groupname)
+
+# Put drive0 in group2 and check that all other devices remain
+# unchanged
+params['group'] = 'group2'
+self.set_io_throttle('drive0', params)
+self.verify_name('drive0', 'group2')
+for i in range(1, self.max_drives):
+devname = "drive%d" % i
+groupname = "group%d" % i
+self.verify_name(devname, groupname)
+
+# Clear throttling settings from drive0 check that all other
+# devices remain unchanged
+params["iops"] = 0
+self.set_io_throttle('drive0', params)
+self.verify_name('drive0', None)
+for i in range(1, self.max_drives):
+devname = "drive%d" % i
+groupname = "group%d" % i
+self.verify_name(devname, groupname)
+
+
 if __name__ == '__main__':
 iotests.main(supported_fmts=["raw"])
diff --git a/tests/qemu-iotests/093.out b/tests/qemu-iotests/093.out
index 89968f3..914e373 100644
--- a/tests/qemu-iotests/093.out
+++ b/tests/qemu-iotests/093.out
@@ -1,5 +1,5 @@
-
+.
 --
-Ran 4 tests
+Ran 5 tests
 
 OK
-- 
1.8.3.1

[Qemu-block] [PULL v2 27/34] Improve block job rate limiting for small bandwidth values

[Kevin Wolf]

From: Sascha Silbe 

ratelimit_calculate_delay() previously reset the accounting every time
slice, no matter how much data had been processed before. This had (at
least) two consequences:

1. The minimum speed is rather large, e.g. 5 MiB/s for commit and stream.

   Not sure if there are real-world use cases where this would be a
   problem. Mirroring and backup over a slow link (e.g. DSL) would
   come to mind, though.

2. Tests for block job operations (e.g. cancel) were rather racy

   All block jobs currently use a time slice of 100ms. That's a
   reasonable value to get smooth output during regular
   operation. However this also meant that the state of block jobs
   changed every 100ms, no matter how low the configured limit was. On
   busy hosts, qemu often transferred additional chunks until the test
   case had a chance to cancel the job.

Fix the block job rate limit code to delay for more than one time
slice to address the above issues. To make it easier to handle
oversized chunks we switch the semantics from returning a delay
_before_ the current request to a delay _after_ the current
request. If necessary, this delay consists of multiple time slice
units.

Since the mirror job sends multiple chunks in one go even if the rate
limit was exceeded in between, we need to keep track of the start of
the current time slice so we can correctly re-compute the delay for
the updated amount of data.

The minimum bandwidth now is 1 data unit per time slice. The block
jobs are currently passing the amount of data transferred in sectors
and using 100ms time slices, so this translates to 5120
bytes/second. With chunk sizes usually being O(512KiB), tests have
plenty of time (O(100s)) to operate on block jobs. The chance of a
race condition now is fairly remote, except possibly on insanely
loaded systems.

Signed-off-by: Sascha Silbe 
Message-id: 1467127721-9564-2-git-send-email-si...@linux.vnet.ibm.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 block/commit.c   | 13 +
 block/mirror.c   |  4 +++-
 block/stream.c   | 12 
 include/qemu/ratelimit.h | 43 ++-
 4 files changed, 46 insertions(+), 26 deletions(-)

diff --git a/block/commit.c b/block/commit.c
index 5d11eb6..553e18d 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -113,6 +113,7 @@ static void coroutine_fn commit_run(void *opaque)
 CommitBlockJob *s = opaque;
 CommitCompleteData *data;
 int64_t sector_num, end;
+uint64_t delay_ns = 0;
 int ret = 0;
 int n = 0;
 void *buf = NULL;
@@ -142,10 +143,8 @@ static void coroutine_fn commit_run(void *opaque)
 buf = blk_blockalign(s->top, COMMIT_BUFFER_SIZE);
 
 for (sector_num = 0; sector_num < end; sector_num += n) {
-uint64_t delay_ns = 0;
 bool copy;
 
-wait:
 /* Note that even when no rate limit is applied we need to yield
  * with no pending I/O here so that bdrv_drain_all() returns.
  */
@@ -161,12 +160,6 @@ wait:
 copy = (ret == 1);
 trace_commit_one_iteration(s, sector_num, n, ret);
 if (copy) {
-if (s->common.speed) {
-delay_ns = ratelimit_calculate_delay(>limit, n);
-if (delay_ns > 0) {
-goto wait;
-}
-}
 ret = commit_populate(s->top, s->base, sector_num, n, buf);
 bytes_written += n * BDRV_SECTOR_SIZE;
 }
@@ -182,6 +175,10 @@ wait:
 }
 /* Publish progress */
 s->common.offset += n * BDRV_SECTOR_SIZE;
+
+if (copy && s->common.speed) {
+delay_ns = ratelimit_calculate_delay(>limit, n);
+}
 }
 
 ret = 0;
diff --git a/block/mirror.c b/block/mirror.c
index 71e5ad4..b1e633e 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -422,7 +422,9 @@ static uint64_t coroutine_fn 
mirror_iteration(MirrorBlockJob *s)
 assert(io_sectors);
 sector_num += io_sectors;
 nb_chunks -= DIV_ROUND_UP(io_sectors, sectors_per_chunk);
-delay_ns += ratelimit_calculate_delay(>limit, io_sectors);
+if (s->common.speed) {
+delay_ns = ratelimit_calculate_delay(>limit, io_sectors);
+}
 }
 return delay_ns;
 }
diff --git a/block/stream.c b/block/stream.c
index 2e7c654..3187481 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -95,6 +95,7 @@ static void coroutine_fn stream_run(void *opaque)
 BlockDriverState *base = s->base;
 int64_t sector_num = 0;
 int64_t end = -1;
+uint64_t delay_ns = 0;
 int error = 0;
 int ret = 0;
 int n = 0;
@@ -123,10 +124,8 @@ static void coroutine_fn stream_run(void *opaque)
 }
 
 for (sector_num = 0; sector_num < end; sector_num += n) {
-uint64_t delay_ns = 0;
 bool copy;
 
-wait:
 /* Note that even when no rate limit is 

[Qemu-block] [PULL v2 31/34] hmp: use snapshot name to determine whether a snapshot is 'fully available'

[Kevin Wolf]

From: Lin Ma 

Currently qemu uses snapshot id to determine whether a snapshot is fully
available, It causes incorrect output in some scenario.

For instance:
(qemu) info block
drive_image1 (#block113): /opt/vms/SLES12-SP1-JeOS-x86_64-GM/disk0.qcow2
(qcow2)
Cache mode:   writeback

drive_image2 (#block349): /opt/vms/SLES12-SP1-JeOS-x86_64-GM/disk1.qcow2
(qcow2)
Cache mode:   writeback
(qemu)
(qemu) info snapshots
There is no snapshot available.
(qemu)
(qemu) snapshot_blkdev_internal drive_image1 snap1
(qemu)
(qemu) info snapshots
There is no suitable snapshot available
(qemu)
(qemu) savevm checkpoint-1
(qemu)
(qemu) info snapshots
IDTAG VM SIZEDATE   VM CLOCK
1 snap1 0 2016-05-22 16:57:31   00:01:30.567
(qemu)

$ qemu-img snapshot -l disk0.qcow2
Snapshot list:
IDTAG VM SIZEDATE   VM CLOCK
1 snap1 0 2016-05-22 16:57:31   00:01:30.567
2 checkpoint-1   165M 2016-05-22 16:58:07   00:02:06.813

$ qemu-img snapshot -l disk1.qcow2
Snapshot list:
IDTAG VM SIZEDATE   VM CLOCK
1 checkpoint-1  0 2016-05-22 16:58:07   00:02:06.813

The patch uses snapshot name instead of snapshot id to determine whether a
snapshot is fully available and uses '--' instead of snapshot id in output
because the snapshot id is not guaranteed to be the same on all images.
For instance:
(qemu) info snapshots
List of snapshots present on all disks:
 IDTAG VM SIZEDATE   VM CLOCK
 --checkpoint-1   165M 2016-05-22 16:58:07   00:02:06.813

Signed-off-by: Lin Ma 
Reviewed-by: Max Reitz 
Message-id: 1467869164-26688-2-git-send-email-...@suse.com
Signed-off-by: Max Reitz 
---
 migration/savevm.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 38b85ee..a8f22da 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2230,7 +2230,7 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
 available_snapshots = g_new0(int, nb_sns);
 total = 0;
 for (i = 0; i < nb_sns; i++) {
-if (bdrv_all_find_snapshot(sn_tab[i].id_str, ) == 0) {
+if (bdrv_all_find_snapshot(sn_tab[i].name, ) == 0) {
 available_snapshots[total] = i;
 total++;
 }
@@ -2241,6 +2241,10 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
 monitor_printf(mon, "\n");
 for (i = 0; i < total; i++) {
 sn = _tab[available_snapshots[i]];
+/* The ID is not guaranteed to be the same on all images, so
+ * overwrite it.
+ */
+pstrcpy(sn->id_str, sizeof(sn->id_str), "--");
 bdrv_snapshot_dump((fprintf_function)monitor_printf, mon, sn);
 monitor_printf(mon, "\n");
 }
-- 
1.8.3.1

[Qemu-block] [PULL v2 25/34] qemu-io: Use correct range limitations

[Kevin Wolf]

From: Max Reitz 

create_iovec() has a comment lamenting the lack of SIZE_T_MAX. Since
there actually is a SIZE_MAX, use it.

Two places use INT_MAX for checking the upper bound of a sector count
that is used as an argument for a blk_*() function (blk_discard() and
blk_write_compressed(), respectively). BDRV_REQUEST_MAX_SECTORS should
be used instead.

And finally, do_co_pwrite_zeroes() used to similarly check that the
sector count does not exceed INT_MAX. However, this function is now
backed by blk_co_pwrite_zeroes() which takes bytes as an argument
instead of sectors. Therefore, it should be the byte count that does not
exceed INT_MAX, not the sector count.

Signed-off-by: Max Reitz 
---
 qemu-io-cmds.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 40fe2eb..6e29edb 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -389,9 +389,9 @@ create_iovec(BlockBackend *blk, QEMUIOVector *qiov, char 
**argv, int nr_iov,
 goto fail;
 }
 
-/* should be SIZE_T_MAX, but that doesn't exist */
-if (len > INT_MAX) {
-printf("Argument '%s' exceeds maximum size %d\n", arg, INT_MAX);
+if (len > SIZE_MAX) {
+printf("Argument '%s' exceeds maximum size %llu\n", arg,
+   (unsigned long long)SIZE_MAX);
 goto fail;
 }
 
@@ -479,7 +479,7 @@ static int do_co_pwrite_zeroes(BlockBackend *blk, int64_t 
offset,
 .done   = false,
 };
 
-if (count >> BDRV_SECTOR_BITS > INT_MAX) {
+if (count > INT_MAX) {
 return -ERANGE;
 }
 
@@ -500,7 +500,7 @@ static int do_write_compressed(BlockBackend *blk, char 
*buf, int64_t offset,
 {
 int ret;
 
-if (count >> 9 > INT_MAX) {
+if (count >> 9 > BDRV_REQUEST_MAX_SECTORS) {
 return -ERANGE;
 }
 
@@ -1688,9 +1688,9 @@ static int discard_f(BlockBackend *blk, int argc, char 
**argv)
 if (count < 0) {
 print_cvtnum_err(count, argv[optind]);
 return 0;
-} else if (count >> BDRV_SECTOR_BITS > INT_MAX) {
+} else if (count >> BDRV_SECTOR_BITS > BDRV_REQUEST_MAX_SECTORS) {
 printf("length cannot exceed %"PRIu64", given %s\n",
-   (uint64_t)INT_MAX << BDRV_SECTOR_BITS,
+   (uint64_t)BDRV_REQUEST_MAX_SECTORS << BDRV_SECTOR_BITS,
argv[optind]);
 return 0;
 }
-- 
1.8.3.1

[Qemu-block] [PULL v2 23/34] qemu-img: Use strerror() for generic resize error

[Kevin Wolf]

From: Max Reitz 

Emitting the plain error number is not very helpful. Use strerror()
instead.

Signed-off-by: Max Reitz 
Message-id: 20160615153630.2116-2-mre...@redhat.com
Reviewed-by: Eric Blake 
Signed-off-by: Max Reitz 
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 969edce..2e40e1f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3283,7 +3283,7 @@ static int img_resize(int argc, char **argv)
 error_report("Image is read-only");
 break;
 default:
-error_report("Error resizing image (%d)", -ret);
+error_report("Error resizing image: %s", strerror(-ret));
 break;
 }
 out:
-- 
1.8.3.1

[Qemu-block] [PULL v2 34/34] iotests: Make 157 actually format-agnostic

[Kevin Wolf]

From: Max Reitz 

iotest 157 pretends not to care about the image format used, but in fact
it does due to the format name not being filtered in its output. This
patch adds filtering and changes the reference output accordingly.

Signed-off-by: Max Reitz 
Message-id: 20160711132246.3152-1-mre...@redhat.com
Reviewed-by: John Snow 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/157 |  3 ++-
 tests/qemu-iotests/157.out | 16 
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/tests/qemu-iotests/157 b/tests/qemu-iotests/157
index 2699168..8d939cb 100755
--- a/tests/qemu-iotests/157
+++ b/tests/qemu-iotests/157
@@ -57,7 +57,8 @@ function do_run_qemu()
 
 function run_qemu()
 {
-do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | 
_filter_generated_node_ids
+do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_imgfmt \
+  | _filter_qemu | _filter_generated_node_ids
 }
 
 
diff --git a/tests/qemu-iotests/157.out b/tests/qemu-iotests/157.out
index 5aa9c0e..77a9c03 100644
--- a/tests/qemu-iotests/157.out
+++ b/tests/qemu-iotests/157.out
@@ -3,20 +3,20 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 
 === Setting WCE with qdev and with manually created BB ===
 
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback 
-device virtio-blk,drive=none0
+Testing: -drive if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writeback 
-device virtio-blk,drive=none0
 Cache mode:   writeback
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback 
-device virtio-blk,drive=none0,write-cache=auto
+Testing: -drive if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writeback 
-device virtio-blk,drive=none0,write-cache=auto
 Cache mode:   writeback
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback 
-device virtio-blk,drive=none0,write-cache=on
+Testing: -drive if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writeback 
-device virtio-blk,drive=none0,write-cache=on
 Cache mode:   writeback
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback 
-device virtio-blk,drive=none0,write-cache=off
+Testing: -drive if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writeback 
-device virtio-blk,drive=none0,write-cache=off
 Cache mode:   writethrough
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough 
-device virtio-blk,drive=none0
+Testing: -drive 
if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writethrough -device 
virtio-blk,drive=none0
 Cache mode:   writethrough
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough 
-device virtio-blk,drive=none0,write-cache=auto
+Testing: -drive 
if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writethrough -device 
virtio-blk,drive=none0,write-cache=auto
 Cache mode:   writethrough
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough 
-device virtio-blk,drive=none0,write-cache=on
+Testing: -drive 
if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writethrough -device 
virtio-blk,drive=none0,write-cache=on
 Cache mode:   writeback
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough 
-device virtio-blk,drive=none0,write-cache=off
+Testing: -drive 
if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writethrough -device 
virtio-blk,drive=none0,write-cache=off
 Cache mode:   writethrough
 *** done
-- 
1.8.3.1

[Qemu-block] [PULL v2 24/34] qcow2: Avoid making the L1 table too big

[Kevin Wolf]

From: Max Reitz 

We refuse to open images whose L1 table we deem "too big". Consequently,
we should not produce such images ourselves.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Max Reitz 
Message-id: 20160615153630.2116-3-mre...@redhat.com
Reviewed-by: Eric Blake 
[mreitz: Added QEMU_BUILD_BUG_ON()]
Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 6b92ce9..00c16dc 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -65,7 +65,8 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t 
min_size,
 }
 }
 
-if (new_l1_size > INT_MAX / sizeof(uint64_t)) {
+QEMU_BUILD_BUG_ON(QCOW_MAX_L1_SIZE > INT_MAX);
+if (new_l1_size > QCOW_MAX_L1_SIZE / sizeof(uint64_t)) {
 return -EFBIG;
 }
 
-- 
1.8.3.1

[Qemu-block] [PULL v2 26/34] qcow2: Fix qcow2_get_cluster_offset()

[Kevin Wolf]

From: Max Reitz 

Recently, qcow2_get_cluster_offset() has been changed to work with bytes
instead of sectors. This invalidated some assertions and introduced a
possible integer multiplication overflow.

This could be reproduced using e.g.

$ qemu-img create -f qcow2 -o cluster_size=1M blub.qcow2 8G
Formatting 'foo.qcow2', fmt=qcow2 size=8589934592 encryption=off
cluster_size=1048576 lazy_refcounts=off refcount_bits=16
$ qemu-io -c map blub.qcow2
qemu-io: qemu/block/qcow2-cluster.c:504: qcow2_get_cluster_offset:
Assertion `bytes_needed <= INT_MAX' failed.
[1]20775 abort (core dumped)  qemu-io -c map foo.qcow2

This patch removes the now wrong assertion, adding comments and more
assertions to prove its correctness (and fixing the overflow which would
become apparent with the original assertion removed).

Signed-off-by: Max Reitz 
Message-id: 20160620142623.24471-3-mre...@redhat.com
Reviewed-by: Eric Blake 
Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 00c16dc..f941835 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -483,8 +483,8 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t 
offset,
 unsigned int l2_index;
 uint64_t l1_index, l2_offset, *l2_table;
 int l1_bits, c;
-unsigned int offset_in_cluster, nb_clusters;
-uint64_t bytes_available, bytes_needed;
+unsigned int offset_in_cluster;
+uint64_t bytes_available, bytes_needed, nb_clusters;
 int ret;
 
 offset_in_cluster = offset_into_cluster(s, offset);
@@ -500,7 +500,6 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t 
offset,
 if (bytes_needed > bytes_available) {
 bytes_needed = bytes_available;
 }
-assert(bytes_needed <= INT_MAX);
 
 *cluster_offset = 0;
 
@@ -537,8 +536,11 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
uint64_t offset,
 l2_index = (offset >> s->cluster_bits) & (s->l2_size - 1);
 *cluster_offset = be64_to_cpu(l2_table[l2_index]);
 
-/* nb_needed <= INT_MAX, thus nb_clusters <= INT_MAX, too */
 nb_clusters = size_to_clusters(s, bytes_needed);
+/* bytes_needed <= *bytes + offset_in_cluster, both of which are unsigned
+ * integers; the minimum cluster size is 512, so this assertion is always
+ * true */
+assert(nb_clusters <= INT_MAX);
 
 ret = qcow2_get_cluster_type(*cluster_offset);
 switch (ret) {
@@ -585,13 +587,17 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
uint64_t offset,
 
 qcow2_cache_put(bs, s->l2_table_cache, (void**) _table);
 
-bytes_available = (c * s->cluster_size);
+bytes_available = (int64_t)c * s->cluster_size;
 
 out:
 if (bytes_available > bytes_needed) {
 bytes_available = bytes_needed;
 }
 
+/* bytes_available <= bytes_needed <= *bytes + offset_in_cluster;
+ * subtracting offset_in_cluster will therefore definitely yield something
+ * not exceeding UINT_MAX */
+assert(bytes_available - offset_in_cluster <= UINT_MAX);
 *bytes = bytes_available - offset_in_cluster;
 
 return ret;
-- 
1.8.3.1

[Qemu-block] [PULL v2 20/34] block/qdev: Allow configuring rerror/werror with qdev properties

[Kevin Wolf]

The rerror/werror policies are implemented in the devices, so that's
where they should be configured. In comparison to the old options in
-drive, the qdev properties are only added to those devices that
actually support them.

If the option isn't given (or "auto" is specified), the setting of the
BlockBackend is used for compatibility with the old options. For block
jobs, "auto" is the same as "enospc".

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 block/block-backend.c|  1 +
 blockjob.c   |  1 +
 hw/block/block.c | 12 
 hw/block/virtio-blk.c|  1 +
 hw/core/qdev-properties.c| 13 +
 hw/ide/qdev.c|  1 +
 hw/scsi/scsi-disk.c  |  1 +
 include/hw/block/block.h |  8 
 include/hw/qdev-properties.h |  4 
 qapi/block-core.json |  4 +++-
 10 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 0d7b801..f9cea1b 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1173,6 +1173,7 @@ BlockErrorAction blk_get_error_action(BlockBackend *blk, 
bool is_read,
 return BLOCK_ERROR_ACTION_REPORT;
 case BLOCKDEV_ON_ERROR_IGNORE:
 return BLOCK_ERROR_ACTION_IGNORE;
+case BLOCKDEV_ON_ERROR_AUTO:
 default:
 abort();
 }
diff --git a/blockjob.c b/blockjob.c
index 6816b78..a5ba3be 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -553,6 +553,7 @@ BlockErrorAction block_job_error_action(BlockJob *job, 
BlockdevOnError on_err,
 
 switch (on_err) {
 case BLOCKDEV_ON_ERROR_ENOSPC:
+case BLOCKDEV_ON_ERROR_AUTO:
 action = (error == ENOSPC) ?
  BLOCK_ERROR_ACTION_STOP : BLOCK_ERROR_ACTION_REPORT;
 break;
diff --git a/hw/block/block.c b/hw/block/block.c
index 396b0d5..8dc9d84 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -54,6 +54,7 @@ void blkconf_blocksizes(BlockConf *conf)
 void blkconf_apply_backend_options(BlockConf *conf)
 {
 BlockBackend *blk = conf->blk;
+BlockdevOnError rerror, werror;
 bool wce;
 
 switch (conf->wce) {
@@ -64,7 +65,18 @@ void blkconf_apply_backend_options(BlockConf *conf)
 abort();
 }
 
+rerror = conf->rerror;
+if (rerror == BLOCKDEV_ON_ERROR_AUTO) {
+rerror = blk_get_on_error(blk, true);
+}
+
+werror = conf->werror;
+if (werror == BLOCKDEV_ON_ERROR_AUTO) {
+werror = blk_get_on_error(blk, false);
+}
+
 blk_set_enable_write_cache(blk, wce);
+blk_set_on_error(blk, rerror, werror);
 }
 
 void blkconf_geometry(BlockConf *conf, int *ptrans,
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index ecd8ea3..357ff90 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -960,6 +960,7 @@ static void virtio_blk_instance_init(Object *obj)
 
 static Property virtio_blk_properties[] = {
 DEFINE_BLOCK_PROPERTIES(VirtIOBlock, conf.conf),
+DEFINE_BLOCK_ERROR_PROPERTIES(VirtIOBlock, conf.conf),
 DEFINE_BLOCK_CHS_PROPERTIES(VirtIOBlock, conf.conf),
 DEFINE_PROP_STRING("serial", VirtIOBlock, conf.serial),
 DEFINE_PROP_BIT("config-wce", VirtIOBlock, conf.config_wce, 0, true),
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 3c20c8e..14e544a 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -539,6 +539,19 @@ PropertyInfo qdev_prop_losttickpolicy = {
 .set   = set_enum,
 };
 
+/* --- Block device error handling policy --- */
+
+QEMU_BUILD_BUG_ON(sizeof(BlockdevOnError) != sizeof(int));
+
+PropertyInfo qdev_prop_blockdev_on_error = {
+.name = "BlockdevOnError",
+.description = "Error handling policy, "
+   "report/ignore/enospc/stop/auto",
+.enum_table = BlockdevOnError_lookup,
+.get = get_enum,
+.set = set_enum,
+};
+
 /* --- BIOS CHS translation */
 
 QEMU_BUILD_BUG_ON(sizeof(BiosAtaTranslation) != sizeof(int));
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 33619f4..67c76bf 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -264,6 +264,7 @@ static int ide_drive_initfn(IDEDevice *dev)
 
 #define DEFINE_IDE_DEV_PROPERTIES() \
 DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),\
+DEFINE_BLOCK_ERROR_PROPERTIES(IDEDrive, dev.conf),  \
 DEFINE_PROP_STRING("ver",  IDEDrive, dev.version),  \
 DEFINE_PROP_UINT64("wwn",  IDEDrive, dev.wwn, 0),\
 DEFINE_PROP_STRING("serial",  IDEDrive, dev.serial),\
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 8c26a4e..8dbfc10 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2849,6 +2849,7 @@ static const TypeInfo scsi_disk_base_info = {
 
 #define DEFINE_SCSI_DISK_PROPERTIES()\
 DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf),   \
+DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf), \
 DEFINE_PROP_STRING("ver", SCSIDiskState, version),  

[Qemu-block] [PULL v2 16/34] coroutine: move entry argument to qemu_coroutine_create

[Kevin Wolf]

From: Paolo Bonzini 

In practice the entry argument is always known at creation time, and
it is confusing that sometimes qemu_coroutine_enter is used with a
non-NULL argument to re-enter a coroutine (this happens in
block/sheepdog.c and tests/test-coroutine.c).  So pass the opaque value
at creation time, for consistency with e.g. aio_bh_new.

Mostly done with the following semantic patch:

@ entry1 @
expression entry, arg, co;
@@
- co = qemu_coroutine_create(entry);
+ co = qemu_coroutine_create(entry, arg);
  ...
- qemu_coroutine_enter(co, arg);
+ qemu_coroutine_enter(co);

@ entry2 @
expression entry, arg;
identifier co;
@@
- Coroutine *co = qemu_coroutine_create(entry);
+ Coroutine *co = qemu_coroutine_create(entry, arg);
  ...
- qemu_coroutine_enter(co, arg);
+ qemu_coroutine_enter(co);

@ entry3 @
expression entry, arg;
@@
- qemu_coroutine_enter(qemu_coroutine_create(entry), arg);
+ qemu_coroutine_enter(qemu_coroutine_create(entry, arg));

@ reentry @
expression co;
@@
- qemu_coroutine_enter(co, NULL);
+ qemu_coroutine_enter(co);

except for the aforementioned few places where the semantic patch
stumbled (as expected) and for test_co_queue, which would otherwise
produce an uninitialized variable warning.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Fam Zheng 
Signed-off-by: Kevin Wolf 
---
 block.c |  4 +--
 block/backup.c  |  4 +--
 block/blkdebug.c|  4 +--
 block/blkreplay.c   |  2 +-
 block/block-backend.c   |  8 +++---
 block/commit.c  |  4 +--
 block/gluster.c |  2 +-
 block/io.c  | 45 
 block/iscsi.c   |  4 +--
 block/linux-aio.c   |  2 +-
 block/mirror.c  |  6 ++---
 block/nbd-client.c  |  6 ++---
 block/nfs.c |  2 +-
 block/qcow.c|  4 +--
 block/qcow2.c   |  4 +--
 block/qed.c |  4 +--
 block/sheepdog.c| 14 +-
 block/ssh.c |  2 +-
 block/stream.c  |  4 +--
 block/vmdk.c|  4 +--
 blockjob.c  |  2 +-
 hw/9pfs/9p.c|  4 +--
 hw/9pfs/coth.c  |  4 +--
 include/qemu/coroutine.h|  8 +++---
 include/qemu/main-loop.h|  4 +--
 io/channel.c|  2 +-
 migration/migration.c   |  4 +--
 nbd/server.c| 12 -
 qemu-io-cmds.c  |  4 +--
 tests/test-blockjob-txn.c   |  4 +--
 tests/test-coroutine.c  | 62 ++---
 tests/test-thread-pool.c|  4 +--
 thread-pool.c   |  2 +-
 util/qemu-coroutine-io.c|  2 +-
 util/qemu-coroutine-lock.c  |  4 +--
 util/qemu-coroutine-sleep.c |  2 +-
 util/qemu-coroutine.c   |  8 +++---
 37 files changed, 130 insertions(+), 131 deletions(-)

diff --git a/block.c b/block.c
index 823ff1d..67894e0 100644
--- a/block.c
+++ b/block.c
@@ -329,8 +329,8 @@ int bdrv_create(BlockDriver *drv, const char* filename,
 /* Fast-path if already in coroutine context */
 bdrv_create_co_entry();
 } else {
-co = qemu_coroutine_create(bdrv_create_co_entry);
-qemu_coroutine_enter(co, );
+co = qemu_coroutine_create(bdrv_create_co_entry, );
+qemu_coroutine_enter(co);
 while (cco.ret == NOT_DONE) {
 aio_poll(qemu_get_aio_context(), true);
 }
diff --git a/block/backup.c b/block/backup.c
index dce6614..2c05323 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -576,9 +576,9 @@ void backup_start(const char *job_id, BlockDriverState *bs,
 
 bdrv_op_block_all(target, job->common.blocker);
 job->common.len = len;
-job->common.co = qemu_coroutine_create(backup_run);
+job->common.co = qemu_coroutine_create(backup_run, job);
 block_job_txn_add_job(txn, >common);
-qemu_coroutine_enter(job->common.co, job);
+qemu_coroutine_enter(job->common.co);
 return;
 
  error:
diff --git a/block/blkdebug.c b/block/blkdebug.c
index bbaa33f..fb29283 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -621,7 +621,7 @@ static int blkdebug_debug_resume(BlockDriverState *bs, 
const char *tag)
 
 QLIST_FOREACH_SAFE(r, >suspended_reqs, next, next) {
 if (!strcmp(r->tag, tag)) {
-qemu_coroutine_enter(r->co, NULL);
+qemu_coroutine_enter(r->co);
 return 0;
 }
 }
@@ -647,7 +647,7 @@ static int 
blkdebug_debug_remove_breakpoint(BlockDriverState *bs,
 }
 QLIST_FOREACH_SAFE(r, >suspended_reqs, next, r_next) {
 if (!strcmp(r->tag, tag)) {
-qemu_coroutine_enter(r->co, NULL);
+qemu_coroutine_enter(r->co);
 ret = 0;
 }
 }
diff --git a/block/blkreplay.c b/block/blkreplay.c
index 70650e4..3368c8c 100755
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -65,7 +65,7 @@ static int64_t 

[Qemu-block] [PULL v2 29/34] blockdev: Fix regression with the default naming of throttling groups

[Kevin Wolf]

From: Alberto Garcia 

When I/O limits are set for a block device, the name of the throttling
group is taken from the BlockBackend if the user doesn't specify one.

Commit efaa7c4eeb7490c6f37f3 moved the naming of the BlockBackend in
blockdev_init() to the end of the function, after I/O limits are set.
The consequence is that the throttling group gets an empty name.

Signed-off-by: Alberto Garcia 
Reported-by: Stefan Hajnoczi 
Cc: Max Reitz 
Cc: qemu-sta...@nongnu.org
Message-id: 
af5cd58bd2c4b9f6c57f260d9cfe586b9fb7d34d.1467986342.git.be...@igalia.com
[mreitz: Use existing "id" variable instead of new "blk_id"]
Signed-off-by: Max Reitz 
---
 blockdev.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index aa23dc2..384ad3b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -512,6 +512,8 @@ static BlockBackend *blockdev_init(const char *file, QDict 
*bs_opts,
 
 writethrough = !qemu_opt_get_bool(opts, BDRV_OPT_CACHE_WB, true);
 
+id = qemu_opts_id(opts);
+
 qdict_extract_subqdict(bs_opts, _dict, "stats-intervals.");
 qdict_array_split(interval_dict, _list);
 
@@ -616,7 +618,7 @@ static BlockBackend *blockdev_init(const char *file, QDict 
*bs_opts,
 /* disk I/O throttling */
 if (throttle_enabled()) {
 if (!throttling_group) {
-throttling_group = blk_name(blk);
+throttling_group = id;
 }
 blk_io_limits_enable(blk, throttling_group);
 blk_set_io_limits(blk, );
@@ -625,7 +627,7 @@ static BlockBackend *blockdev_init(const char *file, QDict 
*bs_opts,
 blk_set_enable_write_cache(blk, !writethrough);
 blk_set_on_error(blk, on_read_error, on_write_error);
 
-if (!monitor_add_blk(blk, qemu_opts_id(opts), errp)) {
+if (!monitor_add_blk(blk, id, errp)) {
 blk_unref(blk);
 blk = NULL;
 goto err_no_bs_opts;
-- 
1.8.3.1

[Qemu-block] [PULL v2 32/34] hmp: show all of snapshot info on every block dev in output of 'info snapshots'

[Kevin Wolf]

From: Lin Ma 

Currently, the output of 'info snapshots' shows fully available snapshots.
It's opaque, hides some snapshot information to users. It's not convenient
if users want to know more about all of snapshot information on every block
device via monitor.

Follow Kevin's and Max's proposals, The patch makes the output more detailed:
(qemu) info snapshots
List of snapshots present on all disks:
 IDTAG VM SIZEDATE   VM CLOCK
 --checkpoint-1   165M 2016-05-22 16:58:07   00:02:06.813

List of partial (non-loadable) snapshots on 'drive_image1':
 IDTAG VM SIZEDATE   VM CLOCK
 1 snap1 0 2016-05-22 16:57:31   00:01:30.567

Signed-off-by: Lin Ma 
Message-id: 1467869164-26688-3-git-send-email-...@suse.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 migration/savevm.c | 97 ++
 1 file changed, 90 insertions(+), 7 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index a8f22da..33a2911 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2200,12 +2200,31 @@ void hmp_delvm(Monitor *mon, const QDict *qdict)
 void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
 {
 BlockDriverState *bs, *bs1;
+BdrvNextIterator it1;
 QEMUSnapshotInfo *sn_tab, *sn;
+bool no_snapshot = true;
 int nb_sns, i;
 int total;
-int *available_snapshots;
+int *global_snapshots;
 AioContext *aio_context;
 
+typedef struct SnapshotEntry {
+QEMUSnapshotInfo sn;
+QTAILQ_ENTRY(SnapshotEntry) next;
+} SnapshotEntry;
+
+typedef struct ImageEntry {
+const char *imagename;
+QTAILQ_ENTRY(ImageEntry) next;
+QTAILQ_HEAD(, SnapshotEntry) snapshots;
+} ImageEntry;
+
+QTAILQ_HEAD(, ImageEntry) image_list =
+QTAILQ_HEAD_INITIALIZER(image_list);
+
+ImageEntry *image_entry, *next_ie;
+SnapshotEntry *snapshot_entry;
+
 bs = bdrv_all_find_vmstate_bs();
 if (!bs) {
 monitor_printf(mon, "No available block device supports snapshots\n");
@@ -,25 +2241,65 @@ void hmp_info_snapshots(Monitor *mon, const QDict 
*qdict)
 return;
 }
 
-if (nb_sns == 0) {
+for (bs1 = bdrv_first(); bs1; bs1 = bdrv_next()) {
+int bs1_nb_sns = 0;
+ImageEntry *ie;
+SnapshotEntry *se;
+AioContext *ctx = bdrv_get_aio_context(bs1);
+
+aio_context_acquire(ctx);
+if (bdrv_can_snapshot(bs1)) {
+sn = NULL;
+bs1_nb_sns = bdrv_snapshot_list(bs1, );
+if (bs1_nb_sns > 0) {
+no_snapshot = false;
+ie = g_new0(ImageEntry, 1);
+ie->imagename = bdrv_get_device_name(bs1);
+QTAILQ_INIT(>snapshots);
+QTAILQ_INSERT_TAIL(_list, ie, next);
+for (i = 0; i < bs1_nb_sns; i++) {
+se = g_new0(SnapshotEntry, 1);
+se->sn = sn[i];
+QTAILQ_INSERT_TAIL(>snapshots, se, next);
+}
+}
+g_free(sn);
+}
+aio_context_release(ctx);
+}
+
+if (no_snapshot) {
 monitor_printf(mon, "There is no snapshot available.\n");
 return;
 }
 
-available_snapshots = g_new0(int, nb_sns);
+global_snapshots = g_new0(int, nb_sns);
 total = 0;
 for (i = 0; i < nb_sns; i++) {
+SnapshotEntry *next_sn;
 if (bdrv_all_find_snapshot(sn_tab[i].name, ) == 0) {
-available_snapshots[total] = i;
+global_snapshots[total] = i;
 total++;
+QTAILQ_FOREACH(image_entry, _list, next) {
+QTAILQ_FOREACH_SAFE(snapshot_entry, _entry->snapshots,
+next, next_sn) {
+if (!strcmp(sn_tab[i].name, snapshot_entry->sn.name)) {
+QTAILQ_REMOVE(_entry->snapshots, snapshot_entry,
+  next);
+g_free(snapshot_entry);
+}
+}
+}
 }
 }
 
+monitor_printf(mon, "List of snapshots present on all disks:\n");
+
 if (total > 0) {
 bdrv_snapshot_dump((fprintf_function)monitor_printf, mon, NULL);
 monitor_printf(mon, "\n");
 for (i = 0; i < total; i++) {
-sn = _tab[available_snapshots[i]];
+sn = _tab[global_snapshots[i]];
 /* The ID is not guaranteed to be the same on all images, so
  * overwrite it.
  */
@@ -2249,11 +2308,35 @@ void hmp_info_snapshots(Monitor *mon, const QDict 
*qdict)
 monitor_printf(mon, "\n");
 }
 } else {
-monitor_printf(mon, "There is no suitable snapshot available\n");
+monitor_printf(mon, "None\n");
 }
 

[Qemu-block] [PULL v2 28/34] vmdk: fix metadata write regression

[Kevin Wolf]

From: Reda Sallahi 

Commit "cdeaf1f vmdk: add bdrv_co_write_zeroes" causes a regression on
writes. It writes metadata after every write instead of doing it only once
for each cluster.

vmdk_pwritev() writes metadata whenever m_data is set as valid so this patch
sets m_data as valid only when we have a new cluster which hasn't been
allocated before or a zero grain.

Signed-off-by: Reda Sallahi 
Message-id: 20160707084249.29084-1-fullma...@gmail.com
Reviewed-by: Fam Zheng 
Signed-off-by: Max Reitz 
---
 block/vmdk.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index c9914f7..46d474e 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1202,13 +1202,6 @@ static int get_cluster_offset(BlockDriverState *bs,
 l2_index = ((offset >> 9) / extent->cluster_sectors) % extent->l2_size;
 cluster_sector = le32_to_cpu(l2_table[l2_index]);
 
-if (m_data) {
-m_data->valid = 1;
-m_data->l1_index = l1_index;
-m_data->l2_index = l2_index;
-m_data->l2_offset = l2_offset;
-m_data->l2_cache_entry = _table[l2_index];
-}
 if (extent->has_zero_grain && cluster_sector == VMDK_GTE_ZEROED) {
 zeroed = true;
 }
@@ -1231,6 +1224,13 @@ static int get_cluster_offset(BlockDriverState *bs,
 if (ret) {
 return ret;
 }
+if (m_data) {
+m_data->valid = 1;
+m_data->l1_index = l1_index;
+m_data->l2_index = l2_index;
+m_data->l2_offset = l2_offset;
+m_data->l2_cache_entry = _table[l2_index];
+}
 }
 *cluster_offset = cluster_sector << BDRV_SECTOR_BITS;
 return VMDK_OK;
-- 
1.8.3.1

[Qemu-block] [PULL v2 14/34] coroutine: use QSIMPLEQ instead of QTAILQ

[Kevin Wolf]

From: Paolo Bonzini 

CoQueue do not need to remove any element but the head of the list;
processing is always strictly FIFO.  Therefore, the simpler singly-linked
QSIMPLEQ can be used instead.

Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Paolo Bonzini 
Reviewed-by: Fam Zheng 
Signed-off-by: Kevin Wolf 
---
 include/qemu/coroutine.h |  2 +-
 include/qemu/coroutine_int.h |  4 ++--
 util/qemu-coroutine-lock.c   | 22 +++---
 util/qemu-coroutine.c|  2 +-
 4 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 305fe76..63ae7fe 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -102,7 +102,7 @@ bool qemu_in_coroutine(void);
  * are built.
  */
 typedef struct CoQueue {
-QTAILQ_HEAD(, Coroutine) entries;
+QSIMPLEQ_HEAD(, Coroutine) entries;
 } CoQueue;
 
 /**
diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h
index 42d6838..581a7f5 100644
--- a/include/qemu/coroutine_int.h
+++ b/include/qemu/coroutine_int.h
@@ -41,8 +41,8 @@ struct Coroutine {
 QSLIST_ENTRY(Coroutine) pool_next;
 
 /* Coroutines that should be woken up when we yield or terminate */
-QTAILQ_HEAD(, Coroutine) co_queue_wakeup;
-QTAILQ_ENTRY(Coroutine) co_queue_next;
+QSIMPLEQ_HEAD(, Coroutine) co_queue_wakeup;
+QSIMPLEQ_ENTRY(Coroutine) co_queue_next;
 };
 
 Coroutine *qemu_coroutine_new(void);
diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
index da37ca7..cf53693 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.c
@@ -31,13 +31,13 @@
 
 void qemu_co_queue_init(CoQueue *queue)
 {
-QTAILQ_INIT(>entries);
+QSIMPLEQ_INIT(>entries);
 }
 
 void coroutine_fn qemu_co_queue_wait(CoQueue *queue)
 {
 Coroutine *self = qemu_coroutine_self();
-QTAILQ_INSERT_TAIL(>entries, self, co_queue_next);
+QSIMPLEQ_INSERT_TAIL(>entries, self, co_queue_next);
 qemu_coroutine_yield();
 assert(qemu_in_coroutine());
 }
@@ -55,8 +55,8 @@ void qemu_co_queue_run_restart(Coroutine *co)
 Coroutine *next;
 
 trace_qemu_co_queue_run_restart(co);
-while ((next = QTAILQ_FIRST(>co_queue_wakeup))) {
-QTAILQ_REMOVE(>co_queue_wakeup, next, co_queue_next);
+while ((next = QSIMPLEQ_FIRST(>co_queue_wakeup))) {
+QSIMPLEQ_REMOVE_HEAD(>co_queue_wakeup, co_queue_next);
 qemu_coroutine_enter(next, NULL);
 }
 }
@@ -66,13 +66,13 @@ static bool qemu_co_queue_do_restart(CoQueue *queue, bool 
single)
 Coroutine *self = qemu_coroutine_self();
 Coroutine *next;
 
-if (QTAILQ_EMPTY(>entries)) {
+if (QSIMPLEQ_EMPTY(>entries)) {
 return false;
 }
 
-while ((next = QTAILQ_FIRST(>entries)) != NULL) {
-QTAILQ_REMOVE(>entries, next, co_queue_next);
-QTAILQ_INSERT_TAIL(>co_queue_wakeup, next, co_queue_next);
+while ((next = QSIMPLEQ_FIRST(>entries)) != NULL) {
+QSIMPLEQ_REMOVE_HEAD(>entries, co_queue_next);
+QSIMPLEQ_INSERT_TAIL(>co_queue_wakeup, next, co_queue_next);
 trace_qemu_co_queue_next(next);
 if (single) {
 break;
@@ -97,19 +97,19 @@ bool qemu_co_enter_next(CoQueue *queue)
 {
 Coroutine *next;
 
-next = QTAILQ_FIRST(>entries);
+next = QSIMPLEQ_FIRST(>entries);
 if (!next) {
 return false;
 }
 
-QTAILQ_REMOVE(>entries, next, co_queue_next);
+QSIMPLEQ_REMOVE_HEAD(>entries, co_queue_next);
 qemu_coroutine_enter(next, NULL);
 return true;
 }
 
 bool qemu_co_queue_empty(CoQueue *queue)
 {
-return QTAILQ_FIRST(>entries) == NULL;
+return QSIMPLEQ_FIRST(>entries) == NULL;
 }
 
 void qemu_co_mutex_init(CoMutex *mutex)
diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
index 5816702..b7cb636 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.c
@@ -76,7 +76,7 @@ Coroutine *qemu_coroutine_create(CoroutineEntry *entry)
 }
 
 co->entry = entry;
-QTAILQ_INIT(>co_queue_wakeup);
+QSIMPLEQ_INIT(>co_queue_wakeup);
 return co;
 }
 
-- 
1.8.3.1

[Qemu-block] [PULL v2 11/34] blockjob: Update description of the 'device' field in the QMP API

[Kevin Wolf]

From: Alberto Garcia 

The 'device' field in all BLOCK_JOB_* events and 'block-job-*' command
is no longer the device name, but the ID of the job. This patch
updates the documentation to clarify that.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 docs/qmp-events.txt  | 12 
 qapi/block-core.json | 35 +--
 2 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
index fa7574d..7967ec4 100644
--- a/docs/qmp-events.txt
+++ b/docs/qmp-events.txt
@@ -92,7 +92,8 @@ Data:
 
 - "type": Job type (json-string; "stream" for image streaming
  "commit" for block commit)
-- "device":   Device name (json-string)
+- "device":   Job identifier. Originally the device name but other
+  values are allowed since QEMU 2.7 (json-string)
 - "len":  Maximum progress value (json-int)
 - "offset":   Current progress value (json-int)
   On success this is equal to len.
@@ -116,7 +117,8 @@ Data:
 
 - "type": Job type (json-string; "stream" for image streaming
  "commit" for block commit)
-- "device":   Device name (json-string)
+- "device":   Job identifier. Originally the device name but other
+  values are allowed since QEMU 2.7 (json-string)
 - "len":  Maximum progress value (json-int)
 - "offset":   Current progress value (json-int)
   On success this is equal to len.
@@ -143,7 +145,8 @@ Emitted when a block job encounters an error.
 
 Data:
 
-- "device": device name (json-string)
+- "device": Job identifier. Originally the device name but other
+values are allowed since QEMU 2.7 (json-string)
 - "operation": I/O operation (json-string, "read" or "write")
 - "action": action that has been taken, it's one of the following 
(json-string):
 "ignore": error has been ignored, the job may fail later
@@ -167,7 +170,8 @@ Data:
 
 - "type": Job type (json-string; "stream" for image streaming
  "commit" for block commit)
-- "device":   Device name (json-string)
+- "device":   Job identifier. Originally the device name but other
+  values are allowed since QEMU 2.7 (json-string)
 - "len":  Maximum progress value (json-int)
 - "offset":   Current progress value (json-int)
   On success this is equal to len.
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1e4b16d..3f77dac 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -713,7 +713,8 @@
 #
 # @type: the job type ('stream' for image streaming)
 #
-# @device: the block device name
+# @device: The job identifier. Originally the device name but other
+#  values are allowed since QEMU 2.7
 #
 # @len: the maximum progress value
 #
@@ -1475,7 +1476,9 @@
 #
 # Throttling can be disabled by setting the speed to 0.
 #
-# @device: the device name
+# @device: The job identifier. This used to be a device name (hence
+#  the name of the parameter), but since QEMU 2.7 it can have
+#  other values.
 #
 # @speed:  the maximum speed, in bytes per second, or 0 for unlimited.
 #  Defaults to 0.
@@ -1506,7 +1509,9 @@
 # operation can be started at a later time to finish copying all data from the
 # backing file.
 #
-# @device: the device name
+# @device: The job identifier. This used to be a device name (hence
+#  the name of the parameter), but since QEMU 2.7 it can have
+#  other values.
 #
 # @force: #optional whether to allow cancellation of a paused job (default
 # false).  Since 1.3.
@@ -1532,7 +1537,9 @@
 # the operation is actually paused.  Cancelling a paused job automatically
 # resumes it.
 #
-# @device: the device name
+# @device: The job identifier. This used to be a device name (hence
+#  the name of the parameter), but since QEMU 2.7 it can have
+#  other values.
 #
 # Returns: Nothing on success
 #  If no background operation is active on this device, DeviceNotActive
@@ -1552,7 +1559,9 @@
 #
 # This command also clears the error status of the job.
 #
-# @device: the device name
+# @device: The job identifier. This used to be a device name (hence
+#  the name of the parameter), but since QEMU 2.7 it can have
+#  other values.
 #
 # Returns: Nothing on success
 #  If no background operation is active on this device, DeviceNotActive
@@ -1578,7 +1587,9 @@
 #
 # A cancelled or paused job cannot be completed.
 #
-# @device: the device name
+# @device: The job identifier. This used to be a device name (hence
+#  the name of the parameter), but since QEMU 2.7 it can have
+#  other values.
 #
 # Returns: Nothing on success
 #  If no background operation is active on this device, 

[Qemu-block] [PULL v2 15/34] test-coroutine: prepare for the next patch

[Kevin Wolf]

From: Paolo Bonzini 

The next patch moves the coroutine argument from first-enter to
creation time.  In this case, coroutine has not been initialized
yet when the coroutine is created, so change to a pointer.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Fam Zheng 
Signed-off-by: Kevin Wolf 
---
 tests/test-coroutine.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/test-coroutine.c b/tests/test-coroutine.c
index 215b92e..5171174 100644
--- a/tests/test-coroutine.c
+++ b/tests/test-coroutine.c
@@ -40,7 +40,8 @@ static void test_in_coroutine(void)
 
 static void coroutine_fn verify_self(void *opaque)
 {
-g_assert(qemu_coroutine_self() == opaque);
+Coroutine **p_co = opaque;
+g_assert(qemu_coroutine_self() == *p_co);
 }
 
 static void test_self(void)
@@ -48,7 +49,7 @@ static void test_self(void)
 Coroutine *coroutine;
 
 coroutine = qemu_coroutine_create(verify_self);
-qemu_coroutine_enter(coroutine, coroutine);
+qemu_coroutine_enter(coroutine, );
 }
 
 /*
-- 
1.8.3.1

[Qemu-block] [PULL v2 22/34] block: Remove BB options from blockdev-add

[Kevin Wolf]

werror/rerror are now available as qdev options. The stats-* options are
removed without an existing replacement; they should probably be
configurable with a separate QMP command like I/O throttling settings.

Removing id is left for another day because this involves updating
qemu-iotests cases to use node-name for everything. Before we can do
that, however, all QMP commands must support node-name.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 qapi/block-core.json | 18 +-
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0d30187..3444a9b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2081,20 +2081,8 @@
 # @discard:   #optional discard-related options (default: ignore)
 # @cache: #optional cache-related options
 # @aio:   #optional AIO backend (default: threads)
-# @rerror:#optional how to handle read errors on the device
-# (default: report)
-# @werror:#optional how to handle write errors on the device
-# (default: enospc)
 # @read-only: #optional whether the block device should be read-only
 # (default: false)
-# @stats-account-invalid: #optional whether to include invalid
-# operations when computing last access statistics
-# (default: true) (Since 2.5)
-# @stats-account-failed: #optional whether to include failed
-# operations when computing latency and last
-# access statistics (default: true) (Since 2.5)
-# @stats-intervals: #optional list of intervals for collecting I/O
-#   statistics, in seconds (default: none) (Since 2.5)
 # @detect-zeroes: #optional detect and optimize zero writes (Since 2.1)
 # (default: off)
 #
@@ -2104,17 +2092,13 @@
 ##
 { 'union': 'BlockdevOptions',
   'base': { 'driver': 'BlockdevDriver',
+# TODO 'id' is a BB-level option, remove it
 '*id': 'str',
 '*node-name': 'str',
 '*discard': 'BlockdevDiscardOptions',
 '*cache': 'BlockdevCacheOptions',
 '*aio': 'BlockdevAioOptions',
-'*rerror': 'BlockdevOnError',
-'*werror': 'BlockdevOnError',
 '*read-only': 'bool',
-'*stats-account-invalid': 'bool',
-'*stats-account-failed': 'bool',
-'*stats-intervals': ['int'],
 '*detect-zeroes': 'BlockdevDetectZeroesOptions' },
   'discriminator': 'driver',
   'data': {
-- 
1.8.3.1

[Qemu-block] [PULL v2 17/34] block/qdev: Allow node name for drive properties

[Kevin Wolf]

If a node name instead of a BlockBackend name is specified as the driver
for a guest device, an anonymous BlockBackend is created now.

The order of operations in release_drive() must be reversed in order to
avoid a use-after-free bug because now blk_detach_dev() frees the last
reference if an anonymous BlockBackend is used.

usb-storage uses a hack where it forwards its BlockBackend as a property
to another device that it internally creates. This hack must be updated
so that it doesn't drop its original BB before it can be passed to the
other device. This used to work because we always had the monitor
reference around, but with node-names the device reference is the only
one now.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 hw/core/qdev-properties-system.c | 39 +--
 hw/usb/dev-storage.c |  5 -
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 65d9fa9..ab1bc5e 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -72,12 +72,21 @@ static void parse_drive(DeviceState *dev, const char *str, 
void **ptr,
 const char *propname, Error **errp)
 {
 BlockBackend *blk;
+bool blk_created = false;
 
 blk = blk_by_name(str);
 if (!blk) {
+BlockDriverState *bs = bdrv_lookup_bs(NULL, str, NULL);
+if (bs) {
+blk = blk_new();
+blk_insert_bs(blk, bs);
+blk_created = true;
+}
+}
+if (!blk) {
 error_setg(errp, "Property '%s.%s' can't find value '%s'",
object_get_typename(OBJECT(dev)), propname, str);
-return;
+goto fail;
 }
 if (blk_attach_dev(blk, dev) < 0) {
 DriveInfo *dinfo = blk_legacy_dinfo(blk);
@@ -91,9 +100,16 @@ static void parse_drive(DeviceState *dev, const char *str, 
void **ptr,
 error_setg(errp, "Drive '%s' is already in use by another device",
str);
 }
-return;
+goto fail;
 }
+
 *ptr = blk;
+
+fail:
+if (blk_created) {
+/* If we need to keep a reference, blk_attach_dev() took it */
+blk_unref(blk);
+}
 }
 
 static void release_drive(Object *obj, const char *name, void *opaque)
@@ -103,8 +119,8 @@ static void release_drive(Object *obj, const char *name, 
void *opaque)
 BlockBackend **ptr = qdev_get_prop_ptr(dev, prop);
 
 if (*ptr) {
-blk_detach_dev(*ptr, dev);
 blockdev_auto_del(*ptr);
+blk_detach_dev(*ptr, dev);
 }
 }
 
@@ -127,7 +143,7 @@ static void set_drive(Object *obj, Visitor *v, const char 
*name, void *opaque,
 
 PropertyInfo qdev_prop_drive = {
 .name  = "str",
-.description = "ID of a drive to use as a backend",
+.description = "Node name or ID of a block device to use as a backend",
 .get   = get_drive,
 .set   = set_drive,
 .release = release_drive,
@@ -362,8 +378,19 @@ PropertyInfo qdev_prop_vlan = {
 void qdev_prop_set_drive(DeviceState *dev, const char *name,
  BlockBackend *value, Error **errp)
 {
-object_property_set_str(OBJECT(dev), value ? blk_name(value) : "",
-name, errp);
+const char *ref = "";
+
+if (value) {
+ref = blk_name(value);
+if (!*ref) {
+BlockDriverState *bs = blk_bs(value);
+if (bs) {
+ref = bdrv_get_node_name(bs);
+}
+}
+}
+
+object_property_set_str(OBJECT(dev), ref, name, errp);
 }
 
 void qdev_prop_set_chr(DeviceState *dev, const char *name,
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 4d605b8..78038a2 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -609,10 +609,12 @@ static void usb_msd_realize_storage(USBDevice *dev, Error 
**errp)
  * a SCSI bus that can serve only a single device, which it
  * creates automatically.  But first it needs to detach from its
  * blockdev, or else scsi_bus_legacy_add_drive() dies when it
- * attaches again.
+ * attaches again. We also need to take another reference so that
+ * blk_detach_dev() doesn't free blk while we still need it.
  *
  * The hack is probably a bad idea.
  */
+blk_ref(blk);
 blk_detach_dev(blk, >dev.qdev);
 s->conf.blk = NULL;
 
@@ -623,6 +625,7 @@ static void usb_msd_realize_storage(USBDevice *dev, Error 
**errp)
 scsi_dev = scsi_bus_legacy_add_drive(>bus, blk, 0, !!s->removable,
  s->conf.bootindex, dev->serial,
  );
+blk_unref(blk);
 if (!scsi_dev) {
 error_propagate(errp, err);
 return;
-- 
1.8.3.1

[Qemu-block] [PULL v2 19/34] commit: Fix use of error handling policy

[Kevin Wolf]

Commit implemented the 'enospc' policy as 'ignore' if the error was not
ENOSPC. The QAPI documentation promises that it's treated as 'stop'.
Using the common block job error handling function fixes this and also
adds the missing QMP event.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 block/commit.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/commit.c b/block/commit.c
index 8b534d7..5d11eb6 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -171,9 +171,9 @@ wait:
 bytes_written += n * BDRV_SECTOR_SIZE;
 }
 if (ret < 0) {
-if (s->on_error == BLOCKDEV_ON_ERROR_STOP ||
-s->on_error == BLOCKDEV_ON_ERROR_REPORT||
-(s->on_error == BLOCKDEV_ON_ERROR_ENOSPC && ret == -ENOSPC)) {
+BlockErrorAction action =
+block_job_error_action(>common, false, s->on_error, -ret);
+if (action == BLOCK_ERROR_ACTION_REPORT) {
 goto out;
 } else {
 n = 0;
-- 
1.8.3.1

[Qemu-block] [PULL v2 10/34] qemu-img: Set the ID of the block job in img_commit()

[Kevin Wolf]

From: Alberto Garcia 

img_commit() creates a block job without an ID. This is no longer
allowed now that we require it to be unique and well-formed. We were
solving this by having a fallback in block_job_create(), but now that
we extended the API of commit_active_start() we can finally set an
explicit ID and revert that change.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 blockjob.c | 6 --
 qemu-img.c | 2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index 511c0db..3b9cec7 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -132,12 +132,6 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
 
 if (job_id == NULL) {
 job_id = bdrv_get_device_name(bs);
-/* Assign a default ID if the BDS does not have a device
- * name. We'll get rid of this soon when we finish extending
- * the API of all commands that create block jobs. */
-if (job_id[0] == '\0') {
-job_id = "default_job";
-}
 }
 
 if (!id_wellformed(job_id)) {
diff --git a/qemu-img.c b/qemu-img.c
index a162f34..969edce 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -920,7 +920,7 @@ static int img_commit(int argc, char **argv)
 .bs   = bs,
 };
 
-commit_active_start(NULL, bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
+commit_active_start("commit", bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
 common_block_job_cb, , _err);
 if (local_err) {
 goto done;
-- 
1.8.3.1

[Qemu-block] [PULL v2 18/34] block/qdev: Allow configuring WCE with qdev properties

[Kevin Wolf]

As cache.writeback is a BlockBackend property and as such more related
to the guest device than the BlockDriverState, we already removed it
from the blockdev-add interface. This patch adds the new way to set it,
as a qdev property of the corresponding guest device.

For example: -drive if=none,file=test.img,node-name=img
 -device ide-hd,drive=img,write-cache=off

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 hw/block/block.c | 16 
 hw/block/nvme.c  |  1 +
 hw/block/virtio-blk.c|  1 +
 hw/ide/qdev.c|  1 +
 hw/scsi/scsi-disk.c  |  1 +
 hw/usb/dev-storage.c |  1 +
 include/hw/block/block.h |  5 -
 7 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/hw/block/block.c b/hw/block/block.c
index 97a59d4..396b0d5 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -51,6 +51,22 @@ void blkconf_blocksizes(BlockConf *conf)
 }
 }
 
+void blkconf_apply_backend_options(BlockConf *conf)
+{
+BlockBackend *blk = conf->blk;
+bool wce;
+
+switch (conf->wce) {
+case ON_OFF_AUTO_ON:wce = true; break;
+case ON_OFF_AUTO_OFF:   wce = false; break;
+case ON_OFF_AUTO_AUTO:  wce = blk_enable_write_cache(blk); break;
+default:
+abort();
+}
+
+blk_set_enable_write_cache(blk, wce);
+}
+
 void blkconf_geometry(BlockConf *conf, int *ptrans,
   unsigned cyls_max, unsigned heads_max, unsigned secs_max,
   Error **errp)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 280d54d..2ded247 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -803,6 +803,7 @@ static int nvme_init(PCIDevice *pci_dev)
 return -1;
 }
 blkconf_blocksizes(>conf);
+blkconf_apply_backend_options(>conf);
 
 pci_conf = pci_dev->config;
 pci_conf[PCI_INTERRUPT_PIN] = 1;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index ae86e94..ecd8ea3 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -897,6 +897,7 @@ static void virtio_blk_device_realize(DeviceState *dev, 
Error **errp)
 }
 
 blkconf_serial(>conf, >serial);
+blkconf_apply_backend_options(>conf);
 s->original_wce = blk_enable_write_cache(conf->conf.blk);
 blkconf_geometry(>conf, NULL, 65535, 255, 255, );
 if (err) {
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index d07b44e..33619f4 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -180,6 +180,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
 return -1;
 }
 }
+blkconf_apply_backend_options(>conf);
 
 if (ide_init_drive(s, dev->conf.blk, kind,
dev->version, dev->serial, dev->model, dev->wwn,
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 36f8a85..8c26a4e 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2309,6 +2309,7 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
 return;
 }
 }
+blkconf_apply_backend_options(>conf);
 
 if (s->qdev.conf.discard_granularity == -1) {
 s->qdev.conf.discard_granularity =
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 78038a2..c607f76 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -603,6 +603,7 @@ static void usb_msd_realize_storage(USBDevice *dev, Error 
**errp)
 
 blkconf_serial(>conf, >serial);
 blkconf_blocksizes(>conf);
+blkconf_apply_backend_options(>conf);
 
 /*
  * Hack alert: this pretends to be a block device, but it's really
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 87c87ed..245ac99 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -25,6 +25,7 @@ typedef struct BlockConf {
 uint32_t discard_granularity;
 /* geometry, not all devices use this */
 uint32_t cyls, heads, secs;
+OnOffAuto wce;
 } BlockConf;
 
 static inline unsigned int get_physical_block_exp(BlockConf *conf)
@@ -49,7 +50,8 @@ static inline unsigned int get_physical_block_exp(BlockConf 
*conf)
 DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
 DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),\
 DEFINE_PROP_UINT32("discard_granularity", _state, \
-   _conf.discard_granularity, -1)
+   _conf.discard_granularity, -1), \
+DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, ON_OFF_AUTO_AUTO)
 
 #define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf)  \
 DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0),  \
@@ -63,6 +65,7 @@ void blkconf_geometry(BlockConf *conf, int *trans,
   unsigned cyls_max, unsigned heads_max, unsigned secs_max,
   Error **errp);
 void blkconf_blocksizes(BlockConf *conf);
+void blkconf_apply_backend_options(BlockConf *conf);
 
 /* Hard disk geometry */
 
-- 
1.8.3.1

[Qemu-block] [PULL v2 13/34] raw-posix: Use qemu_dup

[Kevin Wolf]

From: Fam Zheng 

Signed-off-by: Fam Zheng 
Reviewed-by: Max Reitz 
Reviewed-by: Kevin Wolf 
Reviewed-by: John Snow 
Signed-off-by: Kevin Wolf 
---
 block/raw-posix.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index c979ac3..d1c3bd8 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -639,15 +639,7 @@ static int raw_reopen_prepare(BDRVReopenState *state,
 
 if ((raw_s->open_flags & ~fcntl_flags) == (s->open_flags & ~fcntl_flags)) {
 /* dup the original fd */
-/* TODO: use qemu fcntl wrapper */
-#ifdef F_DUPFD_CLOEXEC
-raw_s->fd = fcntl(s->fd, F_DUPFD_CLOEXEC, 0);
-#else
-raw_s->fd = dup(s->fd);
-if (raw_s->fd != -1) {
-qemu_set_cloexec(raw_s->fd);
-}
-#endif
+raw_s->fd = qemu_dup(s->fd);
 if (raw_s->fd >= 0) {
 ret = fcntl_setfl(raw_s->fd, raw_s->open_flags);
 if (ret) {
-- 
1.8.3.1

[Qemu-block] [PULL v2 12/34] osdep: Introduce qemu_dup

[Kevin Wolf]

From: Fam Zheng 

And use it in qemu_dup_flags.

Signed-off-by: Fam Zheng 
Reviewed-by: Eric Blake 
Reviewed-by: John Snow 
Signed-off-by: Kevin Wolf 
---
 include/qemu/osdep.h |  3 +++
 util/osdep.c | 23 +++
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index e63da28..7361006 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -278,6 +278,9 @@ int qemu_madvise(void *addr, size_t len, int advice);
 
 int qemu_open(const char *name, int flags, ...);
 int qemu_close(int fd);
+#ifndef _WIN32
+int qemu_dup(int fd);
+#endif
 
 #if defined(__HAIKU__) && defined(__i386__)
 #define FMT_pid "%ld"
diff --git a/util/osdep.c b/util/osdep.c
index ff004e8..06fb1cf 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -83,14 +83,7 @@ static int qemu_dup_flags(int fd, int flags)
 int serrno;
 int dup_flags;
 
-#ifdef F_DUPFD_CLOEXEC
-ret = fcntl(fd, F_DUPFD_CLOEXEC, 0);
-#else
-ret = dup(fd);
-if (ret != -1) {
-qemu_set_cloexec(ret);
-}
-#endif
+ret = qemu_dup(fd);
 if (ret == -1) {
 goto fail;
 }
@@ -129,6 +122,20 @@ fail:
 return -1;
 }
 
+int qemu_dup(int fd)
+{
+int ret;
+#ifdef F_DUPFD_CLOEXEC
+ret = fcntl(fd, F_DUPFD_CLOEXEC, 0);
+#else
+ret = dup(fd);
+if (ret != -1) {
+qemu_set_cloexec(ret);
+}
+#endif
+return ret;
+}
+
 static int qemu_parse_fdset(const char *param)
 {
 return qemu_parse_fd(param);
-- 
1.8.3.1

[Qemu-block] [PULL v2 08/34] stream: Add 'job-id' parameter to 'block-stream'

[Kevin Wolf]

From: Alberto Garcia 

This patch adds a new optional 'job-id' parameter to 'block-stream',
allowing the user to specify the ID of the block job to be created.

The HMP 'block_stream' command remains unchanged.

Signed-off-by: Alberto Garcia 
Reviewed-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 block/stream.c| 12 ++--
 blockdev.c|  6 +++---
 hmp.c |  2 +-
 include/block/block_int.h | 10 ++
 qapi/block-core.json  |  8 ++--
 qmp-commands.hx   |  4 +++-
 6 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/block/stream.c b/block/stream.c
index e4319d3..54c8cd8 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -218,15 +218,15 @@ static const BlockJobDriver stream_job_driver = {
 .set_speed = stream_set_speed,
 };
 
-void stream_start(BlockDriverState *bs, BlockDriverState *base,
-  const char *backing_file_str, int64_t speed,
-  BlockdevOnError on_error,
-  BlockCompletionFunc *cb,
-  void *opaque, Error **errp)
+void stream_start(const char *job_id, BlockDriverState *bs,
+  BlockDriverState *base, const char *backing_file_str,
+  int64_t speed, BlockdevOnError on_error,
+  BlockCompletionFunc *cb, void *opaque, Error **errp)
 {
 StreamBlockJob *s;
 
-s = block_job_create(NULL, _job_driver, bs, speed, cb, opaque, 
errp);
+s = block_job_create(job_id, _job_driver, bs, speed,
+ cb, opaque, errp);
 if (!s) {
 return;
 }
diff --git a/blockdev.c b/blockdev.c
index 920d987..d6f1d4d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3005,7 +3005,7 @@ static void block_job_cb(void *opaque, int ret)
 }
 }
 
-void qmp_block_stream(const char *device,
+void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
   bool has_base, const char *base,
   bool has_backing_file, const char *backing_file,
   bool has_speed, int64_t speed,
@@ -3064,8 +3064,8 @@ void qmp_block_stream(const char *device,
 /* backing_file string overrides base bs filename */
 base_name = has_backing_file ? backing_file : base_name;
 
-stream_start(bs, base_bs, base_name, has_speed ? speed : 0,
- on_error, block_job_cb, bs, _err);
+stream_start(has_job_id ? job_id : NULL, bs, base_bs, base_name,
+ has_speed ? speed : 0, on_error, block_job_cb, bs, 
_err);
 if (local_err) {
 error_propagate(errp, local_err);
 goto out;
diff --git a/hmp.c b/hmp.c
index 62eca70..3ca79c3 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1485,7 +1485,7 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
 const char *base = qdict_get_try_str(qdict, "base");
 int64_t speed = qdict_get_try_int(qdict, "speed", 0);
 
-qmp_block_stream(device, base != NULL, base, false, NULL,
+qmp_block_stream(false, NULL, device, base != NULL, base, false, NULL,
  qdict_haskey(qdict, "speed"), speed,
  true, BLOCKDEV_ON_ERROR_REPORT, );
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index a0b9f92..db364bb 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -639,6 +639,8 @@ int is_windows_drive(const char *filename);
 
 /**
  * stream_start:
+ * @job_id: The id of the newly-created job, or %NULL to use the
+ * device name of @bs.
  * @bs: Block device to operate on.
  * @base: Block device that will become the new base, or %NULL to
  * flatten the whole backing file chain onto @bs.
@@ -657,10 +659,10 @@ int is_windows_drive(const char *filename);
  * @backing_file_str in the written image and to @base in the live
  * BlockDriverState.
  */
-void stream_start(BlockDriverState *bs, BlockDriverState *base,
-  const char *backing_file_str, int64_t speed,
-  BlockdevOnError on_error, BlockCompletionFunc *cb,
-  void *opaque, Error **errp);
+void stream_start(const char *job_id, BlockDriverState *bs,
+  BlockDriverState *base, const char *backing_file_str,
+  int64_t speed, BlockdevOnError on_error,
+  BlockCompletionFunc *cb, void *opaque, Error **errp);
 
 /**
  * commit_start:
diff --git a/qapi/block-core.json b/qapi/block-core.json
index ee44ce4..94f4733 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1425,6 +1425,9 @@
 # On successful completion the image file is updated to drop the backing file
 # and the BLOCK_JOB_COMPLETED event is emitted.
 #
+# @job-id: #optional identifier for the newly-created block job. If
+#  omitted, the device name will be used. (Since 2.7)
+#
 # @device: the device name
 #
 # @base:   #optional the common backing file name
@@ -1456,8 

[Qemu-block] [PULL v2 06/34] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror'

[Kevin Wolf]

From: Alberto Garcia 

This patch adds a new optional 'job-id' parameter to 'blockdev-mirror'
and 'drive-mirror', allowing the user to specify the ID of the block
job to be created.

The HMP 'drive_mirror' command remains unchanged.

Signed-off-by: Alberto Garcia 
Reviewed-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 block/mirror.c| 15 ---
 blockdev.c| 15 ---
 hmp.c |  2 +-
 include/block/block_int.h |  6 --
 qapi/block-core.json  | 12 +---
 qmp-commands.hx   | 10 +++---
 6 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 08d88ca..702a686 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -843,8 +843,8 @@ static const BlockJobDriver commit_active_job_driver = {
 .attached_aio_context   = mirror_attached_aio_context,
 };
 
-static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
- const char *replaces,
+static void mirror_start_job(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *target, const char *replaces,
  int64_t speed, uint32_t granularity,
  int64_t buf_size,
  BlockMirrorBackingMode backing_mode,
@@ -873,7 +873,7 @@ static void mirror_start_job(BlockDriverState *bs, 
BlockDriverState *target,
 buf_size = DEFAULT_MIRROR_BUF_SIZE;
 }
 
-s = block_job_create(NULL, driver, bs, speed, cb, opaque, errp);
+s = block_job_create(job_id, driver, bs, speed, cb, opaque, errp);
 if (!s) {
 return;
 }
@@ -906,8 +906,8 @@ static void mirror_start_job(BlockDriverState *bs, 
BlockDriverState *target,
 qemu_coroutine_enter(s->common.co, s);
 }
 
-void mirror_start(BlockDriverState *bs, BlockDriverState *target,
-  const char *replaces,
+void mirror_start(const char *job_id, BlockDriverState *bs,
+  BlockDriverState *target, const char *replaces,
   int64_t speed, uint32_t granularity, int64_t buf_size,
   MirrorSyncMode mode, BlockMirrorBackingMode backing_mode,
   BlockdevOnError on_source_error,
@@ -925,7 +925,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState 
*target,
 }
 is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
 base = mode == MIRROR_SYNC_MODE_TOP ? backing_bs(bs) : NULL;
-mirror_start_job(bs, target, replaces,
+mirror_start_job(job_id, bs, target, replaces,
  speed, granularity, buf_size, backing_mode,
  on_source_error, on_target_error, unmap, cb, opaque, errp,
  _job_driver, is_none_mode, base);
@@ -973,7 +973,8 @@ void commit_active_start(BlockDriverState *bs, 
BlockDriverState *base,
 }
 }
 
-mirror_start_job(bs, base, NULL, speed, 0, 0, MIRROR_LEAVE_BACKING_CHAIN,
+mirror_start_job(NULL, bs, base, NULL, speed, 0, 0,
+ MIRROR_LEAVE_BACKING_CHAIN,
  on_error, on_error, false, cb, opaque, _err,
  _active_job_driver, false, base);
 if (local_err) {
diff --git a/blockdev.c b/blockdev.c
index e649521..2008690 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3422,7 +3422,7 @@ void qmp_blockdev_backup(const char *device, const char 
*target,
 /* Parameter check and block job starting for drive mirroring.
  * Caller should hold @device and @target's aio context (must be the same).
  **/
-static void blockdev_mirror_common(BlockDriverState *bs,
+static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs,
BlockDriverState *target,
bool has_replaces, const char *replaces,
enum MirrorSyncMode sync,
@@ -3482,15 +3482,15 @@ static void blockdev_mirror_common(BlockDriverState *bs,
 /* pass the node name to replace to mirror start since it's loose coupling
  * and will allow to check whether the node still exist at mirror 
completion
  */
-mirror_start(bs, target,
+mirror_start(job_id, bs, target,
  has_replaces ? replaces : NULL,
  speed, granularity, buf_size, sync, backing_mode,
  on_source_error, on_target_error, unmap,
  block_job_cb, bs, errp);
 }
 
-void qmp_drive_mirror(const char *device, const char *target,
-  bool has_format, const char *format,
+void qmp_drive_mirror(bool has_job_id, const char *job_id, const char *device,
+  const char *target, bool has_format, const char *format,
   bool has_node_name, const char *node_name,
   bool has_replaces, const char *replaces,
   

[Qemu-block] [PULL v2 05/34] blockjob: Add 'job_id' parameter to block_job_create()

[Kevin Wolf]

From: Alberto Garcia 

When a new job is created, the job ID is taken from the device name of
the BDS. This patch adds a new 'job_id' parameter to let the caller
provide one instead.

This patch also verifies that the ID is always unique and well-formed.
This causes problems in a couple of places where no ID is being set,
because the BDS does not have a device name.

In the case of test_block_job_start() (from test-blockjob-txn.c) we
can simply use this new 'job_id' parameter to set the missing ID.

In the case of img_commit() (from qemu-img.c) we still don't have the
API to make commit_active_start() set the job ID, so we solve it by
setting a default value. We'll get rid of this as soon as we extend
the API.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 block/backup.c|  3 ++-
 block/commit.c|  2 +-
 block/mirror.c|  2 +-
 block/stream.c|  2 +-
 blockjob.c| 29 +
 include/block/blockjob.h  |  8 +---
 tests/test-blockjob-txn.c |  7 +--
 7 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index f87f8d5..19c587c 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -541,7 +541,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
*target,
 goto error;
 }
 
-job = block_job_create(_job_driver, bs, speed, cb, opaque, errp);
+job = block_job_create(NULL, _job_driver, bs, speed,
+   cb, opaque, errp);
 if (!job) {
 goto error;
 }
diff --git a/block/commit.c b/block/commit.c
index 379efb7..137bb03 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -236,7 +236,7 @@ void commit_start(BlockDriverState *bs, BlockDriverState 
*base,
 return;
 }
 
-s = block_job_create(_job_driver, bs, speed, cb, opaque, errp);
+s = block_job_create(NULL, _job_driver, bs, speed, cb, opaque, 
errp);
 if (!s) {
 return;
 }
diff --git a/block/mirror.c b/block/mirror.c
index 6e3dbd2..08d88ca 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -873,7 +873,7 @@ static void mirror_start_job(BlockDriverState *bs, 
BlockDriverState *target,
 buf_size = DEFAULT_MIRROR_BUF_SIZE;
 }
 
-s = block_job_create(driver, bs, speed, cb, opaque, errp);
+s = block_job_create(NULL, driver, bs, speed, cb, opaque, errp);
 if (!s) {
 return;
 }
diff --git a/block/stream.c b/block/stream.c
index c0efbda..e4319d3 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -226,7 +226,7 @@ void stream_start(BlockDriverState *bs, BlockDriverState 
*base,
 {
 StreamBlockJob *s;
 
-s = block_job_create(_job_driver, bs, speed, cb, opaque, errp);
+s = block_job_create(NULL, _job_driver, bs, speed, cb, opaque, 
errp);
 if (!s) {
 return;
 }
diff --git a/blockjob.c b/blockjob.c
index ca2291b..511c0db 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -33,6 +33,7 @@
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qjson.h"
 #include "qemu/coroutine.h"
+#include "qemu/id.h"
 #include "qmp-commands.h"
 #include "qemu/timer.h"
 #include "qapi-event.h"
@@ -116,9 +117,9 @@ static void block_job_detach_aio_context(void *opaque)
 block_job_unref(job);
 }
 
-void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
-   int64_t speed, BlockCompletionFunc *cb,
-   void *opaque, Error **errp)
+void *block_job_create(const char *job_id, const BlockJobDriver *driver,
+   BlockDriverState *bs, int64_t speed,
+   BlockCompletionFunc *cb, void *opaque, Error **errp)
 {
 BlockBackend *blk;
 BlockJob *job;
@@ -129,6 +130,26 @@ void *block_job_create(const BlockJobDriver *driver, 
BlockDriverState *bs,
 return NULL;
 }
 
+if (job_id == NULL) {
+job_id = bdrv_get_device_name(bs);
+/* Assign a default ID if the BDS does not have a device
+ * name. We'll get rid of this soon when we finish extending
+ * the API of all commands that create block jobs. */
+if (job_id[0] == '\0') {
+job_id = "default_job";
+}
+}
+
+if (!id_wellformed(job_id)) {
+error_setg(errp, "Invalid job ID '%s'", job_id);
+return NULL;
+}
+
+if (block_job_get(job_id)) {
+error_setg(errp, "Job ID '%s' already in use", job_id);
+return NULL;
+}
+
 blk = blk_new();
 blk_insert_bs(blk, bs);
 
@@ -139,7 +160,7 @@ void *block_job_create(const BlockJobDriver *driver, 
BlockDriverState *bs,
 bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
 
 job->driver= driver;
-job->id= g_strdup(bdrv_get_device_name(bs));
+job->id= g_strdup(job_id);
 job->blk   = blk;
 job->cb 

[Qemu-block] [PULL v2 09/34] commit: Add 'job-id' parameter to 'block-commit'

[Kevin Wolf]

From: Alberto Garcia 

This patch adds a new optional 'job-id' parameter to 'block-commit',
allowing the user to specify the ID of the block job to be created.

Signed-off-by: Alberto Garcia 
Reviewed-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 block/commit.c|  7 ---
 block/mirror.c|  6 +++---
 blockdev.c|  9 +
 include/block/block_int.h | 16 ++--
 qapi/block-core.json  |  5 -
 qemu-img.c|  2 +-
 qmp-commands.hx   |  4 +++-
 7 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/block/commit.c b/block/commit.c
index 137bb03..23368fa 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -211,8 +211,8 @@ static const BlockJobDriver commit_job_driver = {
 .set_speed = commit_set_speed,
 };
 
-void commit_start(BlockDriverState *bs, BlockDriverState *base,
-  BlockDriverState *top, int64_t speed,
+void commit_start(const char *job_id, BlockDriverState *bs,
+  BlockDriverState *base, BlockDriverState *top, int64_t speed,
   BlockdevOnError on_error, BlockCompletionFunc *cb,
   void *opaque, const char *backing_file_str, Error **errp)
 {
@@ -236,7 +236,8 @@ void commit_start(BlockDriverState *bs, BlockDriverState 
*base,
 return;
 }
 
-s = block_job_create(NULL, _job_driver, bs, speed, cb, opaque, 
errp);
+s = block_job_create(job_id, _job_driver, bs, speed,
+ cb, opaque, errp);
 if (!s) {
 return;
 }
diff --git a/block/mirror.c b/block/mirror.c
index 702a686..705fbc0 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -931,8 +931,8 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
  _job_driver, is_none_mode, base);
 }
 
-void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
- int64_t speed,
+void commit_active_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *base, int64_t speed,
  BlockdevOnError on_error,
  BlockCompletionFunc *cb,
  void *opaque, Error **errp)
@@ -973,7 +973,7 @@ void commit_active_start(BlockDriverState *bs, 
BlockDriverState *base,
 }
 }
 
-mirror_start_job(NULL, bs, base, NULL, speed, 0, 0,
+mirror_start_job(job_id, bs, base, NULL, speed, 0, 0,
  MIRROR_LEAVE_BACKING_CHAIN,
  on_error, on_error, false, cb, opaque, _err,
  _active_job_driver, false, base);
diff --git a/blockdev.c b/blockdev.c
index d6f1d4d..aa23dc2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3077,7 +3077,7 @@ out:
 aio_context_release(aio_context);
 }
 
-void qmp_block_commit(const char *device,
+void qmp_block_commit(bool has_job_id, const char *job_id, const char *device,
   bool has_base, const char *base,
   bool has_top, const char *top,
   bool has_backing_file, const char *backing_file,
@@ -3168,10 +3168,11 @@ void qmp_block_commit(const char *device,
  " but 'top' is the active layer");
 goto out;
 }
-commit_active_start(bs, base_bs, speed, on_error, block_job_cb,
-bs, _err);
+commit_active_start(has_job_id ? job_id : NULL, bs, base_bs, speed,
+on_error, block_job_cb, bs, _err);
 } else {
-commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs,
+commit_start(has_job_id ? job_id : NULL, bs, base_bs, top_bs, speed,
+ on_error, block_job_cb, bs,
  has_backing_file ? backing_file : NULL, _err);
 }
 if (local_err != NULL) {
diff --git a/include/block/block_int.h b/include/block/block_int.h
index db364bb..8054146 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -666,6 +666,8 @@ void stream_start(const char *job_id, BlockDriverState *bs,
 
 /**
  * commit_start:
+ * @job_id: The id of the newly-created job, or %NULL to use the
+ * device name of @bs.
  * @bs: Active block device.
  * @top: Top block device to be committed.
  * @base: Block device that will be written into, and become the new top.
@@ -677,12 +679,14 @@ void stream_start(const char *job_id, BlockDriverState 
*bs,
  * @errp: Error object.
  *
  */
-void commit_start(BlockDriverState *bs, BlockDriverState *base,
- BlockDriverState *top, int64_t speed,
- BlockdevOnError on_error, BlockCompletionFunc *cb,
- void *opaque, const char *backing_file_str, Error **errp);
+void commit_start(const char *job_id, BlockDriverState *bs,
+  BlockDriverState *base, BlockDriverState *top, int64_t speed,
+   

[Qemu-block] [PULL v2 07/34] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup'

[Kevin Wolf]

From: Alberto Garcia 

This patch adds a new optional 'job-id' parameter to 'blockdev-backup'
and 'drive-backup', allowing the user to specify the ID of the block
job to be created.

The HMP 'drive_backup' command remains unchanged.

Signed-off-by: Alberto Garcia 
Reviewed-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 block/backup.c|  8 
 blockdev.c| 43 ---
 hmp.c |  2 +-
 include/block/block_int.h |  8 +---
 qapi/block-core.json  | 12 +---
 qmp-commands.hx   | 10 +++---
 6 files changed, 50 insertions(+), 33 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 19c587c..dce6614 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -474,9 +474,9 @@ static void coroutine_fn backup_run(void *opaque)
 block_job_defer_to_main_loop(>common, backup_complete, data);
 }
 
-void backup_start(BlockDriverState *bs, BlockDriverState *target,
-  int64_t speed, MirrorSyncMode sync_mode,
-  BdrvDirtyBitmap *sync_bitmap,
+void backup_start(const char *job_id, BlockDriverState *bs,
+  BlockDriverState *target, int64_t speed,
+  MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap,
   BlockdevOnError on_source_error,
   BlockdevOnError on_target_error,
   BlockCompletionFunc *cb, void *opaque,
@@ -541,7 +541,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
*target,
 goto error;
 }
 
-job = block_job_create(NULL, _job_driver, bs, speed,
+job = block_job_create(job_id, _job_driver, bs, speed,
cb, opaque, errp);
 if (!job) {
 goto error;
diff --git a/blockdev.c b/blockdev.c
index 2008690..920d987 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1836,9 +1836,9 @@ typedef struct DriveBackupState {
 BlockJob *job;
 } DriveBackupState;
 
-static void do_drive_backup(const char *device, const char *target,
-bool has_format, const char *format,
-enum MirrorSyncMode sync,
+static void do_drive_backup(const char *job_id, const char *device,
+const char *target, bool has_format,
+const char *format, enum MirrorSyncMode sync,
 bool has_mode, enum NewImageMode mode,
 bool has_speed, int64_t speed,
 bool has_bitmap, const char *bitmap,
@@ -1876,7 +1876,8 @@ static void drive_backup_prepare(BlkActionState *common, 
Error **errp)
 bdrv_drained_begin(blk_bs(blk));
 state->bs = blk_bs(blk);
 
-do_drive_backup(backup->device, backup->target,
+do_drive_backup(backup->has_job_id ? backup->job_id : NULL,
+backup->device, backup->target,
 backup->has_format, backup->format,
 backup->sync,
 backup->has_mode, backup->mode,
@@ -1921,8 +1922,8 @@ typedef struct BlockdevBackupState {
 AioContext *aio_context;
 } BlockdevBackupState;
 
-static void do_blockdev_backup(const char *device, const char *target,
-   enum MirrorSyncMode sync,
+static void do_blockdev_backup(const char *job_id, const char *device,
+   const char *target, enum MirrorSyncMode sync,
bool has_speed, int64_t speed,
bool has_on_source_error,
BlockdevOnError on_source_error,
@@ -1968,8 +1969,8 @@ static void blockdev_backup_prepare(BlkActionState 
*common, Error **errp)
 state->bs = blk_bs(blk);
 bdrv_drained_begin(state->bs);
 
-do_blockdev_backup(backup->device, backup->target,
-   backup->sync,
+do_blockdev_backup(backup->has_job_id ? backup->job_id : NULL,
+   backup->device, backup->target, backup->sync,
backup->has_speed, backup->speed,
backup->has_on_source_error, backup->on_source_error,
backup->has_on_target_error, backup->on_target_error,
@@ -3182,9 +3183,9 @@ out:
 aio_context_release(aio_context);
 }
 
-static void do_drive_backup(const char *device, const char *target,
-bool has_format, const char *format,
-enum MirrorSyncMode sync,
+static void do_drive_backup(const char *job_id, const char *device,
+const char *target, bool has_format,
+const char *format, enum MirrorSyncMode sync,
 bool has_mode, enum NewImageMode mode,
 bool has_speed, int64_t speed,
 bool has_bitmap, 

[Qemu-block] [PULL v2 00/34] Block layer patches

[Kevin Wolf]

The following changes since commit ca3d87d4c84032f19478010b5604cac88b045c25:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-include-2016-07-12' 
into staging (2016-07-12 16:04:36 +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 543d7a42baf39c09db754ba9eca1d386e5958110:

  Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2016-07-13' 
into queue-block (2016-07-13 13:45:55 +0200)



Block layer patches


Alberto Garcia (13):
  stream: Fix prototype of stream_start()
  blockjob: Update description of the 'id' field
  blockjob: Add block_job_get()
  block: Use block_job_get() in find_block_job()
  blockjob: Add 'job_id' parameter to block_job_create()
  mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror'
  backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup'
  stream: Add 'job-id' parameter to 'block-stream'
  commit: Add 'job-id' parameter to 'block-commit'
  qemu-img: Set the ID of the block job in img_commit()
  blockjob: Update description of the 'device' field in the QMP API
  blockdev: Fix regression with the default naming of throttling groups
  qemu-iotests: Test naming of throttling groups

Fam Zheng (2):
  osdep: Introduce qemu_dup
  raw-posix: Use qemu_dup

Kevin Wolf (7):
  block/qdev: Allow node name for drive properties
  block/qdev: Allow configuring WCE with qdev properties
  commit: Fix use of error handling policy
  block/qdev: Allow configuring rerror/werror with qdev properties
  qemu-iotests: Test setting WCE with qdev
  block: Remove BB options from blockdev-add
  Merge remote-tracking branch 
'mreitz/tags/pull-block-for-kevin-2016-07-13' into queue-block

Lin Ma (2):
  hmp: use snapshot name to determine whether a snapshot is 'fully 
available'
  hmp: show all of snapshot info on every block dev in output of 'info 
snapshots'

Max Reitz (6):
  qemu-img: Use strerror() for generic resize error
  qcow2: Avoid making the L1 table too big
  qemu-io: Use correct range limitations
  qcow2: Fix qcow2_get_cluster_offset()
  vvfat: Fix qcow write target driver specification
  iotests: Make 157 actually format-agnostic

Paolo Bonzini (3):
  coroutine: use QSIMPLEQ instead of QTAILQ
  test-coroutine: prepare for the next patch
  coroutine: move entry argument to qemu_coroutine_create

Reda Sallahi (1):
  vmdk: fix metadata write regression

Sascha Silbe (1):
  Improve block job rate limiting for small bandwidth values

 block.c  |   4 +-
 block/backup.c   |  13 +++--
 block/blkdebug.c |   4 +-
 block/blkreplay.c|   2 +-
 block/block-backend.c|   9 +--
 block/commit.c   |  30 +-
 block/gluster.c  |   2 +-
 block/io.c   |  45 +++
 block/iscsi.c|   4 +-
 block/linux-aio.c|   2 +-
 block/mirror.c   |  32 ++-
 block/nbd-client.c   |   6 +-
 block/nfs.c  |   2 +-
 block/qcow.c |   4 +-
 block/qcow2-cluster.c|  19 +--
 block/qcow2.c|   4 +-
 block/qed.c  |   4 +-
 block/raw-posix.c|  10 +---
 block/sheepdog.c |  14 ++---
 block/ssh.c  |   2 +-
 block/stream.c   |  28 -
 block/vmdk.c |  18 +++---
 block/vvfat.c|   3 +-
 blockdev.c   | 119 +++
 blockjob.c   |  42 --
 docs/qmp-events.txt  |  12 ++--
 hmp.c|   6 +-
 hw/9pfs/9p.c |   4 +-
 hw/9pfs/coth.c   |   4 +-
 hw/block/block.c |  28 +
 hw/block/nvme.c  |   1 +
 hw/block/virtio-blk.c|   2 +
 hw/core/qdev-properties-system.c |  39 +++--
 hw/core/qdev-properties.c|  13 +
 hw/ide/qdev.c|   2 +
 hw/scsi/scsi-disk.c  |   2 +
 hw/usb/dev-storage.c |   6 +-
 include/block/block_int.h|  47 ++--
 include/block/blockjob.h |  23 +---
 include/hw/block/block.h |  13 -
 include/hw/qdev-properties.h |   4 ++
 include/qapi/qmp/qerror.h|   3 -
 include/qemu/coroutine.h |  10 ++--
 include/qemu/coroutine_int.h |   4 +-
 include/qemu/main-loop.h |   4 +-
 include/qemu/osdep.h |   3 +
 include/qemu/ratelimit.h |  43 +++---
 io/channel.c |  

[Qemu-block] [PULL v2 04/34] block: Use block_job_get() in find_block_job()

[Kevin Wolf]

From: Alberto Garcia 

find_block_job() looks for a block backend with a specified name,
checks whether it has a block job and acquires its AioContext.

We want to identify jobs by their ID and not by the block backend
they're attached to, so this patch ignores the backends altogether and
gets the job directly. Apart from making the code simpler, this will
allow us to find block jobs once they start having user-specified IDs.

To ensure backward compatibility we keep ERROR_CLASS_DEVICE_NOT_ACTIVE
as the error class if the job doesn't exist. In subsequent patches
we'll also need to keep the device name as the default job ID if the
user doesn't specify a different one.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 blockdev.c | 40 +---
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 0f8065c..e649521 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3704,42 +3704,28 @@ void qmp_blockdev_mirror(const char *device, const char 
*target,
 aio_context_release(aio_context);
 }
 
-/* Get the block job for a given device name and acquire its AioContext */
-static BlockJob *find_block_job(const char *device, AioContext **aio_context,
+/* Get a block job using its ID and acquire its AioContext */
+static BlockJob *find_block_job(const char *id, AioContext **aio_context,
 Error **errp)
 {
-BlockBackend *blk;
-BlockDriverState *bs;
+BlockJob *job;
 
-*aio_context = NULL;
+assert(id != NULL);
 
-blk = blk_by_name(device);
-if (!blk) {
-goto notfound;
-}
-
-*aio_context = blk_get_aio_context(blk);
-aio_context_acquire(*aio_context);
+*aio_context = NULL;
 
-if (!blk_is_available(blk)) {
-goto notfound;
-}
-bs = blk_bs(blk);
+job = block_job_get(id);
 
-if (!bs->job) {
-goto notfound;
+if (!job) {
+error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
+  "Block job '%s' not found", id);
+return NULL;
 }
 
-return bs->job;
+*aio_context = blk_get_aio_context(job->blk);
+aio_context_acquire(*aio_context);
 
-notfound:
-error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
-  "No active block job on device '%s'", device);
-if (*aio_context) {
-aio_context_release(*aio_context);
-*aio_context = NULL;
-}
-return NULL;
+return job;
 }
 
 void qmp_block_job_set_speed(const char *device, int64_t speed, Error **errp)
-- 
1.8.3.1

[Qemu-block] [PULL v2 02/34] blockjob: Update description of the 'id' field

[Kevin Wolf]

From: Alberto Garcia 

The 'id' field of the BlockJob structure will be able to hold any ID,
not only a device name. This patch updates the description of that
field and the error messages where it is being used.

Soon we'll add the ability to set an arbitrary ID when creating a
block job.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 block/mirror.c| 3 ++-
 blockjob.c| 3 ++-
 include/block/blockjob.h  | 5 +
 include/qapi/qmp/qerror.h | 3 ---
 4 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 8d96049..6e3dbd2 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -761,7 +761,8 @@ static void mirror_complete(BlockJob *job, Error **errp)
 target = blk_bs(s->target);
 
 if (!s->synced) {
-error_setg(errp, QERR_BLOCK_JOB_NOT_READY, job->id);
+error_setg(errp, "The active block job '%s' cannot be completed",
+   job->id);
 return;
 }
 
diff --git a/blockjob.c b/blockjob.c
index 205da9d..ce0e27c 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -290,7 +290,8 @@ void block_job_set_speed(BlockJob *job, int64_t speed, 
Error **errp)
 void block_job_complete(BlockJob *job, Error **errp)
 {
 if (job->pause_count || job->cancelled || !job->driver->complete) {
-error_setg(errp, QERR_BLOCK_JOB_NOT_READY, job->id);
+error_setg(errp, "The active block job '%s' cannot be completed",
+   job->id);
 return;
 }
 
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index f7f5687..97b86f1 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -107,10 +107,7 @@ struct BlockJob {
 BlockBackend *blk;
 
 /**
- * The ID of the block job. Currently the BlockBackend name of the BDS
- * owning the job at the time when the job is started.
- *
- * TODO Decouple block job IDs from BlockBackend names
+ * The ID of the block job.
  */
 char *id;
 
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index d08652a..6586c9f 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -19,9 +19,6 @@
 #define QERR_BASE_NOT_FOUND \
 "Base '%s' not found"
 
-#define QERR_BLOCK_JOB_NOT_READY \
-"The active block job for device '%s' cannot be completed"
-
 #define QERR_BUS_NO_HOTPLUG \
 "Bus '%s' does not support hotplugging"
 
-- 
1.8.3.1

[Qemu-block] [PULL v2 01/34] stream: Fix prototype of stream_start()

[Kevin Wolf]

From: Alberto Garcia 

'stream-start' has a parameter called 'backing-file', which is the
string to be written to bs->backing when the job finishes.

In the stream_start() implementation it is called 'backing_file_str',
but it the prototype in the header file it is called 'base_id'.

This patch fixes it so the name is the same in both cases and is
consistent with other cases (like commit_start()).

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 include/block/block_int.h | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 47b9aac..2a27a194 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -642,8 +642,8 @@ int is_windows_drive(const char *filename);
  * @bs: Block device to operate on.
  * @base: Block device that will become the new base, or %NULL to
  * flatten the whole backing file chain onto @bs.
- * @base_id: The file name that will be written to @bs as the new
- * backing file if the job completes.  Ignored if @base is %NULL.
+ * @backing_file_str: The file name that will be written to @bs as the
+ * the new backing file if the job completes. Ignored if @base is %NULL.
  * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
  * @on_error: The action to take upon error.
  * @cb: Completion function for the job.
@@ -654,11 +654,12 @@ int is_windows_drive(const char *filename);
  * in @bs, but allocated in any image between @base and @bs (both
  * exclusive) will be written to @bs.  At the end of a successful
  * streaming job, the backing file of @bs will be changed to
- * @base_id in the written image and to @base in the live BlockDriverState.
+ * @backing_file_str in the written image and to @base in the live
+ * BlockDriverState.
  */
 void stream_start(BlockDriverState *bs, BlockDriverState *base,
-  const char *base_id, int64_t speed, BlockdevOnError on_error,
-  BlockCompletionFunc *cb,
+  const char *backing_file_str, int64_t speed,
+  BlockdevOnError on_error, BlockCompletionFunc *cb,
   void *opaque, Error **errp);
 
 /**
-- 
1.8.3.1

[Qemu-block] [PULL v2 03/34] blockjob: Add block_job_get()

[Kevin Wolf]

From: Alberto Garcia 

Currently the way to look for a specific block job is to iterate the
list manually using block_job_next().

Since we want to be able to identify a job primarily by its ID it
makes sense to have a function that does just that.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 blockjob.c   | 13 +
 include/block/blockjob.h | 10 ++
 2 files changed, 23 insertions(+)

diff --git a/blockjob.c b/blockjob.c
index ce0e27c..ca2291b 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -60,6 +60,19 @@ BlockJob *block_job_next(BlockJob *job)
 return QLIST_NEXT(job, job_list);
 }
 
+BlockJob *block_job_get(const char *id)
+{
+BlockJob *job;
+
+QLIST_FOREACH(job, _jobs, job_list) {
+if (!strcmp(id, job->id)) {
+return job;
+}
+}
+
+return NULL;
+}
+
 /* Normally the job runs in its BlockBackend's AioContext.  The exception is
  * block_job_defer_to_main_loop() where it runs in the QEMU main loop.  Code
  * that supports both cases uses this helper function.
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 97b86f1..934bf1c 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -212,6 +212,16 @@ struct BlockJob {
 BlockJob *block_job_next(BlockJob *job);
 
 /**
+ * block_job_get:
+ * @id: The id of the block job.
+ *
+ * Get the block job identified by @id (which must not be %NULL).
+ *
+ * Returns the requested job, or %NULL if it doesn't exist.
+ */
+BlockJob *block_job_get(const char *id);
+
+/**
  * block_job_create:
  * @job_type: The class object for the newly-created job.
  * @bs: The block
-- 
1.8.3.1

[Qemu-block] [PATCH v4 1/6] block/qdev: Allow node name for drive properties

[Kevin Wolf]

If a node name instead of a BlockBackend name is specified as the driver
for a guest device, an anonymous BlockBackend is created now.

The order of operations in release_drive() must be reversed in order to
avoid a use-after-free bug because now blk_detach_dev() frees the last
reference if an anonymous BlockBackend is used.

usb-storage uses a hack where it forwards its BlockBackend as a property
to another device that it internally creates. This hack must be updated
so that it doesn't drop its original BB before it can be passed to the
other device. This used to work because we always had the monitor
reference around, but with node-names the device reference is the only
one now.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---

v4:
- Fix use-after-free bug in release_drive()

 hw/core/qdev-properties-system.c | 39 +--
 hw/usb/dev-storage.c |  5 -
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 65d9fa9..ab1bc5e 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -72,12 +72,21 @@ static void parse_drive(DeviceState *dev, const char *str, 
void **ptr,
 const char *propname, Error **errp)
 {
 BlockBackend *blk;
+bool blk_created = false;
 
 blk = blk_by_name(str);
 if (!blk) {
+BlockDriverState *bs = bdrv_lookup_bs(NULL, str, NULL);
+if (bs) {
+blk = blk_new();
+blk_insert_bs(blk, bs);
+blk_created = true;
+}
+}
+if (!blk) {
 error_setg(errp, "Property '%s.%s' can't find value '%s'",
object_get_typename(OBJECT(dev)), propname, str);
-return;
+goto fail;
 }
 if (blk_attach_dev(blk, dev) < 0) {
 DriveInfo *dinfo = blk_legacy_dinfo(blk);
@@ -91,9 +100,16 @@ static void parse_drive(DeviceState *dev, const char *str, 
void **ptr,
 error_setg(errp, "Drive '%s' is already in use by another device",
str);
 }
-return;
+goto fail;
 }
+
 *ptr = blk;
+
+fail:
+if (blk_created) {
+/* If we need to keep a reference, blk_attach_dev() took it */
+blk_unref(blk);
+}
 }
 
 static void release_drive(Object *obj, const char *name, void *opaque)
@@ -103,8 +119,8 @@ static void release_drive(Object *obj, const char *name, 
void *opaque)
 BlockBackend **ptr = qdev_get_prop_ptr(dev, prop);
 
 if (*ptr) {
-blk_detach_dev(*ptr, dev);
 blockdev_auto_del(*ptr);
+blk_detach_dev(*ptr, dev);
 }
 }
 
@@ -127,7 +143,7 @@ static void set_drive(Object *obj, Visitor *v, const char 
*name, void *opaque,
 
 PropertyInfo qdev_prop_drive = {
 .name  = "str",
-.description = "ID of a drive to use as a backend",
+.description = "Node name or ID of a block device to use as a backend",
 .get   = get_drive,
 .set   = set_drive,
 .release = release_drive,
@@ -362,8 +378,19 @@ PropertyInfo qdev_prop_vlan = {
 void qdev_prop_set_drive(DeviceState *dev, const char *name,
  BlockBackend *value, Error **errp)
 {
-object_property_set_str(OBJECT(dev), value ? blk_name(value) : "",
-name, errp);
+const char *ref = "";
+
+if (value) {
+ref = blk_name(value);
+if (!*ref) {
+BlockDriverState *bs = blk_bs(value);
+if (bs) {
+ref = bdrv_get_node_name(bs);
+}
+}
+}
+
+object_property_set_str(OBJECT(dev), ref, name, errp);
 }
 
 void qdev_prop_set_chr(DeviceState *dev, const char *name,
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 4d605b8..78038a2 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -609,10 +609,12 @@ static void usb_msd_realize_storage(USBDevice *dev, Error 
**errp)
  * a SCSI bus that can serve only a single device, which it
  * creates automatically.  But first it needs to detach from its
  * blockdev, or else scsi_bus_legacy_add_drive() dies when it
- * attaches again.
+ * attaches again. We also need to take another reference so that
+ * blk_detach_dev() doesn't free blk while we still need it.
  *
  * The hack is probably a bad idea.
  */
+blk_ref(blk);
 blk_detach_dev(blk, >dev.qdev);
 s->conf.blk = NULL;
 
@@ -623,6 +625,7 @@ static void usb_msd_realize_storage(USBDevice *dev, Error 
**errp)
 scsi_dev = scsi_bus_legacy_add_drive(>bus, blk, 0, !!s->removable,
  s->conf.bootindex, dev->serial,
  );
+blk_unref(blk);
 if (!scsi_dev) {
 error_propagate(errp, err);
 return;
-- 
1.8.3.1

Re: [Qemu-block] [Qemu-devel] [PATCH v5 01/10] block: Hide HBitmap in block dirty bitmap interface

[Max Reitz]

On 13.07.2016 09:57, Vladimir Sementsov-Ogievskiy wrote:
> On 13.07.2016 01:49, John Snow wrote:
>>
>> On 06/03/2016 12:32 AM, Fam Zheng wrote:
>>> HBitmap is an implementation detail of block dirty bitmap that should
>>> be hidden
>>> from users. Introduce a BdrvDirtyBitmapIter to encapsulate the
>>> underlying
>>> HBitmapIter.
>>>
>>> A small difference in the interface is, before, an HBitmapIter is
>>> initialized
>>> in place, now the new BdrvDirtyBitmapIter must be dynamically
>>> allocated because
>>> the structure definition is in block/dirty-bitmap.c.
>>>
>>> Two current users are converted too.
>>>
>>> Signed-off-by: Fam Zheng 
>>> ---
>>>   block/backup.c   | 14 --
>>>   block/dirty-bitmap.c | 39
>>> +--
>>>   block/mirror.c   | 24 +---
>>>   include/block/dirty-bitmap.h |  7 +--
>>>   include/qemu/typedefs.h  |  1 +
>>>   5 files changed, 60 insertions(+), 25 deletions(-)
>>>

[...]

>>> @@ -224,6 +231,7 @@ static void
>>> bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
>>>   BdrvDirtyBitmap *bm, *next;
>>>   QLIST_FOREACH_SAFE(bm, >dirty_bitmaps, list, next) {
>>>   if ((!bitmap || bm == bitmap) && (!only_named || bm->name)) {
>>> +assert(!bitmap->active_iterators);
>> No good -- this function is also used to clear out all named bitmaps
>> belonging to a BDS, so 'bitmap' may be NULL, so this assertion is bad.
> 
> Just a note about this. I do not like this hidden behaviour of
> release_bitmap, as it more native for freeing functions to do nothing
> with NULL pointer.. g_free(NULL) do not free all allocations))).. If
> someone agrees, this may be better to be changed.

The function is not called "bdrv_release_dirty_bitmap()", though, but
"bdrv_do_release_matching_dirty_bitmap()". The "do" means that it's an
internal function, called only by bdrv_release_dirty_bitmap() (aha!) and
bdrv_release_named_dirty_bitmaps(); and the "matching" means that if you
pass NULL, it'll release all bitmaps.

What we could do is make bdrv_release_dirty_bitmap() a no-op if NULL is
passed, or add an assertion that the argument is not NULL. I'd be fine
with either, but I don't think bdrv_do_release_matching_dirty_bitmap()
needs to be adjusted.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH v3 01/11] block: Accept node-name for block-stream

[Kevin Wolf]

Am 07.07.2016 um 16:17 hat Kevin Wolf geschrieben:
> Am 07.07.2016 um 14:59 hat Alberto Garcia geschrieben:
> > On Thu 07 Jul 2016 02:11:27 PM CEST, Kevin Wolf wrote:
> > > In order to remove the necessity to use BlockBackend names in the
> > > external API, we want to allow node-names everywhere. This converts
> > > block-stream to accept a node-name without lifting the restriction that
> > > we're operating at a root node.
> > >
> > > In case of an invalid device name, the command returns the GenericError
> > > error class now instead of DeviceNotFound, because this is what
> > > qmp_get_root_bs() returns.
> > >
> > > Signed-off-by: Kevin Wolf 
> > > ---
> > >  blockdev.c | 32 
> > >  qapi/block-core.json   |  5 +
> > >  qmp-commands.hx|  2 +-
> > >  tests/qemu-iotests/030 |  2 +-
> > >  4 files changed, 23 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/blockdev.c b/blockdev.c
> > > index 0f8065c..01e57c9 100644
> > > --- a/blockdev.c
> > > +++ b/blockdev.c
> > > @@ -1172,6 +1172,23 @@ fail:
> > >  return dinfo;
> > >  }
> > >  
> > > +static BlockDriverState *qmp_get_root_bs(const char *name, Error **errp)
> > > +{
> > > +BlockDriverState *bs;
> > > +
> > > +bs = bdrv_lookup_bs(name, name, errp);
> > > +if (bs == NULL) {
> > > +return NULL;
> > > +}
> > > +
> > > +if (!bdrv_has_blk(bs)) {
> > > +error_setg(errp, "Need a root block node");
> > > +return NULL;
> > > +}
> > 
> > Since b6d2e59995f when you create a block job a new BlockBackend is
> > created on top of the BDS. What happens with this check if we allow
> > creating jobs in a non-root BDS? 
> 
> Hm, you mean I'd start first an intermediate streaming job and then I
> can call commands on the target node that I shouldn't be able to call?
> It's a good point, but I think op blockers would prevent that this
> actually works.
> 
> If we wanted to keep exactly the old set of nodes that is allowed, we
> could make qmp_get_root_bs() look for a _named_ BlockBackend. But that
> would kind of defeat the purpose of this series, which wants to allow
> these commands on named nodes that are directly used for -device.
> 
> Another option - and maybe that makes more sense than the old rule
> anyway because you already can have a BB anywhere in the middle of the
> graph - would be to check that the node doesn't have any non-BB parent.

This is what I'm implementing now. The reason for this is that
bdrv_has_blk() obviously rejects configurations where you have only a
node name, but no BB. And the whole point of the series is to move
towards a model without named BBs, so this would mean that you can only
run block job on attached nodes, which doesn't make a lot of sense (and
gives qemu-iotests some trouble).

With this option implemented, a node that isn't attached anywhere can be
used for root node commands, as it should.

Kevin

Re: [Qemu-block] [PATCH 1/1] mirror: double performance of the bulk stage if the disc is full

[Vladimir Sementsov-Ogievskiy]

On 12.07.2016 16:51, Kevin Wolf wrote:

Am 12.07.2016 um 11:36 hat Denis V. Lunev geschrieben:

From: Vladimir Sementsov-Ogievskiy 

Mirror can do up to 16 in-flight requests, but actually on full copy
(the whole source disk is non-zero) in-flight is always 1. This happens
as the request is not limited in size: the data occupies maximum available
capacity of s->buf.

The patch limits the size of the request to some artificial constant
(1 Mb here), which is not that big or small. This effectively enables
back parallelism in mirror code as it was designed.

The result is important: the time to migrate 10 Gb disk is reduced from
~350 sec to 170 sec.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Denis V. Lunev 
CC: Stefan Hajnoczi 
CC: Fam Zheng 
CC: Kevin Wolf 
CC: Max Reitz 
CC: Jeff Cody 
CC: Eric Blake 
---
  block/mirror.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 4fe127e..53d3bcd 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -23,7 +23,9 @@
  
  #define SLICE_TIME1ULL /* ns */

  #define MAX_IN_FLIGHT 16
-#define DEFAULT_MIRROR_BUF_SIZE   (10 << 20)
+#define MAX_IO_SECTORS ((1 << 20) >> BDRV_SECTOR_BITS) /* 1 Mb */
+#define DEFAULT_MIRROR_BUF_SIZE \
+(MAX_IN_FLIGHT * MAX_IO_SECTORS * BDRV_SECTOR_SIZE)
  
  /* The mirroring buffer is a list of granularity-sized chunks.

   * Free chunks are organized in a list.
@@ -387,7 +389,9 @@ static uint64_t coroutine_fn 
mirror_iteration(MirrorBlockJob *s)
nb_chunks * sectors_per_chunk,
_sectors, );
  if (ret < 0) {
-io_sectors = nb_chunks * sectors_per_chunk;
+io_sectors = MIN(nb_chunks * sectors_per_chunk, MAX_IO_SECTORS);
+} else if (ret & BDRV_BLOCK_DATA) {
+io_sectors = MIN(io_sectors, MAX_IO_SECTORS);
  }

Would it make sense to consider the actual buffer size? If we have
s->buf_size / 16 > 1 MB, then this is wasting buffer space.

On the other hand, there is probably a minimum size where using a single
larger buffer performs better than two concurrent small ones. Which size
this is, is hard to tell, though. If we assume that 1 MB is a good
default (should we do some more testing to find the sweet spot?), we
could write this as:

   io_sectors = MIN(io_sectors,
MAX((s->buf_size / BDRV_SECTOR_SIZE) / MAX_IN_FLIGHT,
MAX_IO_SECTORS))

Kevin


Ok, thanks, will resend.

--
Best regards,
Vladimir

Re: [Qemu-block] [Qemu-devel] [PATCH v5 01/10] block: Hide HBitmap in block dirty bitmap interface

[Vladimir Sementsov-Ogievskiy]

On 13.07.2016 01:49, John Snow wrote:


On 06/03/2016 12:32 AM, Fam Zheng wrote:

HBitmap is an implementation detail of block dirty bitmap that should be hidden
from users. Introduce a BdrvDirtyBitmapIter to encapsulate the underlying
HBitmapIter.

A small difference in the interface is, before, an HBitmapIter is initialized
in place, now the new BdrvDirtyBitmapIter must be dynamically allocated because
the structure definition is in block/dirty-bitmap.c.

Two current users are converted too.

Signed-off-by: Fam Zheng 
---
  block/backup.c   | 14 --
  block/dirty-bitmap.c | 39 +--
  block/mirror.c   | 24 +---
  include/block/dirty-bitmap.h |  7 +--
  include/qemu/typedefs.h  |  1 +
  5 files changed, 60 insertions(+), 25 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index feeb9f8..ac7ca45 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -317,14 +317,14 @@ static int coroutine_fn 
backup_run_incremental(BackupBlockJob *job)
  int64_t end;
  int64_t last_cluster = -1;
  int64_t sectors_per_cluster = cluster_size_sectors(job);
-HBitmapIter hbi;
+BdrvDirtyBitmapIter *dbi;
  
  granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap);

  clusters_per_iter = MAX((granularity / job->cluster_size), 1);
-bdrv_dirty_iter_init(job->sync_bitmap, );
+dbi = bdrv_dirty_iter_new(job->sync_bitmap, 0);
  
  /* Find the next dirty sector(s) */

-while ((sector = hbitmap_iter_next()) != -1) {
+while ((sector = bdrv_dirty_iter_next(dbi)) != -1) {
  cluster = sector / sectors_per_cluster;
  
  /* Fake progress updates for any clusters we skipped */

@@ -336,7 +336,7 @@ static int coroutine_fn 
backup_run_incremental(BackupBlockJob *job)
  for (end = cluster + clusters_per_iter; cluster < end; cluster++) {
  do {
  if (yield_and_check(job)) {
-return ret;
+goto out;
  }
  ret = backup_do_cow(job, cluster * sectors_per_cluster,
  sectors_per_cluster, _is_read,
@@ -344,7 +344,7 @@ static int coroutine_fn 
backup_run_incremental(BackupBlockJob *job)
  if ((ret < 0) &&
  backup_error_action(job, error_is_read, -ret) ==
  BLOCK_ERROR_ACTION_REPORT) {
-return ret;
+goto out;
  }
  } while (ret < 0);
  }
@@ -352,7 +352,7 @@ static int coroutine_fn 
backup_run_incremental(BackupBlockJob *job)
  /* If the bitmap granularity is smaller than the backup granularity,
   * we need to advance the iterator pointer to the next cluster. */
  if (granularity < job->cluster_size) {
-bdrv_set_dirty_iter(, cluster * sectors_per_cluster);
+bdrv_set_dirty_iter(dbi, cluster * sectors_per_cluster);
  }
  
  last_cluster = cluster - 1;

@@ -364,6 +364,8 @@ static int coroutine_fn 
backup_run_incremental(BackupBlockJob *job)
  job->common.offset += ((end - last_cluster - 1) * job->cluster_size);
  }
  
+out:

+bdrv_dirty_iter_free(dbi);
  return ret;
  }
  
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c

index 4902ca5..ec073ee 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -42,9 +42,15 @@ struct BdrvDirtyBitmap {
  char *name; /* Optional non-empty unique ID */
  int64_t size;   /* Size of the bitmap (Number of sectors) */
  bool disabled;  /* Bitmap is read-only */
+int active_iterators;   /* How many iterators are active */
  QLIST_ENTRY(BdrvDirtyBitmap) list;
  };
  
+struct BdrvDirtyBitmapIter {

+HBitmapIter hbi;
+BdrvDirtyBitmap *bitmap;
+};
+
  BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char 
*name)
  {
  BdrvDirtyBitmap *bm;
@@ -212,6 +218,7 @@ void bdrv_dirty_bitmap_truncate(BlockDriverState *bs)
  
  QLIST_FOREACH(bitmap, >dirty_bitmaps, list) {

  assert(!bdrv_dirty_bitmap_frozen(bitmap));
+assert(!bitmap->active_iterators);
  hbitmap_truncate(bitmap->bitmap, size);
  bitmap->size = size;
  }
@@ -224,6 +231,7 @@ static void 
bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs,
  BdrvDirtyBitmap *bm, *next;
  QLIST_FOREACH_SAFE(bm, >dirty_bitmaps, list, next) {
  if ((!bitmap || bm == bitmap) && (!only_named || bm->name)) {
+assert(!bitmap->active_iterators);

No good -- this function is also used to clear out all named bitmaps
belonging to a BDS, so 'bitmap' may be NULL, so this assertion is bad.


Just a note about this. I do not like this hidden behaviour of 
release_bitmap, as it more native for freeing functions to do nothing 
with NULL pointer.. g_free(NULL) do not free all 

Re: [Qemu-block] [Qemu-devel] [PATCH] build: Work around SIZE_MAX bug in OSX headers

[Markus Armbruster]

Peter Maydell  writes:

> On 12 July 2016 at 19:23, Eric Blake  wrote:
>> This violates POSIX, which requires that:
>> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stdint.h.html#tag_13_48
>> "Each instance of these macros shall be replaced by a constant
>> expression suitable for use in #if preprocessing directives, and this
>> expression shall have the same type as would an expression that is an
>> object of the corresponding type converted according to the integer
>> promotions."
>>
>> That is, it is valid C to write '#if SIZE_MAX == 0x', while my
>> replacement fails that test:
>>
>> foo.c:6:26: error: missing binary operator before token "("
>>  #define SIZE_MAX ((sizeof(char)) * -1)
>>   ^
>> foo.c:7:5: note: in expansion of macro ‘SIZE_MAX’
>
> I tested this patch with a compile on OSX, and it does compile
> without warnings or errors. (NB: haven't tested that it
> fixes the warning that was being complained about in the
> other patchset.)
>
> I don't have a very strong opinion about whether it's the
> best fix, but a couple of thoughts:
>  * my inclination is to prefer not to override system
>headers except where we've checked and know they're broken
>(ie in a future world where Apple get their headers right
>I'd rather we automatically ended up using their version
>rather than ours)

That's a good point.

>  * we don't have any #if ...SIZE_MAX, but we do have some
>for other kinds of _MAX constant.