Re: [Qemu-block] [Qemu-devel] [PATCH 5/5] block: Move request_alignment into BlockLimit

2016-06-13 Thread Eric Blake
On 06/07/2016 04:08 AM, Kevin Wolf wrote: >> Found it; squash this in (or use it as an argument why we don't want >> request_alignment in bs->bl after all): > > This hunk doesn't make sense to me. For the correctness of the code it > shouldn't make a difference whether the alignment happens

[Qemu-block] [PATCH v3 0/3] coccinelle: Clean up error checks and return value variables

2016-06-13 Thread Eduardo Habkost
Changes v1 -> v2: * The Coccinelle scripts were simplified by using "when" constraints to detect when a variable is not used elsewhere inside the function. * Added script to remove unnecessary variables for function return value. * Coccinelle scripts added to scripts/coccinelle. Changes v2

Re: [Qemu-block] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-13 Thread Alex Bligh
On 13 Jun 2016, at 13:25, Eric Blake wrote: > On 06/13/2016 06:10 AM, Paolo Bonzini wrote: >> >> >> On 12/05/2016 00:39, Eric Blake wrote: >>> - If we report an error to NBD_CMD_READ, we are not writing out >>> any data payload; but the protocol says that a client can

Re: [Qemu-block] [Qemu-devel] [RFC v2 3/3] Remove unnecessary variables for function return value

2016-06-13 Thread Eduardo Habkost
On Mon, Jun 13, 2016 at 01:29:47PM +0200, Markus Armbruster wrote: > Eduardo Habkost writes: > > > Use Coccinelle script to replace 'ret = E; return ret' with > > 'return E'. The script will do the substitution only when the > > function return type and variable type are the

Re: [Qemu-block] [Qemu-devel] [PATCH] macio: Use blk_drain instead of blk_drain_all

2016-06-13 Thread John Snow
On 06/12/2016 02:56 AM, Fam Zheng wrote: > We only care about the associated backend, so blk_drain is more > appropriate here. > > Signed-off-by: Fam Zheng > --- > hw/ide/macio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/ide/macio.c

Re: [Qemu-block] [PATCH 6/6] trace: enable tracing in qemu-img

2016-06-13 Thread Eric Blake
On 06/13/2016 10:58 AM, Denis V. Lunev wrote: > The command will work this way: > qemu-img --trace qcow2* create -f qcow2 1.img 64G > > Signed-off-by: Denis V. Lunev > Suggested by: Daniel P. Berrange > CC: Eric Blake > CC: Paolo

Re: [Qemu-block] [PATCH 5/6] qemu-img: move common options parsing before commands processing

2016-06-13 Thread Eric Blake
On 06/13/2016 10:58 AM, Denis V. Lunev wrote: > This is necessary to enable creation of common qemu-img options which will > be specified before command. > > The patch also enables '-V' alias to '--version' (exactly like in other > block utilities) and documents this change. > > Signed-off-by:

Re: [Qemu-block] [Qemu-devel] [PATCH v2 2/3] error: Remove unnecessary local_err variables

2016-06-13 Thread Eduardo Habkost
On Mon, Jun 13, 2016 at 08:49:37PM +0200, Markus Armbruster wrote: > Eric Blake writes: [...] > >> > >> See, e.g.: > >> > >> void qmp_guest_suspend_disk(Error **errp) > >> { > >> Error *local_err = NULL; > >> GuestSuspendMode *mode = g_new(GuestSuspendMode, 1); > >>

Re: [Qemu-block] [Qemu-devel] [PATCH v2 2/3] error: Remove unnecessary local_err variables

2016-06-13 Thread Eduardo Habkost
On Mon, Jun 13, 2016 at 10:01:16AM -0600, Eric Blake wrote: > On 06/13/2016 09:52 AM, Eduardo Habkost wrote: [...] > > > > See, e.g.: > > > > void qmp_guest_suspend_disk(Error **errp) > > { > > Error *local_err = NULL; > > GuestSuspendMode *mode = g_new(GuestSuspendMode, 1); > > > >

Re: [Qemu-block] [PATCH 4/6] trace: enable tracing in qemu-nbd

2016-06-13 Thread Eric Blake
On 06/13/2016 10:58 AM, Denis V. Lunev wrote: > Please note, trace_init_backends() must be called in the final process, > i.e. after daemonization. This is necessary to keep tracing thread in > the proper process. > > Signed-off-by: Denis V. Lunev > CC: Eric Blake

Re: [Qemu-block] [PATCH 1/6] doc: move text describing --trace to specific .texi file

2016-06-13 Thread Eric Blake
On 06/13/2016 10:58 AM, Denis V. Lunev wrote: > This text will be included to qemu-nbd/qemu-img mans in the next patches. > > Signed-off-by: Denis V. Lunev > CC: Eric Blake > CC: Paolo Bonzini > CC: Stefan Hajnoczi

Re: [Qemu-block] [PATCH] m25p80: fix test on blk_pread() return value

2016-06-13 Thread Eric Blake
On 06/13/2016 11:43 AM, Cédric Le Goater wrote: > On 06/13/2016 06:47 PM, Eric Blake wrote: >> On 06/13/2016 10:25 AM, Cédric Le Goater wrote: >> >>> >>> It seems that commit 243e6f69c129 ("m25p80: Switch to byte-based block >>> access") >>> is bringing another issue : >>> >>> qemu-system-arm:

[Qemu-block] [PATCH v2 2/2] block: Assert that flags are in range

2016-06-13 Thread Eric Blake
Add a new BDRV_REQ_MASK constant, and use it to make sure that caller flags are always valid. Tested with 'make check' and with qemu-iotests on both '-raw' and '-qcow2'; the only failure turned up was fixed in the previous commit. Signed-off-by: Eric Blake ---

[Qemu-block] [PATCH v2 1/2] block: Avoid bogus flags during mirroring

2016-06-13 Thread Eric Blake
Commit e253f4b8 converted mirroring from sector-based bdrv_aio_* to byte-based blk_aio_*, but failed to account for the subtle difference in signatures (the former takes a semi-redundant length, the latter takes a flags parameter). Since all of our flags are currently smaller in size than

[Qemu-block] [PATCH v2 0/2] Fix incomplete aio_preadv byte conversion in mirroring

2016-06-13 Thread Eric Blake
v2: Split the bug fix from the addition of assertions, add a BDRV_REQ_MASK constant [famz], stick assertions at the more common entry points Eric Blake (2): block: Avoid bogus flags during mirroring block: Assert that flags are in range include/block/block.h | 3 +++ block/io.c|

Re: [Qemu-block] [Qemu-devel] [PATCH v2 2/3] error: Remove unnecessary local_err variables

2016-06-13 Thread Markus Armbruster
Eric Blake writes: > On 06/13/2016 09:52 AM, Eduardo Habkost wrote: > >>> >>> There is an (ugly) difference between >>> >>> error_setg(_err, ...); >>> error_propagate(errp, _err); >>> >>> and >>> >>> error_setg(errp, ...); >>> >>> The latter aborts when @errp

Re: [Qemu-block] [Qemu-devel] [PATCH] block-backend: allow flush on devices with open tray

2016-06-13 Thread John Snow
On 06/10/2016 06:42 PM, Eric Blake wrote: > On 06/10/2016 03:59 PM, John Snow wrote: >> If a device still has an attached BDS because the medium has not yet >> been removed, we will be unable to migrate to a new host because >> blk_flush will return an error for that backend. >> >> Replace the

Re: [Qemu-block] [PATCH] m25p80: fix test on blk_pread() return value

2016-06-13 Thread Cédric Le Goater
On 06/13/2016 06:47 PM, Eric Blake wrote: > On 06/13/2016 10:25 AM, Cédric Le Goater wrote: > >> >> It seems that commit 243e6f69c129 ("m25p80: Switch to byte-based block >> access") >> is bringing another issue : >> >> qemu-system-arm: >>

[Qemu-block] [PATCH 6/6] trace: enable tracing in qemu-img

2016-06-13 Thread Denis V. Lunev
The command will work this way: qemu-img --trace qcow2* create -f qcow2 1.img 64G Signed-off-by: Denis V. Lunev Suggested by: Daniel P. Berrange CC: Eric Blake CC: Paolo Bonzini CC: Stefan Hajnoczi

[Qemu-block] [PATCH 1/6] doc: move text describing --trace to specific .texi file

2016-06-13 Thread Denis V. Lunev
This text will be included to qemu-nbd/qemu-img mans in the next patches. Signed-off-by: Denis V. Lunev CC: Eric Blake CC: Paolo Bonzini CC: Stefan Hajnoczi CC: Kevin Wolf --- Makefile

[Qemu-block] [PATCH 2/6] trace: move qemu_trace_opts to trace/control.c

2016-06-13 Thread Denis V. Lunev
The patch also creates trace_opt_parse() helper in trace/control.c to reuse this code in next patches for qemu-nbd and qemu-io. The patch also makes trace_init_events() static, as this call is not used outside the module anymore. Signed-off-by: Denis V. Lunev Reviewed-by: Eric

[Qemu-block] [PATCH 3/6] trace: enable tracing in qemu-io

2016-06-13 Thread Denis V. Lunev
Moving trace_init_backends() into trace_opt_parse() is not possible. This should be called after daemonize() in vl.c. Signed-off-by: Denis V. Lunev Reviewed-by: Eric Blake CC: Paolo Bonzini CC: Stefan Hajnoczi CC:

[Qemu-block] [PATCH 5/6] qemu-img: move common options parsing before commands processing

2016-06-13 Thread Denis V. Lunev
This is necessary to enable creation of common qemu-img options which will be specified before command. The patch also enables '-V' alias to '--version' (exactly like in other block utilities) and documents this change. Signed-off-by: Denis V. Lunev CC: Eric Blake

[Qemu-block] [PATCH v4 0/6] trace: enable tracing in qemu-io/qemu-nbd/qemu-img

2016-06-13 Thread Denis V. Lunev
Changes from v3: - fixed difference in help/man for qemu-img/qemu-nbd - created separate .texi to contain trace description, proper dependency is added to makefile - added --version/--help description to qemu-img - fixed crash induced by new option processing scheme in qemu-img which has

[Qemu-block] [PATCH 4/6] trace: enable tracing in qemu-nbd

2016-06-13 Thread Denis V. Lunev
Please note, trace_init_backends() must be called in the final process, i.e. after daemonization. This is necessary to keep tracing thread in the proper process. Signed-off-by: Denis V. Lunev CC: Eric Blake CC: Paolo Bonzini CC: Stefan

Re: [Qemu-block] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-13 Thread Eric Blake
On 06/13/2016 06:19 AM, Paolo Bonzini wrote: >> +/* Sanity checks, part 2. */ >> +if (request->from + request->len > client->exp->size) { >> +LOG("operation past EOF; From: %" PRIu64 ", Len: %" PRIu32 >> +", Size: %" PRIu64, request->from, request->len, >> +

Re: [Qemu-block] [PATCH] m25p80: fix test on blk_pread() return value

2016-06-13 Thread Eric Blake
On 06/13/2016 10:25 AM, Cédric Le Goater wrote: > > It seems that commit 243e6f69c129 ("m25p80: Switch to byte-based block > access") > is bringing another issue : > > qemu-system-arm: > /home/legoater/work/qemu/qemu-ast2400-mainline.git/block/io.c:1252: > bdrv_aligned_pwritev: Assertion

Re: [Qemu-block] [PATCH v4 00/11] nbd: tighter protocol compliance

2016-06-13 Thread Paolo Bonzini
On 12/05/2016 00:39, Eric Blake wrote: > Fix several corner-case bugs in our implementation of the NBD > protocol, both as client and as server. > > Depends on Kevin's block-next branch: > git://repo.or.cz/qemu/kevin.git block-next > > Also available as a tag at this location: > git fetch

Re: [Qemu-block] [PATCH] m25p80: fix test on blk_pread() return value

2016-06-13 Thread Cédric Le Goater
Hello Eric, On 05/31/2016 04:36 PM, Eric Blake wrote: > On 05/31/2016 08:29 AM, Cédric Le Goater wrote: >> On 05/31/2016 04:26 PM, Eric Blake wrote: >>> On 05/31/2016 05:36 AM, Cédric Le Goater wrote: commit 243e6f69c129 ("m25p80: Switch to byte-based block access") replaced blk_read()

Re: [Qemu-block] [Qemu-devel] [PATCH v2 2/3] error: Remove unnecessary local_err variables

2016-06-13 Thread Eric Blake
On 06/13/2016 09:52 AM, Eduardo Habkost wrote: >> >> There is an (ugly) difference between >> >> error_setg(_err, ...); >> error_propagate(errp, _err); >> >> and >> >> error_setg(errp, ...); >> >> The latter aborts when @errp already contains an error, the former does >> nothing. > >

Re: [Qemu-block] [Qemu-devel] [PATCH v2 2/3] error: Remove unnecessary local_err variables

2016-06-13 Thread Eduardo Habkost
On Mon, Jun 13, 2016 at 01:42:15PM +0200, Markus Armbruster wrote: > Eduardo Habkost writes: > > > This patch simplifies code that uses a local_err variable just to > > immediately use it for an error_propagate() call. > > > > Coccinelle patch used to perform the changes

Re: [Qemu-block] [PATCH v2] block: drop support for using qcow[2] encryption with system emulators

2016-06-13 Thread Kevin Wolf
Am 13.06.2016 um 13:30 hat Daniel P. Berrange geschrieben: > Back in the 2.3.0 release we declared qcow[2] encryption as > deprecated, warning people that it would be removed in a future > release. > > commit a1f688f4152e65260b94f37543521ceff8bfebe4 > Author: Markus Armbruster

Re: [Qemu-block] [PATCH v2] block: drop support for using qcow[2] encryption with system emulators

2016-06-13 Thread Eric Blake
On 06/13/2016 05:30 AM, Daniel P. Berrange wrote: > Back in the 2.3.0 release we declared qcow[2] encryption as > deprecated, warning people that it would be removed in a future > release. > > So the safety net is correctly preventing QEMU reading cipher > text as if it were plain text, during

Re: [Qemu-block] [PATCH v3 0/5] block/mirror: Fix (?) target backing BDS

2016-06-13 Thread Kevin Wolf
Am 10.06.2016 um 20:57 hat Max Reitz geschrieben: > Issue #1: If the target image does not have a backing BDS before mirror > completion, qemu tries really hard to give it a backing BDS. If the > source has a backing BDS, it will actually always "succeed". > In some cases, the target is not

Re: [Qemu-block] [PATCH v3 1/5] block: Allow replacement of a BDS by its overlay

2016-06-13 Thread Kevin Wolf
Am 12.06.2016 um 05:15 hat Fam Zheng geschrieben: > On Fri, 06/10 20:57, Max Reitz wrote: > > change_parent_backing_link() asserts that the BDS to be replaced is not > > used as a backing file. However, we may want to replace a BDS by its > > overlay in which case that very link should not be

Re: [Qemu-block] [Qemu-devel] [PATCH v2] mirror: follow AioContext change gracefully

2016-06-13 Thread Jason J. Herne
On 06/12/2016 02:51 AM, Fam Zheng wrote: ... --- v2: Picked up Stefan's RFC patch and move on towards a more complete fix. Please review! Jason: it would be nice if you could test this version again. It differs from the previous version. No problem. I'll test v3 when it is available. --

Re: [Qemu-block] [PATCH 0/6] block: Enable byte granularity I/O

2016-06-13 Thread Stefan Hajnoczi
On Wed, Jun 08, 2016 at 04:10:05PM +0200, Kevin Wolf wrote: > Previous series have already converted some block drivers to byte-based rather > than sector-based interfaces. However, the common I/O path as well as > raw-posix > still enforced a minimum alignment of 512 bytes because some

Re: [Qemu-block] [PATCH 0/6] block: Enable byte granularity I/O

2016-06-13 Thread Kevin Wolf
Am 13.06.2016 um 15:27 hat Stefan Hajnoczi geschrieben: > On Wed, Jun 08, 2016 at 04:10:05PM +0200, Kevin Wolf wrote: > > Previous series have already converted some block drivers to byte-based > > rather > > than sector-based interfaces. However, the common I/O path as well as > > raw-posix > >

Re: [Qemu-block] [PATCH v2] mirror: follow AioContext change gracefully

2016-06-13 Thread Stefan Hajnoczi
On Sun, Jun 12, 2016 at 02:51:04PM +0800, Fam Zheng wrote: > From: Stefan Hajnoczi > > When dataplane is enabled or disabled the drive switches to a new > AioContext. The mirror block job must also move to the new AioContext > so that drive accesses are always made within

Re: [Qemu-block] [PATCH 5/6] qcow2: Let vmstate call qcow2_co_preadv/pwrite directly

2016-06-13 Thread Eric Blake
On 06/11/2016 08:58 PM, Fam Zheng wrote: >>> -return ret; >>> +return bs->drv->bdrv_co_pwritev(bs, qcow2_vm_state_offset(s) + pos, >>> +qiov->size, qiov, 0); >>> } >> >> bs->drv->bdrv_co_pwritev() is an optional interface; not all the drivers >> have

Re: [Qemu-block] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-13 Thread Eric Blake
[adding nbd list] On 06/13/2016 06:10 AM, Paolo Bonzini wrote: > > > On 12/05/2016 00:39, Eric Blake wrote: >> - If we report an error to NBD_CMD_READ, we are not writing out >> any data payload; but the protocol says that a client can expect >> to read the payload no matter what (and must

Re: [Qemu-block] [PATCH v4 02/11] nbd: More debug typo fixes, use correct formats

2016-06-13 Thread Eric Blake
On 06/13/2016 06:04 AM, Paolo Bonzini wrote: > > > On 12/05/2016 00:39, Eric Blake wrote: >> Clean up some debug message oddities missed earlier; this includes >> some typos, and recognizing that %d is not necessarily compatible >> with uint32_t. > > Actually it should be on any POSIX platform,

Re: [Qemu-block] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-13 Thread Paolo Bonzini
On 12/05/2016 00:39, Eric Blake wrote: > We have a few bugs in how we handle invalid client commands: > > - A client can send an NBD_CMD_DISC where from + len overflows, > convincing us to reply with an error and stay connected, even > though the protocol requires us to silently disconnect. Fix

Re: [Qemu-block] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-13 Thread Paolo Bonzini
On 12/05/2016 00:39, Eric Blake wrote: > - If we report an error to NBD_CMD_READ, we are not writing out > any data payload; but the protocol says that a client can expect > to read the payload no matter what (and must instead ignore it), > which means the client will start reading our next

Re: [Qemu-block] [PATCH v4 02/11] nbd: More debug typo fixes, use correct formats

2016-06-13 Thread Paolo Bonzini
On 12/05/2016 00:39, Eric Blake wrote: > Clean up some debug message oddities missed earlier; this includes > some typos, and recognizing that %d is not necessarily compatible > with uint32_t. Actually it should be on any POSIX platform, since (by way of the requirements on limits.h) POSIX

Re: [Qemu-block] [Qemu-devel] [PATCH v2 2/3] error: Remove unnecessary local_err variables

2016-06-13 Thread Markus Armbruster
Eduardo Habkost writes: > This patch simplifies code that uses a local_err variable just to > immediately use it for an error_propagate() call. > > Coccinelle patch used to perform the changes added to > scripts/coccinelle/remove_local_err.cocci. > > Signed-off-by: Eduardo

Re: [Qemu-block] [Qemu-devel] [RFC v2 3/3] Remove unnecessary variables for function return value

2016-06-13 Thread Markus Armbruster
Eduardo Habkost writes: > Use Coccinelle script to replace 'ret = E; return ret' with > 'return E'. The script will do the substitution only when the > function return type and variable type are the same. > > Sending as RFC because the patch looks more intrusive than the >

[Qemu-block] [PATCH v2] block: drop support for using qcow[2] encryption with system emulators

2016-06-13 Thread Daniel P. Berrange
Back in the 2.3.0 release we declared qcow[2] encryption as deprecated, warning people that it would be removed in a future release. commit a1f688f4152e65260b94f37543521ceff8bfebe4 Author: Markus Armbruster Date: Fri Mar 13 21:09:40 2015 +0100 block: Deprecate

Re: [Qemu-block] [PATCH v2] mirror: follow AioContext change gracefully

2016-06-13 Thread Stefan Hajnoczi
On Sun, Jun 12, 2016 at 02:51:04PM +0800, Fam Zheng wrote: > @@ -119,7 +121,7 @@ static void mirror_iteration_done(MirrorOp *op, int ret) > qemu_iovec_destroy(>qiov); > g_free(op); > > -if (s->waiting_for_io) { > +if (s->waiting_for_io && !s->quiesce_requested) { >

Re: [Qemu-block] [Qemu-devel] [PATCH V3] block/iscsi: allow caching of the allocation map

2016-06-13 Thread Paolo Bonzini
On 30/05/2016 08:33, Peter Lieven wrote: > > The idea of the allocmap in cache.direct = on mode is that we can > still speed up block jobs by skipping large unallocated areas. In this case > the allocmap has only a hint character. If we don't know the status > we issue a get_block_status

Re: [Qemu-block] [PATCH v2] mirror: follow AioContext change gracefully

2016-06-13 Thread Paolo Bonzini
On 12/06/2016 08:51, Fam Zheng wrote: > From: Stefan Hajnoczi > > When dataplane is enabled or disabled the drive switches to a new > AioContext. The mirror block job must also move to the new AioContext > so that drive accesses are always made within its AioContext. > >

Re: [Qemu-block] [PATCH V3] block/iscsi: allow caching of the allocation map

2016-06-13 Thread Paolo Bonzini
On 24/05/2016 10:40, Peter Lieven wrote: > until now the allocation map was used only as a hint if a cluster > is allocated or not. If a block was not allocated (or Qemu had > no info about the allocation status) a get_block_status call was > issued to check the allocation status and possibly

Re: [Qemu-block] [PATCH] macio: Use blk_drain instead of blk_drain_all

2016-06-13 Thread Kevin Wolf
Am 12.06.2016 um 08:56 hat Fam Zheng geschrieben: > We only care about the associated backend, so blk_drain is more > appropriate here. > > Signed-off-by: Fam Zheng [ Cc: John ] > --- > hw/ide/macio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git

Re: [Qemu-block] [PATCH v2 2/3] error: Remove unnecessary local_err variables

2016-06-13 Thread Cornelia Huck
On Fri, 10 Jun 2016 17:12:17 -0300 Eduardo Habkost wrote: > This patch simplifies code that uses a local_err variable just to > immediately use it for an error_propagate() call. > > Coccinelle patch used to perform the changes added to >

Re: [Qemu-block] [PATCH v2 1/3] error: Remove NULL checks on error_propagate() calls

2016-06-13 Thread Cornelia Huck
On Fri, 10 Jun 2016 17:12:16 -0300 Eduardo Habkost wrote: > error_propagate() already ignores local_err==NULL, so there's no > need to check it before calling. > > Coccinelle patch used to perform the changes added to > scripts/coccinelle/error_propagate_null.cocci. > >