Re: [Qemu-block] [PATCH v4 2/6] replication: add shared-disk and shared-disk-id options

2017-05-11 Thread Hailiang Zhang
On 2017/5/12 3:08, Stefan Hajnoczi wrote: On Wed, Apr 12, 2017 at 10:05:17PM +0800, zhanghailiang wrote: We use these two options to identify which disk is shared Signed-off-by: zhanghailiang Signed-off-by: Wen Congyang Signed-off-by: Zhang Chen --- v4: - Add proper comment for primary_disk

Re: [Qemu-block] [PATCH v4 2/6] replication: add shared-disk and shared-disk-id options

2017-05-11 Thread Hailiang Zhang
On 2017/4/18 13:59, Xie Changlong wrote: On 04/12/2017 10:05 PM, zhanghailiang wrote: We use these two options to identify which disk is shared Signed-off-by: zhanghailiang Signed-off-by: Wen Congyang Signed-off-by: Zhang Chen --- v4: - Add proper comment for primary_disk (Stefan) v2: - Mov

Re: [Qemu-block] [PATCH 3/7] curl: avoid recursive locking of BDRVCURLState mutex

2017-05-11 Thread Jeff Cody
On Wed, May 10, 2017 at 04:32:01PM +0200, Paolo Bonzini wrote: > The curl driver has a ugly hack where, if it cannot find an empty CURLState, > it just uses aio_poll to wait for one to be empty. This is probably > buggy when used together with dataplane, and the simplest way to fix it > is to use

Re: [Qemu-block] [PATCH v2 00/16] block: Protect AIO context change with perm API

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:40PM +0800, Fam Zheng wrote: > v2: Address Stefan's comments: > > - Clean up redundancy in bdrv_format_default_perms change. > - Add a test case to check both success/failure cases. > A failure case is not possible at user interface level because of othe

Re: [Qemu-block] [PATCH 2/7] curl: never invoke callbacks with s->mutex held

2017-05-11 Thread Jeff Cody
On Wed, May 10, 2017 at 04:32:00PM +0200, Paolo Bonzini wrote: > All curl callbacks go through curl_multi_do, and hence are called with > s->mutex held. Note that with comments, and make curl_read_cb drop the > lock before invoking the callback. > > Likewise for curl_find_buf, where the callback

Re: [Qemu-block] [Qemu-devel] [PATCH v2 16/16] tests: Add test case for BLK_PERM_AIO_CONTEXT_CHANGE

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:56PM +0800, Fam Zheng wrote: > +static void test_aio_context_failure(void) > +{ > +Error *local_err = NULL; > +BlockBackend *blk1 = blk_new(BLK_PERM_AIO_CONTEXT_CHANGE, > + BLK_PERM_ALL & > ~BLK_PERM_AIO_CONTEXT_CHANGE); > +

Re: [Qemu-block] [PATCH 1/7] curl: strengthen assertion in curl_clean_state

2017-05-11 Thread Jeff Cody
On Wed, May 10, 2017 at 04:31:59PM +0200, Paolo Bonzini wrote: > curl_clean_state should only be called after all AIOCBs have been > completed. This is not so obvious for the call from curl_detach_aio_context, > so assert that. > > Cc: qemu-sta...@nongnu.org > Cc: jc...@redhat.com > Signed-off-by

Re: [Qemu-block] [PATCH v2 15/16] block: Add perm assertion on blk_set_aio_context

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:55PM +0800, Fam Zheng wrote: > Now that all BB users comply with the BLK_PERM_AIO_CONTEXT_CHANGE > rule, we can assert it. > > Signed-off-by: Fam Zheng > --- > block/block-backend.c | 4 > 1 file changed, 4 insertions(+) Reviewed-by: Stefan Hajnoczi signatu

Re: [Qemu-block] [PATCH v2 14/16] nbd: Allow BLK_PERM_AIO_CONTEXT_CHANGE on BB

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:54PM +0800, Fam Zheng wrote: > This is safe because of the aio context notifier we'll register on this > node. So allow it. > > Signed-off-by: Fam Zheng > --- > nbd/server.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi

Re: [Qemu-block] [PATCH v2 13/16] blk: fix aio context loss on media change

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:53PM +0800, Fam Zheng wrote: > From: Vladimir Sementsov-Ogievskiy > > If we have separate iothread for cdrom, we lose connection to it on > qmp_blockdev_change_medium, as aio_context is on bds which is dropped > and switched with new one. > > As an example result, a

Re: [Qemu-block] [PATCH v2 11/16] virtio-scsi: Request BLK_PERM_AIO_CONTEXT_CHANGE for dataplane

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:51PM +0800, Fam Zheng wrote: > blk_set_aio_context is audited by perm API, so follow the protocol and > request for permission first. > > Signed-off-by: Fam Zheng > --- > hw/scsi/virtio-scsi.c | 4 > 1 file changed, 4 insertions(+) Reviewed-by: Stefan Hajnoczi

Re: [Qemu-block] [PATCH v2 12/16] virtio-blk: Request BLK_PERM_AIO_CONTEXT_CHANGE for dataplane

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:52PM +0800, Fam Zheng wrote: > blk_set_aio_context is audited by perm API, so follow the protocol and > request for permission first. > > Previously the return code in error path was hardcoded as -EPERM, but > returning 'r' is more meaningful here both for the new err

Re: [Qemu-block] [PATCH v2 10/16] mirror: Do initial aio context move of target via BB interface

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:50PM +0800, Fam Zheng wrote: > diff --git a/blockdev.c b/blockdev.c > index dfd1385..53f9874 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -3556,8 +3556,6 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp) > goto out; > } > > -bdrv_set_aio

Re: [Qemu-block] [Qemu-devel] [PATCH v2 09/16] commit: Allow aio context change on s->base

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:49PM +0800, Fam Zheng wrote: > The block job has the aio context change notifier, we should allow it > here as well. > > Signed-off-by: Fam Zheng > --- > block/commit.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Aside from my question about another u

Re: [Qemu-block] [PATCH v2 08/16] mirror: Request aio context change permission on target

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:48PM +0800, Fam Zheng wrote: > What's done in the source's context change notifier is moving the > target's context to follow the new one, so we request this permission > here. > > Signed-off-by: Fam Zheng > --- > block/mirror.c | 1 + > 1 file changed, 1 insertion(

Re: [Qemu-block] [PATCH v2 07/16] backup: Do initial aio context move of target via BB interface

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:47PM +0800, Fam Zheng wrote: > The old aio context check is hacky because when it was added we didn't > have the permission system to enforce a general rule. It only checks if > the target BDS has a BB, which is in fact insufficient because there may > be other BBs in

Re: [Qemu-block] [Qemu-devel] [PATCH v2 03/16] blockjob: Add BLK_PERM_AIO_CONTEXT_CHANGE shared perm on bs

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:43PM +0800, Fam Zheng wrote: > Signed-off-by: Fam Zheng > --- > blockjob.c | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature

Re: [Qemu-block] [PATCH v2 05/16] block: Propagate BLK_PERM_AIO_CONTEXT_CHANGE down the graph

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:45PM +0800, Fam Zheng wrote: > bdrv_set_aio_context can take care of children recursively, so it is > okay to pass down the perm. > > Signed-off-by: Fam Zheng > --- > block.c | 10 ++ > block/vvfat.c | 2 +- > 2 files changed, 7 insertions(+), 5 delet

Re: [Qemu-block] [PATCH v2 06/16] backup: Request BLK_PERM_AIO_CONTEXT_CHANGE on target

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:46PM +0800, Fam Zheng wrote: > What's done in the source's context change notifier is moving the > target's context to follow the new one, so we request this permission > here. It's true that the backup block job must be able to set target's AioContext, but does this

Re: [Qemu-block] [PATCH v2 02/16] block-backend: Add blk_request_perm

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:42PM +0800, Fam Zheng wrote: > This function tries to request, if not granted yet, for the given > permissions. > > Signed-off-by: Fam Zheng > --- > block/block-backend.c | 12 > include/sysemu/block-backend.h | 1 + > 2 files changed, 13 inse

Re: [Qemu-block] [PATCH v2 01/16] block: Define BLK_PERM_AIO_CONTEXT_CHANGE

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:41PM +0800, Fam Zheng wrote: > Signed-off-by: Fam Zheng > --- > block.c | 2 ++ > include/block/block.h | 7 ++- > 2 files changed, 8 insertions(+), 1 deletion(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature

Re: [Qemu-block] [Qemu-devel] [PATCH v2 04/16] blockjob: Allow aio context change on intermediate nodes

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 19, 2017 at 05:43:44PM +0800, Fam Zheng wrote: > The intermediate nodes do work with aio context change, so allow that > operations. > > Signed-off-by: Fam Zheng > --- > block/commit.c | 3 ++- > block/mirror.c | 3 ++- > block/stream.c | 3 ++- > 3 files changed, 6 insertions(+), 3

Re: [Qemu-block] [Qemu-devel] [PATCH v2 00/18] Block layer thread safety, part 1

2017-05-11 Thread no-reply
Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 20170511144208.24075-1-pbonz...@redhat.com Subject: [Qemu-devel] [PATCH v2 00/18] Block layer thread safety, part 1 Type: series === TEST SCRIPT BEGIN === #!/bin/bash BASE=base n=1 total

Re: [Qemu-block] [PATCH v4 0/6] COLO block replication supports shared disk case

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 12, 2017 at 10:05:15PM +0800, zhanghailiang wrote: > COLO block replication doesn't support the shared disk case, > Here we try to implement it and this is the 4th version. > > Please review and any commits are welcomed. > > Cc: Dr. David Alan Gilbert (git) > Cc: eddie.d...@intel.com

Re: [Qemu-block] [Qemu-devel] [PATCH v4 5/6] replication: Implement block replication for shared disk case

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 12, 2017 at 10:05:20PM +0800, zhanghailiang wrote: > @@ -612,6 +644,16 @@ static void replication_do_checkpoint(ReplicationState > *rs, Error **errp) > error_propagate(errp, local_err); > break; > } > +} else { > +/* >

Re: [Qemu-block] [PATCH v4 2/6] replication: add shared-disk and shared-disk-id options

2017-05-11 Thread Stefan Hajnoczi
On Wed, Apr 12, 2017 at 10:05:17PM +0800, zhanghailiang wrote: > We use these two options to identify which disk is > shared > > Signed-off-by: zhanghailiang > Signed-off-by: Wen Congyang > Signed-off-by: Zhang Chen > --- > v4: > - Add proper comment for primary_disk (Stefan) > v2: > - Move g_f

Re: [Qemu-block] [Qemu-devel] [PATCH] mirror: add sync mode incremental to drive-mirror and blockdev-mirror

2017-05-11 Thread Denis V. Lunev
On 05/11/2017 08:43 PM, John Snow wrote: > > On 05/11/2017 02:35 PM, Stefan Hajnoczi wrote: >> Daniel's proposed patch isn't large or invasive. The QMP interface is >> consistent with the backup block job's sync=bitmap mode, it's a logical >> extension. >> >> If the concerns about the bitmap lifec

Re: [Qemu-block] [Qemu-devel] [PATCH] mirror: add sync mode incremental to drive-mirror and blockdev-mirror

2017-05-11 Thread John Snow
On 05/11/2017 02:35 PM, Stefan Hajnoczi wrote: > Daniel's proposed patch isn't large or invasive. The QMP interface is > consistent with the backup block job's sync=bitmap mode, it's a logical > extension. > > If the concerns about the bitmap lifecycle are addressed and a test case > is include

Re: [Qemu-block] [Qemu-devel] [PATCH] mirror: add sync mode incremental to drive-mirror and blockdev-mirror

2017-05-11 Thread Stefan Hajnoczi
On Thu, May 11, 2017 at 04:28:14PM +0200, Denis V. Lunev wrote: > On 05/11/2017 04:16 PM, Daniel Kučera wrote: > > > > 2017-05-10 17:05 GMT+02:00 Denis V. Lunev > >: > > > > On 05/10/2017 05:00 PM, Stefan Hajnoczi wrote: > > > On Wed, May 10, 2017 at 03:25:31PM +020

[Qemu-block] [PATCH v4] qemu-img: Check for backing image if specified during create

2017-05-11 Thread John Snow
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1213786 Or, rather, force the open of a backing image if one was specified for creation. Using a similar -unsafe option as rebase, allow qemu-img to ignore the backing file validation if possible. It may not always be possible, as in the existing

Re: [Qemu-block] [PATCH v3] qemu-img: Check for backing image if specified during create

2017-05-11 Thread John Snow
On 05/11/2017 11:17 AM, Eric Blake wrote: > On 05/11/2017 10:11 AM, John Snow wrote: >> Or, rather, force the open of a backing image if one was specified >> for creation. Using a similar -unsafe option as rebase, allow qemu-img >> to ignore the backing file validation if possible. >> >> It may n

Re: [Qemu-block] [Qemu-devel] [PATCH] qcow2: remove extra local_error variable

2017-05-11 Thread Eric Blake
On 05/11/2017 10:03 AM, Alberto Garcia wrote: > Commit d7086422b1c1e75e320519cfe26176db6ec97a37 added a local_err > variable global to the qcow2_amend_options() function, so there's no > need to have this other one. > > Signed-off-by: Alberto Garcia > --- > block/qcow2.c | 5 ++--- > 1 file chan

Re: [Qemu-block] [Qemu-devel] [PATCH 06/18] block: access io_plugged with atomic ops

2017-05-11 Thread Eric Blake
On 05/11/2017 09:41 AM, Paolo Bonzini wrote: > Reviewed-by: Stefan Hajnoczi > Signed-off-by: Paolo Bonzini > --- > block/io.c| 4 ++-- > include/block/block_int.h | 8 +--- > 2 files changed, 7 insertions(+), 5 deletions(-) > @@ -625,6 +622,11 @@ struct BlockDriverState { >

Re: [Qemu-block] [PATCH v3] qemu-img: Check for backing image if specified during create

2017-05-11 Thread Eric Blake
On 05/11/2017 10:11 AM, John Snow wrote: > Or, rather, force the open of a backing image if one was specified > for creation. Using a similar -unsafe option as rebase, allow qemu-img > to ignore the backing file validation if possible. > > It may not always be possible, as in the existing case whe

Re: [Qemu-block] [PATCH v9 01/13] qcow2: Unallocate unmapped zero clusters if no backing file

2017-05-11 Thread Eric Blake
On 05/11/2017 09:56 AM, Eric Blake wrote: > > On IRC, John, Kevin, and I were discussing the current situation with > libvirt NBD storage migration. When libvirt creates a file on the > destination (rather than the user pre-creating it), it currently > defaults to 0.10 [v2] images, even if the so

Re: [Qemu-block] [Qemu-devel] [PATCH] mirror: add sync mode incremental to drive-mirror and blockdev-mirror

2017-05-11 Thread Daniel Kučera
2017-05-11 16:28 GMT+02:00 Denis V. Lunev : > On 05/11/2017 04:16 PM, Daniel Kučera wrote: > > > > 2017-05-10 17:05 GMT+02:00 Denis V. Lunev > >: > > > > On 05/10/2017 05:00 PM, Stefan Hajnoczi wrote: > > > On Wed, May 10, 2017 at 03:25:31PM +0200, Denis V. Lunev w

Re: [Qemu-block] [Qemu-devel] [PATCH v2] qemu-img: Check for backing image if specified during create

2017-05-11 Thread John Snow
On 05/11/2017 05:37 AM, Kevin Wolf wrote: > Am 09.05.2017 um 20:09 hat John Snow geschrieben: >> Or, rather, force the open of a backing image if one was specified >> for creation. Using a similar -unsafe option as rebase, allow qemu-img >> to ignore the backing file validation if possible. >> >>

[Qemu-block] [PATCH v3] qemu-img: Check for backing image if specified during create

2017-05-11 Thread John Snow
Or, rather, force the open of a backing image if one was specified for creation. Using a similar -unsafe option as rebase, allow qemu-img to ignore the backing file validation if possible. It may not always be possible, as in the existing case when a filesize for the new image was not specified.

[Qemu-block] [PATCH] qcow2: remove extra local_error variable

2017-05-11 Thread Alberto Garcia
Commit d7086422b1c1e75e320519cfe26176db6ec97a37 added a local_err variable global to the qcow2_amend_options() function, so there's no need to have this other one. Signed-off-by: Alberto Garcia --- block/qcow2.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/block/qcow2

Re: [Qemu-block] [PATCH v6 13/18] qcow2: add support for LUKS encryption format

2017-05-11 Thread Alberto Garcia
On Tue 25 Apr 2017 05:38:53 PM CEST, Daniel P. Berrange wrote: > This adds support for using LUKS as an encryption format > with the qcow2 file, using the new encrypt.format parameter > to request "luks" format. e.g. > > # qemu-img create --object secret,data=123456,id=sec0 \ >-f qcow2 -o

Re: [Qemu-block] [PATCH v9 01/13] qcow2: Unallocate unmapped zero clusters if no backing file

2017-05-11 Thread Eric Blake
[revisiting this older patch version, even though the final version in today's pull request changed somewhat from this approach] On 04/12/2017 04:49 AM, Kevin Wolf wrote: > Am 11.04.2017 um 03:17 hat Eric Blake geschrieben: >> 'qemu-img map' already coalesces information about unallocated >> clust

[Qemu-block] [PATCH 18/18] block: make accounting thread-safe

2017-05-11 Thread Paolo Bonzini
I'm not trying too hard yet. Later, with multiqueue support, this may cause mutex contention or cacheline bouncing. Signed-off-by: Paolo Bonzini --- v1->v2: QemuSpin -> QemuMutex [Stefan] block/accounting.c | 16 include/block/accounting.h | 8 ++-- 2 file

[Qemu-block] [PATCH 15/18] migration/block: reset dirty bitmap before reading

2017-05-11 Thread Paolo Bonzini
Any data that is returned by read may be stale already, the bitmap has to be cleared before issuing the read. Signed-off-by: Paolo Bonzini --- v1->v2: new migration/block.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/migration/block.c b/migration/block.c index

[Qemu-block] [PATCH 12/18] block: access write_gen with atomics

2017-05-11 Thread Paolo Bonzini
Reviewed-by: Stefan Hajnoczi Signed-off-by: Paolo Bonzini --- block.c | 2 +- block/io.c| 6 +++--- include/block/block_int.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/block.c b/block.c index 3a8e2e0ba4..98f48fe7f8 100644 --- a/block

[Qemu-block] [PATCH 13/18] block: protect tracked_requests and flush_queue with reqs_lock

2017-05-11 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini --- v1->v2: correct and simplify flush queue handling [Fam, me] block.c | 1 + block/io.c| 16 ++-- include/block/block_int.h | 14 +- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/b

[Qemu-block] [PATCH 11/18] block: use Stat64 for wr_highest_offset

2017-05-11 Thread Paolo Bonzini
Reviewed-by: Stefan Hajnoczi Signed-off-by: Paolo Bonzini --- block/io.c| 4 +--- block/qapi.c | 2 +- include/block/block_int.h | 7 --- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/block/io.c b/block/io.c index 8d2854aeaa..25a00d82ed 100644 --

Re: [Qemu-block] [PATCH v6 17/18] block: remove all encryption handling APIs

2017-05-11 Thread Alberto Garcia
On Tue 25 Apr 2017 05:38:57 PM CEST, Daniel P. Berrange wrote: > @@ -220,7 +220,12 @@ static int qcow_open(BlockDriverState *bs, QDict > *options, int flags, > goto fail; > } > bs->encrypted = true; > -bs->valid_key = true; > +} else { > +if (encr

[Qemu-block] [PATCH 16/18] block: protect modification of dirty bitmaps with a mutex

2017-05-11 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini --- v1->v2: remove bdrv_get_dirty from API [Stefan] convert block migration from bdrv_get_dirty to _locked [Stefan] block/dirty-bitmap.c | 68 ++-- block/mirror.c | 11 +-- incl

[Qemu-block] [PATCH 17/18] block: introduce block_account_one_io

2017-05-11 Thread Paolo Bonzini
This is the common code to account operations that produced actual I/O. Signed-off-by: Paolo Bonzini --- v1->v2: new, to hoist qemu_clock_get_ns out of the "if (!stats->account_failed)" case. Failed operations are not a fast path, so getting the current ti

[Qemu-block] [PATCH 08/18] throttle-groups: do not use qemu_co_enter_next

2017-05-11 Thread Paolo Bonzini
Prepare for removing this function; always restart throttled requests from coroutine context. This will matter when restarting throttled requests will have to acquire a CoMutex. Signed-off-by: Paolo Bonzini --- v1->v2: heavily simplified thanks to previous patch [Stefan]

[Qemu-block] [PATCH v2 00/18] Block layer thread safety, part 1

2017-05-11 Thread Paolo Bonzini
This series uses mutexes or atomic operations around core block layer operations. The remaining parts include: I've removed the failing assertion in bdrv_aligned_pwritev in order to test block migration. Paolo v1->v2: add missing comment for 'wakeup' member [Fam] rewrite throttle-groups

[Qemu-block] [PATCH 14/18] block: introduce dirty_bitmap_mutex

2017-05-11 Thread Paolo Bonzini
It protects only the list of dirty bitmaps; in the next patch we will also protect their content. Signed-off-by: Paolo Bonzini --- v1->v2: remove global mutex [Fam] block/dirty-bitmap.c | 44 +++- block/mirror.c| 3 ++- blockdev.

[Qemu-block] [PATCH 10/18] util: add stats64 module

2017-05-11 Thread Paolo Bonzini
This module provides fast paths for 64-bit atomic operations on machines that only have 32-bit atomic access. Signed-off-by: Paolo Bonzini --- v1->v2: use CONFIG_ATOMIC64 [Paolo] fix compilation on 32-bit machines [patchew] simplify "fast" version of stat64

[Qemu-block] [PATCH 05/18] block: access wakeup with atomic ops

2017-05-11 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini --- v1->v2: add comment [Fam] block/io.c| 3 ++- block/nfs.c | 4 +++- block/sheepdog.c | 3 ++- include/block/block.h | 5 +++-- include/block/block_int.h | 7 +-- 5 files changed, 15 insertions(+), 7 deletions(

[Qemu-block] [PULL 54/58] iotests: Add test 179 to cover write zeroes with unmap

2017-05-11 Thread Kevin Wolf
From: Eric Blake No tests were covering write zeroes with unmap. Additionally, I needed to prove that my previous patches for correct status reporting and write zeroes optimizations actually had an impact. The test works for cluster_size between 8k and 2M (for smaller sizes, it fails because ou

[Qemu-block] [PATCH 02/18] block: access quiesce_counter with atomic ops

2017-05-11 Thread Paolo Bonzini
Reviewed-by: Stefan Hajnoczi Signed-off-by: Paolo Bonzini --- block/io.c| 4 ++-- include/block/block_int.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/block/io.c b/block/io.c index a0de60acbc..3204e08c8f 100644 --- a/block/io.c +++ b/block/io.c @@ -241,

[Qemu-block] [PATCH 09/18] throttle-groups: protect throttled requests with a CoMutex

2017-05-11 Thread Paolo Bonzini
Another possibility is to use tg->lock, which we're holding anyway in both schedule_next_request and throttle_group_co_io_limits_intercept. This would require open-coding the CoQueue however, so I've chosen this alternative. Signed-off-by: Paolo Bonzini --- v1->v2: rename CoMutex [Fam]

[Qemu-block] [PULL 53/58] iotests: Improve _filter_qemu_img_map

2017-05-11 Thread Kevin Wolf
From: Eric Blake Although _filter_qemu_img_map documents that it scrubs offsets, it was only doing so for human mode. Of the existing tests using the filter (97, 122, 150, 154, 176), two of them are affected, but it does not hurt the validity of the tests to not require particular mappings (anot

[Qemu-block] [PATCH 07/18] throttle-groups: only start one coroutine from drained_begin

2017-05-11 Thread Paolo Bonzini
Starting all waiting coroutines from bdrv_drain_all is unnecessary; throttle_group_co_io_limits_intercept calls schedule_next_request as soon as the coroutine restarts, which in turn will restart the next request if possible. If we only start the first request and let the coroutines dance from the

[Qemu-block] [PATCH 06/18] block: access io_plugged with atomic ops

2017-05-11 Thread Paolo Bonzini
Reviewed-by: Stefan Hajnoczi Signed-off-by: Paolo Bonzini --- block/io.c| 4 ++-- include/block/block_int.h | 8 +--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/block/io.c b/block/io.c index dcb2b72f91..8d2854aeaa 100644 --- a/block/io.c +++ b/block/io.c @

[Qemu-block] [PATCH 04/18] block: access serialising_in_flight with atomic ops

2017-05-11 Thread Paolo Bonzini
Reviewed-by: Stefan Hajnoczi Signed-off-by: Paolo Bonzini --- block/io.c| 6 +++--- include/block/block_int.h | 10 ++ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/block/io.c b/block/io.c index 3204e08c8f..28d2de6d93 100644 --- a/block/io.c +++ b/block/

[Qemu-block] [PATCH 03/18] block: access io_limits_disabled with atomic ops

2017-05-11 Thread Paolo Bonzini
Reviewed-by: Stefan Hajnoczi Signed-off-by: Paolo Bonzini --- block/block-backend.c | 4 ++-- block/throttle-groups.c| 2 +- include/sysemu/block-backend.h | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index

[Qemu-block] [PULL 51/58] qcow2: Make distinction between zero cluster types obvious

2017-05-11 Thread Kevin Wolf
From: Eric Blake Treat plain zero clusters differently from allocated ones, so that we can simplify the logic of checking whether an offset is present. Do this by splitting QCOW2_CLUSTER_ZERO into two new enums, QCOW2_CLUSTER_ZERO_PLAIN and QCOW2_CLUSTER_ZERO_ALLOC. I tried to arrange the enum s

[Qemu-block] [PULL 49/58] qcow2: Correctly report status of preallocated zero clusters

2017-05-11 Thread Kevin Wolf
From: Eric Blake We were throwing away the preallocation information associated with zero clusters. But we should be matching the well-defined semantics in bdrv_get_block_status(), where (BDRV_BLOCK_ZERO | BDRV_BLOCK_OFFSET_VALID) informs the user which offset is reserved, while still reminding

[Qemu-block] [PATCH 01/18] block: access copy_on_read with atomic ops

2017-05-11 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini --- v1->v2: use atomic_fetch_dec [Stefan] block.c | 6 -- block/io.c| 8 blockdev.c| 2 +- include/block/block_int.h | 11 ++- 4 files changed, 15 insertions(+), 12 deletions(-) diff -

[Qemu-block] [PULL 58/58] MAINTAINERS: Add qemu-progress to the block layer

2017-05-11 Thread Kevin Wolf
From: Max Reitz util/qemu-progress.c is currently unmaintained. The only user of its functionality is qemu-img, so it effectively is part of the block layer. Suggested-by: Fam Zheng Signed-off-by: Max Reitz Message-id: 20170428165517.30341-1-mre...@redhat.com Reviewed-by: Eric Blake Reviewed-

[Qemu-block] [PULL 57/58] qcow2: Discard/zero clusters by byte count

2017-05-11 Thread Kevin Wolf
From: Eric Blake Passing a byte offset, but sector count, when we ultimately want to operate on cluster granularity, is madness. Clean up the external interfaces to take both offset and count as bytes, while still keeping the assertion added previously that the caller must align the values to a

[Qemu-block] [PULL 56/58] qcow2: Assert that cluster operations are aligned

2017-05-11 Thread Kevin Wolf
From: Eric Blake We already audited (in commit 0c1bd469) that qcow2_discard_clusters() is only passed cluster-aligned start values; but we can further tighten the assertion that the only unaligned end value is at EOF. Recent commits have taken advantage of an unaligned tail cluster, for both dis

[Qemu-block] [PULL 48/58] block: Update comments on BDRV_BLOCK_* meanings

2017-05-11 Thread Kevin Wolf
From: Eric Blake We had some conflicting documentation: a nice 8-way table that described all possible combinations of DATA, ZERO, and OFFSET_VALID, contrasted with text that implied that OFFSET_VALID always meant raw data could be read directly. Furthermore, the text refers a lot to bs->file, e

[Qemu-block] [PULL 55/58] qcow2: Optimize write zero of unaligned tail cluster

2017-05-11 Thread Kevin Wolf
From: Eric Blake We've already improved discards to operate efficiently on the tail of an unaligned qcow2 image; it's time to make a similar improvement to write zeroes. The special case is only valid at the tail cluster of a file, where we must recognize that any sectors beyond the image end wo

[Qemu-block] [PULL 50/58] qcow2: Name typedef for cluster type

2017-05-11 Thread Kevin Wolf
From: Eric Blake Although it doesn't add all that much type safety (this is C, after all), it does add a bit of legibility to use the name QCow2ClusterType instead of a plain int. In particular, qcow2_get_cluster_offset() has an overloaded return type; a QCow2ClusterType on success, and -errno o

[Qemu-block] [PULL 52/58] qcow2: Optimize zero_single_l2() to minimize L2 churn

2017-05-11 Thread Kevin Wolf
From: Eric Blake Similar to discard_single_l2(), we should try to avoid dirtying the L2 cache when the cluster we are changing already has the right characteristics. Note that by the time we get to zero_single_l2(), BDRV_REQ_MAY_UNMAP is a requirement to unallocate a cluster (this is because the

[Qemu-block] [PULL 42/58] blkdebug: Add pass-through write_zero and discard support

2017-05-11 Thread Kevin Wolf
From: Eric Blake In order to test the effects of artificial geometry constraints on operations like write zero or discard, we first need blkdebug to manage these actions. It also allows us to inject errors on those operations, just like we can for read/write/flush. We can also test the contract

[Qemu-block] [PULL 39/58] qemu-io: Switch 'map' output to byte-based reporting

2017-05-11 Thread Kevin Wolf
From: Eric Blake Mixing byte offset and sector allocation counts is a bit confusing. Also, reporting n/m sectors, where m decreases according to the remaining size of the file, isn't really adding any useful information; and reporting an offset at both the front and end of the line, with large a

[Qemu-block] [PULL 38/58] qemu-io: Switch 'alloc' command to byte-based length

2017-05-11 Thread Kevin Wolf
From: Eric Blake For the 'alloc' command, accepting an offset in bytes but a length in sectors, and reporting output in sectors, is confusing. Do everything in bytes, and adjust the expected output accordingly. Signed-off-by: Eric Blake Message-id: 20170429191419.30051-3-ebl...@redhat.com Revi

[Qemu-block] [PULL 45/58] tests: Add coverage for recent block geometry fixes

2017-05-11 Thread Kevin Wolf
From: Eric Blake Use blkdebug's new geometry constraints to emulate setups that have needed past regression fixes: write zeroes asserting when running through a loopback block device with max-transfer smaller than cluster size, and discard rounding away portions of requests not aligned to preferr

[Qemu-block] [PULL 47/58] qcow2: Use consistent switch indentation

2017-05-11 Thread Kevin Wolf
From: Eric Blake Fix a couple of inconsistent indentations, before an upcoming patch further tweaks the switch statements. (best viewed with 'git diff -b'). Signed-off-by: Eric Blake Reviewed-by: Max Reitz Message-id: 20170507000552.20847-3-ebl...@redhat.com Signed-off-by: Max Reitz --- bloc

[Qemu-block] [PULL 43/58] blkdebug: Simplify override logic

2017-05-11 Thread Kevin Wolf
From: Eric Blake Rather than store into a local variable, then copy to the struct if the value is valid, then reporting errors otherwise, it is simpler to just store into the struct and report errors if the value is invalid. This however requires that the struct store a 64-bit number, rather tha

[Qemu-block] [PULL 44/58] blkdebug: Add ability to override unmap geometries

2017-05-11 Thread Kevin Wolf
From: Eric Blake Make it easier to simulate various unusual hardware setups (for example, recent commits 3482b9b and b8d0a98 affect the Dell Equallogic iSCSI with its 15M preferred and maximum unmap and write zero sizing, or b2f95fe deals with the Linux loopback block device having a max_transfer

[Qemu-block] [PULL 40/58] blkdebug: Sanity check block layer guarantees

2017-05-11 Thread Kevin Wolf
From: Eric Blake Commits 04ed95f4 and 1a62d0ac updated the block layer to auto-fragment any I/O to fit within device boundaries. Additionally, when using a minimum alignment of 4k, we want to ensure the block layer does proper read-modify-write rather than requesting I/O on a slice of a sector. L

[Qemu-block] [PULL 41/58] blkdebug: Refactor error injection

2017-05-11 Thread Kevin Wolf
From: Eric Blake Rather than repeat the logic at each caller of checking if a Rule exists that warrants an error injection, fold that logic into inject_error(); and rename it to rule_check() for legibility. This will help the next patch, which adds two more callers that need to check rules for th

[Qemu-block] [PULL 46/58] qcow2: Nicer variable names in qcow2_update_snapshot_refcount()

2017-05-11 Thread Kevin Wolf
From: Eric Blake In order to keep checkpatch happy when the next patch changes indentation, we first have to shorten some long lines. The easiest approach is to use a new variable in place of 'offset & L2E_OFFSET_MASK', except that 'offset' is the best name for that variable. Change '[old_]offs

[Qemu-block] [PULL 37/58] qemu-io: Improve alignment checks

2017-05-11 Thread Kevin Wolf
From: Eric Blake Several copy-and-pasted alignment checks exist in qemu-io, which could use some minor improvements: - Manual comparison against 0x1ff is not as clean as using our alignment macros (QEMU_IS_ALIGNED) from osdep.h. - The error messages aren't quite grammatically correct. Suggeste

[Qemu-block] [PULL 36/58] blockdev: use drained_begin/end for qmp_block_resize

2017-05-11 Thread Kevin Wolf
From: John Snow Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1447551 If one tries to issue a block_resize while a guest is busy accessing the disk, it is possible that qemu may deadlock when invoking aio_poll from both the main loop and the iothread. Replace another instance of bdrv_drain

[Qemu-block] [PULL 33/58] file-posix: Remove .bdrv_inactivate/invalidate_cache

2017-05-11 Thread Kevin Wolf
Now that the block layer takes care to request a lot less permissions for inactive nodes, the special-casing in file-posix isn't necessary any more. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/file-posix.c | 33 - 1 file changed, 33 deletions(-)

[Qemu-block] [PULL 29/58] block: New BdrvChildRole.activate() for blk_resume_after_migration()

2017-05-11 Thread Kevin Wolf
Instead of manually calling blk_resume_after_migration() in migration code after doing bdrv_invalidate_cache_all(), integrate the BlockBackend activation with cache invalidation into a single function. This is achieved with a new callback in BdrvChildRole that is called by bdrv_invalidate_cache_all

[Qemu-block] [PULL 30/58] block: Drop permissions when migration completes

2017-05-11 Thread Kevin Wolf
With image locking, permissions affect other qemu processes as well. We want to be sure that the destination can run, so let's drop permissions on the source when migration completes. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 12 +++- block/block-

[Qemu-block] [PULL 27/58] iotests: Extend test 066

2017-05-11 Thread Kevin Wolf
From: Max Reitz 066 was supposed to be a test "for discarding preallocated zero clusters", but it did so incompletely: While it did check the image file's integrity after the operation, it did not confirm that the clusters are indeed freed. This patch adds this test. In addition, new cases for w

[Qemu-block] [PULL 26/58] qcow2: Discard preallocated zero clusters

2017-05-11 Thread Kevin Wolf
From: Max Reitz In discard_single_l2(), we completely discard normal clusters instead of simply turning them into preallocated zero clusters. That means we should probably do the same with such preallocated zero clusters: Discard them instead of keeping them allocated. Reported-by: Eric Blake S

[Qemu-block] [PULL 22/58] qemu-iotests: Add test case 153 for image locking

2017-05-11 Thread Kevin Wolf
From: Fam Zheng Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- tests/qemu-iotests/153 | 233 +++ tests/qemu-iotests/153.out | 390 + tests/qemu-iotests/group | 1 + 3 files changed, 624 insertions(+) create mo

[Qemu-block] [PULL 20/58] osdep: Fall back to posix lock when OFD lock is unavailable

2017-05-11 Thread Kevin Wolf
From: Fam Zheng Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- util/osdep.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/util/osdep.c b/util/osdep.c index 3de4a18..a2863c8 100644 --- a/util/osdep.c +++ b/util/osdep.c @@ -38,6 +38,14 @@ exte

[Qemu-block] [PULL 19/58] osdep: Add qemu_lock_fd and qemu_unlock_fd

2017-05-11 Thread Kevin Wolf
From: Fam Zheng They are wrappers of POSIX fcntl "file private locking", with a convenient "try lock" wrapper implemented with F_OFD_GETLK. Signed-off-by: Fam Zheng Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- include/qemu/osdep.h | 3 +++ util/osdep.c | 48 +

[Qemu-block] [PULL 16/58] file-win32: Error out if locking=on

2017-05-11 Thread Kevin Wolf
From: Fam Zheng We share the same set of QAPI options with file-posix, but locking is not supported here. So error out if it is specified as 'on' for now. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/file-win32.c | 5 + 1 file changed, 5 insertions(+) diff --git a/block/f

[Qemu-block] [PULL 18/58] block: Reuse bs as backing hd for drive-backup sync=none

2017-05-11 Thread Kevin Wolf
From: Fam Zheng Opening the backing image for the second time is bad, especially here when it is also in use as the active image as the source. The drive-backup job itself doesn't read from target->backing for COW, instead it gets data from the write notifier, so it's not a big problem. However,

[Qemu-block] [PULL 14/58] tests: Use null-co:// instead of /dev/null as the dummy image

2017-05-11 Thread Kevin Wolf
From: Fam Zheng Signed-off-by: Fam Zheng Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/drive_del-test.c| 2 +- tests/nvme-test.c | 2 +- tests/usb-hcd-uhci-test.c | 2 +- tests/usb-hcd-xhci-test.c | 2 +- tests/virtio-blk-test.c | 2 +- tests/virtio-scsi-test.c | 5

[Qemu-block] [PULL 17/58] tests: Disable image lock in test-replication

2017-05-11 Thread Kevin Wolf
From: Fam Zheng The COLO block replication architecture requires one disk to be shared between primary and secondary, in the test both processes use posix file protocol (instead of over NBD) so it is affected by image locking. Disable the lock. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf

[Qemu-block] [PULL 35/58] nvme: Implement Write Zeroes

2017-05-11 Thread Kevin Wolf
From: Christoph Hellwig Signed-off-by: Keith Busch [hch: ported over from qemu-nvme.git to mainline] Signed-off-by: Christoph Hellwig Acked-by: Keith Busch Signed-off-by: Kevin Wolf --- hw/block/nvme.c | 26 ++ hw/block/nvme.h | 1 + 2 files changed, 27 insertions(+)

[Qemu-block] [PULL 32/58] block: Fix write/resize permissions for inactive images

2017-05-11 Thread Kevin Wolf
Format drivers for inactive nodes don't need write/resize permissions on their bs->file and can share write/resize with another VM (in fact, this is the whole point of keeping images inactive). Represent this fact in the op blocker system, so that image locking does the right thing without special-

[Qemu-block] [PULL 31/58] block: Inactivate parents before children

2017-05-11 Thread Kevin Wolf
The proper order for inactivating block nodes is that first the parents get inactivated and then the children. If we do things in this order, we can assert that we didn't accidentally leave a parent activated when one of its child nodes is inactive. Signed-off-by: Kevin Wolf Reviewed-by: Eric Bla

[Qemu-block] [PULL 34/58] qemu-img: wait for convert coroutines to complete

2017-05-11 Thread Kevin Wolf
From: Anton Nefedov On error path (like i/o error in one of the coroutines), it's required to - wait for coroutines completion before cleaning the common structures - reenter dependent coroutines so they ever finish Introduced in 2d9187bc65. Cc: qemu-sta...@nongnu.org Signed-off-by: Anton N

  1   2   >