On Mon, 2019-09-16 at 15:39 +0200, Max Reitz wrote:
> On 15.09.19 22:36, Maxim Levitsky wrote:
> > Commit 8ac0f15f335 accidently broke the COW of non changed areas
> > of newly allocated clusters, when the write spans multiple clusters,
> > and needs COW both prior and after the write.
> > This
From: Maxim Levitsky
This fixes subtle corruption introduced by luks threaded encryption
in commit 8ac0f15f335
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1745922
The corruption happens when we do a write that
* writes to two or more unallocated clusters at once
* doesn't fully
From: Thomas Huth
When running "make check -j8" or something similar, the iotests are
running in parallel with the other tests. So when they are printing
out "Passed all xx tests" or a similar status message at the end,
it might not be quite clear that this message belongs to the iotests,
since
From: Sergio Lopez
block_job_remove_all_bdrv() iterates through job->nodes, calling
bdrv_root_unref_child() for each entry. The call to the latter may
reach child_job_[can_]set_aio_ctx(), which will also attempt to
traverse job->nodes, potentially finding entries that where freed
on previous
On 9/16/19 12:40 PM, Philippe Mathieu-Daudé wrote:
> On 9/16/19 6:36 PM, Eric Blake wrote:
>> On 9/16/19 4:51 AM, Philippe Mathieu-Daudé wrote:
>>> While the tracing frawework does not forbid trailing newline in
>>
>> framework
>>
>>> events format string, using them lead to confuse output.
>>>
From: Nir Soffer
Replace instances of:
(n & (BDRV_SECTOR_SIZE - 1)) == 0
And:
(n & ~BDRV_SECTOR_MASK) == 0
With:
QEMU_IS_ALIGNED(n, BDRV_SECTOR_SIZE)
Which reveals the intent of the code better, and makes it easier to
locate the code checking alignment.
Signed-off-by: Nir
From: Maxim Levitsky
Signed-off-by: Maxim Levitsky
Tested-by: Vladimir Sementsov-Ogievskiy
Message-id: 20190915203655.21638-4-mlevi...@redhat.com
Reviewed-by: Max Reitz
Signed-off-by: Max Reitz
---
tests/qemu-iotests/263 | 91 ++
Instead of reporting all sockets to cURL, only report the one that has
caused curl_multi_do_locked() to be called. This lets us get rid of the
QLIST_FOREACH_SAFE() list, which was actually wrong: SAFE foreaches are
only safe when the current element is removed in each iteration. If it
possible
Am 16.09.2019 um 11:52 hat Max Reitz geschrieben:
> On 13.09.19 16:16, Kevin Wolf wrote:
> > Am 09.08.2019 um 18:13 hat Max Reitz geschrieben:
> >> @@ -261,16 +272,19 @@ void stream_start(const char *job_id,
> >> BlockDriverState *bs,
> >> * disappear from the chain after this operation.
On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote:
> 12.09.2019 3:16, John Snow wrote:
>> Like script_main, but doesn't require a single point of entry.
>> Replace all existing initialization sections with this drop-in replacement.
>>
>> This brings debug support to all existing
Hi all!
Here I suggest some refactoring patches for bitmaps.
Vladimir Sementsov-Ogievskiy (4):
block/dirty-bitmap: drop meta
block/dirty-bitmap: add bs link
block/dirty-bitmap: drop BdrvDirtyBitmap.mutex
block/dirty-bitmap: refactor bdrv_dirty_bitmap_next
include/block/dirty-bitmap.h
mutex field is just a pointer to bs->dirty_bitmap_mutex, so no needs
to store it in BdrvDirtyBitmap when we have bs pointer in it (since
previous patch).
Drop mutex field. Constantly use bdrv_dirty_bitmaps_lock/unlock in
block/dirty-bitmap.c to make it more obvious that it's not per-bitmap
lock.
From: Nir Soffer
Replace confusing usage:
~BDRV_SECTOR_MASK
With more clear:
(BDRV_SECTOR_SIZE - 1)
Remove BDRV_SECTOR_MASK and the unused BDRV_BLOCK_OFFSET_MASK which was
it's last user.
Signed-off-by: Nir Soffer
Message-id: 20190827185913.27427-3-nsof...@redhat.com
Reviewed-by:
A follow-up patch will make curl_multi_do() and curl_multi_read() take a
CURLSocket instead of the CURLState. They still need the latter,
though, so add a pointer to it to the former.
Cc: qemu-sta...@nongnu.org
Signed-off-by: Max Reitz
Reviewed-by: John Snow
Message-id:
While it is more likely that transfers complete after some file
descriptor has data ready to read, we probably should not rely on it.
Better be safe than sorry and call curl_multi_check_completion() in
curl_multi_do(), too, just like it is done in curl_multi_read().
With this change,
From: Vladimir Sementsov-Ogievskiy
qemu-io now prefixes its error and warnings with "qemu-io:".
36b9986b08787019e fixed a lot of iotests output but forget about
026.out.nocache. Fix it too.
Fixes: 99e98d7c9fc1a1639fad ("qemu-io: Use error_[gs]et_progname()")
Signed-off-by: Vladimir
12.09.2019 3:16, John Snow wrote:
> Like script_main, but doesn't require a single point of entry.
> Replace all existing initialization sections with this drop-in replacement.
>
> This brings debug support to all existing script-style iotests.
>
> Note: supported_oses=['linux'] was omitted, as
Add bs field to BdrvDirtyBitmap structure. Drop BlockDriverState
parameter from bitmap APIs where possible.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
include/block/dirty-bitmap.h | 14 +-
block/backup.c | 14 ++
block/dirty-bitmap.c | 24
On Mon, 2019-09-16 at 16:59 +0300, Maxim Levitsky wrote:
> On Mon, 2019-09-16 at 15:39 +0200, Max Reitz wrote:
> > On 15.09.19 22:36, Maxim Levitsky wrote:
> > > Commit 8ac0f15f335 accidently broke the COW of non changed areas
> > > of newly allocated clusters, when the write spans multiple
On 9/16/19 4:51 AM, Philippe Mathieu-Daudé wrote:
> While the tracing frawework does not forbid trailing newline in
framework
> events format string, using them lead to confuse output.
> It is the responsibility of the backend to properly end an event
> line.
Why just trailing newline? Should
On 9/16/19 6:36 PM, Eric Blake wrote:
> On 9/16/19 4:51 AM, Philippe Mathieu-Daudé wrote:
>> While the tracing frawework does not forbid trailing newline in
>
> framework
>
>> events format string, using them lead to confuse output.
>> It is the responsibility of the backend to properly end an
On 9/16/19 12:39 PM, Vladimir Sementsov-Ogievskiy wrote:
> 16.09.2019 19:32, John Snow wrote:
>>
>>
>> On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> 12.09.2019 3:16, John Snow wrote:
Like script_main, but doesn't require a single point of entry.
Replace all existing
Drop meta bitmaps, as they are unused.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
include/block/dirty-bitmap.h | 5
block/dirty-bitmap.c | 46
2 files changed, 51 deletions(-)
diff --git a/include/block/dirty-bitmap.h
From: Thomas Huth
The check script is already printing out which iotest is currently
running, so printing out the name of the check-block.sh shell script
looks superfluous here.
Signed-off-by: Thomas Huth
Message-id: 20190906113534.10907-1-th...@redhat.com
Acked-by: John Snow
Reviewed-by: Max
From: Maxim Levitsky
* Change the qcow2_co_{encrypt|decrypt} to just receive full host and
guest offsets and use this function directly instead of calling
do_perform_cow_encrypt (which is removed by that patch).
* Adjust qcow2_co_encdec to take full host and guest offsets as well.
*
If we had done that all along, debugging would have been much simpler.
(Also, I/O errors are better than hangs.)
Signed-off-by: Max Reitz
Message-id: 20190910124136.10565-8-mre...@redhat.com
Reviewed-by: Maxim Levitsky
Reviewed-by: John Snow
Signed-off-by: Max Reitz
---
block/curl.c | 8
16.09.2019 19:32, John Snow wrote:
>
>
> On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 12.09.2019 3:16, John Snow wrote:
>>> Like script_main, but doesn't require a single point of entry.
>>> Replace all existing initialization sections with this drop-in replacement.
>>>
>>> This
bdrv_dirty_bitmap_next is always used in same pattern. So, split it
into _next and _first, instead of combining two functions into one and
add FOR_EACH_DIRTY_BITMAP macro.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
include/block/dirty-bitmap.h | 9 +++--
block.c
The following changes since commit dd25f97c66a75d1508f1d4c6478ed2c95bec428f:
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20190913'
into staging (2019-09-16 10:15:15 +0100)
are available in the Git repository at:
https://github.com/XanClic/qemu.git
Background: As of cURL 7.59.0, it verifies that several functions are
not called from within a callback. Among these functions is
curl_multi_add_handle().
curl_read_cb() is a callback from cURL and not a coroutine. Waking up
acb->co will lead to entering it then and there, which means the
On 15.09.19 22:36, Maxim Levitsky wrote:
> Commit 8ac0f15f335 accidently broke the COW of non changed areas
> of newly allocated clusters, when the write spans multiple clusters,
> and needs COW both prior and after the write.
> This results in 'after' COW area being encrypted with wrong
> sector
Patchew URL: https://patchew.org/QEMU/20190916142246.31474-1-mre...@redhat.com/
Hi,
This series failed the docker-quick@centos7 build test. Please find the testing
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
16.09.2019 16:26, Max Reitz wrote:
> On 13.09.19 10:58, Max Reitz wrote:
>> On 16.08.19 17:30, Vladimir Sementsov-Ogievskiy wrote:
>>> Hi all!
>>>
>>> Here is an asynchronous scheme for handling fragmented qcow2
>>> reads and writes. Both qcow2 read and write functions loops through
>>> sequential
curl_multi_do_locked() currently marks all sockets as ready. That is
not only inefficient, but in fact unsafe (the loop is). A follow-up
patch will change that, but to do so, curl_multi_do_locked() needs to
know exactly which socket is ready; and that is accomplished by this
patch here.
Cc:
This does not really change anything, but it makes the code a bit easier
to follow once we use @socket as the opaque pointer for
aio_set_fd_handler().
Cc: qemu-sta...@nongnu.org
Signed-off-by: Max Reitz
Message-id: 20190910124136.10565-3-mre...@redhat.com
Reviewed-by: Maxim Levitsky
On 9/16/19 4:00 AM, Kevin Wolf wrote:
> Am 13.09.2019 um 20:49 hat John Snow geschrieben:
>> On 9/12/19 4:20 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> Also, I'm not sure about "are" suggested by Max. "are" is for plural, but
>>> here I meant
>>> one object: sum of @start and @count.
>>>
>>
Further patch will run partial requests of iterations of
qcow2_co_preadv in parallel for performance reasons. To prepare for
this, separate part which may be parallelized into separate function
(qcow2_co_preadv_task).
While being here, also separate encrypted clusters reading to own
function,
Similarly to previous commit, prepare for parallelizing write-loop
iterations.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Max Reitz
---
block/qcow2.c | 154 +-
1 file changed, 90 insertions(+), 64 deletions(-)
diff --git
Common interface for aio task loops. To be used for improving
performance of synchronous io loops in qcow2, block-stream,
copy-on-read, and may be other places.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Max Reitz
---
include/block/aio_task.h | 54 +
On 9/16/19 10:19 AM, Vladimir Sementsov-Ogievskiy wrote:
> Drop meta bitmaps, as they are unused.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
believe it or not, I had a local patch that does the same :)
Reviewed-by: John Snow
Upcoming asynchronous handling of sub-parts of qcow2 requests will
change number of leaked clusters and even make it racy. As a
preparation, ignore leaks on failure parts in 026.
It's not trivial to just grep or substitute qemu-img output for such
thing. Instead do better: 3 is a error code of
Hi all!
Here is an asynchronous scheme for handling fragmented qcow2
reads and writes. Both qcow2 read and write functions loops through
sequential portions of data. The series aim it to parallelize these
loops iterations.
It improves performance for fragmented qcow2 images, I've tested it
as
On 9/12/19 1:37 AM, Sergio Lopez wrote:
>> I tried to test this patch, but even with it applied, I still got an
>> aio-context crasher by attempting an nbd-server-start, nbd-server-add,
>> nbd-server-stop (intentionally skipping the nbd-server-remove step) on a
>> domain using iothreads, with a
It improves performance for fragmented qcow2 images.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/qcow2.h | 3 ++
block/qcow2.c | 125 -
block/trace-events | 1 +
3 files changed, 117 insertions(+), 12 deletions(-)
diff --git
On 9/12/19 6:31 AM, Kevin Wolf wrote:
>>
>> Yes, I think locking the context during the "if (exp->blk) {" block at
>> nbd/server.c:1646 should do the trick.
That line number has moved over time; which function are you referring to?
>
> We need to be careful to avoid locking things twice, so
When iothreads are in use, the failure to grab the aio context results
in an assertion failure when trying to unlock things during blk_unref,
when trying to unlock a mutex that was not locked. In short, all
calls to nbd_export_put need to done while within the correct aio
context. But since
On 14.09.19 00:53, John Snow wrote:
>
>
> On 9/12/19 9:56 AM, Max Reitz wrote:
>> Callers can use this new parameter to expect failure during the
>> completion process.
>>
>> Signed-off-by: Max Reitz
>> ---
>> tests/qemu-iotests/iotests.py | 18 --
>> 1 file changed, 12
Am 13.09.2019 um 12:56 hat Sergio Lopez geschrieben:
> virtio_notify_config() needs to acquire the global mutex, which isn't
> allowed from an iothread, and may lead to a deadlock like this:
>
> - main thead
> * Has acquired: qemu_global_mutex.
> * Is trying the acquire: iothread AioContext
15.09.2019 23:36, Maxim Levitsky wrote:
> * Change the qcow2_co_{encrypt|decrypt} to just receive full host and
>guest offsets and use this function directly instead of calling
>do_perform_cow_encrypt (which is removed by that patch).
>
> * Adjust qcow2_co_encdec to take full host and
Am 13.09.2019 um 21:54 hat John Snow geschrieben:
>
>
> On 9/13/19 11:25 AM, Sergio Lopez wrote:
> > do_drive_backup() already acquires the AioContext, so release it
> > before the call.
> >
> > Signed-off-by: Sergio Lopez
> > ---
> > blockdev.c | 6 +-
> > 1 file changed, 1 insertion(+),
Am 13.09.2019 um 20:49 hat John Snow geschrieben:
> On 9/12/19 4:20 AM, Vladimir Sementsov-Ogievskiy wrote:
> > Also, I'm not sure about "are" suggested by Max. "are" is for plural, but
> > here I meant
> > one object: sum of @start and @count.
> >
>
> There's not great agreement universally
14.09.2019 1:01, John Snow wrote:
>
>
> On 9/11/19 11:00 AM, Vladimir Sementsov-Ogievskiy wrote:
>> It's more comfortable to not deal with local_err.
>>
>
> I agree.
>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy
>> ---
>> block/qcow2.h| 5 ++---
>>
Am 13.09.2019 um 12:52 hat Philippe Mathieu-Daudé geschrieben:
> Hi Stefan,
>
> I'v been confused by trailing newline in trace reports,
> so this series aims to fix this, by cleaning current
> formats and add a check to catch new one introduced.
Good idea.
Reviewed-by: Kevin Wolf
On 15.09.19 22:36, Maxim Levitsky wrote:
> This fixes subtle corruption introduced by luks threaded encryption
> in commit 8ac0f15f335
>
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1745922
>
> The corruption happens when we do a write that
>* writes to two or more unallocated
Kevin Wolf writes:
> Am 13.09.2019 um 21:54 hat John Snow geschrieben:
>>
>>
>> On 9/13/19 11:25 AM, Sergio Lopez wrote:
>> > do_drive_backup() already acquires the AioContext, so release it
>> > before the call.
>> >
>> > Signed-off-by: Sergio Lopez
>> > ---
>> > blockdev.c | 6 +-
>>
John Snow writes:
> On 9/13/19 11:25 AM, Sergio Lopez wrote:
>> do_drive_backup() already acquires the AioContext, so release it
>> before the call.
>>
>> Signed-off-by: Sergio Lopez
>> ---
>> blockdev.c | 6 +-
>> 1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git
virtio_notify_config() needs to acquire the global mutex, which isn't
allowed from an iothread, and may lead to a deadlock like this:
- main thead
* Has acquired: qemu_global_mutex.
* Is trying the acquire: iothread AioContext lock via
AIO_WAIT_WHILE (after aio_poll).
- iothread
*
While the tracing frawework does not forbid trailing newline in
events format string, using them lead to confuse output.
It is the responsibility of the backend to properly end an event
line.
Some of our formats have trailing newlines, remove them.
Reviewed-by: John Snow
Reviewed-by: Kevin Wolf
On 13.09.19 16:16, Kevin Wolf wrote:
> Am 09.08.2019 um 18:13 hat Max Reitz geschrieben:
>> Because of the recent changes that make the stream job independent of
>> the base node and instead track the node above it, we have to split that
>> "bottom" node into two cases: The bottom COW node, and
Hi Stefan,
I'v been confused by trailing newline in trace reports,
so this series aims to fix this, by cleaning current
formats and add a check to catch new one introduced.
v2:
- Use regex format (easier to review)
- Added R-b
Regards,
Phil.
Philippe Mathieu-Daudé (2):
trace: Remove
Event format ending with newlines confuse the trace reports.
Forbid them.
Add a check to refuse new format added with trailing newline:
$ make
[...]
GEN hw/misc/trace.h
Traceback (most recent call last):
File "scripts/tracetool.py", line 152, in
main(sys.argv)
File
On 13.09.19 14:55, Kevin Wolf wrote:
> Am 09.08.2019 um 18:13 hat Max Reitz geschrieben:
>> This includes some permission limiting (for example, we only need to
>> take the RESIZE permission for active commits where the base is smaller
>> than the top).
>>
>> Signed-off-by: Max Reitz
>> ---
>>
On 15.09.19 22:36, Maxim Levitsky wrote:
> * Change the qcow2_co_{encrypt|decrypt} to just receive full host and
> guest offsets and use this function directly instead of calling
> do_perform_cow_encrypt (which is removed by that patch).
>
> * Adjust qcow2_co_encdec to take full host and
On 15.09.19 22:36, Maxim Levitsky wrote:
> Signed-off-by: Maxim Levitsky
> Tested-by: Vladimir Sementsov-Ogievskiy
> ---
> tests/qemu-iotests/263 | 91 ++
> tests/qemu-iotests/263.out | 40 +
> tests/qemu-iotests/group | 1 +
> 3 files
On 13.09.19 10:58, Max Reitz wrote:
> On 16.08.19 17:30, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> Here is an asynchronous scheme for handling fragmented qcow2
>> reads and writes. Both qcow2 read and write functions loops through
>> sequential portions of data. The series aim it to
65 matches
Mail list logo
