Re: [Qemu-block] [PATCH v3 4/7] block/sheepdog: code beautification

2017-11-08 Thread Darren Kenny
On Tue, Nov 07, 2017 at 05:27:21PM -0500, Jeff Cody wrote: No functional changes, just whitespace manipulation. Signed-off-by: Jeff Cody Reviewed-by: Eric Blake Reviewed-by: Darren Kenny --- block/sheepdog.c | 164

Re: [Qemu-block] [PATCH v3 2/7] block/ssh: make compliant with coding guidelines

2017-11-08 Thread Darren Kenny
On Tue, Nov 07, 2017 at 05:27:19PM -0500, Jeff Cody wrote: Signed-off-by: Jeff Cody Reviewed-by: Eric Blake Reviewed-by: Darren Kenny --- block/ssh.c | 32 ++-- 1 file changed, 18 insertions(+), 14

Re: [Qemu-block] [PATCH v3 5/7] block/curl: check error return of curl_global_init()

2017-11-08 Thread Darren Kenny
On Tue, Nov 07, 2017 at 05:27:22PM -0500, Jeff Cody wrote: If curl_global_init() fails, per the documentation no other curl functions may be called, so make sure to check the return value. Also, some minor changes to the initialization latch variable 'inited': - Make it static in the file, for

Re: [Qemu-block] [PATCH v3 2/7] block/ssh: make compliant with coding guidelines

2017-11-08 Thread Richard W.M. Jones
On Tue, Nov 07, 2017 at 05:27:19PM -0500, Jeff Cody wrote: > Signed-off-by: Jeff Cody > Reviewed-by: Eric Blake This one is just simple coding style fixes, so: Reviewed-by: Richard W.M. Jones Rich. > block/ssh.c | 32

Re: [Qemu-block] [Qemu-devel] [PATCH v3] util/async: use atomic_mb_set in qemu_bh_cancel

2017-11-08 Thread Pavel Butsykin
On 08.11.2017 09:34, Sergio Lopez wrote: Commit b7a745d added a qemu_bh_cancel call to the completion function as an optimization to prevent it from unnecessarily rescheduling itself. This completion function is scheduled from worker_thread, after setting the state of a ThreadPoolElement to

Re: [Qemu-block] [PATCH v3 5/7] block/curl: check error return of curl_global_init()

2017-11-08 Thread Richard W.M. Jones
On Tue, Nov 07, 2017 at 05:27:22PM -0500, Jeff Cody wrote: > If curl_global_init() fails, per the documentation no other curl > functions may be called, so make sure to check the return value. > > Also, some minor changes to the initialization latch variable 'inited': > > - Make it static in the

Re: [Qemu-block] [Qemu-devel] [PATCH] block: Deprecate bdrv_set_read_only() and users

2017-11-08 Thread Kevin Wolf
Am 08.11.2017 um 11:49 hat Daniel P. Berrange geschrieben: > On Wed, Nov 08, 2017 at 11:44:01AM +0100, Paolo Bonzini wrote: > > On 07/11/2017 18:39, Daniel P. Berrange wrote: > > > On Tue, Nov 07, 2017 at 06:26:38PM +0100, Kevin Wolf wrote: > > >> bdrv_set_read_only() is used by some block drivers

Re: [Qemu-block] [PATCH v2 4/7] qcow2: Don't open images with header.refcount_table_clusters == 0

2017-11-08 Thread Alberto Garcia
On Tue 07 Nov 2017 05:43:49 PM CET, Kevin Wolf wrote: >> +echo >> +echo "=== Testing zero refcount table size ===" >> +echo >> +_make_test_img 64M >> +poke_file "$TEST_IMG" "56""\x00\x00\x00\x00" >> +$QEMU_IO -c "write 0 64k" "$TEST_IMG" 2>&1 | _filter_testdir | >> _filter_imgfmt

Re: [Qemu-block] [PATCH] block: Deprecate bdrv_set_read_only() and users

2017-11-08 Thread Kevin Wolf
Am 07.11.2017 um 21:29 hat Eric Blake geschrieben: > On 11/07/2017 11:26 AM, Kevin Wolf wrote: > > bdrv_set_read_only() is used by some block drivers to override the > > read-only option given by the user. This is not how read-only images > > generally work in QEMU: Instead of second guessing what

Re: [Qemu-block] [PATCH] qcow2: don't permit changing encryption parameters

2017-11-08 Thread Kevin Wolf
Am 03.11.2017 um 15:39 hat Daniel P. Berrange geschrieben: > Currently if trying to change encryption parameters on a qcow2 image, qemu-img > will abort. We already explicitly check for attempt to change encrypt.format > but missed other parameters like encrypt.key-secret. Rather than list each >

[Qemu-block] [PATCH 1/1] qcow2: Check that corrupted images can be repaired in iotest 060

2017-11-08 Thread Alberto Garcia
We just fixed a few bugs that caused QEMU to crash when trying to write to corrupted qcow2 images, and iotest 060 was expanded to test all those scenarios. In almost all cases the corrupted images can be repaired using qemu-img, so this patch verifies that. Signed-off-by: Alberto Garcia

Re: [Qemu-block] [Qemu-devel] [PATCH] util/async: use atomic_mb_set in qemu_bh_cancel

2017-11-08 Thread Stefan Hajnoczi
On Tue, Nov 07, 2017 at 04:09:37PM +0100, Sergio Lopez wrote: > Commit b7a745d added a qemu_bh_cancel call to the completion function > as an optimization to prevent it from unnecessarily rescheduling itself. > > This completion function is scheduled from worker_thread, after setting > the state

Re: [Qemu-block] [Qemu-devel] [PATCH] block: Deprecate bdrv_set_read_only() and users

2017-11-08 Thread Paolo Bonzini
On 07/11/2017 18:39, Daniel P. Berrange wrote: > On Tue, Nov 07, 2017 at 06:26:38PM +0100, Kevin Wolf wrote: >> bdrv_set_read_only() is used by some block drivers to override the >> read-only option given by the user. This is not how read-only images >> generally work in QEMU: Instead of second

Re: [Qemu-block] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style

2017-11-08 Thread Richard W.M. Jones
On Tue, Nov 07, 2017 at 05:27:24PM -0500, Jeff Cody wrote: > This addresses non-functional changes to help curl.c better comply > with the coding styles (comments, indentation, brackets, etc.). > > One minor code change is the combination of two if statements into > a single if statement. > >

Re: [Qemu-block] [PATCH v3 6/7] block/curl: fix minor memory leaks

2017-11-08 Thread Richard W.M. Jones
On Tue, Nov 07, 2017 at 05:27:23PM -0500, Jeff Cody wrote: > Signed-off-by: Jeff Cody > --- > block/curl.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/block/curl.c b/block/curl.c > index 00a9879..35cf417 100644 > --- a/block/curl.c > +++ b/block/curl.c > @@

Re: [Qemu-block] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style

2017-11-08 Thread Darren Kenny
Hi Jeff, While I'm relatively new to this community, I do have some comments about the styling in this file. I don't see anything in the CODING_STYLE file that tells me I'm wrong here, but it's certainly possible... More inline. On Tue, Nov 07, 2017 at 05:27:24PM -0500, Jeff Cody wrote: This

Re: [Qemu-block] [Qemu-devel] [PATCH] block: Deprecate bdrv_set_read_only() and users

2017-11-08 Thread Paolo Bonzini
On 08/11/2017 12:51, Kevin Wolf wrote: > Am 08.11.2017 um 11:49 hat Daniel P. Berrange geschrieben: >> On Wed, Nov 08, 2017 at 11:44:01AM +0100, Paolo Bonzini wrote: >>> I am not sure this counts as deprecation, but it should go in the >>> release notes as "future incompatible changes", and that

Re: [Qemu-block] [PATCH v3] util/async: use atomic_mb_set in qemu_bh_cancel

2017-11-08 Thread Stefan Hajnoczi
On Wed, Nov 08, 2017 at 07:34:47AM +0100, Sergio Lopez wrote: > Commit b7a745d added a qemu_bh_cancel call to the completion function > as an optimization to prevent it from unnecessarily rescheduling itself. > > This completion function is scheduled from worker_thread, after setting > the state

Re: [Qemu-block] [PATCH v6 19/25] block: Add BlockDriver.bdrv_gather_child_options

2017-11-08 Thread Alberto Garcia
On Fri 29 Sep 2017 06:53:41 PM CEST, Max Reitz wrote: > Some follow-up patches will rework the way bs->full_open_options is > refreshed in bdrv_refresh_filename(). The new implementation will remove > the need for the block drivers' bdrv_refresh_filename() implementations > to set

Re: [Qemu-block] [Qemu-devel] [PATCH] block: Deprecate bdrv_set_read_only() and users

2017-11-08 Thread Kevin Wolf
Am 08.11.2017 um 13:00 hat Paolo Bonzini geschrieben: > On 08/11/2017 12:51, Kevin Wolf wrote: > > Am 08.11.2017 um 11:49 hat Daniel P. Berrange geschrieben: > >> On Wed, Nov 08, 2017 at 11:44:01AM +0100, Paolo Bonzini wrote: > >>> I am not sure this counts as deprecation, but it should go in the

[Qemu-block] [PATCH 0/1] qcow2: Check that corrupted images can be repaired in iotest 060

2017-11-08 Thread Alberto Garcia
Hi, I sent the 'Misc qcow2 corruption checks' series the other day, and Kevin suggested that we check that the corrupted images can be repaired using qemu-img. This patch extends the tests that I wrote in order to do just that. Since the series is already in Max's branch I decided to write this

Re: [Qemu-block] [PATCH v6 18/25] block: Add sgfnt_runtime_opts to BlockDriver

2017-11-08 Thread Alberto Garcia
On Thu 02 Nov 2017 05:18:31 PM CET, Max Reitz wrote: > On 2017-11-02 17:11, Alberto Garcia wrote: >> On Fri 29 Sep 2017 06:53:40 PM CEST, Max Reitz wrote: >>> QLIST_ENTRY(BlockDriver) list; >>> + >>> +/* Pointer to a NULL-terminated array of names of significant options >>> that >>> +

Re: [Qemu-block] [Qemu-devel] [PATCH] block: Deprecate bdrv_set_read_only() and users

2017-11-08 Thread Daniel P. Berrange
On Wed, Nov 08, 2017 at 11:44:01AM +0100, Paolo Bonzini wrote: > On 07/11/2017 18:39, Daniel P. Berrange wrote: > > On Tue, Nov 07, 2017 at 06:26:38PM +0100, Kevin Wolf wrote: > >> bdrv_set_read_only() is used by some block drivers to override the > >> read-only option given by the user. This is

Re: [Qemu-block] [PATCH v3 3/7] block/sheepdog: remove spurious NULL check

2017-11-08 Thread Darren Kenny
On Tue, Nov 07, 2017 at 05:27:20PM -0500, Jeff Cody wrote: 'tag' is already checked in the lines immediately preceding this check, and set to non-NULL if NULL. No need to check again, it hasn't changed. Signed-off-by: Jeff Cody Reviewed-by: Eric Blake

Re: [Qemu-block] [PATCH] block: Deprecate bdrv_set_read_only() and users

2017-11-08 Thread Kevin Wolf
Am 08.11.2017 um 11:04 hat Kevin Wolf geschrieben: > Am 07.11.2017 um 21:29 hat Eric Blake geschrieben: > > On 11/07/2017 11:26 AM, Kevin Wolf wrote: > > > bdrv_set_read_only() is used by some block drivers to override the > > > read-only option given by the user. This is not how read-only images

Re: [Qemu-block] segfault in parallel blockjobs (iotest 30)

2017-11-08 Thread Alberto Garcia
On Tue 07 Nov 2017 05:19:41 PM CET, Anton Nefedov wrote: > One more drainage-related thing. > We have recently encountered an issue with parallel block jobs and it's > not quite clear how to fix it properly, so would appreciate your ideas. > > The small attached tweak makes iotest 30 (-qcow2

Re: [Qemu-block] [PATCH] block: Close a BlockDriverState completely even when bs->drv is NULL

2017-11-08 Thread Kevin Wolf
Am 06.11.2017 um 15:53 hat Alberto Garcia geschrieben: > bdrv_close() skips much of its logic when bs->drv is NULL. This is > fine when we're closing a BlockDriverState that has just been created > (because e.g the initialization process failed), but it's not enough > in other cases. > > For

Re: [Qemu-block] [Qemu-devel] [PATCH v3 01/46] Replace all occurances of __FUNCTION__ with __func__

2017-11-08 Thread Alistair Francis
On Wed, Nov 8, 2017 at 7:00 AM, Eric Blake wrote: > On 11/08/2017 08:51 AM, Alistair Francis wrote: > > Let me rephrase the question: do we really support compilers that don't > understand __func__? The presence of numerous unconditional uses of > __func__ in the

Re: [Qemu-block] [Qemu-devel] [PATCH v3] util/async: use atomic_mb_set in qemu_bh_cancel

2017-11-08 Thread Sergio Lopez
On Wed, Nov 8, 2017 at 2:50 PM, Pavel Butsykin wrote: > On 08.11.2017 09:34, Sergio Lopez wrote: >> This was considered to be safe, as the completion function restarts the >> loop just after the call to qemu_bh_cancel. But, under certain access >> patterns and scheduling

Re: [Qemu-block] [Qemu-devel] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style

2017-11-08 Thread Paolo Bonzini
On 08/11/2017 15:50, Darren Kenny wrote: >> But, since checkpatch.pl doesn't flag it, and since it is easier to >> remove the leading and trailing /* and */ to enable the debug #defines >> (compared to editing every single line of the comment), I don't see a >> problem with the style chosen here.

Re: [Qemu-block] [Qemu-devel] [PATCH v3] util/async: use atomic_mb_set in qemu_bh_cancel

2017-11-08 Thread Sergio Lopez
On Wed, Nov 8, 2017 at 3:15 PM, Paolo Bonzini wrote: > On 08/11/2017 15:10, Sergio Lopez wrote: >>> I'm not quite sure that the pre-fetched is involved in this issue, >>> because pre-fetch reading a certain addresses should be invalidated by >>> write on another core to the

Re: [Qemu-block] [Qemu-devel] [PATCH v3] util/async: use atomic_mb_set in qemu_bh_cancel

2017-11-08 Thread Paolo Bonzini
On 08/11/2017 15:10, Sergio Lopez wrote: >> I'm not quite sure that the pre-fetched is involved in this issue, >> because pre-fetch reading a certain addresses should be invalidated by >> write on another core to the same addresses. In our case write >> req->state = THREAD_DONE should invalidate

Re: [Qemu-block] [Qemu-devel] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style

2017-11-08 Thread Eric Blake
On 11/08/2017 04:47 AM, Darren Kenny wrote: > Hi Jeff, > > While I'm relatively new to this community, I do have some comments > about the styling in this file. > > I don't see anything in the CODING_STYLE file that tells me I'm > wrong here, but it's certainly possible... > > More inline. >

Re: [Qemu-block] [PATCH] block: Deprecate bdrv_set_read_only() and users

2017-11-08 Thread Eric Blake
On 11/08/2017 06:20 AM, Kevin Wolf wrote: >> Well, they don't only need an explicitly set option, but the important >> point is that they don't work with the default value. But I can add >> something to this effect. > > I'll squash this in if it looks good to you: > > diff --git

Re: [Qemu-block] [Qemu-devel] [PATCH v3 01/46] Replace all occurances of __FUNCTION__ with __func__

2017-11-08 Thread Alistair Francis
On Tue, Nov 7, 2017 at 11:52 PM, Markus Armbruster wrote: > Eric Blake writes: > >> On 11/07/2017 04:12 AM, Markus Armbruster wrote: >>> Juan Quintela writes: >>> Alistair Francis wrote: > Replace

Re: [Qemu-block] [Qemu-devel] [PATCH v3] util/async: use atomic_mb_set in qemu_bh_cancel

2017-11-08 Thread Pavel Butsykin
On 08.11.2017 17:24, Sergio Lopez wrote: On Wed, Nov 8, 2017 at 3:15 PM, Paolo Bonzini wrote: On 08/11/2017 15:10, Sergio Lopez wrote: I'm not quite sure that the pre-fetched is involved in this issue, because pre-fetch reading a certain addresses should be invalidated by

Re: [Qemu-block] [Qemu-devel] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style

2017-11-08 Thread Darren Kenny
On Wed, Nov 08, 2017 at 08:26:57AM -0600, Eric Blake wrote: On 11/08/2017 04:47 AM, Darren Kenny wrote: Hi Jeff, While I'm relatively new to this community, I do have some comments about the styling in this file. I don't see anything in the CODING_STYLE file that tells me I'm wrong here, but

Re: [Qemu-block] segfault in parallel blockjobs (iotest 30)

2017-11-08 Thread Anton Nefedov
On 8/11/2017 5:45 PM, Alberto Garcia wrote: On Tue 07 Nov 2017 05:19:41 PM CET, Anton Nefedov wrote: BlockBackend gets deleted by another job's stream_complete(), deferred to the main loop, so the fact that the job is put to sleep by bdrv_drain_all_begin() doesn't really stop it from

Re: [Qemu-block] [PATCH v3 7/7] block/curl: code cleanup to comply with coding style

2017-11-08 Thread Paolo Bonzini
On 08/11/2017 11:47, Darren Kenny wrote: >> >> >> -// #define DEBUG_CURL >> -// #define DEBUG_VERBOSE >> +/* >> + #define DEBUG_CURL >> + #define DEBUG_VERBOSE >> +*/ > > Is it not more common to use the style: > >    /* >     * text >     */ > > This is visible in almost every file at the top,

Re: [Qemu-block] segfault in parallel blockjobs (iotest 30)

2017-11-08 Thread Alberto Garcia
On Tue 07 Nov 2017 05:19:41 PM CET, Anton Nefedov wrote: > BlockBackend gets deleted by another job's stream_complete(), deferred > to the main loop, so the fact that the job is put to sleep by > bdrv_drain_all_begin() doesn't really stop it from execution. I was debugging this a bit, and the

Re: [Qemu-block] [PATCH v6 19/25] block: Add BlockDriver.bdrv_gather_child_options

2017-11-08 Thread Kevin Wolf
Am 29.09.2017 um 18:53 hat Max Reitz geschrieben: > Some follow-up patches will rework the way bs->full_open_options is > refreshed in bdrv_refresh_filename(). The new implementation will remove > the need for the block drivers' bdrv_refresh_filename() implementations > to set

Re: [Qemu-block] [PATCH v6 18/25] block: Add sgfnt_runtime_opts to BlockDriver

2017-11-08 Thread Kevin Wolf
Am 02.11.2017 um 17:18 hat Max Reitz geschrieben: > On 2017-11-02 17:11, Alberto Garcia wrote: > > On Fri 29 Sep 2017 06:53:40 PM CEST, Max Reitz wrote: > >> QLIST_ENTRY(BlockDriver) list; > >> + > >> +/* Pointer to a NULL-terminated array of names of significant options > >> that > >> +

Re: [Qemu-block] [PATCH v6 18/25] block: Add sgfnt_runtime_opts to BlockDriver

2017-11-08 Thread Max Reitz
On 2017-11-08 18:14, Kevin Wolf wrote: > Am 02.11.2017 um 17:18 hat Max Reitz geschrieben: >> On 2017-11-02 17:11, Alberto Garcia wrote: >>> On Fri 29 Sep 2017 06:53:40 PM CEST, Max Reitz wrote: QLIST_ENTRY(BlockDriver) list; + +/* Pointer to a NULL-terminated array of

Re: [Qemu-block] [Qemu-devel] [PATCH v3] util/async: use atomic_mb_set in qemu_bh_cancel

2017-11-08 Thread Pavel Butsykin
On 08.11.2017 17:15, Paolo Bonzini wrote: On 08/11/2017 15:10, Sergio Lopez wrote: I'm not quite sure that the pre-fetched is involved in this issue, because pre-fetch reading a certain addresses should be invalidated by write on another core to the same addresses. In our case write

Re: [Qemu-block] [PATCH v6 19/25] block: Add BlockDriver.bdrv_gather_child_options

2017-11-08 Thread Max Reitz
On 2017-11-08 18:18, Kevin Wolf wrote: > Am 29.09.2017 um 18:53 hat Max Reitz geschrieben: >> Some follow-up patches will rework the way bs->full_open_options is >> refreshed in bdrv_refresh_filename(). The new implementation will remove >> the need for the block drivers' bdrv_refresh_filename()

Re: [Qemu-block] [PATCH v6 16/25] block: Add 'base-directory' BDS option

2017-11-08 Thread Max Reitz
On 2017-11-08 18:09, Kevin Wolf wrote: > Am 07.11.2017 um 13:07 hat Alberto Garcia geschrieben: >> On Thu 02 Nov 2017 11:06:28 PM CET, Eric Blake wrote: >> >> Using this option, one can directly override what bdrv_dirname() >> will return. This is useful if one uses e.g. qcow2 on top of

Re: [Qemu-block] [PATCH v6 16/25] block: Add 'base-directory' BDS option

2017-11-08 Thread Kevin Wolf
Am 07.11.2017 um 13:07 hat Alberto Garcia geschrieben: > On Thu 02 Nov 2017 11:06:28 PM CET, Eric Blake wrote: > > Using this option, one can directly override what bdrv_dirname() > will return. This is useful if one uses e.g. qcow2 on top of quorum > (with only protocol BDSs under

Re: [Qemu-block] [PATCH v3 1/7] block/ssh: don't call libssh2_init() in block_init()

2017-11-08 Thread Richard W.M. Jones
On Tue, Nov 07, 2017 at 05:27:18PM -0500, Jeff Cody wrote: > We don't need libssh2 failure to be fatal (we could just opt to not > register the driver on failure). But, it is probably a good idea to > avoid external library calls during the block_init(), and call the > libssh2 global init function

Re: [Qemu-block] [Qemu-devel] [PATCH] block: Deprecate bdrv_set_read_only() and users

2017-11-08 Thread Daniel P. Berrange
On Wed, Nov 08, 2017 at 12:51:27PM +0100, Kevin Wolf wrote: > Am 08.11.2017 um 11:49 hat Daniel P. Berrange geschrieben: > > On Wed, Nov 08, 2017 at 11:44:01AM +0100, Paolo Bonzini wrote: > > > On 07/11/2017 18:39, Daniel P. Berrange wrote: > > > > On Tue, Nov 07, 2017 at 06:26:38PM +0100, Kevin

[Qemu-block] [PATCH v2 5/7] nbd-client: Short-circuit 0-length operations

2017-11-08 Thread Eric Blake
The NBD spec was recently clarified to state that clients should not send 0-length requests to the server, as the server behavior is undefined [1]. We know that qemu-nbd's behavior is a successful no-op (once it has filtered for read-only exports), but other NBD implementations might return an

[Qemu-block] [PATCH v2 6/7] nbd-client: Stricter enforcing of structured reply spec

2017-11-08 Thread Eric Blake
Ensure that the server is not sending unexpected chunk lengths for either the NONE or the OFFSET_DATA chunk, nor unexpected hole length for OFFSET_HOLE. This will flag any server that responds to a zero-length read with an OFFSET_DATA as broken, even though we previously fixed our client to never

Re: [Qemu-block] [Qemu-devel] [PATCH v2 2/7] nbd-client: Refuse read-only client with BDRV_O_RDWR

2017-11-08 Thread Eric Blake
On 11/08/2017 03:56 PM, Eric Blake wrote: > Fix iotest 147 to comply with the new behavior (since nbd-server-add > defaults to a read-only export, we must tell blockdev-add to set up a > read-only client). My bad for testing only './check -nbd' and not also './check -qcow2'; iotest 58 and 140

[Qemu-block] [PATCH v2 4/7] nbd: Fix struct name for structured reads

2017-11-08 Thread Eric Blake
A closer read of the NBD spec shows that a structured reply chunk for a hole is not quite identical to the prefix of a data chunk, because the hole has to also send a 32-bit size field. Although we do not yet send holes, we should fix the misleading information in our header and make it easier

[Qemu-block] [PATCH v2 3/7] nbd/client: Nicer trace of structured reply

2017-11-08 Thread Eric Blake
It's useful to know which structured reply chunk is being processed. Signed-off-by: Eric Blake --- nbd/client.c | 4 +++- nbd/trace-events | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index 3d680e63e1..1880103d2a

[Qemu-block] [PATCH v2 0/7] various NBD fixes for 2.11

2017-11-08 Thread Eric Blake
Sparked by my question on whether we were handling zero-length reads correctly according to the NBD spec, but grew to fix other things as well such as some typos, tracing weaknesses, and better handling of read-only exports. The order these patches are listed here is bisection-friendly, but

[Qemu-block] [PATCH v2 1/7] nbd-client: Fix error message typos

2017-11-08 Thread Eric Blake
Provide missing spaces that are required when using string concatenation to break error messages across source lines. Signed-off-by: Eric Blake --- block/nbd-client.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/block/nbd-client.c

[Qemu-block] [PATCH v2 7/7] nbd/server: Fix structured read of length 0

2017-11-08 Thread Eric Blake
The NBD spec was recently clarified to state that a read of length 0 should not be attempted by a compliant client; but that a server must still handle it correctly in an unspecified manner (that is, either a successful no-op or an error reply, but not a crash) [1]. However, it also implies that

[Qemu-block] [PATCH v2 2/7] nbd-client: Refuse read-only client with BDRV_O_RDWR

2017-11-08 Thread Eric Blake
The NBD spec says that clients should not try to write/trim to an export advertised as read-only by the server. But we failed to check that, and would allow the block layer to use NBD with BDRV_O_RDWR even when the server is read-only, which meant we were depending on the server sending a proper

[Qemu-block] [PATCH v2 2.5/7] fixup! nbd-client: Refuse read-only client with BDRV_O_RDWR

2017-11-08 Thread Eric Blake
... Fix several iotests to comply with the new behavior (since qemu-nbd of an internal snapshot, as well as nbd-server-add over QMP, default to a read-only export, we must tell blockdev-add/qemu-io to set up a read-only client). Signed-off-by: Eric Blake --- Will squash when

Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/7] various NBD fixes for 2.11

2017-11-08 Thread Eric Blake
On 11/08/2017 04:24 PM, no-re...@patchew.org wrote: > Hi, > > This series failed automatic build test. Please find the testing commands and > their output below. If you have docker installed, you can probably reproduce > it > locally. > > 140 - output mismatch (see 140.out.bad) > ---

[Qemu-block] [PATCH v4 04/45] tests: Replace fprintf(stderr, "*\n" with error_report()

2017-11-08 Thread Alistair Francis
Replace a large number of the fprintf(stderr, "*\n" calls with error_report(). The functions were renamed with these commands and then compiler issues where manually fixed. find ./* -type f -exec sed -i \ 'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr,

[Qemu-block] [PATCH v4 42/45] util: Replace fprintf(stderr, "*\n" with error_report()

2017-11-08 Thread Alistair Francis
Replace a large number of the fprintf(stderr, "*\n" calls with error_report(). The functions were renamed with these commands and then compiler issues where manually fixed. find ./* -type f -exec sed -i \ 'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr,

[Qemu-block] [PATCH v4 02/45] Replace all occurances of __FUNCTION__ with __func__

2017-11-08 Thread Alistair Francis
Replace all occurs of __FUNCTION__ except for the check in checkpatch with the non GCC specific __func__. One line in hcd-musb.c was manually tweaked to pass checkpatch. Signed-off-by: Alistair Francis Cc: Gerd Hoffmann Cc: Andrzej Zaborowski

[Qemu-block] [PATCH v4 16/45] hw/ide: Replace fprintf(stderr, "*\n" with error_report()

2017-11-08 Thread Alistair Francis
Replace a large number of the fprintf(stderr, "*\n" calls with error_report(). The functions were renamed with these commands and then compiler issues where manually fixed. find ./* -type f -exec sed -i \ 'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr,

[Qemu-block] Intermittent hang of iotest 194 (bdrv_drain_all after non-shared storage migration)

2017-11-08 Thread Max Reitz
Hi, More exciting news from the bdrv_drain() front! I've noticed in the past that iotest 194 sometimes hangs. I usually run the tests on tmpfs, but I've just now verified that it happens on my SSD just as well. So the reproducer is a plain: while ./check -raw 194; do; done (No difference

[Qemu-block] [PATCH v4 06/45] hw/block: Replace fprintf(stderr, "*\n" with error_report()

2017-11-08 Thread Alistair Francis
Replace a large number of the fprintf(stderr, "*\n" calls with error_report(). The functions were renamed with these commands and then compiler issues where manually fixed. find ./* -type f -exec sed -i \ 'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr,

Re: [Qemu-block] Drainage in bdrv_replace_child_noperm()

2017-11-08 Thread Max Reitz
On 2017-11-07 15:22, Kevin Wolf wrote: > Am 06.11.2017 um 19:49 hat Max Reitz geschrieben: >> Hi everyone, >> >> On my quest to fix some flaky iotests, I came to a bit of a halt on 129. >> (Details: Its issue is that block jobs now generally ignore throttling >> in a BB (because they use their

Re: [Qemu-block] [Qemu-devel] [PATCH v3] util/async: use atomic_mb_set in qemu_bh_cancel

2017-11-08 Thread Stefan Hajnoczi
On Wed, Nov 08, 2017 at 05:32:23PM +0300, Pavel Butsykin wrote: > On 08.11.2017 17:24, Sergio Lopez wrote: > > On Wed, Nov 8, 2017 at 3:15 PM, Paolo Bonzini wrote: > > > On 08/11/2017 15:10, Sergio Lopez wrote: > > > > > I'm not quite sure that the pre-fetched is involved in

Re: [Qemu-block] [Qemu-devel] Drainage in bdrv_replace_child_noperm()

2017-11-08 Thread Max Reitz
On 2017-11-07 06:21, Fam Zheng wrote: > On Mon, 11/06 19:49, Max Reitz wrote: [...] >> I have two ideas: >> >> One is to assume that (un-)draining a parent will always (un-)drain all >> children, including the one the (un-)drain comes from. This assumption >> seems wrong, see [1], but maybe it

Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/7] various NBD fixes for 2.11

2017-11-08 Thread Fam Zheng
On Wed, 11/08 17:05, Eric Blake wrote: > But I'm not sure if there is an easy way to tell patchew to include a > fixup patch and re-test the series, short of sending a v3. Yeah, I don't know a way either. Fam

Re: [Qemu-block] [Qemu-devel] segfault in parallel blockjobs (iotest 30)

2017-11-08 Thread Fam Zheng
On Wed, 11/08 18:50, Anton Nefedov wrote: > > > On 8/11/2017 5:45 PM, Alberto Garcia wrote: > > On Tue 07 Nov 2017 05:19:41 PM CET, Anton Nefedov wrote: > > > BlockBackend gets deleted by another job's stream_complete(), deferred > > > to the main loop, so the fact that the job is put to sleep

[Qemu-block] [PATCH 1/5] iotests: Make 030 less flaky

2017-11-08 Thread Max Reitz
This patch fixes two race conditions in 030: 1. The first is in TestENSPC.test_enospc(). After resuming the job, querying it to confirm it is no longer paused may fail because in the meantime it might have completed already. The same was fixed in TestEIO.test_ignore() already (in

[Qemu-block] [PATCH 2/5] iotests: Add missing 'blkdebug::' in 040

2017-11-08 Thread Max Reitz
040 tries to invoke pause_drive() on a drive that does not use blkdebug. Good idea, but let's use blkdebug to make it actually work. Signed-off-by: Max Reitz --- tests/qemu-iotests/040 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/040

[Qemu-block] [PATCH 5/5] iotests: Make 136 less flaky

2017-11-08 Thread Max Reitz
136 executes some AIO requests without a final aio_flush; then it advances the virtual clock and thus expects the last access time of the device to be less than the current time when queried (i.e. idle_time_ns to be greater than 0). However, without the aio_flush, some requests may be settled

[Qemu-block] [PATCH 4/5] iotests: Make 083 less flaky

2017-11-08 Thread Max Reitz
083 has (at least) two issues: 1. By launching the nbd-fault-injector in background, it may not be scheduled until the first grep on its output file is executed. However, until then, that file may not have been created yet -- so it either does not exist yet (thus making the grep emit an

[Qemu-block] [PATCH 0/5] iotests: Make some tests less flaky

2017-11-08 Thread Max Reitz
There are a couple of tests that fail (on my machine) from time to time (and by that I mean that recently I've rarely ever had a test run with both 083 and 136 working on first try). This series should fix most (at least the issues I am aware of). Notes: - 083 might have another issue, but if so

Re: [Qemu-block] [Qemu-devel] Intermittent hang of iotest 194 (bdrv_drain_all after non-shared storage migration)

2017-11-08 Thread Fam Zheng
On Thu, 11/09 01:48, Max Reitz wrote: > Hi, > > More exciting news from the bdrv_drain() front! > > I've noticed in the past that iotest 194 sometimes hangs. I usually run > the tests on tmpfs, but I've just now verified that it happens on my SSD > just as well. > > So the reproducer is a

Re: [Qemu-block] [Qemu-devel] [PATCH v4 06/45] hw/block: Replace fprintf(stderr, "*\n" with error_report()

2017-11-08 Thread Philippe Mathieu-Daudé
On 11/08/2017 07:56 PM, Alistair Francis wrote: > Replace a large number of the fprintf(stderr, "*\n" calls with > error_report(). The functions were renamed with these commands and then > compiler issues where manually fixed. > [...] > diff --git a/hw/block/tc58128.c b/hw/block/tc58128.c > index