Squeak Squeak...
...Any objections to me staging this?
(This patchset removes the accommodations in iotests for allowing
either library to run and always forces the new one. Point of no
return for iotests.)
--js
On Wed, Feb 2, 2022 at 9:24 PM John Snow wrote:
>
> Based-on:
On 08/02/2022 14.12, Hanna Reitz wrote:
On 08.02.22 11:13, Thomas Huth wrote:
We can get a nicer progress indication if we add the iotests
individually via the 'check' script instead of going through
the check-block.sh wrapper.
For this, we have to add some of the sanity checks that have
Using bdrv_do_drained_begin_quiesce() in bdrv_child_cb_drained_begin()
is not a good idea: the callback might be called when running
a drain in a coroutine, and bdrv_drained_begin_poll() does not
handle that case, resulting in assertion failure.
Instead, bdrv_do_drained_begin with no recursion
Doing the opposite can make ->detach() (more precisely
bdrv_unapply_subtree_drain() in bdrv_child_cb_detach) undo the subtree_drain
just performed to protect the removal of the child from the graph,
thus making the fully-enabled assert_bdrv_graph_writable fail.
Note that
This test uses a callback of an I/O function (blk_aio_preadv)
to modify the graph, using bdrv_attach_child.
This is simply not allowed anymore. I/O cannot change the graph.
Before "block/io.c: make bdrv_do_drained_begin_quiesce static
and introduce bdrv_drained_begin_no_poll", the test would
On 2/8/22 16:10, Thomas Huth wrote:
Before we go down that road, I think it would be better to get rid of
the "auto" group and add the list of the iotests that should get run
during "make check" to the meson.build file directly. That's easier to
understand and less confusing.
There are other
On 08/02/2022 13.36, Paolo Bonzini wrote:
On 2/8/22 11:13, Thomas Huth wrote:
We can get a nicer progress indication if we add the iotests
individually via the 'check' script instead of going through
the check-block.sh wrapper.
For this, we have to add some of the sanity checks that have
job-driver.h contains all functions of job.h that are used by
the drivers (JobDriver, BlockJobDriver).
These functions are unaware of the job_mutex,
so they all take and release the lock internally.
No functional change intended.
Signed-off-by: Emanuele Giuseppe Esposito
---
job-monitor.h contains all functions of job.h that are used by the
monitor and essentially all functions that do not define a
JobDriver/Blockdriver.
No functional change intended.
Signed-off-by: Emanuele Giuseppe Esposito
---
include/qemu/job-monitor.h | 249
If a drain happens while a job is sleeping, the timeout
gets cancelled and the job continues once the drain ends.
This is especially bad for the sleep performed in commit and stream
jobs, since that is dictated by ratelimit to maintain a certain speed.
Basically the execution path is the
This serie aims to remove and clean up some bugs that came up
when trying to replace the AioContext lock and still protect
BlockDriverState fields.
They were part of the serie "Removal of Aiocontext lock
through drains: protect bdrv_replace_child_noperm", but since
that serie is still a work in
From: Paolo Bonzini
We want to make sure access of job->aio_context is always done
under either BQL or job_mutex. The problem is that using
aio_co_enter(job->aiocontext, job->co) in job_start and job_enter_cond
makes the coroutine immediately resume, so we can't hold the job lock.
And caching it
Just as for the job API, rename block_job functions that are
always called under job lock.
No functional change intended.
Signed-off-by: Emanuele Giuseppe Esposito
---
block.c | 3 ++-
block/backup.c | 4 ++--
blockdev.c | 12 +++-
blockjob.c
Not sure what the atomic here was supposed to do, since job.busy
is protected by the job lock. Since the whole function
is called under job_mutex, just remove the atomic.
Signed-off-by: Emanuele Giuseppe Esposito
---
blockjob.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
We are always using the given bs AioContext, so there is no need
to take the job ones (which is identical anyways).
This also reduces the point we need to check when protecting
job.aio_context field.
Signed-off-by: Emanuele Giuseppe Esposito
---
block/commit.c | 4 ++--
block/mirror.c | 2 +-
2
Doing the opposite can make adding the child node to a non-drained node,
as apply_subtree_drain is only done in ->attach() and thus make
assert_bdrv_graph_writable fail.
This can happen for example during a transaction rollback (test 245,
test_io_with_graph_changes):
1. a node is removed from the
There will be 2 problems in this test when we will add
subtree drains in bdrv_replace_child_noperm:
- First, the test is inconsistent about taking the AioContext lock when
calling bdrv_replace_child_noperm. bdrv_replace_child_noperm is reached
in two places: from blk_insert_bs directly, and
All these functions assume that the lock is not held, and acquire
it internally.
These functions will be useful when job_lock is globally applied,
as they will allow callers to access the job struct fields
without worrying about the job lock.
Update also the comments in blockjob.c (and move them
In order to make it thread safe, implement a "fake rwlock",
where we allow reads under BQL *or* job_mutex held, but
writes only under BQL *and* job_mutex.
The only write we have is in child_job_set_aio_ctx, which always
happens under drain (so the job is paused).
For this reason, introduce
With the *nop* job_lock/unlock placed, rename the static
functions that are always under job_mutex, adding "_locked" suffix.
List of functions that get this suffix:
job_txn_refjob_txn_del_job
job_txn_apply job_state_transition
job_should_pause
Once job lock is used and aiocontext is removed, mirror has
to perform job operations under the same critical section,
using the helpers prepared in previous commit.
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Signed-off-by: Emanuele Giuseppe Esposito
---
Add missing job synchronization in the unit tests, with
explicit locks.
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Signed-off-by: Emanuele Giuseppe Esposito
---
tests/unit/test-bdrv-drain.c | 76 -
tests/unit/test-block-iothread.c |
Same as AIO_WAIT_WHILE macro, but if we are in the Main loop
do not release and then acquire ctx_ 's aiocontext.
Once all Aiocontext locks go away, this macro will replace
AIO_WAIT_WHILE.
Signed-off-by: Emanuele Giuseppe Esposito
---
include/block/aio-wait.h | 15 +++
1 file
Both blockdev.c and job-qmp.c have TOC/TOU conditions, because
they first search for the job and then perform an action on it.
Therefore, we need to do the search + action under the same
job mutex critical section.
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Categorize the fields in struct Job to understand which ones
need to be protected by the job mutex and which don't.
Signed-off-by: Emanuele Giuseppe Esposito
---
include/qemu/job.h | 59 ++
1 file changed, 34 insertions(+), 25 deletions(-)
diff --git
job mutex will be used to protect the job struct elements and list,
replacing AioContext locks.
Right now use a shared lock for all jobs, in order to keep things
simple. Once the AioContext lock is gone, we can introduce per-job
locks.
To simplify the switch from aiocontext to job lock,
Instead of having the lock in job_tnx_apply, move it inside
in the callback. This will be helpful for next commits, when
we introduce job_lock/unlock pairs.
job_transition_to_pending() and job_needs_finalize() do not
need to be protected by the aiocontext lock.
No functional change intended.
job_event_* functions can all be static, as they are not used
outside job.c.
Same applies for job_txn_add_job().
Signed-off-by: Emanuele Giuseppe Esposito
---
include/qemu/job.h | 18 --
job.c | 12 +---
2 files changed, 9 insertions(+), 21 deletions(-)
On 08/02/2022 14.11, Philippe Mathieu-Daudé wrote:
On 8/2/22 13:38, Thomas Huth wrote:
On 08/02/2022 13.28, Hanna Reitz wrote:
On 08.02.22 13:13, Thomas Huth wrote:
On 08/02/2022 12.46, Hanna Reitz wrote:
On 08.02.22 11:13, Thomas Huth wrote:
Instead of failing the iotests if GNU sed is not
On Thu, 3 Feb 2022 at 23:22, John Snow wrote:
>
> On Thu, Feb 3, 2022 at 11:52 AM Peter Maydell
> wrote:
> >
> > On Thu, 3 Feb 2022 at 16:38, John Snow wrote:
> >
> > > On Thu, Feb 3, 2022, 11:20 AM Peter Maydell
> > > wrote:
> > >> Summary of Failures:
> > >>
> > >> 1/1 qemu:block /
job-common.h contains all struct and common function that currently
are in job.h and will be shared by job-monitor and job-driver in
the next commits.
Also move job_type(), job_type_str() and job_is_internal there,
as they are common helper functions.
No functional change intended.
In preparation to the job_lock/unlock patch, remove these
aiocontext locks.
The main reason these two locks are removed here is because
they are inside a loop iterating on the jobs list. Once the
job_lock is added, it will have to protect the whole loop,
wrapping also the aiocontext
In this series, we split the job API in two headers:
job-driver.h and job-monitor.h.
As explained in job.c, job-monitor are the functions mainly used
by the monitor, and require consistency between the search of
a specific job (job_get) and the actual operation/action on it
(e.g. job_user_cancel).
With the *nop* job_lock/unlock placed, rename the job functions
of the job API that are always under job_mutex, adding "_locked"
suffix.
List of functions that get this suffix:
job_txn_unref job_txn_add_job
job_ref job_unref
job_enter_cond job_finish_sync
Change the job_{lock/unlock} and macros to use job_mutex.
Now that they are not nop anymore, remove the aiocontext
to avoid deadlocks.
Therefore:
- when possible, remove completely the aiocontext lock/unlock pair
- if it is used by some other functions too, reduce the locking
section as much as
On 8/2/22 13:38, Thomas Huth wrote:
On 08/02/2022 13.28, Hanna Reitz wrote:
On 08.02.22 13:13, Thomas Huth wrote:
On 08/02/2022 12.46, Hanna Reitz wrote:
On 08.02.22 11:13, Thomas Huth wrote:
Instead of failing the iotests if GNU sed is not available (or
skipping
them completely in the
On 2/8/22 11:13, Thomas Huth wrote:
We can get a nicer progress indication if we add the iotests
individually via the 'check' script instead of going through
the check-block.sh wrapper.
For this, we have to add some of the sanity checks that have
originally been done in the tests/check-block.sh
Introduce the job locking mechanism through the whole job API,
following the comments in job.h and requirements of job-monitor
(like the functions in job-qmp.c, assume lock is held) and
job-driver (like in mirror.c and all other JobDriver, lock is not held).
Use the _locked helpers introduced
In preparation to the job_lock/unlock usage, create _locked
duplicates of some functions, since they will be sometimes called with
job_mutex held (mostly within job.c),
and sometimes without (mostly from JobDrivers using the job API).
Therefore create a _locked version of such function, so that
Am 08.02.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
>
>
> On 07/02/2022 19:30, Kevin Wolf wrote:
> > Am 21.01.2022 um 18:05 hat Emanuele Giuseppe Esposito geschrieben:
> >> Each function in the GS API will have an assertion, checking
> >> that it is always running under BQL.
> >>
In this series, we want to remove the AioContext lock and instead
use the already existent job_mutex to protect the job structures
and list. This is part of the work to get rid of AioContext lock
usage in favour of smaller granularity locks.
In order to simplify reviewer's job, job lock/unlock
On 08.02.22 11:13, Thomas Huth wrote:
By using subdir_done(), we can get rid of one level of indentation
in this file. This will make it easier to add more conditions to
skip the iotests in future patches.
Signed-off-by: Thomas Huth
---
tests/qemu-iotests/meson.build | 61
On 07/02/2022 19:30, Kevin Wolf wrote:
> Am 21.01.2022 um 18:05 hat Emanuele Giuseppe Esposito geschrieben:
>> Each function in the GS API will have an assertion, checking
>> that it is always running under BQL.
>> I/O functions are instead thread safe (or so should be), meaning
>> that they
On 08.02.22 13:13, Thomas Huth wrote:
On 08/02/2022 12.46, Hanna Reitz wrote:
On 08.02.22 11:13, Thomas Huth wrote:
Instead of failing the iotests if GNU sed is not available (or skipping
them completely in the check-block.sh script), it would be better to
simply skip the bash-based tests, so
On 08.02.22 11:13, Thomas Huth wrote:
For better integration of the iotests into the meson build system, it
would be very helpful to get the list of the tests in the "auto" group
during the "configure" step already. However, "check -n -g auto"
currently only works if the binaries have already
On 08.02.22 11:13, Thomas Huth wrote:
We can get a nicer progress indication if we add the iotests
individually via the 'check' script instead of going through
the check-block.sh wrapper.
For this, we have to add some of the sanity checks that have
originally been done in the
On 08.02.22 11:13, Thomas Huth wrote:
Instead of failing the iotests if GNU sed is not available (or skipping
them completely in the check-block.sh script), it would be better to
simply skip the bash-based tests, so that the python-based tests could
still be run. Thus add the check for BusyBox
On 08/02/2022 13.28, Hanna Reitz wrote:
On 08.02.22 13:13, Thomas Huth wrote:
On 08/02/2022 12.46, Hanna Reitz wrote:
On 08.02.22 11:13, Thomas Huth wrote:
Instead of failing the iotests if GNU sed is not available (or skipping
them completely in the check-block.sh script), it would be better
On 2/8/22 12:16, Thomas Huth wrote:
This code being deleted claims to be doing something to ensure that
the tests get run and output the useful messages on failure.
No, AFAIK that --verbose switch just influences how meson prints the
progress during the test runs (i.e. either a brief or a
On 08/02/2022 12.46, Hanna Reitz wrote:
On 08.02.22 11:13, Thomas Huth wrote:
Instead of failing the iotests if GNU sed is not available (or skipping
them completely in the check-block.sh script), it would be better to
simply skip the bash-based tests, so that the python-based tests could
still
On 08/02/2022 11.26, Peter Maydell wrote:
On Tue, 8 Feb 2022 at 10:18, Thomas Huth wrote:
Now that we add the single iotests directly in meson.build, we do
not have to separate the block suite from the other suites anymore.
Signed-off-by: Thomas Huth
---
meson.build| 6 +++---
On Tue, 8 Feb 2022 at 11:16, Thomas Huth wrote:
>
> On 08/02/2022 11.26, Peter Maydell wrote:
> > What is the mechanism for this in the new meson setup ?
>
> cat meson-logs/testlog.txt
>
> ... I guess we should either dump that to stdout
Yes, it needs to actually appear in the stdout for CI
On 07/02/2022 19:14, Kevin Wolf wrote:
> Am 21.01.2022 um 18:05 hat Emanuele Giuseppe Esposito geschrieben:
>> Introduce .pre_run() job callback. This cb will run in job_start,
>> before the coroutine is created and runs run() in the job aiocontext.
>>
>> Therefore, .pre_run() always runs in
On 8/2/22 11:13, Thomas Huth wrote:
By using subdir_done(), we can get rid of one level of indentation
in this file. This will make it easier to add more conditions to
skip the iotests in future patches.
Signed-off-by: Thomas Huth
---
tests/qemu-iotests/meson.build | 61
On 07/02/2022 17:53, Kevin Wolf wrote:
> Am 01.02.2022 um 11:30 hat Paolo Bonzini geschrieben:
>> On 2/1/22 10:45, Emanuele Giuseppe Esposito wrote:
That said, even if they are a different category, I think it makes sense
to leave them in the same header file as I/O functions, because
On 07/02/2022 18:26, Kevin Wolf wrote:
> Am 21.01.2022 um 18:05 hat Emanuele Giuseppe Esposito geschrieben:
>> blockjob functions run always under the BQL lock.
>>
>> Signed-off-by: Emanuele Giuseppe Esposito
>> Reviewed-by: Stefan Hajnoczi
>> ---
>> include/block/blockjob.h | 9 +
>>
On Tue, 8 Feb 2022 at 10:18, Thomas Huth wrote:
>
> Now that we add the single iotests directly in meson.build, we do
> not have to separate the block suite from the other suites anymore.
>
> Signed-off-by: Thomas Huth
> ---
> meson.build| 6 +++---
> scripts/mtest2make.py | 4
Now that the iotests are added by the meson.build file already,
we do not need the check-block.sh wrapper script anymore.
Signed-off-by: Thomas Huth
---
tests/check-block.sh | 73
1 file changed, 73 deletions(-)
delete mode 100755
Now that we add the single iotests directly in meson.build, we do
not have to separate the block suite from the other suites anymore.
Signed-off-by: Thomas Huth
---
meson.build| 6 +++---
scripts/mtest2make.py | 4
tests/Makefile.include | 9 +
3 files changed, 4
We can get a nicer progress indication if we add the iotests
individually via the 'check' script instead of going through
the check-block.sh wrapper.
For this, we have to add some of the sanity checks that have
originally been done in the tests/check-block.sh script (whether
"bash" is available
For better integration of the iotests into the meson build system, it
would be very helpful to get the list of the tests in the "auto" group
during the "configure" step already. However, "check -n -g auto"
currently only works if the binaries have already been built. Re-order
the code in the
Instead of failing the iotests if GNU sed is not available (or skipping
them completely in the check-block.sh script), it would be better to
simply skip the bash-based tests, so that the python-based tests could
still be run. Thus add the check for BusyBox sed to common.rc and mark
the tests as
By using subdir_done(), we can get rid of one level of indentation
in this file. This will make it easier to add more conditions to
skip the iotests in future patches.
Signed-off-by: Thomas Huth
---
tests/qemu-iotests/meson.build | 61 ++
1 file changed, 32
Though "make check-block" is currently already run via the meson test
runner, it still looks like an oddball in the output of "make check" since
the tests are still run separately via the check-block.sh script. It would
be nicer if the iotests would show up like the other tests suites. For this
we
On Tue, Feb 8, 2022 at 4:09 PM Stefan Hajnoczi wrote:
>
> On Tue, Feb 08, 2022 at 03:35:27PM +0800, Yongji Xie wrote:
> > On Mon, Feb 7, 2022 at 10:39 PM Stefan Hajnoczi wrote:
> > >
> > > On Tue, Jan 25, 2022 at 09:18:00PM +0800, Xie Yongji wrote:
> > > > +static void *vduse_log_get(const char
On Tue, Feb 08, 2022 at 03:35:27PM +0800, Yongji Xie wrote:
> On Mon, Feb 7, 2022 at 10:39 PM Stefan Hajnoczi wrote:
> >
> > On Tue, Jan 25, 2022 at 09:18:00PM +0800, Xie Yongji wrote:
> > > +static void *vduse_log_get(const char *dir, const char *name, size_t
> > > size)
> > > +{
> > > +
On Tue, Feb 08, 2022 at 02:42:41PM +0800, Yongji Xie wrote:
> On Mon, Feb 7, 2022 at 10:01 PM Stefan Hajnoczi wrote:
> >
> > On Tue, Jan 25, 2022 at 09:17:57PM +0800, Xie Yongji wrote:
> > > +int vduse_dev_handler(VduseDev *dev)
> > > +{
> > > +struct vduse_dev_request req;
> > > +struct
On Mon, Feb 7, 2022 at 10:39 PM Stefan Hajnoczi wrote:
>
> On Tue, Jan 25, 2022 at 09:18:00PM +0800, Xie Yongji wrote:
> > To support reconnecting after restart or crash, VDUSE backend
> > might need to resubmit inflight I/Os. This stores the metadata
> > such as the index of inflight I/O's
68 matches
Mail list logo
