Re: [Qemu-block] [PULL v2 39/40] iotests: Add tests for the x-blockdev-del command

[Stefan Hajnoczi]

On Tue, Nov 10, 2015 at 2:09 PM, Kevin Wolf  wrote:
> From: Alberto Garcia 
>
> Signed-off-by: Alberto Garcia 
> Message-id: 
> 57c3b0d4d0c73ddadd19e5bded9492c359cc4568.1446475331.git.be...@igalia.com
> Reviewed-by: Max Reitz 
> Signed-off-by: Max Reitz 
> ---
>  tests/qemu-iotests/139 | 414 
> +
>  tests/qemu-iotests/139.out |   5 +
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 420 insertions(+)
>  create mode 100644 tests/qemu-iotests/139
>  create mode 100644 tests/qemu-iotests/139.out

I'm seeing the following failure:

 ./check -qcow2 139
QEMU  -- "./qemu" -nodefaults
QEMU_IMG  -- "./qemu-img"
QEMU_IO   -- "./qemu-io"  -f qcow2 --cache writeback
QEMU_NBD  -- "./qemu-nbd"
IMGFMT-- qcow2 (compat=1.1)
IMGPROTO  -- file
PLATFORM  -- Linux/x86_64 stefanha-x1 4.2.5-300.fc23.x86_64
TEST_DIR  -- /home/stefanha/qemu/tests/qemu-iotests/scratch
SOCKET_SCM_HELPER -- /home/stefanha/qemu/tests/qemu-iotests/socket_scm_helper

139 [failed, exit status 1] - output mismatch (see 139.out.bad)
--- /home/stefanha/qemu/tests/qemu-iotests/139.out2015-11-10
14:24:03.728322694 +
+++ 139.out.bad2015-11-10 14:54:51.617899443 +
@@ -1,5 +1,19 @@
-
+F...
+==
+FAIL: testQuorum (__main__.TestBlockdevDel)
+--
+Traceback (most recent call last):
+  File "139", line 403, in testQuorum
+self.addQuorum('quorum0', 'node0', 'node1')
+  File "139", line 291, in addQuorum
+self.assert_qmp(result, 'return', {})
+  File "/home/stefanha/qemu/tests/qemu-iotests/iotests.py", line 314,
in assert_qmp
+result = self.dictpath(d, path)
+  File "/home/stefanha/qemu/tests/qemu-iotests/iotests.py", line 293,
in dictpath
+self.fail('failed path traversal for "%s" in "%s"' % (path, str(d)))
+AssertionError: failed path traversal for "return" in "{u'error':
{u'class': u'GenericError', u'desc': u"Unknown driver 'quorum'"}}"
+
 --
 Ran 12 tests

-OK
+FAILED (failures=1)
Failures: 139
Failed 1 of 1 tests

Re: [Qemu-block] [PULL v2 39/40] iotests: Add tests for the x-blockdev-del command

[Kevin Wolf]

Am 10.11.2015 um 15:59 hat Stefan Hajnoczi geschrieben:
> On Tue, Nov 10, 2015 at 2:09 PM, Kevin Wolf  wrote:
> > From: Alberto Garcia 
> >
> > Signed-off-by: Alberto Garcia 
> > Message-id: 
> > 57c3b0d4d0c73ddadd19e5bded9492c359cc4568.1446475331.git.be...@igalia.com
> > Reviewed-by: Max Reitz 
> > Signed-off-by: Max Reitz 
> > ---
> >  tests/qemu-iotests/139 | 414 
> > +
> >  tests/qemu-iotests/139.out |   5 +
> >  tests/qemu-iotests/group   |   1 +
> >  3 files changed, 420 insertions(+)
> >  create mode 100644 tests/qemu-iotests/139
> >  create mode 100644 tests/qemu-iotests/139.out
> 
> I'm seeing the following failure:
> 
>  ./check -qcow2 139
> QEMU  -- "./qemu" -nodefaults
> QEMU_IMG  -- "./qemu-img"
> QEMU_IO   -- "./qemu-io"  -f qcow2 --cache writeback
> QEMU_NBD  -- "./qemu-nbd"
> IMGFMT-- qcow2 (compat=1.1)
> IMGPROTO  -- file
> PLATFORM  -- Linux/x86_64 stefanha-x1 4.2.5-300.fc23.x86_64
> TEST_DIR  -- /home/stefanha/qemu/tests/qemu-iotests/scratch
> SOCKET_SCM_HELPER -- /home/stefanha/qemu/tests/qemu-iotests/socket_scm_helper
> 
> 139 [failed, exit status 1] - output mismatch (see 139.out.bad)
> --- /home/stefanha/qemu/tests/qemu-iotests/139.out2015-11-10
> 14:24:03.728322694 +
> +++ 139.out.bad2015-11-10 14:54:51.617899443 +
> @@ -1,5 +1,19 @@
> -
> +F...
> +==
> +FAIL: testQuorum (__main__.TestBlockdevDel)
> +--
> +Traceback (most recent call last):
> +  File "139", line 403, in testQuorum
> +self.addQuorum('quorum0', 'node0', 'node1')
> +  File "139", line 291, in addQuorum
> +self.assert_qmp(result, 'return', {})
> +  File "/home/stefanha/qemu/tests/qemu-iotests/iotests.py", line 314,
> in assert_qmp
> +result = self.dictpath(d, path)
> +  File "/home/stefanha/qemu/tests/qemu-iotests/iotests.py", line 293,
> in dictpath
> +self.fail('failed path traversal for "%s" in "%s"' % (path, str(d)))
> +AssertionError: failed path traversal for "return" in "{u'error':
> {u'class': u'GenericError', u'desc': u"Unknown driver 'quorum'"}}"
> +

I guess we need a follow-up patch for -rc1 that simply skips the test
case if quorum isn't available.

Kevin

[Qemu-block] [PATCH] iotests: Check for quorum support in test 139

[Alberto Garcia]

The quorum driver is always built in, but it is disabled during
run-time if there's no SHA256 support available (see commit e94867e).

This patch skips the quorum test in iotest 139 in that case.

Signed-off-by: Alberto Garcia 
---
 tests/qemu-iotests/139 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139
index b5470f7..42f78c7 100644
--- a/tests/qemu-iotests/139
+++ b/tests/qemu-iotests/139
@@ -400,6 +400,8 @@ class TestBlockdevDel(iotests.QMPTestCase):
 self.checkBlockDriverState('node1', False)
 
 def testQuorum(self):
+if not 'quorum' in iotests.qemu_img_pipe('--help'):
+return
 self.addQuorum('quorum0', 'node0', 'node1')
 # We cannot remove the children of a Quorum device
 self.delBlockDriverState('node0', expect_error = True)
-- 
2.6.2

Re: [Qemu-block] [PATCH v4 00/21] Extended I/O accounting

[Stefan Hajnoczi]

On Wed, Oct 28, 2015 at 05:32:57PM +0200, Alberto Garcia wrote:
> Here's v4 of the series that implements extended I/O accounting for
> block devices.
> 
> Since part of Max's BlockBackend series has already been merged, this
> series can now be applied cleanly on top of the master branch without
> additional dependencies.
> 
> Here's the summary of what this series provides:
> 
>  - New block_acct_failed() and block_acct_invalid() calls.
>We keep track now of the number of successful, failed and invalid
>operations (each one separated into read, write and flush). So from
>the API point of view, BlockDeviceStats contains 6 new fields for
>those.
> 
>  - idle_time_ns: time since the last I/O operation.
> 
>  - New BlockDeviceTimedStats struct: it has statistics for the I/O
>during a given interval of time. It keeps minimum, maximum and
>average latencies for read, write and flush operations.
> 
>It also keeps the average read and write queue depths.
> 
>  - New 'stats-intervals' option that allows the user to define the
>intervals used to keep the aforementioned statistics. An arbitrary
>number of intervals can be specified, the length of each one is in
>seconds.
> 
>For the API I opted for a colon-separated list of numbers,
> 
>   stats-intervals=60:3600:86400
> 
>I also considered something a different syntax,
> 
>   stats-intervals.0.length=60,
>   stats-intervals.1.length=3600,
>   stats-intervals.2.length=86400
> 
>This one could be useful if we want to specify any other attribute
>for each interval, but I couldn't come up with any, so I chose the
>simpler solution.
> 
>  - Two new options, stats-account-invalid and stats-account-failed,
>which allow the user to decide whether to count invalid and failed
>operations when computing the idle time and total latency.
> 
> Regards,
> 
> Berto
> 
> v4:
> - Rebase on top of the current master. This series no longer depends
>   on any other.
> - patch 8: clarify that interval_length is in seconds [Stefan]
> - patch 9: rewrite timed_average_sum() so it does not call
>   qemu_clock_get_ns() twice [Stefan]
> 
> v3: https://lists.gnu.org/archive/html/qemu-block/2015-10/msg00785.html
> - Rebased on top of the current master and on Max's BlockBackend
>   series v7
> - patch 4: minor documentation fixes [Stefan]
> - patch 5: s/miliseconds/nanoseconds/ [Stefan]
> - patch 6: dropped, there's no "supports_stats" anymore [Stefan]
> - patch 7 (now 6): explain why block_acct_invalid() does not update
>   total_time_ns[] [Stefan]
> - patch 12 (now 11): don't initialize BlockAcctCookie to { 0 }, it's
>not needed.
> 
> v2: https://lists.gnu.org/archive/html/qemu-block/2015-10/msg00161.html
> - First complete implementation of the new statistics
> 
> v1: https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg03321.html
> - Initial series containing only the timed average infrastructure.
> 
> Alberto Garcia (21):
>   xen_disk: Account for flush operations
>   ide: Account for write operations correctly
>   block: define 'clock_type' for the accounting code
>   util: Infrastructure for computing recent averages
>   block: Add idle_time_ns to BlockDeviceStats
>   block: Add statistics for failed and invalid I/O operations
>   block: Allow configuring whether to account failed and invalid ops
>   block: Compute minimum, maximum and average I/O latencies
>   block: Add average I/O queue depth to BlockDeviceTimedStats
>   block: New option to define the intervals for collecting I/O
> statistics
>   qemu-io: Account for failed, invalid and flush operations
>   block: Use QEMU_CLOCK_VIRTUAL for the accounting code in qtest mode
>   iotests: Add test for the block device statistics
>   nvme: Account for failed and invalid operations
>   virtio-blk: Account for failed and invalid operations
>   xen_disk: Account for failed and invalid operations
>   atapi: Account for failed and invalid operations
>   ide: Account for failed and invalid operations
>   macio: Account for failed operations
>   scsi-disk: Account for failed operations
>   block: Update copyright of the accounting code
> 
>  block/accounting.c   | 123 ++-
>  block/block-backend.c|   1 +
>  block/qapi.c |  51 +++
>  blockdev.c   |  53 +++
>  hmp.c|   4 +-
>  hw/block/nvme.c  |  11 +-
>  hw/block/virtio-blk.c|   4 +-
>  hw/block/xen_disk.c  |  27 +++-
>  hw/ide/atapi.c   |  31 ++--
>  hw/ide/core.c|  12 +-
>  hw/ide/macio.c   |  12 +-
>  hw/scsi/scsi-disk.c  |  46 --
>  include/block/accounting.h   |  28 
>  include/qemu/timed-average.h |  64 
>  qapi/block-core.json | 103 -
>  qemu-io-cmds.c   |   9 ++
>  qmp-commands.hx  |  80 +-
>  tests/Makefile   |   4 +
>  

Re: [Qemu-block] [Qemu-devel] [PULL v2 00/40] Block layer patches

[Peter Maydell]

On 10 November 2015 at 14:09, Kevin Wolf  wrote:
> The following changes since commit a8b4f9585a0bf5186fca793ce2c5d754cd8ec49a:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-11-10' 
> into staging (2015-11-10 09:39:24 +)
>
> are available in the git repository at:
>
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to c400bddb916268394e352f82809eb4728424a5b1:
>
>   Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-11-10' 
> into queue-block (2015-11-10 14:59:26 +0100)
>
> 
>
> Block layer patches

Fails to build on OSX :-(

/Users/pm215/src/qemu-for-merges/ui/cocoa.m:1121:40: error: too few
arguments to function call, expected 7, have 5
   );
   ^
./qmp-commands.h:61:1: note: 'qmp_blockdev_change_medium' declared here
void qmp_blockdev_change_medium(const char *device, const char
*filename, bool has_format, const char *format, bool
has_read_only_mode, BlockdevChangeReadOnlyMode read_only_mode, Error
**errp);
^
1 error generated.

Also some warnings:

/Users/pm215/src/qemu-for-merges/qemu-io-cmds.c:772:56: warning:
format specifies type 'size_t' (aka 'unsigned long') but the argument
has type 'unsigned long lo
ng' [-Wformat]
printf("length cannot exceed %zu, given %s\n", SIZE_MAX, argv[optind]);
 ~~~   ^~~~
 %llu
/usr/include/stdint.h:153:20: note: expanded from macro 'SIZE_MAX'
#define SIZE_MAX  UINT64_MAX
  ^~
/usr/include/stdint.h:87:27: note: expanded from macro 'UINT64_MAX'
#define UINT64_MAX18446744073709551615ULL
  ^~~
/Users/pm215/src/qemu-for-merges/qemu-io-cmds.c:1082:56: warning:
format specifies type 'size_t' (aka 'unsigned long') but the argument
has type 'unsigned long l
ong' [-Wformat]
printf("length cannot exceed %zu, given %s\n", SIZE_MAX, argv[optind]);
 ~~~   ^~~~
 %llu
/usr/include/stdint.h:153:20: note: expanded from macro 'SIZE_MAX'
#define SIZE_MAX  UINT64_MAX
  ^~
/usr/include/stdint.h:87:27: note: expanded from macro 'UINT64_MAX'
#define UINT64_MAX18446744073709551615ULL
  ^~~

thanks
-- PMM

Re: [Qemu-block] [PATCH v4 10/21] block: New option to define the intervals for collecting I/O statistics

[Eric Blake]

On 10/28/2015 09:33 AM, Alberto Garcia wrote:
> The BlockAcctStats structure contains a list of BlockAcctTimedStats.
> Each one of these collects statistics about the minimum, maximum and
> average latencies of all I/O operations in a certain interval of time.
> 
> This patch adds a new "stats-intervals" option that allows defining
> these intervals.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  blockdev.c   | 37 +
>  qapi/block-core.json |  4 
>  2 files changed, 41 insertions(+)

> +++ b/qapi/block-core.json
> @@ -1503,6 +1503,9 @@
>  # @stats-account-failed: #optional whether to include failed
>  # operations when computing latency and last
>  # access statistics (default: true) (Since 2.5)
> +# @stats-intervals: #optional colon-separated 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)
>  #
> @@ -1520,6 +1523,7 @@
>  '*read-only': 'bool',
>  '*stats-account-invalid': 'bool',
>  '*stats-account-failed': 'bool',
> +'*stats-intervals': 'str',

My fault for not reviewing this change to a .json file prior to the PULL
request, but I think this should be '*stats-intervals':['int'] so that
we aren't post-processing to parse out colons.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH 1/4] hw/ide: Remove superfluous return statements

[John Snow]

On 11/10/2015 03:16 PM, Thomas Huth wrote:
> The "return;" statements at the end of functions do not make
> much sense, so let's remove them.
> 
> Cc: John Snow 
> Cc: qemu-block@nongnu.org
> Signed-off-by: Thomas Huth 
> ---
>  hw/ide/atapi.c | 1 -
>  hw/ide/macio.c | 2 --
>  2 files changed, 3 deletions(-)
> 
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index 747f466..347c6e7 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -737,7 +737,6 @@ static void cmd_inquiry(IDEState *s, uint8_t *buf)
>   out:
>  buf[size_idx] = idx - preamble_len;
>  ide_atapi_cmd_reply(s, idx, max_len);
> -return;
>  }
>  
>  static void cmd_get_configuration(IDEState *s, uint8_t *buf)
> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
> index 893c9b9..8a56115 100644
> --- a/hw/ide/macio.c
> +++ b/hw/ide/macio.c
> @@ -288,8 +288,6 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int 
> ret)
>  done:
>  block_acct_done(blk_get_stats(s->blk), >acct);
>  io->dma_end(opaque);
> -
> -return;
>  }
>  
>  static void pmac_ide_transfer_cb(void *opaque, int ret)
> 

Fair enough.

Reviewed-by: John Snow 

Re: [Qemu-block] [PATCH v6 4/4] hmp: add monitor command to add/remove a child

[Wen Congyang]

On 11/09/2015 10:54 PM, Alberto Garcia wrote:
> On Fri 16 Oct 2015 10:57:46 AM CEST, Wen Congyang wrote:
> 
>> +.name   = "blockdev_change",
>> +.args_type  = "op:s,parent:B,child:B?,node:?",
>> +.params = "operation parent [child] [node]",
>   [...]
>> +/*
>> + * FIXME: we must specify the parameter child, otherwise,
>> + * we can't specify the parameter node.
>> + */
>> +if (op == CHANGE_OPERATION_ADD) {
>> +has_child = false;
>> +}
> 
> So if you want to perform the 'add' operation you must pass both 'child'
> and 'node' but the former will be discarded.
> 
> I don't think you really need to do this for the HMP interface, but it's
> anyway one more good reason to merge 'child' and 'node'.

Do you mean there is no need to implement the HMP interface?

Thanks
Wen Congyang

> 
> Berto
> .
> 

Re: [Qemu-block] [PATCH] mirror: Improve zero-write and discard with fragmented image

[Paolo Bonzini]

On 10/11/2015 07:14, Fam Zheng wrote:
> On Mon, 11/09 17:29, Kevin Wolf wrote:
>> Am 09.11.2015 um 17:18 hat Paolo Bonzini geschrieben:
>>>
>>>
>>> On 09/11/2015 17:04, Kevin Wolf wrote:
 Am 06.11.2015 um 11:22 hat Fam Zheng geschrieben:
> The "pnum < nb_sectors" condition in deciding whether to actually copy
> data is unnecessarily strict, and the qiov initialization is
> unnecessarily too, for both bdrv_aio_write_zeroes and bdrv_aio_discard
> branches.
>
> Reorganize mirror_iteration flow so that we:
>
> 1) Find the contiguous zero/discarded sectors with
> bdrv_get_block_status_above() before deciding what to do. We query
> s->buf_size sized blocks at a time.
>
> 2) If the sectors in question are zeroed/discarded and aligned to
> target cluster, issue zero write or discard accordingly. It's done
> in mirror_do_zero_or_discard, where we don't add buffer to qiov.
>
> 3) Otherwise, do the same loop as before in mirror_do_read.
>
> Signed-off-by: Fam Zheng 

 I'm not sure where in the patch to comment on this, so I'll just do it
 here right in the beginning.

 I'm concerned that we need to be more careful about races in this patch,
 in particular regarding the bitmaps. I think the conditions for the two
 bitmaps are:

 * Dirty bitmap: We must clear the bit after finding the next piece of
   data to be mirrored, but before we yield after getting information
   that we use for the decision which kind of operation we need.

   In other words, we need to clear the dirty bitmap bit before calling
   bdrv_get_block_status_above(), because that's both the function that
   retrieves information about the next chunk and also a function that
   can yield.

   If after this point the data is written to, we need to mirror it
   again.
>>>
>>> With Fam's patch, that's not trivial for two reasons:
>>>
>>> 1) bdrv_get_block_status_above() can return a smaller amount than what
>>> is asked.
>>>
>>> 2) the "read and write" case can handle s->granularity sectors per
>>> iteration (many of them can be coalesced, but still that's how the
>>> iteration works).
>>>
>>> The simplest solution is to perform the query with s->granularity size
>>> rather than s->buf_size.
>>
>> Then we end up with many small operations, that's not what we want.
>>
>> Why can't we mark up to s->buf_size dirty clusters as clean first, then
>> query the status, and mark all of those that we can't handle dirty
>> again?
> 
> Then we may end up marking more clusters as dirty than it should be.

You're both right.

> Because all bdrv_set_dirty() and bdrv_set_dirty_bitmap() callers are 
> coroutine,
> we can introduce a CoMutex to let bitmap reader block bdrv_set_dirty and
> bdrv_set_dirty_bitmap.

I think this is not necessary.

I think the following is safe:

1) before calling bdrv_get_block_status_above(), find out how many
consecutive bits in the dirty bitmap are 1

2) zero all those bits in the dirty bitmap

3) call bdrv_get_block_status_above() with a size equivalent to the
number of dirty bits

4) if bdrv_get_block_status_above() only returns a partial result, loop
step (3) until all the dirty bits are processed

For full mirroring, this strategy will probably make the first
incremental iteration more expensive.

Paolo

Re: [Qemu-block] [PATCH] mirror: Improve zero-write and discard with fragmented image

[Paolo Bonzini]

On 10/11/2015 11:12, Kevin Wolf wrote:
> > For full mirroring, this strategy will probably make the first
> > incremental iteration more expensive.
>
> You mean because we issue smaller, interleaved write and write_zeroes
> requests now instead of only large writes? That's probably right, but
> getting the right result should be more important than speed. :-)

No, because you might end up clearing the whole dirty bitmap before
issuing the first bdrv_get_block_status_above().  Blocks are actually
read much later; if someone sets the dirty bitmap in between, you will
re-write those blocks unnecessarily during the first incremental
iteration.  It's not specific to the first iteration, it's just more likely.

However, it may be enough to clamp the number of dirty bitmap bits that
you process in one go (e.g. to 100 MB of so).

Paolo

[Qemu-block] [PATCH v3 2/2] mirror: Improve zero-write and discard with fragmented image

[Fam Zheng]

The "pnum < nb_sectors" condition in deciding whether to actually copy
data is unnecessarily strict, and the qiov initialization is
unnecessarily too, for both bdrv_aio_write_zeroes and bdrv_aio_discard
branches.

Reorganize mirror_iteration flow so that we:

1) Find the contiguous zero/discarded sectors with
bdrv_get_block_status_above() before deciding what to do. We query
s->buf_size sized blocks at a time.

2) If the sectors in question are zeroed/discarded and aligned to
target cluster, issue zero write or discard accordingly. It's done
in mirror_do_zero_or_discard, where we don't add buffer to qiov.

3) Otherwise, do the same loop as before in mirror_do_read.

Signed-off-by: Fam Zheng 
---
 block/mirror.c | 160 +
 1 file changed, 128 insertions(+), 32 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index b1252a1..ade0412 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -157,23 +157,13 @@ static void mirror_read_complete(void *opaque, int ret)
 mirror_write_complete, op);
 }
 
-static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
+static uint64_t mirror_do_read(MirrorBlockJob *s)
 {
 BlockDriverState *source = s->common.bs;
-int nb_sectors, sectors_per_chunk, nb_chunks;
-int64_t end, sector_num, next_chunk, next_sector, hbitmap_next_sector;
+int sectors_per_chunk, nb_sectors, nb_chunks;
+int64_t end, next_chunk, next_sector, hbitmap_next_sector, sector_num;
 uint64_t delay_ns = 0;
 MirrorOp *op;
-int pnum;
-int64_t ret;
-
-s->sector_num = hbitmap_iter_next(>hbi);
-if (s->sector_num < 0) {
-bdrv_dirty_iter_init(s->dirty_bitmap, >hbi);
-s->sector_num = hbitmap_iter_next(>hbi);
-trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap));
-assert(s->sector_num >= 0);
-}
 
 hbitmap_next_sector = s->sector_num;
 sector_num = s->sector_num;
@@ -198,14 +188,6 @@ static uint64_t coroutine_fn 
mirror_iteration(MirrorBlockJob *s)
 next_sector = sector_num;
 next_chunk = sector_num / sectors_per_chunk;
 
-/* Wait for I/O to this cluster (from a previous iteration) to be done.  */
-while (test_bit(next_chunk, s->in_flight_bitmap)) {
-trace_mirror_yield_in_flight(s, sector_num, s->in_flight);
-s->waiting_for_io = true;
-qemu_coroutine_yield();
-s->waiting_for_io = false;
-}
-
 do {
 int added_sectors, added_chunks;
 
@@ -301,24 +283,138 @@ static uint64_t coroutine_fn 
mirror_iteration(MirrorBlockJob *s)
 s->sectors_in_flight += nb_sectors;
 trace_mirror_one_iteration(s, sector_num, nb_sectors);
 
-ret = bdrv_get_block_status_above(source, NULL, sector_num,
-  nb_sectors, );
-if (ret < 0 || pnum < nb_sectors ||
-(ret & BDRV_BLOCK_DATA && !(ret & BDRV_BLOCK_ZERO))) {
-bdrv_aio_readv(source, sector_num, >qiov, nb_sectors,
-   mirror_read_complete, op);
-} else if (ret & BDRV_BLOCK_ZERO) {
+bdrv_aio_readv(source, sector_num, >qiov, nb_sectors,
+   mirror_read_complete, op);
+return delay_ns;
+}
+
+static uint64_t mirror_do_zero_or_discard(MirrorBlockJob *s,
+  int64_t sector_num,
+  int nb_sectors,
+  bool is_discard)
+{
+int sectors_per_chunk, nb_chunks;
+int64_t next_chunk, next_sector, hbitmap_next_sector;
+uint64_t delay_ns = 0;
+MirrorOp *op;
+
+sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
+assert(nb_sectors >= sectors_per_chunk);
+next_chunk = sector_num / sectors_per_chunk;
+nb_chunks = DIV_ROUND_UP(nb_sectors, sectors_per_chunk);
+bitmap_set(s->in_flight_bitmap, next_chunk, nb_chunks);
+delay_ns = ratelimit_calculate_delay(>limit, nb_sectors);
+
+/* Allocate a MirrorOp that is used as an AIO callback. The qiov is zeroed
+ * out so the freeing in iteration is nop. */
+op = g_new0(MirrorOp, 1);
+op->s = s;
+op->sector_num = sector_num;
+op->nb_sectors = nb_sectors;
+
+/* Advance the HBitmapIter in parallel, so that we do not examine
+ * the same sector twice.
+ */
+hbitmap_next_sector = sector_num;
+next_sector = sector_num + nb_sectors;
+while (next_sector > hbitmap_next_sector) {
+hbitmap_next_sector = hbitmap_iter_next(>hbi);
+if (hbitmap_next_sector < 0) {
+break;
+}
+}
+
+bdrv_reset_dirty_bitmap(s->dirty_bitmap, sector_num, nb_sectors);
+s->in_flight++;
+s->sectors_in_flight += nb_sectors;
+if (is_discard) {
+bdrv_aio_discard(s->target, sector_num, op->nb_sectors,
+ mirror_write_complete, op);
+} else {
 bdrv_aio_write_zeroes(s->target, sector_num, op->nb_sectors,
  

Re: [Qemu-block] [PATCH v3] virtio-blk: trivial code optimization

[Stefan Hajnoczi]

On Tue, Nov 10, 2015 at 02:35:19PM +0800, Gonglei wrote:
> On 2015/11/9 21:57, Stefan Hajnoczi wrote:
> > On Mon, Nov 09, 2015 at 05:03:30PM +0800, arei.gong...@huawei.com wrote:
> >> From: Gonglei 
> >>
> >> 1. avoid possible superflous checking
> >> 2. make code more robustness
> >>
> >> Signed-off-by: Gonglei 
> >> Reviewed-by: Fam Zheng 
> >> ---
> >> v3: change the third condition too [Paolo]
> >> add Fam's R-by
> >> ---
> >>  hw/block/virtio-blk.c | 27 +--
> >>  1 file changed, 9 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> >> index 093e475..9124358 100644
> >> --- a/hw/block/virtio-blk.c
> >> +++ b/hw/block/virtio-blk.c
> >> @@ -404,24 +404,15 @@ void virtio_blk_submit_multireq(BlockBackend *blk, 
> >> MultiReqBuffer *mrb)
> >>  for (i = 0; i < mrb->num_reqs; i++) {
> >>  VirtIOBlockReq *req = mrb->reqs[i];
> >>  if (num_reqs > 0) {
> >> -bool merge = true;
> >> -
> >> -/* merge would exceed maximum number of IOVs */
> >> -if (niov + req->qiov.niov > IOV_MAX) {
> >> -merge = false;
> >> -}
> >> -
> >> -/* merge would exceed maximum transfer length of backend 
> >> device */
> >> -if (req->qiov.size / BDRV_SECTOR_SIZE + nb_sectors > 
> >> max_xfer_len) {
> >> -merge = false;
> >> -}
> >> -
> >> -/* requests are not sequential */
> >> -if (sector_num + nb_sectors != req->sector_num) {
> >> -merge = false;
> >> -}
> >> -
> >> -if (!merge) {
> >> +/*
> >> + * NOTE: We cannot merge the requests in below situations:
> >> + * 1. requests are not sequential
> >> + * 2. merge would exceed maximum number of IOVs
> >> + * 3. merge would exceed maximum transfer length of backend 
> >> device
> >> + */
> >> +if (sector_num + nb_sectors != req->sector_num ||
> >> +niov > IOV_MAX - req->qiov.niov ||
> >> +nb_sectors > max_xfer_len - req->qiov.size / 
> >> BDRV_SECTOR_SIZE) {
> > 
> > nb_sectors - int
> > max_xfer_len - int
> > req->qiov.size - size_t
> > BDRV_SECTOR_SIZE - unsigned long long
> > 
> > Therefore this expression is an int > unsigned long long comparison.
> > 
> Sorry, I'm confused.
> max_xfer_len is int,
> "req->qiov.size / BDRV_SECTOR_SIZE" is  unsigned long long,
> so, "max_xfer_len - req->qiov.size / BDRV_SECTOR_SIZE" will be int,

The type of "max_xfer_len - req->qiov.size / BDRV_SECTOR_SIZE" cannot be
int because you said req->qiov.size / BDRV_SECTOR_SIZE" is  unsigned
long long.

The C99 standard says:

6.3.1.1 Boolean, characters, and integers

- The rank of a signed integer type shall be greater than the rank of
any signed integer type with less precision.

...

- The rank of any unsigned integer type shall equal the rank of the
corresponding signed integer type, if any.

6.3.1.8 Usual arithmetic conversions

Otherwise, if the operand that has unsigned integer type has rank
greater or equal to the rank of the type of the other operand, then the
operand with signed integer type is converted to the type of the operand
with unsigned integer type.

So the max_xfer_len int operand must be converted to the higher ranking
unsigned long long.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-block] [PATCH] mirror: Improve zero-write and discard with fragmented image

[Kevin Wolf]

Am 10.11.2015 um 10:01 hat Paolo Bonzini geschrieben:
> 
> 
> On 10/11/2015 07:14, Fam Zheng wrote:
> > On Mon, 11/09 17:29, Kevin Wolf wrote:
> >> Am 09.11.2015 um 17:18 hat Paolo Bonzini geschrieben:
> >>>
> >>>
> >>> On 09/11/2015 17:04, Kevin Wolf wrote:
>  Am 06.11.2015 um 11:22 hat Fam Zheng geschrieben:
> > The "pnum < nb_sectors" condition in deciding whether to actually copy
> > data is unnecessarily strict, and the qiov initialization is
> > unnecessarily too, for both bdrv_aio_write_zeroes and bdrv_aio_discard
> > branches.
> >
> > Reorganize mirror_iteration flow so that we:
> >
> > 1) Find the contiguous zero/discarded sectors with
> > bdrv_get_block_status_above() before deciding what to do. We query
> > s->buf_size sized blocks at a time.
> >
> > 2) If the sectors in question are zeroed/discarded and aligned to
> > target cluster, issue zero write or discard accordingly. It's done
> > in mirror_do_zero_or_discard, where we don't add buffer to qiov.
> >
> > 3) Otherwise, do the same loop as before in mirror_do_read.
> >
> > Signed-off-by: Fam Zheng 
> 
>  I'm not sure where in the patch to comment on this, so I'll just do it
>  here right in the beginning.
> 
>  I'm concerned that we need to be more careful about races in this patch,
>  in particular regarding the bitmaps. I think the conditions for the two
>  bitmaps are:
> 
>  * Dirty bitmap: We must clear the bit after finding the next piece of
>    data to be mirrored, but before we yield after getting information
>    that we use for the decision which kind of operation we need.
> 
>    In other words, we need to clear the dirty bitmap bit before calling
>    bdrv_get_block_status_above(), because that's both the function that
>    retrieves information about the next chunk and also a function that
>    can yield.
> 
>    If after this point the data is written to, we need to mirror it
>    again.
> >>>
> >>> With Fam's patch, that's not trivial for two reasons:
> >>>
> >>> 1) bdrv_get_block_status_above() can return a smaller amount than what
> >>> is asked.
> >>>
> >>> 2) the "read and write" case can handle s->granularity sectors per
> >>> iteration (many of them can be coalesced, but still that's how the
> >>> iteration works).
> >>>
> >>> The simplest solution is to perform the query with s->granularity size
> >>> rather than s->buf_size.
> >>
> >> Then we end up with many small operations, that's not what we want.
> >>
> >> Why can't we mark up to s->buf_size dirty clusters as clean first, then
> >> query the status, and mark all of those that we can't handle dirty
> >> again?
> > 
> > Then we may end up marking more clusters as dirty than it should be.
> 
> You're both right.
> 
> > Because all bdrv_set_dirty() and bdrv_set_dirty_bitmap() callers are 
> > coroutine,
> > we can introduce a CoMutex to let bitmap reader block bdrv_set_dirty and
> > bdrv_set_dirty_bitmap.
> 
> I think this is not necessary.
> 
> I think the following is safe:
> 
> 1) before calling bdrv_get_block_status_above(), find out how many
> consecutive bits in the dirty bitmap are 1
> 
> 2) zero all those bits in the dirty bitmap
> 
> 3) call bdrv_get_block_status_above() with a size equivalent to the
> number of dirty bits
> 
> 4) if bdrv_get_block_status_above() only returns a partial result, loop
> step (3) until all the dirty bits are processed

Right, you can always implement one iteration with more than one I/O
request. And maybe that would be the time to start a coroutine for the
requests already in the mirror code instead of complicating the AIO
state machine and letting block.c start coroutines.

> For full mirroring, this strategy will probably make the first
> incremental iteration more expensive.

You mean because we issue smaller, interleaved write and write_zeroes
requests now instead of only large writes? That's probably right, but
getting the right result should be more important than speed. :-)

Kevin

[Qemu-block] [PATCH v3 1/2] block: Introduce coroutine lock to dirty bitmap

[Fam Zheng]

Typically, what a dirty bit consumer does is 1) get the next dirty
sectors; 2) do something with the sectors; 3) clear the dirty bits; 4)
goto 1). This works as long as 2) is simple and atomic in the coroutine
sense.  Anything sophisticated requires either moving 3) before 2) or
using locks, because the dirty bits may get cleared in the middle when
the coroutine yield.

This will be the case for mirror.c in following patches, so introduce
CoMutex in BdrvDirtyBitmap to allowing blocking the producer.

Also mark all involved dirty bitmap functions as coroutine_fn.

Signed-off-by: Fam Zheng 
---
 block.c   | 26 +-
 include/block/block.h |  6 --
 include/block/block_int.h |  4 +++-
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index e9f40dc..34a4109 100644
--- a/block.c
+++ b/block.c
@@ -69,6 +69,7 @@ struct BdrvDirtyBitmap {
 int64_t size;   /* Size of the bitmap (Number of sectors) */
 bool disabled;  /* Bitmap is read-only */
 QLIST_ENTRY(BdrvDirtyBitmap) list;
+CoMutex lock;
 };
 
 #define NOT_DONE 0x7fff /* used while emulated sync operation in progress 
*/
@@ -3173,6 +3174,7 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs,
 bitmap->size = bitmap_size;
 bitmap->name = g_strdup(name);
 bitmap->disabled = false;
+qemu_co_mutex_init(>lock);
 QLIST_INSERT_HEAD(>dirty_bitmaps, bitmap, list);
 return bitmap;
 }
@@ -3385,11 +3387,24 @@ void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, 
HBitmapIter *hbi)
 hbitmap_iter_init(hbi, bitmap->bitmap, 0);
 }
 
-void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
-   int64_t cur_sector, int nr_sectors)
+void coroutine_fn bdrv_lock_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+qemu_co_mutex_lock(>lock);
+}
+
+void coroutine_fn bdrv_unlock_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+qemu_co_mutex_unlock(>lock);
+}
+
+void coroutine_fn bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
+int64_t cur_sector,
+int nr_sectors)
 {
 assert(bdrv_dirty_bitmap_enabled(bitmap));
+bdrv_lock_dirty_bitmap(bitmap);
 hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
+bdrv_unlock_dirty_bitmap(bitmap);
 }
 
 void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
@@ -3405,15 +3420,16 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap)
 hbitmap_reset_all(bitmap->bitmap);
 }
 
-void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
-int nr_sectors)
+void coroutine_fn bdrv_set_dirty(BlockDriverState *bs,
+ int64_t cur_sector,
+ int nr_sectors)
 {
 BdrvDirtyBitmap *bitmap;
 QLIST_FOREACH(bitmap, >dirty_bitmaps, list) {
 if (!bdrv_dirty_bitmap_enabled(bitmap)) {
 continue;
 }
-hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
+bdrv_set_dirty_bitmap(bitmap, cur_sector, nr_sectors);
 }
 }
 
diff --git a/include/block/block.h b/include/block/block.h
index 610db92..592f317 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -488,9 +488,11 @@ uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap 
*bitmap);
 bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
 DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap);
+void coroutine_fn bdrv_lock_dirty_bitmap(BdrvDirtyBitmap *bitmap);
+void coroutine_fn bdrv_unlock_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t 
sector);
-void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
-   int64_t cur_sector, int nr_sectors);
+void coroutine_fn bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
+int64_t cur_sector, int nr_sectors);
 void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
  int64_t cur_sector, int nr_sectors);
 void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3ceeb5a..e17712c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -672,7 +672,9 @@ bool blk_dev_is_tray_open(BlockBackend *blk);
 bool blk_dev_is_medium_locked(BlockBackend *blk);
 void blk_dev_resize_cb(BlockBackend *blk);
 
-void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
+void coroutine_fn bdrv_set_dirty(BlockDriverState *bs,
+ int64_t cur_sector,
+ int nr_sectors);
 bool bdrv_requests_pending(BlockDriverState *bs);
 
 #endif /* BLOCK_INT_H */
-- 
2.4.3

[Qemu-block] [PATCH v3 0/2] mirror: Improve zero write and discard

[Fam Zheng]

The first patch adds a lock between bdrv_set_dirty{,_bitmap} and non-atomic
(coroutine) readers,

The second patch makes use of it and fixes mirror thin writing.

Fam Zheng (2):
  block: Introduce coroutine lock to dirty bitmap
  mirror: Improve zero-write and discard with fragmented image

 block.c   |  26 ++--
 block/mirror.c| 160 --
 include/block/block.h |   6 +-
 include/block/block_int.h |   4 +-
 4 files changed, 156 insertions(+), 40 deletions(-)

-- 
2.4.3

Re: [Qemu-block] [Qemu-devel] [PATCH v6 3/4] qmp: add monitor command to add/remove a child

[Markus Armbruster]

Wen Congyang  writes:

> On 11/09/2015 10:42 PM, Alberto Garcia wrote:
>> Sorry again for the late review, here are my comments:
>> 
>> On Fri 16 Oct 2015 10:57:45 AM CEST, Wen Congyang wrote:
>>> +void qmp_x_blockdev_change(ChangeOperation op, const char *parent,
>>> +   bool has_child, const char *child,
>>> +   bool has_new_node, const char *new_node,
>>> +   Error **errp)
>> 
>> You are using different names for the parameters here: 'op', 'parent',
>> 'child', 'new_node'; in the JSON file the first and last one are named
>> 'operation' and 'node'.
>
> OK, I will fix it in the next version
>
>> 
>>> +parent_bs = bdrv_lookup_bs(parent, parent, _err);
>>> +if (!parent_bs) {
>>> +error_propagate(errp, local_err);
>>> +return;
>>> +}
>> 
>> You don't need to change it if you don't want but you can use errp
>> directly here and spare the error_propagate() call.
>
> Too many codes in qemu use local_err and error_propagate(). I think
> errp can be NOT NULL here(in which case?).

It's usually advisable not to rely on "all callers pass non-null value
to parameter errp" arguments, because they're non-local and tend to be
brittle.

error.h attempts to provide guidance:

 * Receive an error and pass it on to the caller:
 * Error *err = NULL;
 * foo(arg, );
 * if (err) {
 * handle the error...
 * error_propagate(errp, err);
 * }
 * where Error **errp is a parameter, by convention the last one.
 *
 * Do *not* "optimize" this to
 * foo(arg, errp);
 * if (*errp) { // WRONG!
 * handle the error...
 * }
 * because errp may be NULL!
 *
 * But when all you do with the error is pass it on, please use
 * foo(arg, errp);
 * for readability.

Since all you do with local_err in the quoted code snippet is pass it
on, the last paragraph applies, and you can simplify to:

parent_bs = bdrv_lookup_bs(parent, parent, errp);
if (!parent_bs) {
return;
}

Whether errp can be null doesn't matter.

[...]

[Qemu-block] [PATCH v4] virtio-blk: trivial code optimization

[arei.gonglei]

From: Gonglei 

1. avoid possible superflous checking
2. make code more robustness

Signed-off-by: Gonglei 
---
 v4: address possible integer underover [Stefan]
   please review again, thanks
---
 hw/block/virtio-blk.c | 28 ++--
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 093e475..433ff92 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -404,24 +404,16 @@ void virtio_blk_submit_multireq(BlockBackend *blk, 
MultiReqBuffer *mrb)
 for (i = 0; i < mrb->num_reqs; i++) {
 VirtIOBlockReq *req = mrb->reqs[i];
 if (num_reqs > 0) {
-bool merge = true;
-
-/* merge would exceed maximum number of IOVs */
-if (niov + req->qiov.niov > IOV_MAX) {
-merge = false;
-}
-
-/* merge would exceed maximum transfer length of backend device */
-if (req->qiov.size / BDRV_SECTOR_SIZE + nb_sectors > max_xfer_len) 
{
-merge = false;
-}
-
-/* requests are not sequential */
-if (sector_num + nb_sectors != req->sector_num) {
-merge = false;
-}
-
-if (!merge) {
+/*
+ * NOTE: We cannot merge the requests in below situations:
+ * 1. requests are not sequential
+ * 2. merge would exceed maximum number of IOVs
+ * 3. merge would exceed maximum transfer length of backend device
+ */
+if (sector_num + nb_sectors != req->sector_num ||
+niov > IOV_MAX - req->qiov.niov ||
+req->qiov.size / BDRV_SECTOR_SIZE > max_xfer_len ||
+nb_sectors > max_xfer_len - req->qiov.size / BDRV_SECTOR_SIZE) 
{
 submit_requests(blk, mrb, start, num_reqs, niov);
 num_reqs = 0;
 }
-- 
1.7.12.4

[Qemu-block] [PATCH v11 19/28] qapi: Change munging of CamelCase enum values

[Eric Blake]

When munging enum values, the fact that we were passing the entire
prefix + value through camel_to_upper() meant that enum values
spelled with CamelCase could be turned into CAMEL_CASE.  However,
this provides a potential collision (both OneTwo and One-Two would
munge into ONE_TWO).  By changing the generation of enum constants
to always be prefix + '_' + c_name(value).upper(), we can avoid
any risk of collisions (if we can also ensure no case collisions,
in the next patch) without having to think about what the
heuristics in camel_to_upper() will actually do to the value.

Thankfully, only two enums are affected: ErrorClass and InputButton.

Suggested by: Markus Armbruster 
Signed-off-by: Eric Blake 

---
v11: new patch
---
 backends/rng-egd.c|  2 +-
 balloon.c |  4 ++--
 block.c   |  2 +-
 blockdev-nbd.c|  2 +-
 blockdev.c| 24 
 docs/writing-qmp-commands.txt |  2 +-
 hmp.c |  6 +++---
 hw/input/hid.c|  4 ++--
 hw/input/ps2.c|  4 ++--
 hw/input/virtio-input-hid.c   |  4 ++--
 include/qapi/error.h  |  6 +++---
 include/ui/qemu-spice.h   |  2 +-
 monitor.c |  8 
 net/net.c |  4 ++--
 qapi/qmp-dispatch.c   |  2 +-
 qdev-monitor.c|  4 ++--
 qmp.c |  8 
 qom/object.c  |  4 ++--
 scripts/qapi.py   |  2 +-
 ui/cocoa.m|  4 ++--
 ui/gtk.c  |  4 ++--
 ui/input-legacy.c |  4 ++--
 ui/input.c|  2 +-
 ui/sdl.c  |  4 ++--
 ui/sdl2.c |  4 ++--
 ui/spice-input.c  |  4 ++--
 ui/vnc.c  |  4 ++--
 util/error.c  |  6 +++---
 28 files changed, 65 insertions(+), 65 deletions(-)

diff --git a/backends/rng-egd.c b/backends/rng-egd.c
index 6c13409..d92af31 100644
--- a/backends/rng-egd.c
+++ b/backends/rng-egd.c
@@ -147,7 +147,7 @@ static void rng_egd_opened(RngBackend *b, Error **errp)

 s->chr = qemu_chr_find(s->chr_name);
 if (s->chr == NULL) {
-error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+error_set(errp, ERROR_CLASS_DEVICENOTFOUND,
   "Device '%s' not found", s->chr_name);
 return;
 }
diff --git a/balloon.c b/balloon.c
index 0f45d1b..8aab0b8 100644
--- a/balloon.c
+++ b/balloon.c
@@ -51,12 +51,12 @@ void qemu_balloon_inhibit(bool state)
 static bool have_balloon(Error **errp)
 {
 if (kvm_enabled() && !kvm_has_sync_mmu()) {
-error_set(errp, ERROR_CLASS_KVM_MISSING_CAP,
+error_set(errp, ERROR_CLASS_KVMMISSINGCAP,
   "Using KVM without synchronous MMU, balloon unavailable");
 return false;
 }
 if (!balloon_event_fn) {
-error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
+error_set(errp, ERROR_CLASS_DEVICENOTACTIVE,
   "No balloon device has been activated");
 return false;
 }
diff --git a/block.c b/block.c
index 53a978a..0e0058d 100644
--- a/block.c
+++ b/block.c
@@ -2584,7 +2584,7 @@ void bdrv_add_key(BlockDriverState *bs, const char *key, 
Error **errp)
 }
 } else {
 if (bdrv_key_required(bs)) {
-error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED,
+error_set(errp, ERROR_CLASS_DEVICEENCRYPTED,
   "'%s' (%s) is encrypted",
   bdrv_get_device_or_node_name(bs),
   bdrv_get_encrypted_filename(bs));
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index bcdd18b..6184eef 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -90,7 +90,7 @@ void qmp_nbd_server_add(const char *device, bool 
has_writable, bool writable,

 blk = blk_by_name(device);
 if (!blk) {
-error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+error_set(errp, ERROR_CLASS_DEVICENOTFOUND,
   "Device '%s' not found", device);
 return;
 }
diff --git a/blockdev.c b/blockdev.c
index 97be42f..115f46e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1215,7 +1215,7 @@ SnapshotInfo 
*qmp_blockdev_snapshot_delete_internal_sync(const char *device,

 blk = blk_by_name(device);
 if (!blk) {
-error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+error_set(errp, ERROR_CLASS_DEVICENOTFOUND,
   "Device '%s' not found", device);
 return NULL;
 }
@@ -1411,7 +1411,7 @@ static void internal_snapshot_prepare(BlkTransactionState 
*common,
 /* 2. check for validation */
 blk = blk_by_name(device);
 if (!blk) {
-error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+error_set(errp, ERROR_CLASS_DEVICENOTFOUND,
   "Device '%s' not found", device);
 return;
 }
@@ -1697,7 +1697,7 @@ static void 

[Qemu-block] [PATCH v11 21/28] qapi: Convert qtype_code into qapi enum type

[Eric Blake]

What's more meta than using qapi to define qapi? :)

Convert qtype_code into a full-fledged[*] builtin qapi enum type,
so that a subsequent patch can then use it as the discriminator
type of qapi alternate types.  Doing so is easiest when renaming
it to qapi conventions, as QTypeCode.  Fortunately, there are not
many places in the tree that were actually spelling the type name
out, and the judicious use of 'prefix' in the qapi defintion
avoids churn to the spelling of the enum constants.

To avoid circular definitions, we have to flip the order of
inclusion between "qobject.h" vs. "qapi-types.h".  Back in commit
28770e0, we had the latter include the former, so that we could
use 'QObject *' for our implementation of 'any'.  But that usage
also works with only a forward declaration, whereas the
definition of QType requires QTypeCode to be a complete type.

[*] The type has to be builtin, rather than declared in
qapi/common.json, because we want to use it for alternates even
when common.json is not included. But since it is the first
builtin enum type, we have to add special cases to qapi-types
and qapi-visit to only emit definitions once, even when two
qapi files are being compiled into the same binary (the way we
already handled builtin list types like 'intList').  We may
need to revisit how multiple qapi files share common types,
but that's a project for another day.

Signed-off-by: Eric Blake 

---
v11: new patch
---
 block/qapi.c |  4 ++--
 docs/qapi-code-gen.txt   |  1 +
 include/hw/qdev-core.h   |  2 +-
 include/qapi/qmp/qobject.h   | 19 +++
 qobject/qdict.c  |  2 +-
 scripts/qapi-types.py| 13 ++---
 scripts/qapi-visit.py| 10 --
 scripts/qapi.py  |  7 ++-
 tests/qapi-schema/alternate-empty.out|  2 ++
 tests/qapi-schema/comments.out   |  2 ++
 tests/qapi-schema/empty.out  |  2 ++
 tests/qapi-schema/event-case.out |  2 ++
 tests/qapi-schema/flat-union-empty.out   |  2 ++
 tests/qapi-schema/ident-with-escape.out  |  2 ++
 tests/qapi-schema/include-relpath.out|  2 ++
 tests/qapi-schema/include-repetition.out |  2 ++
 tests/qapi-schema/include-simple.out |  2 ++
 tests/qapi-schema/indented-expr.out  |  2 ++
 tests/qapi-schema/qapi-schema-test.out   |  2 ++
 tests/qapi-schema/union-clash-data.out   |  2 ++
 tests/qapi-schema/union-empty.out|  2 ++
 21 files changed, 58 insertions(+), 26 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index ec0f513..4211f11 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -539,7 +539,7 @@ static void dump_qlist(fprintf_function func_fprintf, void 
*f, int indentation,
 int i = 0;

 for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
-qtype_code type = qobject_type(entry->value);
+QTypeCode type = qobject_type(entry->value);
 bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
 const char *format = composite ? "%*s[%i]:\n" : "%*s[%i]: ";

@@ -557,7 +557,7 @@ static void dump_qdict(fprintf_function func_fprintf, void 
*f, int indentation,
 const QDictEntry *entry;

 for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) {
-qtype_code type = qobject_type(entry->value);
+QTypeCode type = qobject_type(entry->value);
 bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
 const char *format = composite ? "%*s%s:\n" : "%*s%s: ";
 char key[strlen(entry->key) + 1];
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 54a6a7b..35301c5 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -163,6 +163,7 @@ The following types are predefined, and map to C as follows:
accepts size suffixes
   bool  bool   JSON true or false
   any   QObject *  any JSON value
+  QTypeCode QTypeCode  JSON string of enum QTypeCode values


 === Includes ===
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 8057aed..8e7df8e 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -239,7 +239,7 @@ struct Property {
 PropertyInfo *info;
 int  offset;
 uint8_t  bitnr;
-qtype_code   qtype;
+QTypeCodeqtype;
 int64_t  defval;
 int  arrayoffset;
 PropertyInfo *arrayinfo;
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index 4b96ed5..8d6322b 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -34,23 +34,10 @@

 #include 
 #include 
-
-typedef enum {
-QTYPE_NONE,/* sentinel value, no QObject has this type code */
-QTYPE_QNULL,
-QTYPE_QINT,
-QTYPE_QSTRING,
-QTYPE_QDICT,
-QTYPE_QLIST,
-QTYPE_QFLOAT,
-QTYPE_QBOOL,
-QTYPE_MAX,
-} qtype_code;
-
-struct QObject;
+#include 

[Qemu-block] [PATCH v11 18/28] qerror: more error_setg() usage

[Eric Blake]

A few uses of error_set(ERROR_CLASS_GENERIC_ERROR) have snuck in
since c6bd8c706.  Nuke them.

Signed-off-by: Eric Blake 

---
v11: new patch
---
 block.c   |  3 +--
 docs/writing-qmp-commands.txt | 20 +---
 hw/i386/pc.c  |  2 +-
 hw/net/rocker/rocker.c|  6 ++
 hw/net/rocker/rocker_of_dpa.c | 12 
 qom/object.c  |  4 ++--
 6 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/block.c b/block.c
index e9f40dc..53a978a 100644
--- a/block.c
+++ b/block.c
@@ -1795,8 +1795,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
BlockReopenQueue *queue,

 ret = bdrv_flush(reopen_state->bs);
 if (ret) {
-error_set(errp, ERROR_CLASS_GENERIC_ERROR, "Error (%s) flushing drive",
-  strerror(-ret));
+error_setg_errno(errp, -ret, "Error flushing drive");
 goto error;
 }

diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
index 8647cac..59aa77a 100644
--- a/docs/writing-qmp-commands.txt
+++ b/docs/writing-qmp-commands.txt
@@ -210,7 +210,7 @@ if you don't see these strings, then something went wrong.
 === Errors ===

 QMP commands should use the error interface exported by the error.h header
-file. Basically, errors are set by calling the error_set() function.
+file. Basically, most errors are set by calling the error_setg() function.

 Let's say we don't accept the string "message" to contain the word "love". If
 it does contain it, we want the "hello-world" command to return an error:
@@ -219,8 +219,7 @@ void qmp_hello_world(bool has_message, const char *message, 
Error **errp)
 {
 if (has_message) {
 if (strstr(message, "love")) {
-error_set(errp, ERROR_CLASS_GENERIC_ERROR,
-  "the word 'love' is not allowed");
+error_setg(errp, "the word 'love' is not allowed");
 return;
 }
 printf("%s\n", message);
@@ -229,10 +228,8 @@ void qmp_hello_world(bool has_message, const char 
*message, Error **errp)
 }
 }

-The first argument to the error_set() function is the Error pointer to pointer,
-which is passed to all QMP functions. The second argument is a ErrorClass
-value, which should be ERROR_CLASS_GENERIC_ERROR most of the time (more
-details about error classes are given below). The third argument is a human
+The first argument to the error_setg() function is the Error pointer
+to pointer, which is passed to all QMP functions. The next argument is a human
 description of the error, this is a free-form printf-like string.

 Let's test the example above. Build qemu, run it as defined in the "Testing"
@@ -249,8 +246,9 @@ The QMP server's response should be:
 }
 }

-As a general rule, all QMP errors should use ERROR_CLASS_GENERIC_ERROR. There
-are two exceptions to this rule:
+As a general rule, all QMP errors should use ERROR_CLASS_GENERIC_ERROR
+(done by default when using error_setg()). There are two exceptions to
+this rule:

  1. A non-generic ErrorClass value exists* for the failure you want to report
 (eg. DeviceNotFound)
@@ -259,8 +257,8 @@ are two exceptions to this rule:
 want to report, hence you have to add a new ErrorClass value so that they
 can check for it

-If the failure you want to report doesn't fall in one of the two cases above,
-just report ERROR_CLASS_GENERIC_ERROR.
+If the failure you want to report falls into one of the two cases above,
+use error_set() with a second argument of an ErrorClass value.

  * All existing ErrorClass values are defined in the qapi-schema.json file

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0cb8afd..dfb57a8 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1795,7 +1795,7 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, 
Visitor *v,
 return;
 }
 if (value > (1ULL << 32)) {
-error_set(, ERROR_CLASS_GENERIC_ERROR,
+error_setg(,
   "Machine option 'max-ram-below-4g=%"PRIu64
   "' expects size less than or equal to 4G", value);
 error_propagate(errp, error);
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index bb6fdc3..c57f1a6 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -101,8 +101,7 @@ RockerSwitch *qmp_query_rocker(const char *name, Error 
**errp)

 r = rocker_find(name);
 if (!r) {
-error_set(errp, ERROR_CLASS_GENERIC_ERROR,
-  "rocker %s not found", name);
+error_setg(errp, "rocker %s not found", name);
 return NULL;
 }

@@ -122,8 +121,7 @@ RockerPortList *qmp_query_rocker_ports(const char *name, 
Error **errp)

 r = rocker_find(name);
 if (!r) {
-error_set(errp, ERROR_CLASS_GENERIC_ERROR,
-  "rocker %s not found", name);
+error_setg(errp, "rocker %s not found", name);
 return NULL;
 }

diff --git a/hw/net/rocker/rocker_of_dpa.c 

[Qemu-block] [PULL v2 40/40] qcow2: Fix qcow2_get_cluster_offset() for zero clusters

[Kevin Wolf]

When searching for contiguous zero clusters, we only need to check the
cluster type. Before this patch, an increasing offset (L2E_OFFSET_MASK)
was expected, so that the function never returned more than a single
zero cluster in practice. This patch fixes it to actually return as many
contiguous zero clusters as it can.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Kevin Wolf 
Message-id: 1446657384-5907-1-git-send-email-kw...@redhat.com
Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 67be0ce..24a60e2 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -312,7 +312,7 @@ static int count_contiguous_clusters(int nb_clusters, int 
cluster_size,
 if (!offset)
 return 0;
 
-assert(qcow2_get_cluster_type(first_entry) != QCOW2_CLUSTER_COMPRESSED);
+assert(qcow2_get_cluster_type(first_entry) == QCOW2_CLUSTER_NORMAL);
 
 for (i = 0; i < nb_clusters; i++) {
 uint64_t l2_entry = be64_to_cpu(l2_table[i]) & mask;
@@ -324,14 +324,16 @@ static int count_contiguous_clusters(int nb_clusters, int 
cluster_size,
return i;
 }
 
-static int count_contiguous_free_clusters(int nb_clusters, uint64_t *l2_table)
+static int count_contiguous_clusters_by_type(int nb_clusters,
+ uint64_t *l2_table,
+ int wanted_type)
 {
 int i;
 
 for (i = 0; i < nb_clusters; i++) {
 int type = qcow2_get_cluster_type(be64_to_cpu(l2_table[i]));
 
-if (type != QCOW2_CLUSTER_UNALLOCATED) {
+if (type != wanted_type) {
 break;
 }
 }
@@ -554,13 +556,14 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
uint64_t offset,
 ret = -EIO;
 goto fail;
 }
-c = count_contiguous_clusters(nb_clusters, s->cluster_size,
-_table[l2_index], QCOW_OFLAG_ZERO);
+c = count_contiguous_clusters_by_type(nb_clusters, _table[l2_index],
+  QCOW2_CLUSTER_ZERO);
 *cluster_offset = 0;
 break;
 case QCOW2_CLUSTER_UNALLOCATED:
 /* how many empty clusters ? */
-c = count_contiguous_free_clusters(nb_clusters, _table[l2_index]);
+c = count_contiguous_clusters_by_type(nb_clusters, _table[l2_index],
+  QCOW2_CLUSTER_UNALLOCATED);
 *cluster_offset = 0;
 break;
 case QCOW2_CLUSTER_NORMAL:
-- 
1.8.3.1

[Qemu-block] [PULL v2 34/40] qemu-iotests: fix cleanup of background processes

[Kevin Wolf]

From: Jeff Cody 

Commit 934659c switched the iotests to run qemu and qemu-nbd from a bash
subshell, in order to catch segfaults.  Unfortunately, this means the
process PID cannot be captured via '$!'. We stopped killing qemu and
qemu-nbd processes, leaving a lot of orphaned, running qemu processes
after executing iotests.

Since the process is using exec in the subshell, the PID is the
same as the subshell PID.

Track these PIDs for cleanup using pidfiles in the $TEST_DIR. Only
track the qemu PID, however, if requested - not all usage requires
killing the process.

Reported-by: John Snow 
Signed-off-by: Jeff Cody 
Message-id: 
9e4f958b3895b7259b98d845bb46f000ba362869.1446232490.git.jc...@redhat.com
[mre...@redhat.com: Replaced '! -z "..."' by '-n "..."']
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/058   | 12 
 tests/qemu-iotests/common.config | 14 --
 tests/qemu-iotests/common.qemu   | 18 --
 tests/qemu-iotests/common.rc |  8 +---
 4 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058
index f2bdd0b..63a6598 100755
--- a/tests/qemu-iotests/058
+++ b/tests/qemu-iotests/058
@@ -32,11 +32,17 @@ status=1# failure is the default!
 
 nbd_unix_socket=$TEST_DIR/test_qemu_nbd_socket
 nbd_snapshot_img="nbd:unix:$nbd_unix_socket"
+rm -f "${TEST_DIR}/qemu-nbd.pid"
 
 _cleanup_nbd()
 {
-if [ -n "$NBD_SNAPSHOT_PID" ]; then
-kill "$NBD_SNAPSHOT_PID"
+local NBD_SNAPSHOT_PID
+if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then
+read NBD_SNAPSHOT_PID < "${TEST_DIR}/qemu-nbd.pid"
+rm -f "${TEST_DIR}/qemu-nbd.pid"
+if [ -n "$NBD_SNAPSHOT_PID" ]; then
+kill "$NBD_SNAPSHOT_PID"
+fi
 fi
 rm -f "$nbd_unix_socket"
 }
@@ -60,7 +66,6 @@ _export_nbd_snapshot()
 {
 _cleanup_nbd
 $QEMU_NBD -v -t -k "$nbd_unix_socket" "$TEST_IMG" -l $1 &
-NBD_SNAPSHOT_PID=$!
 _wait_for_nbd
 }
 
@@ -68,7 +73,6 @@ _export_nbd_snapshot1()
 {
 _cleanup_nbd
 $QEMU_NBD -v -t -k "$nbd_unix_socket" "$TEST_IMG" -l snapshot.name=$1 &
-NBD_SNAPSHOT_PID=$!
 _wait_for_nbd
 }
 
diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index 596bb2b..be99730 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -44,6 +44,8 @@ export HOST_OPTIONS=${HOST_OPTIONS:=local.config}
 export CHECK_OPTIONS=${CHECK_OPTIONS:="-g auto"}
 export PWD=`pwd`
 
+export _QEMU_HANDLE=0
+
 # $1 = prog to look for, $2* = default pathnames if not found in $PATH
 set_prog_path()
 {
@@ -105,7 +107,12 @@ fi
 
 _qemu_wrapper()
 {
-(exec "$QEMU_PROG" $QEMU_OPTIONS "$@")
+(
+if [ -n "${QEMU_NEED_PID}" ]; then
+echo $BASHPID > "${TEST_DIR}/qemu-${_QEMU_HANDLE}.pid"
+fi
+exec "$QEMU_PROG" $QEMU_OPTIONS "$@"
+)
 }
 
 _qemu_img_wrapper()
@@ -120,7 +127,10 @@ _qemu_io_wrapper()
 
 _qemu_nbd_wrapper()
 {
-(exec "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS "$@")
+(
+echo $BASHPID > "${TEST_DIR}/qemu-nbd.pid"
+exec "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS "$@"
+)
 }
 
 export QEMU=_qemu_wrapper
diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu
index e3faa53..8bf3969 100644
--- a/tests/qemu-iotests/common.qemu
+++ b/tests/qemu-iotests/common.qemu
@@ -30,8 +30,6 @@ QEMU_COMM_TIMEOUT=10
 QEMU_FIFO_IN="${TEST_DIR}/qmp-in-$$"
 QEMU_FIFO_OUT="${TEST_DIR}/qmp-out-$$"
 
-QEMU_PID=
-_QEMU_HANDLE=0
 QEMU_HANDLE=0
 
 # If bash version is >= 4.1, these will be overwritten and dynamic
@@ -153,11 +151,11 @@ function _launch_qemu()
 mkfifo "${fifo_out}"
 mkfifo "${fifo_in}"
 
+QEMU_NEED_PID='y'\
 ${QEMU} -nographic -serial none ${comm} -machine accel=qtest "${@}" \
 >"${fifo_out}" 
\
 2>&1 \
 <"${fifo_in}" &
-QEMU_PID[${_QEMU_HANDLE}]=$!
 
 if [[ "${BASH_VERSINFO[0]}" -ge "5" ||
 ("${BASH_VERSINFO[0]}" -ge "4"  &&  "${BASH_VERSINFO[1]}" -ge "1") ]]
@@ -196,10 +194,18 @@ function _cleanup_qemu()
 # QEMU_PID[], QEMU_IN[], QEMU_OUT[] all use same indices
 for i in "${!QEMU_OUT[@]}"
 do
-if [ -z "${wait}" ]; then
-kill -KILL ${QEMU_PID[$i]} 2>/dev/null
+local QEMU_PID
+if [ -f "${TEST_DIR}/qemu-${i}.pid" ]; then
+read QEMU_PID < "${TEST_DIR}/qemu-${i}.pid"
+rm -f "${TEST_DIR}/qemu-${i}.pid"
+if [ -z "${wait}" ] && [ -n "${QEMU_PID}" ]; then
+kill -KILL ${QEMU_PID} 2>/dev/null
+fi
+if [ -n "${QEMU_PID}" ]; then
+wait ${QEMU_PID} 2>/dev/null # silent kill
+fi
 fi
-wait ${QEMU_PID[$i]} 2>/dev/null # silent kill

[Qemu-block] [PULL v2 38/40] block: Add 'x-blockdev-del' QMP command

[Kevin Wolf]

From: Alberto Garcia 

This command is still experimental, hence the name.

This is the companion to 'blockdev-add'. It allows deleting a
BlockBackend with its associated BlockDriverState tree, or a
BlockDriverState that is not attached to any backend.

In either case, the command fails if the reference count is greater
than 1 or the BlockDriverState has any parents.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Message-id: 
6cfc148c77aca1da942b094d811bfa3fcf7ac7bb.1446475331.git.be...@igalia.com
Signed-off-by: Max Reitz 
---
 blockdev.c   | 66 
 qapi/block-core.json | 32 +++--
 qmp-commands.hx  | 61 ++--
 3 files changed, 155 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 3197791..8607df9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3479,6 +3479,72 @@ fail:
 qmp_output_visitor_cleanup(ov);
 }
 
+void qmp_x_blockdev_del(bool has_id, const char *id,
+bool has_node_name, const char *node_name, Error 
**errp)
+{
+AioContext *aio_context;
+BlockBackend *blk;
+BlockDriverState *bs;
+
+if (has_id && has_node_name) {
+error_setg(errp, "Only one of id and node-name must be specified");
+return;
+} else if (!has_id && !has_node_name) {
+error_setg(errp, "No block device specified");
+return;
+}
+
+if (has_id) {
+blk = blk_by_name(id);
+if (!blk) {
+error_setg(errp, "Cannot find block backend %s", id);
+return;
+}
+if (blk_get_refcnt(blk) > 1) {
+error_setg(errp, "Block backend %s is in use", id);
+return;
+}
+bs = blk_bs(blk);
+aio_context = blk_get_aio_context(blk);
+} else {
+bs = bdrv_find_node(node_name);
+if (!bs) {
+error_setg(errp, "Cannot find node %s", node_name);
+return;
+}
+blk = bs->blk;
+if (blk) {
+error_setg(errp, "Node %s is in use by %s",
+   node_name, blk_name(blk));
+return;
+}
+aio_context = bdrv_get_aio_context(bs);
+}
+
+aio_context_acquire(aio_context);
+
+if (bs) {
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, errp)) {
+goto out;
+}
+
+if (bs->refcnt > 1 || !QLIST_EMPTY(>parents)) {
+error_setg(errp, "Block device %s is in use",
+   bdrv_get_device_or_node_name(bs));
+goto out;
+}
+}
+
+if (blk) {
+blk_unref(blk);
+} else {
+bdrv_unref(bs);
+}
+
+out:
+aio_context_release(aio_context);
+}
+
 BlockJobInfoList *qmp_query_block_jobs(Error **errp)
 {
 BlockJobInfoList *head = NULL, **p_next = 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 083d2cd..470e86c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1895,8 +1895,8 @@
 # level and no BlockBackend will be created.
 #
 # This command is still a work in progress.  It doesn't support all
-# block drivers, it lacks a matching blockdev-del, and more.  Stay
-# away from it unless you want to help with its development.
+# block drivers among other things.  Stay away from it unless you want
+# to help with its development.
 #
 # @options: block device options for the new device
 #
@@ -1905,6 +1905,34 @@
 { 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
 
 ##
+# @x-blockdev-del:
+#
+# Deletes a block device that has been added using blockdev-add.
+# The selected device can be either a block backend or a graph node.
+#
+# In the former case the backend will be destroyed, along with its
+# inserted medium if there's any. The command will fail if the backend
+# or its medium are in use.
+#
+# In the latter case the node will be destroyed. The command will fail
+# if the node is attached to a block backend or is otherwise being
+# used.
+#
+# One of @id or @node-name must be specified, but not both.
+#
+# This command is still a work in progress and is considered
+# experimental. Stay away from it unless you want to help with its
+# development.
+#
+# @id: #optional Name of the block backend device to delete.
+#
+# @node-name: #optional Name of the graph node to delete.
+#
+# Since: 2.5
+##
+{ 'command': 'x-blockdev-del', 'data': { '*id': 'str', '*node-name': 'str' } }
+
+##
 # @blockdev-open-tray:
 #
 # Opens a block device's tray. If there is a block driver state tree inserted 
as
diff --git a/qmp-commands.hx b/qmp-commands.hx
index bd4d38f..658e7da 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3927,8 +3927,8 @@ blockdev-add
 Add a block device.
 
 This command is still a work in progress.  It doesn't support all
-block drivers, it lacks a matching blockdev-del, and more.  Stay away
-from it 

[Qemu-block] [PULL v2 33/40] qemu-io: Correct error messages

[Kevin Wolf]

From: John Snow 

Reported-by: Max Reitz 
Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 qemu-io-cmds.c | 53 ++---
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 238b1da..3dddae8 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -146,6 +146,21 @@ static int64_t cvtnum(const char *s)
 return ret;
 }
 
+static void print_cvtnum_err(int64_t rc, const char *arg)
+{
+switch (rc) {
+case -EINVAL:
+printf("Parsing error: non-numeric argument,"
+   " or extraneous/unrecognized suffix -- %s\n", arg);
+break;
+case -ERANGE:
+printf("Parsing error: argument too large -- %s\n", arg);
+break;
+default:
+printf("Parsing error: %s\n", arg);
+}
+}
+
 #define EXABYTES(x) ((long long)(x) << 60)
 #define PETABYTES(x)((long long)(x) << 50)
 #define TERABYTES(x)((long long)(x) << 40)
@@ -367,13 +382,13 @@ create_iovec(BlockBackend *blk, QEMUIOVector *qiov, char 
**argv, int nr_iov,
 
 len = cvtnum(arg);
 if (len < 0) {
-printf("non-numeric length argument -- %s\n", arg);
+print_cvtnum_err(len, arg);
 goto fail;
 }
 
 /* should be SIZE_T_MAX, but that doesn't exist */
 if (len > INT_MAX) {
-printf("too large length argument -- %s\n", arg);
+printf("Argument '%s' exceeds maximum size %d\n", arg, INT_MAX);
 goto fail;
 }
 
@@ -700,7 +715,7 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
 lflag = 1;
 pattern_count = cvtnum(optarg);
 if (pattern_count < 0) {
-printf("non-numeric length argument -- %s\n", optarg);
+print_cvtnum_err(pattern_count, optarg);
 return 0;
 }
 break;
@@ -721,7 +736,7 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
 sflag = 1;
 pattern_offset = cvtnum(optarg);
 if (pattern_offset < 0) {
-printf("non-numeric length argument -- %s\n", optarg);
+print_cvtnum_err(pattern_offset, optarg);
 return 0;
 }
 break;
@@ -744,14 +759,14 @@ static int read_f(BlockBackend *blk, int argc, char 
**argv)
 
 offset = cvtnum(argv[optind]);
 if (offset < 0) {
-printf("non-numeric length argument -- %s\n", argv[optind]);
+print_cvtnum_err(offset, argv[optind]);
 return 0;
 }
 
 optind++;
 count = cvtnum(argv[optind]);
 if (count < 0) {
-printf("non-numeric length argument -- %s\n", argv[optind]);
+print_cvtnum_err(count, argv[optind]);
 return 0;
 } else if (count > SIZE_MAX) {
 printf("length cannot exceed %zu, given %s\n", SIZE_MAX, argv[optind]);
@@ -905,7 +920,7 @@ static int readv_f(BlockBackend *blk, int argc, char **argv)
 
 offset = cvtnum(argv[optind]);
 if (offset < 0) {
-printf("non-numeric length argument -- %s\n", argv[optind]);
+print_cvtnum_err(offset, argv[optind]);
 return 0;
 }
 optind++;
@@ -1054,14 +1069,14 @@ static int write_f(BlockBackend *blk, int argc, char 
**argv)
 
 offset = cvtnum(argv[optind]);
 if (offset < 0) {
-printf("non-numeric length argument -- %s\n", argv[optind]);
+print_cvtnum_err(offset, argv[optind]);
 return 0;
 }
 
 optind++;
 count = cvtnum(argv[optind]);
 if (count < 0) {
-printf("non-numeric length argument -- %s\n", argv[optind]);
+print_cvtnum_err(count, argv[optind]);
 return 0;
 } else if (count > SIZE_MAX) {
 printf("length cannot exceed %zu, given %s\n", SIZE_MAX, argv[optind]);
@@ -1189,7 +1204,7 @@ static int writev_f(BlockBackend *blk, int argc, char 
**argv)
 
 offset = cvtnum(argv[optind]);
 if (offset < 0) {
-printf("non-numeric length argument -- %s\n", argv[optind]);
+print_cvtnum_err(offset, argv[optind]);
 return 0;
 }
 optind++;
@@ -1316,7 +1331,7 @@ static int multiwrite_f(BlockBackend *blk, int argc, char 
**argv)
 /* Read the offset of the request */
 offset = cvtnum(argv[optind]);
 if (offset < 0) {
-printf("non-numeric offset argument -- %s\n", argv[optind]);
+print_cvtnum_err(offset, argv[optind]);
 goto out;
 }
 optind++;
@@ -1543,7 +1558,7 @@ static int aio_read_f(BlockBackend *blk, int argc, char 
**argv)
 
 ctx->offset = cvtnum(argv[optind]);
 if (ctx->offset < 0) {
-printf("non-numeric length argument -- %s\n", argv[optind]);
+print_cvtnum_err(ctx->offset, argv[optind]);
 g_free(ctx);
   

[Qemu-block] [PULL v2 35/40] qemu-iotests: fix -valgrind option for check

[Kevin Wolf]

From: Jeff Cody 

Commit 934659c switched the iotests to run qemu-io from a bash subshell,
in order to catch segfaults.  This method is incompatible with the
current valgrind_qemu_io() bash function.

Move the valgrind usage into the exec subshell in _qemu_io_wrapper(),
while making sure the original return value is passed back to the
caller.

Update test output for tests 039, 061, and 137 as it looks for the
specific subshell command when the process is terminated.

Reported-by: Kevin Wolf 
Signed-off-by: Jeff Cody 
Message-id: 
0066fd85d26ca641a1c25135ff2479b7985701cf.1446232490.git.jc...@redhat.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/039.out   | 30 +-
 tests/qemu-iotests/061.out   | 12 ++--
 tests/qemu-iotests/137.out   |  6 +-
 tests/qemu-iotests/common|  9 ++---
 tests/qemu-iotests/common.config | 18 +-
 tests/qemu-iotests/common.rc | 10 --
 6 files changed, 59 insertions(+), 26 deletions(-)

diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
index 03a31c5..32c8846 100644
--- a/tests/qemu-iotests/039.out
+++ b/tests/qemu-iotests/039.out
@@ -11,7 +11,11 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed  ( exec "$QEMU_IO_PROG" 
$QEMU_IO_OPTIONS "$@" )
+./common.config: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
then
+exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
"$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+else
+exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+fi )
 incompatible_features 0x1
 ERROR cluster 5 refcount=0 reference=1
 ERROR OFLAG_COPIED data cluster: l2_entry=8005 refcount=0
@@ -46,7 +50,11 @@ read 512/512 bytes at offset 0
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed  ( exec "$QEMU_IO_PROG" 
$QEMU_IO_OPTIONS "$@" )
+./common.config: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
then
+exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
"$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+else
+exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+fi )
 incompatible_features 0x1
 ERROR cluster 5 refcount=0 reference=1
 Rebuilding refcount structure
@@ -60,7 +68,11 @@ incompatible_features 0x0
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed  ( exec "$QEMU_IO_PROG" 
$QEMU_IO_OPTIONS "$@" )
+./common.config: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
then
+exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
"$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+else
+exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+fi )
 incompatible_features 0x0
 No errors were found on the image.
 
@@ -79,7 +91,11 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed  ( exec "$QEMU_IO_PROG" 
$QEMU_IO_OPTIONS "$@" )
+./common.config: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
then
+exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
"$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+else
+exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+fi )
 incompatible_features 0x1
 ERROR cluster 5 refcount=0 reference=1
 ERROR OFLAG_COPIED data cluster: l2_entry=8005 refcount=0
@@ -89,7 +105,11 @@ Data may be corrupted, or further writes to the image may 
corrupt it.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed  ( exec "$QEMU_IO_PROG" 
$QEMU_IO_OPTIONS "$@" )
+./common.config: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
then
+exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
"$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+else
+exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+fi )
 incompatible_features 0x0
 No errors were found on the image.
 *** done
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index b16bea9..f2598a8 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -57,7 +57,11 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

[Qemu-block] [PULL v2 37/40] block: Add blk_get_refcnt()

[Kevin Wolf]

From: Alberto Garcia 

This function returns the reference count of a given BlockBackend.
For convenience, it returns 0 if the BlockBackend pointer is NULL.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Message-id: 
dfdd8a17dbe3288842840636d2cfe5bb895abcb0.1446475331.git.be...@igalia.com
Signed-off-by: Max Reitz 
---
 block/block-backend.c  | 5 +
 include/sysemu/block-backend.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 1ac6982..6f9309f 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -189,6 +189,11 @@ static void drive_info_del(DriveInfo *dinfo)
 g_free(dinfo);
 }
 
+int blk_get_refcnt(BlockBackend *blk)
+{
+return blk ? blk->refcnt : 0;
+}
+
 /*
  * Increment @blk's reference count.
  * @blk must not be null.
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 40e315b..f4a68e2 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -65,6 +65,7 @@ BlockBackend *blk_new_with_bs(const char *name, Error **errp);
 BlockBackend *blk_new_open(const char *name, const char *filename,
const char *reference, QDict *options, int flags,
Error **errp);
+int blk_get_refcnt(BlockBackend *blk);
 void blk_ref(BlockBackend *blk);
 void blk_unref(BlockBackend *blk);
 const char *blk_name(BlockBackend *blk);
-- 
1.8.3.1

Re: [Qemu-block] [PATCH v9 11/15] qmp: Introduce blockdev-change-medium

[Kevin Wolf]

Am 06.11.2015 um 16:27 hat Max Reitz geschrieben:
> Introduce a new QMP command 'blockdev-change-medium' which is intended
> to replace the 'change' command for block devices. The existing function
> qmp_change_blockdev() is accordingly renamed to
> qmp_blockdev_change_medium().
> 
> Signed-off-by: Max Reitz 

Thanks, updated the queued patch with this one.

Kevin

[Qemu-block] [PULL v2 10/40] blockdev: Implement change with basic operations

[Kevin Wolf]

From: Max Reitz 

Implement 'change' on block devices by calling blockdev-open-tray,
blockdev-remove-medium, blockdev-insert-medium (a variation of that
which does not need a node-name) and blockdev-close-tray.

Signed-off-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 blockdev.c | 178 +++--
 1 file changed, 68 insertions(+), 110 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 1624297..53d4edf 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1940,44 +1940,6 @@ exit:
 }
 }
 
-
-static void eject_device(BlockBackend *blk, int force, Error **errp)
-{
-BlockDriverState *bs = blk_bs(blk);
-AioContext *aio_context;
-
-if (!bs) {
-/* No medium inserted, so there is nothing to do */
-return;
-}
-
-aio_context = bdrv_get_aio_context(bs);
-aio_context_acquire(aio_context);
-
-if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
-goto out;
-}
-if (!blk_dev_has_removable_media(blk)) {
-error_setg(errp, "Device '%s' is not removable",
-   bdrv_get_device_name(bs));
-goto out;
-}
-
-if (blk_dev_is_medium_locked(blk) && !blk_dev_is_tray_open(blk)) {
-blk_dev_eject_request(blk, force);
-if (!force) {
-error_setg(errp, "Device '%s' is locked",
-   bdrv_get_device_name(bs));
-goto out;
-}
-}
-
-bdrv_close(bs);
-
-out:
-aio_context_release(aio_context);
-}
-
 void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
 {
 Error *local_err = NULL;
@@ -2015,78 +1977,6 @@ void qmp_block_passwd(bool has_device, const char 
*device,
 aio_context_release(aio_context);
 }
 
-/* Assumes AioContext is held */
-static void qmp_bdrv_open_encrypted(BlockDriverState **pbs,
-const char *filename,
-int bdrv_flags, const char *format,
-const char *password, Error **errp)
-{
-Error *local_err = NULL;
-QDict *options = NULL;
-int ret;
-
-if (format) {
-options = qdict_new();
-qdict_put(options, "driver", qstring_from_str(format));
-}
-
-ret = bdrv_open(pbs, filename, NULL, options, bdrv_flags, _err);
-if (ret < 0) {
-error_propagate(errp, local_err);
-return;
-}
-
-bdrv_add_key(*pbs, password, errp);
-}
-
-void qmp_change_blockdev(const char *device, const char *filename,
- const char *format, Error **errp)
-{
-BlockBackend *blk;
-BlockDriverState *bs;
-AioContext *aio_context;
-int bdrv_flags;
-bool new_bs;
-Error *err = NULL;
-
-blk = blk_by_name(device);
-if (!blk) {
-error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-  "Device '%s' not found", device);
-return;
-}
-bs = blk_bs(blk);
-new_bs = !bs;
-
-aio_context = blk_get_aio_context(blk);
-aio_context_acquire(aio_context);
-
-eject_device(blk, 0, );
-if (err) {
-error_propagate(errp, err);
-goto out;
-}
-
-bdrv_flags = blk_is_read_only(blk) ? 0 : BDRV_O_RDWR;
-bdrv_flags |= blk_get_root_state(blk)->open_flags & ~BDRV_O_RDWR;
-
-qmp_bdrv_open_encrypted(, filename, bdrv_flags, format, NULL, );
-if (err) {
-error_propagate(errp, err);
-goto out;
-}
-
-if (new_bs) {
-blk_insert_bs(blk, bs);
-/* Has been sent automatically by bdrv_open() if blk_bs(blk) was not
- * NULL */
-blk_dev_change_media_cb(blk, true);
-}
-
-out:
-aio_context_release(aio_context);
-}
-
 void qmp_blockdev_open_tray(const char *device, bool has_force, bool force,
 Error **errp)
 {
@@ -2253,6 +2143,74 @@ void qmp_blockdev_insert_medium(const char *device, 
const char *node_name,
 qmp_blockdev_insert_anon_medium(device, bs, errp);
 }
 
+void qmp_change_blockdev(const char *device, const char *filename,
+ const char *format, Error **errp)
+{
+BlockBackend *blk;
+BlockDriverState *medium_bs = NULL;
+int bdrv_flags, ret;
+QDict *options = NULL;
+Error *err = NULL;
+
+blk = blk_by_name(device);
+if (!blk) {
+error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+  "Device '%s' not found", device);
+goto fail;
+}
+
+if (blk_bs(blk)) {
+blk_update_root_state(blk);
+}
+
+bdrv_flags = blk_get_open_flags_from_root_state(blk);
+
+if (format) {
+options = qdict_new();
+qdict_put(options, "driver", qstring_from_str(format));
+}
+
+assert(!medium_bs);
+ret = bdrv_open(_bs, filename, NULL, options, bdrv_flags, errp);
+if (ret < 0) {
+goto fail;
+}
+
+blk_apply_root_state(blk, medium_bs);
+
+bdrv_add_key(medium_bs, NULL, );
+if (err) {
+

[Qemu-block] [PULL v2 05/40] blockdev: Add blockdev-open-tray

[Kevin Wolf]

From: Max Reitz 

Signed-off-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 blockdev.c   | 36 
 qapi/block-core.json | 32 
 qmp-commands.hx  | 48 
 3 files changed, 116 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 97be42f..d01a99e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2088,6 +2088,42 @@ out:
 aio_context_release(aio_context);
 }
 
+void qmp_blockdev_open_tray(const char *device, bool has_force, bool force,
+Error **errp)
+{
+BlockBackend *blk;
+bool locked;
+
+if (!has_force) {
+force = false;
+}
+
+blk = blk_by_name(device);
+if (!blk) {
+error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+  "Device '%s' not found", device);
+return;
+}
+
+if (!blk_dev_has_removable_media(blk)) {
+error_setg(errp, "Device '%s' is not removable", device);
+return;
+}
+
+if (blk_dev_is_tray_open(blk)) {
+return;
+}
+
+locked = blk_dev_is_medium_locked(blk);
+if (locked) {
+blk_dev_eject_request(blk, force);
+}
+
+if (!locked || force) {
+blk_dev_change_media_cb(blk, false);
+}
+}
+
 /* 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,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 425fdab..b65ab81 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1876,6 +1876,38 @@
 ##
 { 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
 
+##
+# @blockdev-open-tray:
+#
+# Opens a block device's tray. If there is a block driver state tree inserted 
as
+# a medium, it will become inaccessible to the guest (but it will remain
+# associated to the block device, so closing the tray will make it accessible
+# again).
+#
+# If the tray was already open before, this will be a no-op.
+#
+# Once the tray opens, a DEVICE_TRAY_MOVED event is emitted. There are cases in
+# which no such event will be generated, these include:
+# - if the guest has locked the tray, @force is false and the guest does not
+#   respond to the eject request
+# - if the BlockBackend denoted by @device does not have a guest device 
attached
+#   to it
+# - if the guest device does not have an actual tray and is empty, for instance
+#   for floppy disk drives
+#
+# @device: block device name
+#
+# @force:  #optional if false (the default), an eject request will be sent to
+#  the guest if it has locked the tray (and the tray will not be opened
+#  immediately); if true, the tray will be opened regardless of whether
+#  it is locked
+#
+# Since: 2.5
+##
+{ 'command': 'blockdev-open-tray',
+  'data': { 'device': 'str',
+'*force': 'bool' } }
+
 
 ##
 # @BlockErrorAction
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d7cf0ff..0b796f9 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3936,6 +3936,54 @@ Example (2):
 EQMP
 
 {
+.name   = "blockdev-open-tray",
+.args_type  = "device:s,force:b?",
+.mhandler.cmd_new = qmp_marshal_blockdev_open_tray,
+},
+
+SQMP
+blockdev-open-tray
+--
+
+Opens a block device's tray. If there is a block driver state tree inserted as 
a
+medium, it will become inaccessible to the guest (but it will remain associated
+to the block device, so closing the tray will make it accessible again).
+
+If the tray was already open before, this will be a no-op.
+
+Once the tray opens, a DEVICE_TRAY_MOVED event is emitted. There are cases in
+which no such event will be generated, these include:
+- if the guest has locked the tray, @force is false and the guest does not
+  respond to the eject request
+- if the BlockBackend denoted by @device does not have a guest device attached
+  to it
+- if the guest device does not have an actual tray and is empty, for instance
+  for floppy disk drives
+
+Arguments:
+
+- "device": block device name (json-string)
+- "force": if false (the default), an eject request will be sent to the guest 
if
+   it has locked the tray (and the tray will not be opened 
immediately);
+   if true, the tray will be opened regardless of whether it is locked
+   (json-bool, optional)
+
+Example:
+
+-> { "execute": "blockdev-open-tray",
+ "arguments": { "device": "ide1-cd0" } }
+
+<- { "timestamp": { "seconds": 1418751016,
+"microseconds": 716996 },
+ "event": "DEVICE_TRAY_MOVED",
+ "data": { "device": "ide1-cd0",
+   "tray-open": true } }
+
+<- { "return": {} }
+
+EQMP
+
+{
 .name   = "query-named-block-nodes",
 .args_type  = "",
 .mhandler.cmd_new = qmp_marshal_query_named_block_nodes,
-- 
1.8.3.1

[Qemu-block] [PULL v2 02/40] block: Add blk_remove_bs()

[Kevin Wolf]

From: Max Reitz 

This function removes the BlockDriverState associated with the given
BlockBackend from that BB and sets the BDS pointer in the BB to NULL.

Signed-off-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 block/block-backend.c  | 12 
 include/sysemu/block-backend.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 19fdaae..878c448 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -334,6 +334,18 @@ void blk_hide_on_behalf_of_hmp_drive_del(BlockBackend *blk)
 }
 
 /*
+ * Disassociates the currently associated BlockDriverState from @blk.
+ */
+void blk_remove_bs(BlockBackend *blk)
+{
+blk_update_root_state(blk);
+
+blk->bs->blk = NULL;
+bdrv_unref(blk->bs);
+blk->bs = NULL;
+}
+
+/*
  * Associates a new BlockDriverState with @blk.
  */
 void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 9306a52..14a6d32 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -72,6 +72,7 @@ BlockBackend *blk_by_name(const char *name);
 BlockBackend *blk_next(BlockBackend *blk);
 
 BlockDriverState *blk_bs(BlockBackend *blk);
+void blk_remove_bs(BlockBackend *blk);
 void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs);
 
 void blk_hide_on_behalf_of_hmp_drive_del(BlockBackend *blk);
-- 
1.8.3.1

[Qemu-block] [PULL v2 22/40] commit: reopen overlay_bs before base

[Kevin Wolf]

From: Alberto Garcia 

'block-commit' needs write access to two different nodes of the chain:

- 'base', because that's where the data is written to.
- the overlay of 'top', because it needs to update the backing file
  string to point to 'base' after the operation.

Both images have to be opened in read-write mode, and commit_start()
takes care of reopening them if necessary.

With the current implementation, however, when overlay_bs is reopened
in read-write mode it has the side effect of making 'base' read-only
again, eventually making 'block-commit' fail.

This needs to be fixed in bdrv_reopen(), but until we get to that it
can be worked around simply by swapping the order of base and
overlay_bs in the reopen queue.

In order to reproduce this bug, overlay_bs needs to be initially in
read-only mode. That is: the 'top' parameter of 'block-commit' cannot
be the active layer nor its immediate backing chain.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 block/commit.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/commit.c b/block/commit.c
index fdebe87..a5d02aa 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -236,14 +236,14 @@ void commit_start(BlockDriverState *bs, BlockDriverState 
*base,
 orig_overlay_flags = bdrv_get_flags(overlay_bs);
 
 /* convert base & overlay_bs to r/w, if necessary */
-if (!(orig_base_flags & BDRV_O_RDWR)) {
-reopen_queue = bdrv_reopen_queue(reopen_queue, base, NULL,
- orig_base_flags | BDRV_O_RDWR);
-}
 if (!(orig_overlay_flags & BDRV_O_RDWR)) {
 reopen_queue = bdrv_reopen_queue(reopen_queue, overlay_bs, NULL,
  orig_overlay_flags | BDRV_O_RDWR);
 }
+if (!(orig_base_flags & BDRV_O_RDWR)) {
+reopen_queue = bdrv_reopen_queue(reopen_queue, base, NULL,
+ orig_base_flags | BDRV_O_RDWR);
+}
 if (reopen_queue) {
 bdrv_reopen_multiple(reopen_queue, _err);
 if (local_err != NULL) {
-- 
1.8.3.1

[Qemu-block] [PULL v2 13/40] hmp: Use blockdev-change-medium for change command

[Kevin Wolf]

From: Max Reitz 

Use separate code paths for the two overloaded functions of the 'change'
HMP command, and invoke the 'blockdev-change-medium' QMP command if used
on a block device (by calling qmp_blockdev_change_medium()).

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 hmp.c | 27 +++
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/hmp.c b/hmp.c
index a15d00c..5e5358f 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1338,22 +1338,25 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 const char *arg = qdict_get_try_str(qdict, "arg");
 Error *err = NULL;
 
-if (strcmp(device, "vnc") == 0 &&
-(strcmp(target, "passwd") == 0 ||
- strcmp(target, "password") == 0)) {
-if (!arg) {
-monitor_read_password(mon, hmp_change_read_arg, NULL);
+if (strcmp(device, "vnc") == 0) {
+if (strcmp(target, "passwd") == 0 ||
+strcmp(target, "password") == 0) {
+if (!arg) {
+monitor_read_password(mon, hmp_change_read_arg, NULL);
+return;
+}
+}
+qmp_change("vnc", target, !!arg, arg, );
+} else {
+qmp_blockdev_change_medium(device, target, !!arg, arg, );
+if (err &&
+error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
+error_free(err);
+monitor_read_block_device_key(mon, device, NULL, NULL);
 return;
 }
 }
 
-qmp_change(device, target, !!arg, arg, );
-if (err &&
-error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
-error_free(err);
-monitor_read_block_device_key(mon, device, NULL, NULL);
-return;
-}
 hmp_handle_error(mon, );
 }
 
-- 
1.8.3.1

[Qemu-block] [PULL v2 03/40] block: Make bdrv_states public

[Kevin Wolf]

From: Max Reitz 

When inserting a BDS tree into a BB, we will need to add the root BDS to
this list. Since we will want to do that in the blockdev-insert-medium
implementation in blockdev.c, we will need access to it there.

This patch is not exactly elegant, but bdrv_states will be removed in
the future anyway because we no longer need it since we have BBs.

Signed-off-by: Max Reitz 
Reviewed-by: Alberto Garcia 
Signed-off-by: Kevin Wolf 
---
 block.c   | 3 +--
 include/block/block_int.h | 2 ++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index eb8158a..a99e6d8 100644
--- a/block.c
+++ b/block.c
@@ -73,8 +73,7 @@ struct BdrvDirtyBitmap {
 
 #define NOT_DONE 0x7fff /* used while emulated sync operation in progress 
*/
 
-static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
-QTAILQ_HEAD_INITIALIZER(bdrv_states);
+struct BdrvStates bdrv_states = QTAILQ_HEAD_INITIALIZER(bdrv_states);
 
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
 QTAILQ_HEAD_INITIALIZER(graph_bdrv_states);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3ceeb5a..6a3f64d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -473,6 +473,8 @@ extern BlockDriver bdrv_file;
 extern BlockDriver bdrv_raw;
 extern BlockDriver bdrv_qcow2;
 
+extern QTAILQ_HEAD(BdrvStates, BlockDriverState) bdrv_states;
+
 /**
  * bdrv_setup_io_funcs:
  *
-- 
1.8.3.1

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

[Kevin Wolf]

The following changes since commit a8b4f9585a0bf5186fca793ce2c5d754cd8ec49a:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-11-10' into 
staging (2015-11-10 09:39:24 +)

are available in the git repository at:


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

for you to fetch changes up to c400bddb916268394e352f82809eb4728424a5b1:

  Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-11-10' 
into queue-block (2015-11-10 14:59:26 +0100)



Block layer patches


Alberto Garcia (17):
  block: Don't call blk_bs() twice in bdrv_lookup_bs()
  block: check for existing device IDs in external_snapshot_prepare()
  block: rename BlockdevSnapshot to BlockdevSnapshotSync
  block: support passing 'backing': '' to 'blockdev-add'
  block: add a 'blockdev-snapshot' QMP command
  block: add tests for the 'blockdev-snapshot' command
  commit: reopen overlay_bs before base
  qemu-iotests: Test the reopening of overlay_bs in 'block-commit'
  throttle: Check for pending requests in throttle_group_unregister_bs()
  throttle: Use bs->throttle_state instead of bs->io_limits_enabled
  block: Disallow snapshots if the overlay doesn't support backing files
  block: Remove inner quotation marks in iotest 085
  block: test 'blockdev-snapshot' using a file BDS as the overlay
  mirror: block all operations on the target image during the job
  block: Add blk_get_refcnt()
  block: Add 'x-blockdev-del' QMP command
  iotests: Add tests for the x-blockdev-del command

Jeff Cody (2):
  qemu-iotests: fix cleanup of background processes
  qemu-iotests: fix -valgrind option for check

John Snow (5):
  qcow2: avoid misaligned 64bit bswap
  qemu-img: add check for zero-length job len
  qemu-io: fix cvtnum lval types
  qemu-io: Check for trailing chars
  qemu-io: Correct error messages

Kevin Wolf (2):
  qcow2: Fix qcow2_get_cluster_offset() for zero clusters
  Merge remote-tracking branch 
'mreitz/tags/pull-block-for-kevin-2015-11-10' into queue-block

Max Reitz (15):
  block: Add blk_remove_bs()
  block: Make bdrv_states public
  block: Add functions for inheriting a BBRS
  blockdev: Add blockdev-open-tray
  blockdev: Add blockdev-close-tray
  blockdev: Add blockdev-remove-medium
  blockdev: Add blockdev-insert-medium
  blockdev: Implement eject with basic operations
  blockdev: Implement change with basic operations
  block: Inquire tray state before tray-moved events
  qmp: Introduce blockdev-change-medium
  hmp: Use blockdev-change-medium for change command
  blockdev: read-only-mode for blockdev-change-medium
  hmp: Add read-only-mode option to change command
  iotests: Add test for change-related QMP commands

 block.c  |  22 +-
 block/block-backend.c|  61 +++-
 block/commit.c   |   8 +-
 block/mirror.c   |   4 +
 block/qapi.c |   2 +-
 block/qcow2-cluster.c|  15 +-
 block/qcow2-refcount.c   |  11 +-
 block/throttle-groups.c  |   7 +
 blockdev.c   | 527 
 hmp-commands.hx  |  20 +-
 hmp.c|  47 ++-
 include/block/block_int.h|   7 +-
 include/sysemu/block-backend.h   |   4 +
 include/sysemu/blockdev.h|   2 -
 qapi-schema.json |  10 +-
 qapi/block-core.json | 192 ++-
 qemu-img.c   |   3 +-
 qemu-io-cmds.c   | 185 +++---
 qmp-commands.hx  | 317 -
 qmp.c|   3 +-
 tests/qemu-iotests/039.out   |  30 +-
 tests/qemu-iotests/040   |  30 ++
 tests/qemu-iotests/040.out   |   4 +-
 tests/qemu-iotests/058   |  12 +-
 tests/qemu-iotests/061.out   |  12 +-
 tests/qemu-iotests/085   | 120 ++-
 tests/qemu-iotests/085.out   |  38 ++-
 tests/qemu-iotests/118   | 720 +++
 tests/qemu-iotests/118.out   |   5 +
 tests/qemu-iotests/137.out   |   6 +-
 tests/qemu-iotests/139   | 414 ++
 tests/qemu-iotests/139.out   |   5 +
 tests/qemu-iotests/common|   9 +-
 tests/qemu-iotests/common.config |  32 +-
 tests/qemu-iotests/common.qemu   |  18 +-
 tests/qemu-iotests/common.rc |  18 +-
 tests/qemu-iotests/group |   2 +
 ui/cocoa.m   |  10 +-
 38 files changed, 2621 insertions(+), 311 deletions(-)
 create mode 100755 tests/qemu-iotests/118
 create mode 100644 tests/qemu-iotests/118.out
 create mode 100644 tests/qemu-iotests/139
 create mode 100644 tests/qemu-iotests/139.out

[Qemu-block] [PULL v2 11/40] block: Inquire tray state before tray-moved events

[Kevin Wolf]

From: Max Reitz 

blk_dev_change_media_cb() is called for all potential tray movements;
however, it is possible to request closing the tray but nothing actually
happening (on a floppy disk drive without a medium).

Thus, the actual tray status should be inquired before sending a
tray-moved event (and an event should be sent whenever the status
changed).

Checking @load is now superfluous; it was necessary because it was
possible to change a medium without having explicitly opened the tray
and closed it again (or it might have been possible, at least). This is
no longer possible, though.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 block/block-backend.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 7d49539..1ac6982 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -429,18 +429,15 @@ void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps 
*ops,
 void blk_dev_change_media_cb(BlockBackend *blk, bool load)
 {
 if (blk->dev_ops && blk->dev_ops->change_media_cb) {
-bool tray_was_closed = !blk_dev_is_tray_open(blk);
+bool tray_was_open, tray_is_open;
 
+tray_was_open = blk_dev_is_tray_open(blk);
 blk->dev_ops->change_media_cb(blk->dev_opaque, load);
-if (tray_was_closed) {
-/* tray open */
-qapi_event_send_device_tray_moved(blk_name(blk),
-  true, _abort);
-}
-if (load) {
-/* tray close */
-qapi_event_send_device_tray_moved(blk_name(blk),
-  false, _abort);
+tray_is_open = blk_dev_is_tray_open(blk);
+
+if (tray_was_open != tray_is_open) {
+qapi_event_send_device_tray_moved(blk_name(blk), tray_is_open,
+  _abort);
 }
 }
 }
-- 
1.8.3.1

[Qemu-block] [PULL v2 16/40] iotests: Add test for change-related QMP commands

[Kevin Wolf]

From: Max Reitz 

Signed-off-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/118 | 720 +
 tests/qemu-iotests/118.out |   5 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 726 insertions(+)
 create mode 100755 tests/qemu-iotests/118
 create mode 100644 tests/qemu-iotests/118.out

diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118
new file mode 100755
index 000..a2bcd54
--- /dev/null
+++ b/tests/qemu-iotests/118
@@ -0,0 +1,720 @@
+#!/usr/bin/env python
+#
+# Test case for the QMP 'change' command and all other associated
+# commands
+#
+# Copyright (C) 2015 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import os
+import stat
+import time
+import iotests
+from iotests import qemu_img
+
+old_img = os.path.join(iotests.test_dir, 'test0.img')
+new_img = os.path.join(iotests.test_dir, 'test1.img')
+
+class ChangeBaseClass(iotests.QMPTestCase):
+has_opened = False
+has_closed = False
+
+def process_events(self):
+for event in self.vm.get_qmp_events(wait=False):
+if (event['event'] == 'DEVICE_TRAY_MOVED' and
+event['data']['device'] == 'drive0'):
+if event['data']['tray-open'] == False:
+self.has_closed = True
+else:
+self.has_opened = True
+
+def wait_for_open(self):
+timeout = time.clock() + 3
+while not self.has_opened and time.clock() < timeout:
+self.process_events()
+if not self.has_opened:
+self.fail('Timeout while waiting for the tray to open')
+
+def wait_for_close(self):
+timeout = time.clock() + 3
+while not self.has_closed and time.clock() < timeout:
+self.process_events()
+if not self.has_opened:
+self.fail('Timeout while waiting for the tray to close')
+
+class GeneralChangeTestsBaseClass(ChangeBaseClass):
+def test_change(self):
+result = self.vm.qmp('change', device='drive0', target=new_img,
+   arg=iotests.imgfmt)
+self.assert_qmp(result, 'return', {})
+
+self.wait_for_open()
+self.wait_for_close()
+
+result = self.vm.qmp('query-block')
+self.assert_qmp(result, 'return[0]/tray_open', False)
+self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+def test_blockdev_change_medium(self):
+result = self.vm.qmp('blockdev-change-medium', device='drive0',
+   filename=new_img,
+   format=iotests.imgfmt)
+self.assert_qmp(result, 'return', {})
+
+self.wait_for_open()
+self.wait_for_close()
+
+result = self.vm.qmp('query-block')
+self.assert_qmp(result, 'return[0]/tray_open', False)
+self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+def test_eject(self):
+result = self.vm.qmp('eject', device='drive0', force=True)
+self.assert_qmp(result, 'return', {})
+
+self.wait_for_open()
+
+result = self.vm.qmp('query-block')
+self.assert_qmp(result, 'return[0]/tray_open', True)
+self.assert_qmp_absent(result, 'return[0]/inserted')
+
+def test_tray_eject_change(self):
+result = self.vm.qmp('eject', device='drive0', force=True)
+self.assert_qmp(result, 'return', {})
+
+self.wait_for_open()
+
+result = self.vm.qmp('query-block')
+self.assert_qmp(result, 'return[0]/tray_open', True)
+self.assert_qmp_absent(result, 'return[0]/inserted')
+
+result = self.vm.qmp('blockdev-change-medium', device='drive0',
+   filename=new_img,
+   format=iotests.imgfmt)
+self.assert_qmp(result, 'return', {})
+
+self.wait_for_close()
+
+result = self.vm.qmp('query-block')
+self.assert_qmp(result, 'return[0]/tray_open', False)
+self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+def test_tray_open_close(self):
+result = self.vm.qmp('blockdev-open-tray', device='drive0', force=True)
+

[Qemu-block] [PULL v2 18/40] block: rename BlockdevSnapshot to BlockdevSnapshotSync

[Kevin Wolf]

From: Alberto Garcia 

We will introduce the 'blockdev-snapshot' command that will require
its own struct for the parameters, so we need to rename this one in
order to avoid name clashes.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Reviewed-by: Kevin Wolf 
Reviewed-by: Jeff Cody 
Signed-off-by: Kevin Wolf 
---
 blockdev.c   | 2 +-
 qapi-schema.json | 2 +-
 qapi/block-core.json | 8 
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 0c6c2a7..86a29d8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1168,7 +1168,7 @@ void qmp_blockdev_snapshot_sync(bool has_device, const 
char *device,
 bool has_format, const char *format,
 bool has_mode, NewImageMode mode, Error **errp)
 {
-BlockdevSnapshot snapshot = {
+BlockdevSnapshotSync snapshot = {
 .has_device = has_device,
 .device = (char *) device,
 .has_node_name = has_node_name,
diff --git a/qapi-schema.json b/qapi-schema.json
index ed7fc99..42694d2 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1534,7 +1534,7 @@
 ##
 { 'union': 'TransactionAction',
   'data': {
-   'blockdev-snapshot-sync': 'BlockdevSnapshot',
+   'blockdev-snapshot-sync': 'BlockdevSnapshotSync',
'drive-backup': 'DriveBackup',
'blockdev-backup': 'BlockdevBackup',
'abort': 'Abort',
diff --git a/qapi/block-core.json b/qapi/block-core.json
index fa08ba9..f592adc 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -682,7 +682,7 @@
   'data': [ 'existing', 'absolute-paths' ] }
 
 ##
-# @BlockdevSnapshot
+# @BlockdevSnapshotSync
 #
 # Either @device or @node-name must be set but not both.
 #
@@ -699,7 +699,7 @@
 # @mode: #optional whether and how QEMU should create a new image, default is
 #'absolute-paths'.
 ##
-{ 'struct': 'BlockdevSnapshot',
+{ 'struct': 'BlockdevSnapshotSync',
   'data': { '*device': 'str', '*node-name': 'str',
 'snapshot-file': 'str', '*snapshot-node-name': 'str',
 '*format': 'str', '*mode': 'NewImageMode' } }
@@ -790,7 +790,7 @@
 #
 # Generates a synchronous snapshot of a block device.
 #
-# For the arguments, see the documentation of BlockdevSnapshot.
+# For the arguments, see the documentation of BlockdevSnapshotSync.
 #
 # Returns: nothing on success
 #  If @device is not a valid block device, DeviceNotFound
@@ -798,7 +798,7 @@
 # Since 0.14.0
 ##
 { 'command': 'blockdev-snapshot-sync',
-  'data': 'BlockdevSnapshot' }
+  'data': 'BlockdevSnapshotSync' }
 
 ##
 # @change-backing-file
-- 
1.8.3.1

[Qemu-block] [PULL v2 04/40] block: Add functions for inheriting a BBRS

[Kevin Wolf]

From: Max Reitz 

In order to open a BDS which inherits a BB's root state,
blk_get_open_flags_from_root_state() is used to inquire the flags to be
passed to bdrv_open(), and blk_apply_root_state() is used to apply the
remaining state after the BDS has been opened.

Signed-off-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 block/block-backend.c  | 27 +++
 include/sysemu/block-backend.h |  2 ++
 2 files changed, 29 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 878c448..7d49539 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1239,6 +1239,33 @@ void blk_update_root_state(BlockBackend *blk)
 }
 }
 
+/*
+ * Applies the information in the root state to the given BlockDriverState. 
This
+ * does not include the flags which have to be specified for bdrv_open(), use
+ * blk_get_open_flags_from_root_state() to inquire them.
+ */
+void blk_apply_root_state(BlockBackend *blk, BlockDriverState *bs)
+{
+bs->detect_zeroes = blk->root_state.detect_zeroes;
+if (blk->root_state.throttle_group) {
+bdrv_io_limits_enable(bs, blk->root_state.throttle_group);
+}
+}
+
+/*
+ * Returns the flags to be used for bdrv_open() of a BlockDriverState which is
+ * supposed to inherit the root state.
+ */
+int blk_get_open_flags_from_root_state(BlockBackend *blk)
+{
+int bs_flags;
+
+bs_flags = blk->root_state.read_only ? 0 : BDRV_O_RDWR;
+bs_flags |= blk->root_state.open_flags & ~BDRV_O_RDWR;
+
+return bs_flags;
+}
+
 BlockBackendRootState *blk_get_root_state(BlockBackend *blk)
 {
 return >root_state;
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 14a6d32..40e315b 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -167,6 +167,8 @@ void blk_io_unplug(BlockBackend *blk);
 BlockAcctStats *blk_get_stats(BlockBackend *blk);
 BlockBackendRootState *blk_get_root_state(BlockBackend *blk);
 void blk_update_root_state(BlockBackend *blk);
+void blk_apply_root_state(BlockBackend *blk, BlockDriverState *bs);
+int blk_get_open_flags_from_root_state(BlockBackend *blk);
 
 void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
   BlockCompletionFunc *cb, void *opaque);
-- 
1.8.3.1

[Qemu-block] [PULL v2 07/40] blockdev: Add blockdev-remove-medium

[Kevin Wolf]

From: Max Reitz 

Signed-off-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 blockdev.c   | 51 +++
 qapi/block-core.json | 16 
 qmp-commands.hx  | 45 +
 3 files changed, 112 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index cb8a2f0..25635e3 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2147,6 +2147,57 @@ void qmp_blockdev_close_tray(const char *device, Error 
**errp)
 blk_dev_change_media_cb(blk, true);
 }
 
+void qmp_blockdev_remove_medium(const char *device, Error **errp)
+{
+BlockBackend *blk;
+BlockDriverState *bs;
+AioContext *aio_context;
+bool has_device;
+
+blk = blk_by_name(device);
+if (!blk) {
+error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+  "Device '%s' not found", device);
+return;
+}
+
+/* For BBs without a device, we can exchange the BDS tree at will */
+has_device = blk_get_attached_dev(blk);
+
+if (has_device && !blk_dev_has_removable_media(blk)) {
+error_setg(errp, "Device '%s' is not removable", device);
+return;
+}
+
+if (has_device && !blk_dev_is_tray_open(blk)) {
+error_setg(errp, "Tray of device '%s' is not open", device);
+return;
+}
+
+bs = blk_bs(blk);
+if (!bs) {
+return;
+}
+
+aio_context = bdrv_get_aio_context(bs);
+aio_context_acquire(aio_context);
+
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
+goto out;
+}
+
+/* This follows the convention established by bdrv_make_anon() */
+if (bs->device_list.tqe_prev) {
+QTAILQ_REMOVE(_states, bs, device_list);
+bs->device_list.tqe_prev = NULL;
+}
+
+blk_remove_bs(blk);
+
+out:
+aio_context_release(aio_context);
+}
+
 /* 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,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1cb719a..e19e82c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1924,6 +1924,22 @@
 { 'command': 'blockdev-close-tray',
   'data': { 'device': 'str' } }
 
+##
+# @blockdev-remove-medium:
+#
+# Removes a medium (a block driver state tree) from a block device. That block
+# device's tray must currently be open (unless there is no attached guest
+# device).
+#
+# If the tray is open and there is no medium inserted, this will be a no-op.
+#
+# @device: block device name
+#
+# Since: 2.5
+##
+{ 'command': 'blockdev-remove-medium',
+  'data': { 'device': 'str' } }
+
 
 ##
 # @BlockErrorAction
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4b16d73..6165398 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4019,6 +4019,51 @@ Example:
 EQMP
 
 {
+.name   = "blockdev-remove-medium",
+.args_type  = "device:s",
+.mhandler.cmd_new = qmp_marshal_blockdev_remove_medium,
+},
+
+SQMP
+blockdev-remove-medium
+--
+
+Removes a medium (a block driver state tree) from a block device. That block
+device's tray must currently be open (unless there is no attached guest 
device).
+
+If the tray is open and there is no medium inserted, this will be a no-op.
+
+Arguments:
+
+- "device": block device name (json-string)
+
+Example:
+
+-> { "execute": "blockdev-remove-medium",
+ "arguments": { "device": "ide1-cd0" } }
+
+<- { "error": { "class": "GenericError",
+"desc": "Tray of device 'ide1-cd0' is not open" } }
+
+-> { "execute": "blockdev-open-tray",
+ "arguments": { "device": "ide1-cd0" } }
+
+<- { "timestamp": { "seconds": 1418751627,
+"microseconds": 549958 },
+ "event": "DEVICE_TRAY_MOVED",
+ "data": { "device": "ide1-cd0",
+   "tray-open": true } }
+
+<- { "return": {} }
+
+-> { "execute": "blockdev-remove-medium",
+ "arguments": { "device": "ide1-cd0" } }
+
+<- { "return": {} }
+
+EQMP
+
+{
 .name   = "query-named-block-nodes",
 .args_type  = "",
 .mhandler.cmd_new = qmp_marshal_query_named_block_nodes,
-- 
1.8.3.1

[Qemu-block] [PULL v2 17/40] block: check for existing device IDs in external_snapshot_prepare()

[Kevin Wolf]

From: Alberto Garcia 

The 'snapshot-node-name' parameter of blockdev-snapshot-sync allows
setting the node name of the image that is going to be created.

Before creating the image, external_snapshot_prepare() checks that the
name is not already being used. The check is however incomplete since
it only considers existing node names, but node names must not clash
with device IDs either because they share the same namespace.

If the user attempts to create a snapshot using the name of an
existing device for the 'snapshot-node-name' parameter the operation
will eventually fail, but only after the new image has been created.

This patch replaces bdrv_find_node() with bdrv_lookup_bs() to extend
the check to existing device IDs, and thus detect possible name
clashes before the new image is created.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Jeff Cody 
Signed-off-by: Kevin Wolf 
---
 blockdev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 34f6e5b..0c6c2a7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1579,8 +1579,9 @@ static void external_snapshot_prepare(BlkTransactionState 
*common,
 return;
 }
 
-if (has_snapshot_node_name && bdrv_find_node(snapshot_node_name)) {
-error_setg(errp, "New snapshot node name already existing");
+if (has_snapshot_node_name &&
+bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) {
+error_setg(errp, "New snapshot node name already in use");
 return;
 }
 
-- 
1.8.3.1

[Qemu-block] [PULL v2 21/40] block: add tests for the 'blockdev-snapshot' command

[Kevin Wolf]

From: Alberto Garcia 

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Jeff Cody 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/085 | 102 ++---
 tests/qemu-iotests/085.out |  34 ++-
 2 files changed, 128 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
index 56cd6f8..9484117 100755
--- a/tests/qemu-iotests/085
+++ b/tests/qemu-iotests/085
@@ -7,6 +7,7 @@
 # snapshots are performed.
 #
 # Copyright (C) 2014 Red Hat, Inc.
+# Copyright (C) 2015 Igalia, S.L.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -34,17 +35,17 @@ status=1# failure is the default!
 snapshot_virt0="snapshot-v0.qcow2"
 snapshot_virt1="snapshot-v1.qcow2"
 
-MAX_SNAPSHOTS=10
+SNAPSHOTS=10
 
 _cleanup()
 {
 _cleanup_qemu
-for i in $(seq 1 ${MAX_SNAPSHOTS})
+for i in $(seq 1 ${SNAPSHOTS})
 do
 rm -f "${TEST_DIR}/${i}-${snapshot_virt0}"
 rm -f "${TEST_DIR}/${i}-${snapshot_virt1}"
 done
-   _cleanup_test_img
+rm -f "${TEST_IMG}.1" "${TEST_IMG}.2"
 
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -85,18 +86,50 @@ function create_group_snapshot()
 _send_qemu_cmd $h "${cmd}" "return"
 }
 
+# ${1}: unique identifier for the snapshot filename
+# ${2}: true: open backing images; false: don't open them (default)
+function add_snapshot_image()
+{
+if [ "${2}" = "true" ]; then
+extra_params=""
+else
+extra_params="'backing': '', "
+fi
+base_image="${TEST_DIR}/$((${1}-1))-${snapshot_virt0}"
+snapshot_file="${TEST_DIR}/${1}-${snapshot_virt0}"
+_make_test_img -b "${base_image}" "$size"
+mv "${TEST_IMG}" "${snapshot_file}"
+cmd="{ 'execute': 'blockdev-add', 'arguments':
+   { 'options':
+ { 'driver': 'qcow2', 'node-name': 'snap_"${1}"', "${extra_params}"
+   'file':
+   { 'driver': 'file', 'filename': '"${snapshot_file}"' } } } }"
+_send_qemu_cmd $h "${cmd}" "return"
+}
+
+# ${1}: unique identifier for the snapshot filename
+# ${2}: expected response, defaults to 'return'
+function blockdev_snapshot()
+{
+cmd="{ 'execute': 'blockdev-snapshot',
+  'arguments': { 'node': 'virtio0',
+ 'overlay':'snap_"${1}"' } }"
+_send_qemu_cmd $h "${cmd}" "${2:-return}"
+}
+
 size=128M
 
 _make_test_img $size
-mv "${TEST_IMG}" "${TEST_IMG}.orig"
+mv "${TEST_IMG}" "${TEST_IMG}.1"
 _make_test_img $size
+mv "${TEST_IMG}" "${TEST_IMG}.2"
 
 echo
 echo === Running QEMU ===
 echo
 
 qemu_comm_method="qmp"
-_launch_qemu -drive file="${TEST_IMG}.orig",if=virtio -drive 
file="${TEST_IMG}",if=virtio
+_launch_qemu -drive file="${TEST_IMG}.1",if=virtio -drive 
file="${TEST_IMG}.2",if=virtio
 h=$QEMU_HANDLE
 
 echo
@@ -105,6 +138,8 @@ echo
 
 _send_qemu_cmd $h "{ 'execute': 'qmp_capabilities' }" "return"
 
+# Tests for the blockdev-snapshot-sync command
+
 echo
 echo === Create a single snapshot on virtio0 ===
 echo
@@ -132,11 +167,66 @@ echo
 echo === Create several transactional group snapshots ===
 echo
 
-for i in $(seq 2 ${MAX_SNAPSHOTS})
+for i in $(seq 2 ${SNAPSHOTS})
 do
 create_group_snapshot ${i}
 done
 
+# Tests for the blockdev-snapshot command
+
+echo
+echo === Create a couple of snapshots using blockdev-snapshot ===
+echo
+
+SNAPSHOTS=$((${SNAPSHOTS}+1))
+add_snapshot_image ${SNAPSHOTS}
+blockdev_snapshot ${SNAPSHOTS}
+
+SNAPSHOTS=$((${SNAPSHOTS}+1))
+add_snapshot_image ${SNAPSHOTS}
+blockdev_snapshot ${SNAPSHOTS}
+
+echo
+echo === Invalid command - snapshot node used as active layer ===
+echo
+
+blockdev_snapshot ${SNAPSHOTS} error
+
+_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
+ 'arguments': { 'node':'virtio0',
+'overlay':'virtio0' }
+   }" "error"
+
+_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
+ 'arguments': { 'node':'virtio0',
+'overlay':'virtio1' }
+   }" "error"
+
+echo
+echo === Invalid command - snapshot node used as backing hd ===
+echo
+
+blockdev_snapshot $((${SNAPSHOTS}-1)) error
+
+echo
+echo === Invalid command - snapshot node has a backing image ===
+echo
+
+SNAPSHOTS=$((${SNAPSHOTS}+1))
+add_snapshot_image ${SNAPSHOTS} true
+blockdev_snapshot ${SNAPSHOTS} error
+
+echo
+echo === Invalid command - The node does not exist ===
+echo
+
+blockdev_snapshot $((${SNAPSHOTS}+1)) error
+
+_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
+ 'arguments': { 'node':'nodevice',
+'overlay':'snap_"${SNAPSHOTS}"' }
+   }" "error"
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git 

[Qemu-block] [PULL v2 19/40] block: support passing 'backing': '' to 'blockdev-add'

[Kevin Wolf]

From: Alberto Garcia 

Passing an empty string allows opening an image but not its backing
file. This was already described in the API documentation, only the
implementation was missing.

This is useful for creating snapshots using images opened with
blockdev-add, since they are not supposed to have a backing image
before the operation.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Kevin Wolf 
Reviewed-by: Jeff Cody 
Signed-off-by: Kevin Wolf 
---
 block.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/block.c b/block.c
index a99e6d8..3493501 100644
--- a/block.c
+++ b/block.c
@@ -1393,6 +1393,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 BlockDriverState *bs;
 BlockDriver *drv = NULL;
 const char *drvname;
+const char *backing;
 Error *local_err = NULL;
 int snapshot_flags = 0;
 
@@ -1460,6 +1461,12 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 
 assert(drvname || !(flags & BDRV_O_PROTOCOL));
 
+backing = qdict_get_try_str(options, "backing");
+if (backing && *backing == '\0') {
+flags |= BDRV_O_NO_BACKING;
+qdict_del(options, "backing");
+}
+
 bs->open_flags = flags;
 bs->options = options;
 options = qdict_clone_shallow(options);
-- 
1.8.3.1

[Qemu-block] [PULL v2 06/40] blockdev: Add blockdev-close-tray

[Kevin Wolf]

From: Max Reitz 

Signed-off-by: Max Reitz 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 blockdev.c   | 23 +++
 qapi/block-core.json | 16 
 qmp-commands.hx  | 35 +++
 3 files changed, 74 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index d01a99e..cb8a2f0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2124,6 +2124,29 @@ void qmp_blockdev_open_tray(const char *device, bool 
has_force, bool force,
 }
 }
 
+void qmp_blockdev_close_tray(const char *device, Error **errp)
+{
+BlockBackend *blk;
+
+blk = blk_by_name(device);
+if (!blk) {
+error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+  "Device '%s' not found", device);
+return;
+}
+
+if (!blk_dev_has_removable_media(blk)) {
+error_setg(errp, "Device '%s' is not removable", device);
+return;
+}
+
+if (!blk_dev_is_tray_open(blk)) {
+return;
+}
+
+blk_dev_change_media_cb(blk, true);
+}
+
 /* 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,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b65ab81..1cb719a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1908,6 +1908,22 @@
   'data': { 'device': 'str',
 '*force': 'bool' } }
 
+##
+# @blockdev-close-tray:
+#
+# Closes a block device's tray. If there is a block driver state tree 
associated
+# with the block device (which is currently ejected), that tree will be loaded
+# as the medium.
+#
+# If the tray was already closed before, this will be a no-op.
+#
+# @device: block device name
+#
+# Since: 2.5
+##
+{ 'command': 'blockdev-close-tray',
+  'data': { 'device': 'str' } }
+
 
 ##
 # @BlockErrorAction
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 0b796f9..4b16d73 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3984,6 +3984,41 @@ Example:
 EQMP
 
 {
+.name   = "blockdev-close-tray",
+.args_type  = "device:s",
+.mhandler.cmd_new = qmp_marshal_blockdev_close_tray,
+},
+
+SQMP
+blockdev-close-tray
+---
+
+Closes a block device's tray. If there is a block driver state tree associated
+with the block device (which is currently ejected), that tree will be loaded as
+the medium.
+
+If the tray was already closed before, this will be a no-op.
+
+Arguments:
+
+- "device": block device name (json-string)
+
+Example:
+
+-> { "execute": "blockdev-close-tray",
+ "arguments": { "device": "ide1-cd0" } }
+
+<- { "timestamp": { "seconds": 1418751345,
+"microseconds": 272147 },
+ "event": "DEVICE_TRAY_MOVED",
+ "data": { "device": "ide1-cd0",
+   "tray-open": false } }
+
+<- { "return": {} }
+
+EQMP
+
+{
 .name   = "query-named-block-nodes",
 .args_type  = "",
 .mhandler.cmd_new = qmp_marshal_query_named_block_nodes,
-- 
1.8.3.1

[Qemu-block] [PULL v2 20/40] block: add a 'blockdev-snapshot' QMP command

[Kevin Wolf]

From: Alberto Garcia 

One of the limitations of the 'blockdev-snapshot-sync' command is that
it does not allow passing BlockdevOptions to the newly created
snapshots, so they are always opened using the default values.

Extending the command to allow passing options is not a practical
solution because there is overlap between those options and some of
the existing parameters of the command.

This patch introduces a new 'blockdev-snapshot' command with a simpler
interface: it just takes two references to existing block devices that
will be used as the source and target for the snapshot.

Since the main difference between the two commands is that one of them
creates and opens the target image, while the other uses an already
opened one, the bulk of the implementation is shared.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 blockdev.c   | 165 ---
 qapi-schema.json |   2 +
 qapi/block-core.json |  28 +
 qmp-commands.hx  |  38 
 4 files changed, 172 insertions(+), 61 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 86a29d8..7645d49 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1185,6 +1185,18 @@ void qmp_blockdev_snapshot_sync(bool has_device, const 
char *device,
, errp);
 }
 
+void qmp_blockdev_snapshot(const char *node, const char *overlay,
+   Error **errp)
+{
+BlockdevSnapshot snapshot_data = {
+.node = (char *) node,
+.overlay = (char *) overlay
+};
+
+blockdev_do_action(TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT,
+   _data, errp);
+}
+
 void qmp_blockdev_snapshot_internal_sync(const char *device,
  const char *name,
  Error **errp)
@@ -1530,58 +1542,48 @@ typedef struct ExternalSnapshotState {
 static void external_snapshot_prepare(BlkTransactionState *common,
   Error **errp)
 {
-int flags, ret;
-QDict *options;
+int flags = 0, ret;
+QDict *options = NULL;
 Error *local_err = NULL;
-bool has_device = false;
+/* Device and node name of the image to generate the snapshot from */
 const char *device;
-bool has_node_name = false;
 const char *node_name;
-bool has_snapshot_node_name = false;
-const char *snapshot_node_name;
+/* Reference to the new image (for 'blockdev-snapshot') */
+const char *snapshot_ref;
+/* File name of the new image (for 'blockdev-snapshot-sync') */
 const char *new_image_file;
-const char *format = "qcow2";
-enum NewImageMode mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
 ExternalSnapshotState *state =
  DO_UPCAST(ExternalSnapshotState, common, common);
 TransactionAction *action = common->action;
 
-/* get parameters */
-g_assert(action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC);
-
-has_device = action->u.blockdev_snapshot_sync->has_device;
-device = action->u.blockdev_snapshot_sync->device;
-has_node_name = action->u.blockdev_snapshot_sync->has_node_name;
-node_name = action->u.blockdev_snapshot_sync->node_name;
-has_snapshot_node_name =
-action->u.blockdev_snapshot_sync->has_snapshot_node_name;
-snapshot_node_name = action->u.blockdev_snapshot_sync->snapshot_node_name;
-
-new_image_file = action->u.blockdev_snapshot_sync->snapshot_file;
-if (action->u.blockdev_snapshot_sync->has_format) {
-format = action->u.blockdev_snapshot_sync->format;
-}
-if (action->u.blockdev_snapshot_sync->has_mode) {
-mode = action->u.blockdev_snapshot_sync->mode;
+/* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar
+ * purpose but a different set of parameters */
+switch (action->type) {
+case TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT:
+{
+BlockdevSnapshot *s = action->u.blockdev_snapshot;
+device = s->node;
+node_name = s->node;
+new_image_file = NULL;
+snapshot_ref = s->overlay;
+}
+break;
+case TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC:
+{
+BlockdevSnapshotSync *s = action->u.blockdev_snapshot_sync;
+device = s->has_device ? s->device : NULL;
+node_name = s->has_node_name ? s->node_name : NULL;
+new_image_file = s->snapshot_file;
+snapshot_ref = NULL;
+}
+break;
+default:
+g_assert_not_reached();
 }
 
 /* start processing */
-state->old_bs = bdrv_lookup_bs(has_device ? device : NULL,
-   has_node_name ? node_name : NULL,
-   _err);
-if (local_err) {
-error_propagate(errp, local_err);
-return;
-}

[Qemu-block] [PULL v2 23/40] qemu-iotests: Test the reopening of overlay_bs in 'block-commit'

[Kevin Wolf]

From: Alberto Garcia 

The 'block-commit' command needs the overlay image of 'top' to
be opened in read-write mode in order to update the backing file
string. If 'top' is not the active layer or its backing file then its
overlay needs to be reopened during the block job.

This is a test case for that scenario.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/040 | 30 ++
 tests/qemu-iotests/040.out |  4 ++--
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index ea2f98e..5bdaf3d 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -41,6 +41,7 @@ class ImageCommitTestCase(iotests.QMPTestCase):
 while not completed:
 for event in self.vm.get_qmp_events(wait=True):
 if event['event'] == 'BLOCK_JOB_COMPLETED':
+self.assert_qmp_absent(event, 'data/error')
 self.assert_qmp(event, 'data/type', 'commit')
 self.assert_qmp(event, 'data/device', 'drive0')
 self.assert_qmp(event, 'data/offset', event['data']['len'])
@@ -251,5 +252,34 @@ class TestSetSpeed(ImageCommitTestCase):
 class TestActiveZeroLengthImage(TestSingleDrive):
 image_len = 0
 
+class TestReopenOverlay(ImageCommitTestCase):
+image_len = 1024 * 1024
+img0 = os.path.join(iotests.test_dir, '0.img')
+img1 = os.path.join(iotests.test_dir, '1.img')
+img2 = os.path.join(iotests.test_dir, '2.img')
+img3 = os.path.join(iotests.test_dir, '3.img')
+
+def setUp(self):
+iotests.create_image(self.img0, self.image_len)
+qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % 
self.img0, self.img1)
+qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % 
self.img1, self.img2)
+qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % 
self.img2, self.img3)
+qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xab 0 128K', self.img1)
+self.vm = iotests.VM().add_drive(self.img3)
+self.vm.launch()
+
+def tearDown(self):
+self.vm.shutdown()
+os.remove(self.img0)
+os.remove(self.img1)
+os.remove(self.img2)
+os.remove(self.img3)
+
+# This tests what happens when the overlay image of the 'top' node
+# needs to be reopened in read-write mode in order to update the
+# backing image string.
+def test_reopen_overlay(self):
+self.run_commit_test(self.img1, self.img0)
+
 if __name__ == '__main__':
 iotests.main(supported_fmts=['qcow2', 'qed'])
diff --git a/tests/qemu-iotests/040.out b/tests/qemu-iotests/040.out
index 42314e9..4fd1c2d 100644
--- a/tests/qemu-iotests/040.out
+++ b/tests/qemu-iotests/040.out
@@ -1,5 +1,5 @@
-
+.
 --
-Ran 24 tests
+Ran 25 tests
 
 OK
-- 
1.8.3.1

[Qemu-block] [PULL v2 15/40] hmp: Add read-only-mode option to change command

[Kevin Wolf]

From: Max Reitz 

Expose the new read-only-mode option of 'blockdev-change-medium' for the
'change' HMP command.

Signed-off-by: Max Reitz 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 hmp-commands.hx | 20 +---
 hmp.c   | 22 +-
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 3a4ae39..814ea86 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -194,8 +194,8 @@ ETEXI
 
 {
 .name   = "change",
-.args_type  = "device:B,target:F,arg:s?",
-.params = "device filename [format]",
+.args_type  = "device:B,target:F,arg:s?,read-only-mode:s?",
+.params = "device filename [format [read-only-mode]]",
 .help   = "change a removable medium, optional format",
 .mhandler.cmd = hmp_change,
 },
@@ -206,7 +206,7 @@ STEXI
 Change the configuration of a device.
 
 @table @option
-@item change @var{diskdevice} @var{filename} [@var{format}]
+@item change @var{diskdevice} @var{filename} [@var{format} 
[@var{read-only-mode}]]
 Change the medium for a removable disk device to point to @var{filename}. eg
 
 @example
@@ -215,6 +215,20 @@ Change the medium for a removable disk device to point to 
@var{filename}. eg
 
 @var{format} is optional.
 
+@var{read-only-mode} may be used to change the read-only status of the device.
+It accepts the following values:
+
+@table @var
+@item retain
+Retains the current status; this is the default.
+
+@item read-only
+Makes the device read-only.
+
+@item read-write
+Makes the device writable.
+@end table
+
 @item change vnc @var{display},@var{options}
 Change the configuration of the VNC server. The valid syntax for @var{display}
 and @var{options} are described at @ref{sec_invocation}. eg
diff --git a/hmp.c b/hmp.c
index efa07a2..8166fd2 100644
--- a/hmp.c
+++ b/hmp.c
@@ -27,6 +27,7 @@
 #include "qapi/opts-visitor.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/string-output-visitor.h"
+#include "qapi/util.h"
 #include "qapi-visit.h"
 #include "ui/console.h"
 #include "block/qapi.h"
@@ -1336,9 +1337,16 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 const char *device = qdict_get_str(qdict, "device");
 const char *target = qdict_get_str(qdict, "target");
 const char *arg = qdict_get_try_str(qdict, "arg");
+const char *read_only = qdict_get_try_str(qdict, "read-only-mode");
+BlockdevChangeReadOnlyMode read_only_mode = 0;
 Error *err = NULL;
 
 if (strcmp(device, "vnc") == 0) {
+if (read_only) {
+monitor_printf(mon,
+   "Parameter 'read-only-mode' is invalid for VNC\n");
+return;
+}
 if (strcmp(target, "passwd") == 0 ||
 strcmp(target, "password") == 0) {
 if (!arg) {
@@ -1348,7 +1356,19 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 }
 qmp_change("vnc", target, !!arg, arg, );
 } else {
-qmp_blockdev_change_medium(device, target, !!arg, arg, false, 0, );
+if (read_only) {
+read_only_mode =
+qapi_enum_parse(BlockdevChangeReadOnlyMode_lookup,
+read_only, BLOCKDEV_CHANGE_READ_ONLY_MODE_MAX,
+BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN, );
+if (err) {
+hmp_handle_error(mon, );
+return;
+}
+}
+
+qmp_blockdev_change_medium(device, target, !!arg, arg,
+   !!read_only, read_only_mode, );
 if (err &&
 error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
 error_free(err);
-- 
1.8.3.1

[Qemu-block] [PULL v2 29/40] block: Remove inner quotation marks in iotest 085

[Kevin Wolf]

From: Alberto Garcia 

This patch removes the inner quotation marks in all cases like this:

   cmd=" ... "${variable}" ... "

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/085 | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
index 9484117..80e547d 100755
--- a/tests/qemu-iotests/085
+++ b/tests/qemu-iotests/085
@@ -65,7 +65,7 @@ function create_single_snapshot()
 {
 cmd="{ 'execute': 'blockdev-snapshot-sync',
   'arguments': { 'device': 'virtio0',
- 
'snapshot-file':'"${TEST_DIR}/${1}-${snapshot_virt0}"',
+ 
'snapshot-file':'${TEST_DIR}/${1}-${snapshot_virt0}',
  'format': 'qcow2' } }"
 _send_qemu_cmd $h "${cmd}" "return"
 }
@@ -77,10 +77,10 @@ function create_group_snapshot()
{'actions': [
{ 'type': 'blockdev-snapshot-sync', 'data' :
{ 'device': 'virtio0',
-  'snapshot-file': '"${TEST_DIR}/${1}-${snapshot_virt0}"' 
} },
+  'snapshot-file': '${TEST_DIR}/${1}-${snapshot_virt0}' } 
},
{ 'type': 'blockdev-snapshot-sync', 'data' :
{ 'device': 'virtio1',
-   'snapshot-file': '"${TEST_DIR}/${1}-${snapshot_virt1}"' 
} } ]
+   'snapshot-file': '${TEST_DIR}/${1}-${snapshot_virt1}' } 
} ]
  } }"
 
 _send_qemu_cmd $h "${cmd}" "return"
@@ -101,9 +101,9 @@ function add_snapshot_image()
 mv "${TEST_IMG}" "${snapshot_file}"
 cmd="{ 'execute': 'blockdev-add', 'arguments':
{ 'options':
- { 'driver': 'qcow2', 'node-name': 'snap_"${1}"', "${extra_params}"
+ { 'driver': 'qcow2', 'node-name': 'snap_${1}', ${extra_params}
'file':
-   { 'driver': 'file', 'filename': '"${snapshot_file}"' } } } }"
+   { 'driver': 'file', 'filename': '${snapshot_file}' } } } }"
 _send_qemu_cmd $h "${cmd}" "return"
 }
 
@@ -113,7 +113,7 @@ function blockdev_snapshot()
 {
 cmd="{ 'execute': 'blockdev-snapshot',
   'arguments': { 'node': 'virtio0',
- 'overlay':'snap_"${1}"' } }"
+ 'overlay':'snap_${1}' } }"
 _send_qemu_cmd $h "${cmd}" "${2:-return}"
 }
 
@@ -152,7 +152,7 @@ echo === Invalid command - missing device and nodename ===
 echo
 
 _send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot-sync',
- 'arguments': { 
'snapshot-file':'"${TEST_DIR}/1-${snapshot_virt0}"',
+ 'arguments': { 
'snapshot-file':'${TEST_DIR}/1-${snapshot_virt0}',
  'format': 'qcow2' } }" "error"
 
 echo
@@ -224,7 +224,7 @@ blockdev_snapshot $((${SNAPSHOTS}+1)) error
 
 _send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
  'arguments': { 'node':'nodevice',
-'overlay':'snap_"${SNAPSHOTS}"' }
+'overlay':'snap_${SNAPSHOTS}' }
}" "error"
 
 # success, all done
-- 
1.8.3.1

[Qemu-block] [PULL v2 26/40] throttle: Check for pending requests in throttle_group_unregister_bs()

[Kevin Wolf]

From: Alberto Garcia 

throttle_group_unregister_bs() removes a BlockDriverState from its
throttling group and destroys the timers. This means that there must
be no pending throttled requests at that point (because it would be
impossible to complete them), so the caller has to drain them first.

At the moment throttle_group_unregister_bs() is only called from
bdrv_io_limits_disable(), which already takes care of draining the
requests, so there's nothing to worry about, but this patch makes
this invariant explicit in the documentation and adds the relevant
assertions.

Signed-off-by: Alberto Garcia 
Signed-off-by: Kevin Wolf 
---
 block/throttle-groups.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 3419af7..13b5baa 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -437,6 +437,9 @@ void throttle_group_register_bs(BlockDriverState *bs, const 
char *groupname)
  * list, destroying the timers and setting the throttle_state pointer
  * to NULL.
  *
+ * The BlockDriverState must not have pending throttled requests, so
+ * the caller has to drain them first.
+ *
  * The group will be destroyed if it's empty after this operation.
  *
  * @bs: the BlockDriverState to remove
@@ -446,6 +449,10 @@ void throttle_group_unregister_bs(BlockDriverState *bs)
 ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
 int i;
 
+assert(bs->pending_reqs[0] == 0 && bs->pending_reqs[1] == 0);
+assert(qemu_co_queue_empty(>throttled_reqs[0]));
+assert(qemu_co_queue_empty(>throttled_reqs[1]));
+
 qemu_mutex_lock(>lock);
 for (i = 0; i < 2; i++) {
 if (tg->tokens[i] == bs) {
-- 
1.8.3.1

[Qemu-block] [PULL v2 14/40] blockdev: read-only-mode for blockdev-change-medium

[Kevin Wolf]

From: Max Reitz 

Add an option to qmp_blockdev_change_medium() which allows changing the
read-only status of the block device whose medium is changed.

Some drives do not have a inherently fixed read-only status; for
instance, floppy disks can be set read-only or writable independently of
the drive. Some users may find it useful to be able to therefore change
the read-only status of a block device when changing the medium.

Signed-off-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 blockdev.c   | 22 ++
 hmp.c|  2 +-
 qapi/block-core.json | 24 +++-
 qmp-commands.hx  | 24 +++-
 qmp.c|  3 ++-
 5 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index b3a958c..34f6e5b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2145,6 +2145,8 @@ void qmp_blockdev_insert_medium(const char *device, const 
char *node_name,
 
 void qmp_blockdev_change_medium(const char *device, const char *filename,
 bool has_format, const char *format,
+bool has_read_only,
+BlockdevChangeReadOnlyMode read_only,
 Error **errp)
 {
 BlockBackend *blk;
@@ -2166,6 +2168,26 @@ void qmp_blockdev_change_medium(const char *device, 
const char *filename,
 
 bdrv_flags = blk_get_open_flags_from_root_state(blk);
 
+if (!has_read_only) {
+read_only = BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN;
+}
+
+switch (read_only) {
+case BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN:
+break;
+
+case BLOCKDEV_CHANGE_READ_ONLY_MODE_READ_ONLY:
+bdrv_flags &= ~BDRV_O_RDWR;
+break;
+
+case BLOCKDEV_CHANGE_READ_ONLY_MODE_READ_WRITE:
+bdrv_flags |= BDRV_O_RDWR;
+break;
+
+default:
+abort();
+}
+
 if (has_format) {
 options = qdict_new();
 qdict_put(options, "driver", qstring_from_str(format));
diff --git a/hmp.c b/hmp.c
index 5e5358f..efa07a2 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1348,7 +1348,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 }
 qmp_change("vnc", target, !!arg, arg, );
 } else {
-qmp_blockdev_change_medium(device, target, !!arg, arg, );
+qmp_blockdev_change_medium(device, target, !!arg, arg, false, 0, );
 if (err &&
 error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
 error_free(err);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index e9fa649..fa08ba9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1959,6 +1959,24 @@
 
 
 ##
+# @BlockdevChangeReadOnlyMode:
+#
+# Specifies the new read-only mode of a block device subject to the
+# @blockdev-change-medium command.
+#
+# @retain:  Retains the current read-only mode
+#
+# @read-only:   Makes the device read-only
+#
+# @read-write:  Makes the device writable
+#
+# Since: 2.3
+##
+{ 'enum': 'BlockdevChangeReadOnlyMode',
+  'data': ['retain', 'read-only', 'read-write'] }
+
+
+##
 # @blockdev-change-medium:
 #
 # Changes the medium inserted into a block device by ejecting the current 
medium
@@ -1973,12 +1991,16 @@
 # @format:  #optional, format to open the new image with (defaults to
 #   the probed format)
 #
+# @read-only-mode:  #optional, change the read-only mode of the device; 
defaults
+#   to 'retain'
+#
 # Since: 2.5
 ##
 { 'command': 'blockdev-change-medium',
   'data': { 'device': 'str',
 'filename': 'str',
-'*format': 'str' } }
+'*format': 'str',
+'*read-only-mode': 'BlockdevChangeReadOnlyMode' } }
 
 
 ##
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d6e4d7e..707e0bd 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4164,7 +4164,7 @@ EQMP
 
 {
 .name   = "blockdev-change-medium",
-.args_type  = "device:B,filename:F,format:s?",
+.args_type  = "device:B,filename:F,format:s?,read-only-mode:s?",
 .mhandler.cmd_new = qmp_marshal_blockdev_change_medium,
 },
 
@@ -4180,6 +4180,8 @@ Arguments:
 - "device": device name (json-string)
 - "filename": filename of the new image (json-string)
 - "format": format of the new image (json-string, optional)
+- "read-only-mode": new read-only mode (json-string, optional)
+  - Possible values: "retain" (default), "read-only", "read-write"
 
 Examples:
 
@@ -4191,6 +4193,26 @@ Examples:
 "format": "raw" } }
 <- { "return": {} }
 
+2. Load a read-only medium into a writable drive
+
+-> { "execute": "blockdev-change-medium",
+ "arguments": { "device": "isa-fd0",
+"filename": "/srv/images/ro.img",
+"format": "raw",
+"read-only-mode": "retain" } }
+
+<- { "error":
+ { "class": 

[Qemu-block] [PULL v2 24/40] qcow2: avoid misaligned 64bit bswap

[Kevin Wolf]

From: John Snow 

If we create a buffer directly on the stack by using 12 bytes, there's
no guarantee the 64bit value we want to swap will be aligned, which
could cause errors with undefined behavior.

Spotted with clang -fsanitize=undefined and observed in iotests 15, 26,
44, 115 and 121.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Signed-off-by: Kevin Wolf 
---
 block/qcow2-refcount.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 4b81c8d..6e0e5bd 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -560,13 +560,16 @@ static int alloc_refcount_block(BlockDriverState *bs,
 }
 
 /* Hook up the new refcount table in the qcow2 header */
-uint8_t data[12];
-cpu_to_be64w((uint64_t*)data, table_offset);
-cpu_to_be32w((uint32_t*)(data + 8), table_clusters);
+struct QEMU_PACKED {
+uint64_t d64;
+uint32_t d32;
+} data;
+cpu_to_be64w(, table_offset);
+cpu_to_be32w(, table_clusters);
 BLKDBG_EVENT(bs->file, BLKDBG_REFBLOCK_ALLOC_SWITCH_TABLE);
 ret = bdrv_pwrite_sync(bs->file->bs,
offsetof(QCowHeader, refcount_table_offset),
-   data, sizeof(data));
+   , sizeof(data));
 if (ret < 0) {
 goto fail_table;
 }
-- 
1.8.3.1

[Qemu-block] [PULL v2 27/40] throttle: Use bs->throttle_state instead of bs->io_limits_enabled

[Kevin Wolf]

From: Alberto Garcia 

There are two ways to check for I/O limits in a BlockDriverState:

- bs->throttle_state: if this pointer is not NULL, it means that this
  BDS is member of a throttling group, its ThrottleTimers structure
  has been initialized and its I/O limits are ready to be applied.

- bs->io_limits_enabled: if true it means that the throttle_state
  pointer is valid _and_ the limits are currently enabled.

The latter is used in several places to check whether a BDS has I/O
limits configured, but what it really checks is whether requests
are being throttled or not. For example, io_limits_enabled can be
temporarily set to false in cases like bdrv_read_unthrottled() without
otherwise touching the throtting configuration of that BDS.

This patch replaces bs->io_limits_enabled with bs->throttle_state in
all cases where what we really want to check is the existence of I/O
limits, not whether they are currently enabled or not.

Signed-off-by: Alberto Garcia 
Signed-off-by: Kevin Wolf 
---
 block.c   | 6 +++---
 block/qapi.c  | 2 +-
 blockdev.c| 4 ++--
 include/block/block_int.h | 5 -
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index 3493501..cffac75 100644
--- a/block.c
+++ b/block.c
@@ -1907,7 +1907,7 @@ void bdrv_close(BlockDriverState *bs)
 }
 
 /* Disable I/O limits and drain all pending throttled requests */
-if (bs->io_limits_enabled) {
+if (bs->throttle_state) {
 bdrv_io_limits_disable(bs);
 }
 
@@ -3712,7 +3712,7 @@ void bdrv_detach_aio_context(BlockDriverState *bs)
 baf->detach_aio_context(baf->opaque);
 }
 
-if (bs->io_limits_enabled) {
+if (bs->throttle_state) {
 throttle_timers_detach_aio_context(>throttle_timers);
 }
 if (bs->drv->bdrv_detach_aio_context) {
@@ -3748,7 +3748,7 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
 if (bs->drv->bdrv_attach_aio_context) {
 bs->drv->bdrv_attach_aio_context(bs, new_context);
 }
-if (bs->io_limits_enabled) {
+if (bs->throttle_state) {
 throttle_timers_attach_aio_context(>throttle_timers, new_context);
 }
 
diff --git a/block/qapi.c b/block/qapi.c
index ec0f513..89d4274 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -64,7 +64,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs, 
Error **errp)
 info->backing_file_depth = bdrv_get_backing_file_depth(bs);
 info->detect_zeroes = bs->detect_zeroes;
 
-if (bs->io_limits_enabled) {
+if (bs->throttle_state) {
 ThrottleConfig cfg;
 
 throttle_group_get_config(bs, );
diff --git a/blockdev.c b/blockdev.c
index 7645d49..3598b01 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2361,14 +2361,14 @@ void qmp_block_set_io_throttle(const char *device, 
int64_t bps, int64_t bps_rd,
 if (throttle_enabled()) {
 /* Enable I/O limits if they're not enabled yet, otherwise
  * just update the throttling group. */
-if (!bs->io_limits_enabled) {
+if (!bs->throttle_state) {
 bdrv_io_limits_enable(bs, has_group ? group : device);
 } else if (has_group) {
 bdrv_io_limits_update_group(bs, group);
 }
 /* Set the new throttling configuration */
 bdrv_set_io_limits(bs, );
-} else if (bs->io_limits_enabled) {
+} else if (bs->throttle_state) {
 /* If all throttling settings are set to 0, disable I/O limits */
 bdrv_io_limits_disable(bs);
 }
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 6a3f64d..603145a 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -390,7 +390,10 @@ struct BlockDriverState {
 /* number of in-flight serialising requests */
 unsigned int serialising_in_flight;
 
-/* I/O throttling */
+/* I/O throttling.
+ * throttle_state tells us if this BDS has I/O limits configured.
+ * io_limits_enabled tells us if they are currently being
+ * enforced, but it can be temporarily set to false */
 CoQueue  throttled_reqs[2];
 bool io_limits_enabled;
 /* The following fields are protected by the ThrottleGroup lock.
-- 
1.8.3.1

[Qemu-block] [PULL v2 31/40] qemu-io: fix cvtnum lval types

[Kevin Wolf]

From: John Snow 

cvtnum() returns int64_t: we should not be storing this
result inside of an int.

In a few cases, we need an extra sprinkling of error handling
where we expect to pass this number on towards a function that
expects something smaller than int64_t.

Reported-by: Max Reitz 
Signed-off-by: John Snow 
Signed-off-by: Kevin Wolf 
---
 qemu-io-cmds.c | 123 -
 1 file changed, 87 insertions(+), 36 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 6e5d1e4..20605f2 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -294,9 +294,10 @@ static void qemu_io_free(void *p)
 qemu_vfree(p);
 }
 
-static void dump_buffer(const void *buffer, int64_t offset, int len)
+static void dump_buffer(const void *buffer, int64_t offset, int64_t len)
 {
-int i, j;
+uint64_t i;
+int j;
 const uint8_t *p;
 
 for (i = 0, p = buffer; i < len; i += 16) {
@@ -319,7 +320,7 @@ static void dump_buffer(const void *buffer, int64_t offset, 
int len)
 }
 
 static void print_report(const char *op, struct timeval *t, int64_t offset,
- int count, int total, int cnt, int Cflag)
+ int64_t count, int64_t total, int cnt, int Cflag)
 {
 char s1[64], s2[64], ts[64];
 
@@ -327,12 +328,12 @@ static void print_report(const char *op, struct timeval 
*t, int64_t offset,
 if (!Cflag) {
 cvtstr((double)total, s1, sizeof(s1));
 cvtstr(tdiv((double)total, *t), s2, sizeof(s2));
-printf("%s %d/%d bytes at offset %" PRId64 "\n",
+printf("%s %"PRId64"/%"PRId64" bytes at offset %" PRId64 "\n",
op, total, count, offset);
 printf("%s, %d ops; %s (%s/sec and %.4f ops/sec)\n",
s1, cnt, ts, s2, tdiv((double)cnt, *t));
 } else {/* bytes,ops,time,bytes/sec,ops/sec */
-printf("%d,%d,%s,%.3f,%.3f\n",
+printf("%"PRId64",%d,%s,%.3f,%.3f\n",
 total, cnt, ts,
 tdiv((double)total, *t),
 tdiv((double)cnt, *t));
@@ -393,11 +394,15 @@ fail:
 return buf;
 }
 
-static int do_read(BlockBackend *blk, char *buf, int64_t offset, int count,
-   int *total)
+static int do_read(BlockBackend *blk, char *buf, int64_t offset, int64_t count,
+   int64_t *total)
 {
 int ret;
 
+if (count >> 9 > INT_MAX) {
+return -ERANGE;
+}
+
 ret = blk_read(blk, offset >> 9, (uint8_t *)buf, count >> 9);
 if (ret < 0) {
 return ret;
@@ -406,11 +411,15 @@ static int do_read(BlockBackend *blk, char *buf, int64_t 
offset, int count,
 return 1;
 }
 
-static int do_write(BlockBackend *blk, char *buf, int64_t offset, int count,
-int *total)
+static int do_write(BlockBackend *blk, char *buf, int64_t offset, int64_t 
count,
+int64_t *total)
 {
 int ret;
 
+if (count >> 9 > INT_MAX) {
+return -ERANGE;
+}
+
 ret = blk_write(blk, offset >> 9, (uint8_t *)buf, count >> 9);
 if (ret < 0) {
 return ret;
@@ -419,9 +428,13 @@ static int do_write(BlockBackend *blk, char *buf, int64_t 
offset, int count,
 return 1;
 }
 
-static int do_pread(BlockBackend *blk, char *buf, int64_t offset, int count,
-int *total)
+static int do_pread(BlockBackend *blk, char *buf, int64_t offset,
+int64_t count, int64_t *total)
 {
+if (count > INT_MAX) {
+return -ERANGE;
+}
+
 *total = blk_pread(blk, offset, (uint8_t *)buf, count);
 if (*total < 0) {
 return *total;
@@ -429,9 +442,13 @@ static int do_pread(BlockBackend *blk, char *buf, int64_t 
offset, int count,
 return 1;
 }
 
-static int do_pwrite(BlockBackend *blk, char *buf, int64_t offset, int count,
- int *total)
+static int do_pwrite(BlockBackend *blk, char *buf, int64_t offset,
+ int64_t count, int64_t *total)
 {
+if (count > INT_MAX) {
+return -ERANGE;
+}
+
 *total = blk_pwrite(blk, offset, (uint8_t *)buf, count);
 if (*total < 0) {
 return *total;
@@ -442,8 +459,8 @@ static int do_pwrite(BlockBackend *blk, char *buf, int64_t 
offset, int count,
 typedef struct {
 BlockBackend *blk;
 int64_t offset;
-int count;
-int *total;
+int64_t count;
+int64_t *total;
 int ret;
 bool done;
 } CoWriteZeroes;
@@ -463,8 +480,8 @@ static void coroutine_fn co_write_zeroes_entry(void *opaque)
 *data->total = data->count;
 }
 
-static int do_co_write_zeroes(BlockBackend *blk, int64_t offset, int count,
-  int *total)
+static int do_co_write_zeroes(BlockBackend *blk, int64_t offset, int64_t count,
+  int64_t *total)
 {
 Coroutine *co;
 CoWriteZeroes data = {
@@ -475,6 +492,10 @@ static int do_co_write_zeroes(BlockBackend *blk, int64_t 
offset, int count,
   

[Qemu-block] [PULL v2 30/40] block: test 'blockdev-snapshot' using a file BDS as the overlay

[Kevin Wolf]

From: Alberto Garcia 

This test checks that it is not possible to create a snapshot if the
requested overlay node is a BDS which does not support backing images.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/085 | 12 +++-
 tests/qemu-iotests/085.out |  4 
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
index 80e547d..aa77eca 100755
--- a/tests/qemu-iotests/085
+++ b/tests/qemu-iotests/085
@@ -103,7 +103,8 @@ function add_snapshot_image()
{ 'options':
  { 'driver': 'qcow2', 'node-name': 'snap_${1}', ${extra_params}
'file':
-   { 'driver': 'file', 'filename': '${snapshot_file}' } } } }"
+   { 'driver': 'file', 'filename': '${snapshot_file}',
+ 'node-name': 'file_${1}' } } } }"
 _send_qemu_cmd $h "${cmd}" "return"
 }
 
@@ -187,6 +188,15 @@ add_snapshot_image ${SNAPSHOTS}
 blockdev_snapshot ${SNAPSHOTS}
 
 echo
+echo === Invalid command - cannot create a snapshot using a file BDS ===
+echo
+
+_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
+ 'arguments': { 'node':'virtio0',
+'overlay':'file_${SNAPSHOTS}' }
+   }" "error"
+
+echo
 echo === Invalid command - snapshot node used as active layer ===
 echo
 
diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
index 52292ea..01c78d6 100644
--- a/tests/qemu-iotests/085.out
+++ b/tests/qemu-iotests/085.out
@@ -62,6 +62,10 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
backing_file=TEST_DIR/
 {"return": {}}
 {"return": {}}
 
+=== Invalid command - cannot create a snapshot using a file BDS ===
+
+{"error": {"class": "GenericError", "desc": "The snapshot does not support 
backing images"}}
+
 === Invalid command - snapshot node used as active layer ===
 
 {"error": {"class": "GenericError", "desc": "The snapshot is already in use by 
virtio0"}}
-- 
1.8.3.1

[Qemu-block] [PULL v2 28/40] block: Disallow snapshots if the overlay doesn't support backing files

[Kevin Wolf]

From: Alberto Garcia 

This addresses scenarios like this one:

  { 'execute': 'blockdev-add', 'arguments':
{ 'options': { 'driver': 'qcow2',
   'node-name': 'new0',
   'file': { 'driver': 'file',
 'filename': 'new.qcow2',
 'node-name': 'file0' } } } }

  { 'execute': 'blockdev-snapshot', 'arguments':
{ 'node': 'virtio0',
  'overlay': 'file0' } }

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 blockdev.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 3598b01..3197791 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1678,6 +1678,11 @@ static void 
external_snapshot_prepare(BlkTransactionState *common,
 
 if (state->new_bs->backing != NULL) {
 error_setg(errp, "The snapshot already has a backing image");
+return;
+}
+
+if (!state->new_bs->drv->supports_backing) {
+error_setg(errp, "The snapshot does not support backing images");
 }
 }
 
-- 
1.8.3.1

[Qemu-block] [PULL v2 25/40] qemu-img: add check for zero-length job len

[Kevin Wolf]

From: John Snow 

The mirror job doesn't update its total length until
it has already started running, so we should translate
a zero-length job-len as meaning 0%.

Otherwise, we may get divide-by-zero faults.

Signed-off-by: John Snow 
Reviewed-by: Jeff Cody 
Reviewed-by: Eric Blake 
Signed-off-by: Kevin Wolf 
---
 qemu-img.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 3025776..9831db7 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -656,7 +656,8 @@ static void run_block_job(BlockJob *job, Error **errp)
 
 do {
 aio_poll(aio_context, true);
-qemu_progress_print((float)job->offset / job->len * 100.f, 0);
+qemu_progress_print(job->len ?
+((float)job->offset / job->len * 100.f) : 0.0f, 0);
 } while (!job->ready);
 
 block_job_complete_sync(job, errp);
-- 
1.8.3.1

[Qemu-block] [PULL v2 36/40] mirror: block all operations on the target image during the job

[Kevin Wolf]

From: Alberto Garcia 

There's nothing preventing the target image from being used by other
operations during the 'drive-mirror' job, so we should block them all
until the job is done.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Message-id: 
82b88fd04cde918a08a6f9a4ab85626d7fd7b502.1446475331.git.be...@igalia.com
Signed-off-by: Max Reitz 
---
 block/mirror.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/block/mirror.c b/block/mirror.c
index b1252a1..60f1cb5 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -384,6 +384,7 @@ static void mirror_exit(BlockJob *job, void *opaque)
 aio_context_release(replace_aio_context);
 }
 g_free(s->replaces);
+bdrv_op_unblock_all(s->target, s->common.blocker);
 bdrv_unref(s->target);
 block_job_completed(>common, data->ret);
 g_free(data);
@@ -744,6 +745,9 @@ static void mirror_start_job(BlockDriverState *bs, 
BlockDriverState *target,
 block_job_release(bs);
 return;
 }
+
+bdrv_op_block_all(s->target, s->common.blocker);
+
 bdrv_set_enable_write_cache(s->target, true);
 if (s->target->blk) {
 blk_set_on_error(s->target->blk, on_target_error, on_target_error);
-- 
1.8.3.1

Re: [Qemu-block] [PATCH] docs: update bitmaps.md

[Eric Blake]

On 11/10/2015 04:00 PM, John Snow wrote:
> Include new error handling scenarios for 2.5.
> 
> Signed-off-by: John Snow 
> ---
>  docs/bitmaps.md | 157 
> 
>  1 file changed, 157 insertions(+)
> 
> diff --git a/docs/bitmaps.md b/docs/bitmaps.md
> index 9fd8ea6..a2e8d51 100644
> --- a/docs/bitmaps.md
> +++ b/docs/bitmaps.md
> @@ -19,12 +19,20 @@ which is included at the end of this document.
>  * A dirty bitmap's name is unique to the node, but bitmaps attached to 
> different
>nodes can share the same name.
>  
> +* Dirty bitmaps created for internal use by QEMU may be anonymous and have no
> +  name, but any user-created bitmaps may not be. There can be any number of
> +  anonymous bitmaps per node.

may not be what?  Maybe:

Dirty bitmaps ... have no name, but any user-created bitmaps will have a
name.  There can be...


> +
> +### Grouped Completion Mode
> +

> +* Later, QEMU sends notice that the second job has errored out,
> +  but that the first job was also cancelled:
> +```json
> +{ "timestamp": { "seconds": 1447193702, "microseconds": 632377 },
> +  "data": { "device": "drive1", "action": "report",
> +"operation": "read" },
> +  "event": "BLOCK_JOB_ERROR" }
> +```
> +
> +```json
> +{ "timestamp": { "seconds": 1447193702, "microseconds": 640074 },
> +  "data": { "speed": 0, "offset": 0, "len": 67108864,
> +"error": "Input/output error",
> +"device": "drive1", "type": "backup" },
> +  "event": "BLOCK_JOB_COMPLETED" }
> +```

So we get both an error and a completion notice on failed jobs?  I guess
it's because you can configure jobs to report errors but continue on, so
the error notification alone doesn't say whether the job ends.

> +
> +```json
> +{ "timestamp": { "seconds": 1447193702, "microseconds": 640163 },
> +  "data": { "device": "drive0", "type": "backup", "speed": 0,
> +"len": 67108864, "offset": 16777216 },
> +  "event": "BLOCK_JOB_CANCELLED" }
> +```
> +

Thanks; these examples are very useful.

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH] docs: update bitmaps.md

[John Snow]

On 11/10/2015 06:09 PM, Eric Blake wrote:
> On 11/10/2015 04:00 PM, John Snow wrote:
>> Include new error handling scenarios for 2.5.
>>
>> Signed-off-by: John Snow 
>> ---
>>  docs/bitmaps.md | 157 
>> 
>>  1 file changed, 157 insertions(+)
>>
>> diff --git a/docs/bitmaps.md b/docs/bitmaps.md
>> index 9fd8ea6..a2e8d51 100644
>> --- a/docs/bitmaps.md
>> +++ b/docs/bitmaps.md
>> @@ -19,12 +19,20 @@ which is included at the end of this document.
>>  * A dirty bitmap's name is unique to the node, but bitmaps attached to 
>> different
>>nodes can share the same name.
>>  
>> +* Dirty bitmaps created for internal use by QEMU may be anonymous and have 
>> no
>> +  name, but any user-created bitmaps may not be. There can be any number of
>> +  anonymous bitmaps per node.
> 
> may not be what?  Maybe:
> 
> Dirty bitmaps ... have no name, but any user-created bitmaps will have a
> name.  There can be...
> 
> 
>> +
>> +### Grouped Completion Mode
>> +
> 
>> +* Later, QEMU sends notice that the second job has errored out,
>> +  but that the first job was also cancelled:
>> +```json
>> +{ "timestamp": { "seconds": 1447193702, "microseconds": 632377 },
>> +  "data": { "device": "drive1", "action": "report",
>> +"operation": "read" },
>> +  "event": "BLOCK_JOB_ERROR" }
>> +```
>> +
>> +```json
>> +{ "timestamp": { "seconds": 1447193702, "microseconds": 640074 },
>> +  "data": { "speed": 0, "offset": 0, "len": 67108864,
>> +"error": "Input/output error",
>> +"device": "drive1", "type": "backup" },
>> +  "event": "BLOCK_JOB_COMPLETED" }
>> +```
> 
> So we get both an error and a completion notice on failed jobs?  I guess
> it's because you can configure jobs to report errors but continue on, so
> the error notification alone doesn't say whether the job ends.
> 

Not a design choice of mine; that's just what already happens when a
block job fails. You get the error notice *AND* the "completion" notice
with the error field set.

>> +
>> +```json
>> +{ "timestamp": { "seconds": 1447193702, "microseconds": 640163 },
>> +  "data": { "device": "drive0", "type": "backup", "speed": 0,
>> +"len": 67108864, "offset": 16777216 },
>> +  "event": "BLOCK_JOB_CANCELLED" }
>> +```
>> +
> 
> Thanks; these examples are very useful.
> 

I'm glad.

> Reviewed-by: Eric Blake 
> 

Thanks,
--js

Re: [Qemu-block] [PATCH v11 00/14] block: incremental backup transactions using BlockJobTxn

[Stefan Hajnoczi]

On Thu, Nov 05, 2015 at 06:13:06PM -0500, John Snow wrote:
> Welcome to the Incremental Backup Transactions Newsletter!
> 
> What's new?
> 
> I replaced the per-action "transactional-cancel" parameter with
> a per-transaction paremeter named "completion-mode" which is implemented
> as an enum in case we want to add new behaviors in the future, such
> as a "jobs only" cancel mode.
> 
> For now, it's "grouped" or "individual", and if you use it with actions
> that do not support the latent transactional cancel, you will receive
> an error for your troubles.
> 
> Version 10 primarily changed V7's patches 10-11 and replaced them
> with patches 10-12 that are cut a little differently.
> 
> This is based on top of the work by Stefan Hajnoczi and Fam Zheng.
> 
> Recap: motivation for block job transactions
> 
> If an incremental backup block job fails then we reclaim the bitmap so
> the job can be retried.  The problem comes when multiple jobs are started as
> part of a qmp 'transaction' command.  We need to group these jobs in a
> transaction so that either all jobs complete successfully or all bitmaps are
> reclaimed.
> 
> Without transactions, there is a case where some jobs complete successfully 
> and
> throw away their bitmaps, making it impossible to retry the backup by 
> rerunning
> the command if one of the jobs fails.
> 
> How does this implementation work?
> --
> These patches add a BlockJobTxn object with the following API:
> 
>   txn = block_job_txn_new();
>   block_job_txn_add_job(txn, job1);
>   block_job_txn_add_job(txn, job2);
> 
> The jobs either both complete successfully or they both fail/cancel.  If the
> user cancels job1 then job2 will also be cancelled and vice versa.
> 
> Jobs objects stay alive waiting for other jobs to complete, even if the
> coroutines have returned.  They can be cancelled by the user during this time.
> Job blockers are still in effect and no other block job can run on this device
> in the meantime (since QEMU currently only allows 1 job per device).  This is
> the main drawback to this approach but reasonable since you probably don't 
> want
> to run other jobs/operations until you're sure the backup was successful (you
> won't be able to retry a failed backup if there's a new job running).
> 
> [History]
> 
> v11: Renamed "err-cancel" to "completion-mode"
>  "none" becomes "individual"
>  "all" becomes "grouped"
> 
> v10: Took series back from Fam. (jsnow)
>  Replaced per-action parameter with per-transaction properties.
>  Patches 10,11 were split into 10-12.
> 
> v9: this version fixes a reference count problem with job->bs,
> in patch 05.
> 
> v8: Rebase on to master.
> Minor fixes addressing John Snow's comments.
> 
> v7: Add Eric's rev-by in 1, 11.
> Add Max's rev-by in 4, 5, 9, 10, 11.
> Add John's rev-by in 5, 6, 8.
> Fix wording for 6. [John]
> Fix comment of block_job_txn_add_job() in 9. [Max]
> Remove superfluous hunks, and document default value in 11. [Eric]
> Update Makefile dep in 14. [Max]
> 
> 
> 
> For convenience, this branch is available at:
> https://github.com/jnsnow/qemu.git branch block-transpop
> https://github.com/jnsnow/qemu/tree/block-transpop
> 
> This version is tagged block-transpop-v11:
> https://github.com/jnsnow/qemu/releases/tag/block-transpop-v11
> 
> Fam Zheng (6):
>   backup: Extract dirty bitmap handling as a separate function
>   blockjob: Introduce reference count and fix reference to job->bs
>   blockjob: Add .commit and .abort block job actions
>   blockjob: Add "completed" and "ret" in BlockJob
>   blockjob: Simplify block_job_finish_sync
>   block: Add block job transactions
> 
> John Snow (7):
>   qapi: Add transaction support to block-dirty-bitmap operations
>   iotests: add transactional incremental backup test
>   block: rename BlkTransactionState and BdrvActionOps
>   block/backup: Rely on commit/abort for cleanup
>   block: Add BlockJobTxn support to backup_run
>   block: add transactional properties
>   iotests: 124 - transactional failure test
> 
> Stefan Hajnoczi (1):
>   tests: add BlockJobTxn unit test
> 
>  block.c|  19 +-
>  block/backup.c |  50 --
>  block/mirror.c |   2 +-
>  blockdev.c | 432 
> ++---
>  blockjob.c | 189 
>  docs/bitmaps.md|   6 +-
>  include/block/block.h  |   2 +-
>  include/block/block_int.h  |   6 +-
>  include/block/blockjob.h   |  85 -
>  qapi-schema.json   |  56 +-
>  qemu-img.c |   3 -
>  qmp-commands.hx|   2 +-
>  tests/Makefile |   3 +
>  tests/qemu-iotests/124 | 182 ++-
>  tests/qemu-iotests/124.out |   4 +-
>  tests/test-blockjob-txn.c  | 250 

[Qemu-block] [PATCH] nand: fix address overflow

[Rabin Vincent]

The shifts of the address mask and value shift beyond 32 bits when there
are 5 address cycles.

Signed-off-by: Rabin Vincent 
---
 hw/block/nand.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/block/nand.c b/hw/block/nand.c
index 61d2cec..a68266f 100644
--- a/hw/block/nand.c
+++ b/hw/block/nand.c
@@ -522,8 +522,8 @@ void nand_setio(DeviceState *dev, uint32_t value)
 
 if (s->ale) {
 unsigned int shift = s->addrlen * 8;
-unsigned int mask = ~(0xff << shift);
-unsigned int v = value << shift;
+uint64_t mask = ~(0xffull << shift);
+uint64_t v = (uint64_t)value << shift;
 
 s->addr = (s->addr & mask) | v;
 s->addrlen ++;
-- 
1.7.10.4

[Qemu-block] [PATCH v10 00/15] blockdev: BlockBackend and media

[Max Reitz]

I'm sorry for having singlehandedly stalled the block pull request twice
now, and I sure hope everything is going to be fine this time.

v9 was missing a change to patch 13: While it did (probably?) fix
cocoa.m in patch 11 for patch 11, patch 13 changed the signature of the
function in question yet again, so with the whole series applied, it
still broke.

Just as v9, this series consists of the changed patch (patch 13) only.
The sole difference to v8/v9 is an added "false, 0," line in cocoa.m
(clang tells me that putting plain integers into enums is fine in Obj-C
(unlike in C++), so I hope it works).

“Third time's the charm”, that's what they say.


git-backport-diff against v9:

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

001/15:[] [--] 'block: Add blk_remove_bs()'
002/15:[] [--] 'block: Make bdrv_states public'
003/15:[] [--] 'block: Add functions for inheriting a BBRS'
004/15:[] [--] 'blockdev: Add blockdev-open-tray'
005/15:[] [--] 'blockdev: Add blockdev-close-tray'
006/15:[] [--] 'blockdev: Add blockdev-remove-medium'
007/15:[] [--] 'blockdev: Add blockdev-insert-medium'
008/15:[] [--] 'blockdev: Implement eject with basic operations'
009/15:[] [--] 'blockdev: Implement change with basic operations'
010/15:[] [--] 'block: Inquire tray state before tray-moved events'
011/15:[] [--] 'qmp: Introduce blockdev-change-medium'
012/15:[] [--] 'hmp: Use blockdev-change-medium for change command'
013/15:[0001] [FC] 'blockdev: read-only-mode for blockdev-change-medium'
014/15:[] [--] 'hmp: Add read-only-mode option to change command'
015/15:[] [--] 'iotests: Add test for change-related QMP commands'


Max Reitz (15):
  block: Add blk_remove_bs()
  block: Make bdrv_states public
  block: Add functions for inheriting a BBRS
  blockdev: Add blockdev-open-tray
  blockdev: Add blockdev-close-tray
  blockdev: Add blockdev-remove-medium
  blockdev: Add blockdev-insert-medium
  blockdev: Implement eject with basic operations
  blockdev: Implement change with basic operations
  block: Inquire tray state before tray-moved events
  qmp: Introduce blockdev-change-medium
  hmp: Use blockdev-change-medium for change command
  blockdev: read-only-mode for blockdev-change-medium
  hmp: Add read-only-mode option to change command
  iotests: Add test for change-related QMP commands

 block.c|   3 +-
 block/block-backend.c  |  56 +++-
 blockdev.c | 286 
 hmp-commands.hx|  20 +-
 hmp.c  |  47 ++-
 include/block/block_int.h  |   2 +
 include/sysemu/block-backend.h |   3 +
 include/sysemu/blockdev.h  |   2 -
 qapi-schema.json   |   6 +-
 qapi/block-core.json   | 126 
 qmp-commands.hx| 218 +
 qmp.c  |   3 +-
 tests/qemu-iotests/118 | 720 +
 tests/qemu-iotests/118.out |   5 +
 tests/qemu-iotests/group   |   1 +
 ui/cocoa.m |  11 +-
 16 files changed, 1403 insertions(+), 106 deletions(-)
 create mode 100755 tests/qemu-iotests/118
 create mode 100644 tests/qemu-iotests/118.out

-- 
2.6.2

[Qemu-block] [PATCH v10 13/15] blockdev: read-only-mode for blockdev-change-medium

[Max Reitz]

Add an option to qmp_blockdev_change_medium() which allows changing the
read-only status of the block device whose medium is changed.

Some drives do not have a inherently fixed read-only status; for
instance, floppy disks can be set read-only or writable independently of
the drive. Some users may find it useful to be able to therefore change
the read-only status of a block device when changing the medium.

Signed-off-by: Max Reitz 
---
 blockdev.c   | 22 ++
 hmp.c|  2 +-
 qapi/block-core.json | 24 +++-
 qmp-commands.hx  | 24 +++-
 qmp.c|  3 ++-
 ui/cocoa.m   |  1 +
 6 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index b3a958c..34f6e5b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2145,6 +2145,8 @@ void qmp_blockdev_insert_medium(const char *device, const 
char *node_name,
 
 void qmp_blockdev_change_medium(const char *device, const char *filename,
 bool has_format, const char *format,
+bool has_read_only,
+BlockdevChangeReadOnlyMode read_only,
 Error **errp)
 {
 BlockBackend *blk;
@@ -2166,6 +2168,26 @@ void qmp_blockdev_change_medium(const char *device, 
const char *filename,
 
 bdrv_flags = blk_get_open_flags_from_root_state(blk);
 
+if (!has_read_only) {
+read_only = BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN;
+}
+
+switch (read_only) {
+case BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN:
+break;
+
+case BLOCKDEV_CHANGE_READ_ONLY_MODE_READ_ONLY:
+bdrv_flags &= ~BDRV_O_RDWR;
+break;
+
+case BLOCKDEV_CHANGE_READ_ONLY_MODE_READ_WRITE:
+bdrv_flags |= BDRV_O_RDWR;
+break;
+
+default:
+abort();
+}
+
 if (has_format) {
 options = qdict_new();
 qdict_put(options, "driver", qstring_from_str(format));
diff --git a/hmp.c b/hmp.c
index e5ad944..16006aa 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1355,7 +1355,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 }
 qmp_change("vnc", target, !!arg, arg, );
 } else {
-qmp_blockdev_change_medium(device, target, !!arg, arg, );
+qmp_blockdev_change_medium(device, target, !!arg, arg, false, 0, );
 if (err &&
 error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
 error_free(err);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index e9fa649..fa08ba9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1959,6 +1959,24 @@
 
 
 ##
+# @BlockdevChangeReadOnlyMode:
+#
+# Specifies the new read-only mode of a block device subject to the
+# @blockdev-change-medium command.
+#
+# @retain:  Retains the current read-only mode
+#
+# @read-only:   Makes the device read-only
+#
+# @read-write:  Makes the device writable
+#
+# Since: 2.3
+##
+{ 'enum': 'BlockdevChangeReadOnlyMode',
+  'data': ['retain', 'read-only', 'read-write'] }
+
+
+##
 # @blockdev-change-medium:
 #
 # Changes the medium inserted into a block device by ejecting the current 
medium
@@ -1973,12 +1991,16 @@
 # @format:  #optional, format to open the new image with (defaults to
 #   the probed format)
 #
+# @read-only-mode:  #optional, change the read-only mode of the device; 
defaults
+#   to 'retain'
+#
 # Since: 2.5
 ##
 { 'command': 'blockdev-change-medium',
   'data': { 'device': 'str',
 'filename': 'str',
-'*format': 'str' } }
+'*format': 'str',
+'*read-only-mode': 'BlockdevChangeReadOnlyMode' } }
 
 
 ##
diff --git a/qmp-commands.hx b/qmp-commands.hx
index f6d9c25..39d6e25 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4183,7 +4183,7 @@ EQMP
 
 {
 .name   = "blockdev-change-medium",
-.args_type  = "device:B,filename:F,format:s?",
+.args_type  = "device:B,filename:F,format:s?,read-only-mode:s?",
 .mhandler.cmd_new = qmp_marshal_blockdev_change_medium,
 },
 
@@ -4199,6 +4199,8 @@ Arguments:
 - "device": device name (json-string)
 - "filename": filename of the new image (json-string)
 - "format": format of the new image (json-string, optional)
+- "read-only-mode": new read-only mode (json-string, optional)
+  - Possible values: "retain" (default), "read-only", "read-write"
 
 Examples:
 
@@ -4210,6 +4212,26 @@ Examples:
 "format": "raw" } }
 <- { "return": {} }
 
+2. Load a read-only medium into a writable drive
+
+-> { "execute": "blockdev-change-medium",
+ "arguments": { "device": "isa-fd0",
+"filename": "/srv/images/ro.img",
+"format": "raw",
+"read-only-mode": "retain" } }
+
+<- { "error":
+ { "class": "GenericError",
+   "desc": "Could not open