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
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
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
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
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
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
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
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
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
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
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)
> ---
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,
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
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,
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,
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,
...
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
> >> +
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
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
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
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
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.
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,
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
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
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
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
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
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
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.
>
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
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
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
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
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
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
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
>>> +
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
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
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
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
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
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
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
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
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
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.
>
>
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
> @@
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
>
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
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
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
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
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
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
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
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
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
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
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
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
78 matches
Mail list logo