On Sat, Sep 24, 2022 at 04:00:34PM +0800, Bin Meng wrote:
> From: Bin Meng
>
> At present there are two callers of get_tmp_filename() and they are
> inconsistent.
>
> One does:
>
> /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
> char *tmp_filename = g_malloc0(PAT
On Sun, Sep 25, 2022 at 12:32:00AM +0800, Bin Meng wrote:
> From: Bin Meng
>
> At present there are two callers of get_tmp_filename() and they are
> inconsistent.
>
> One does:
>
> /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
> char *tmp_filename = g_malloc0(PAT
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 job_set
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
Reviewed-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Kevin Wolf
Reviewed-by: Stefan Hajnoczi
---
include/qemu/job.h | 61
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, introduce
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*.
Signed
Just as done with job.h, create _locked() functions in blockjob.h
These functions will be later useful when caller has already taken
the lock. All blockjob _locked functions call job _locked functions.
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Signed-off-by: Ema
With "intact" we mean that all job.h functions implicitly
take the lock. Therefore API callers are unmodified.
This means that:
- many static functions that will be always called with job lock held
become _locked, and call _locked functions
- all public functions take the lock internally if need
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
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Kevin Wolf
---
include/qemu/job.h | 18 --
These public functions are not used anywhere, thus can be dropped.
Signed-off-by: Emanuele Giuseppe Esposito
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Kevin Wolf
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
blockjob.c | 16 ++--
include/block/blockjob.h | 31 +++
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 fun
iostatus is the only field (together with .job) that needs
protection using the job mutex.
It is set in the main loop (GLOBAL_STATE functions) but read
in I/O code (block_job_error_action).
In order to protect it, change block_job_iostatus_set_err
to block_job_iostatus_set_err_locked(), always ca
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
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Vladimir Sementsov-Ogie
This comment applies more on job, it was left in blockjob as in the past
the whole job logic was implemented there.
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
No functional change intended.
Signed-off-by: Emanuele Giuseppe Esposito
Reviewed-by: Vladimir Sementso
Add missing job synchronization in the unit tests, with
explicit locks.
We are deliberately using _locked functions wrapped by a guard
instead of a normal call because the normal call will be removed
in future, as the only usage is limited to the tests.
In other words, if a function like job_paus
They all are called with job_lock held, in job_event_*_locked()
Signed-off-by: Emanuele Giuseppe Esposito
Reviewed-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Kevin Wolf
---
blockjob.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletion
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 function too, reduce the locking
section as much as
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
Once job lock is used and aiocontext is removed, mirror has
to perform job operations under the same critical section,
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Signed-off-by: Emanuele Giuseppe Esposito
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
block/mirror
Now that the API offers also _locked() functions, take advantage
of it and give also the caller control to take the lock and call
_locked functions.
This makes sense especially when we have for loops, because it
makes no sense to have:
for(job = job_next(); ...)
where each job_next() takes the l
Some callbacks implementation use bdrv_* APIs that assume the
AioContext lock is held. Make sure this invariant is documented.
Signed-off-by: Emanuele Giuseppe Esposito
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
include/qemu/job.h | 27 +--
1 file changed, 25 insertio
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
Reviewed-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Stefan Hajnoczi
Reviewed-by
The same job lock is being used also to protect some of blockjob fields.
Categorize them just as done in job.h.
Reviewed-by: Vladimir Sementsov-Ogievskiy
Signed-off-by: Emanuele Giuseppe Esposito
---
include/block/blockjob.h | 32 ++--
1 file changed, 26 insertions(+
These public functions are not used anywhere, thus can be dropped.
Also, since this is the final job API that doesn't use AioContext
lock and replaces it with job_lock, adjust all remaining function
documentation to clearly specify if the job lock is taken or not.
Also document the locking require
[+ Den]
On 9/25/22 16:53, luzhipeng wrote:
From: lu zhipeng
Prevent the NBD socket stuck all the time, So
set timeout.
Signed-off-by: lu zhipeng
---
nbd/client.c | 8
1 file changed, 8 insertions(+)
diff --git a/nbd/client.c b/nbd/client.c
index 30d5383cb1..89dde53a0f 100644
---
Bin Meng writes:
> From: Bin Meng
>
> At present there are two callers of get_tmp_filename() and they are
> inconsistent.
>
> One does:
>
> /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
> char *tmp_filename = g_malloc0(PATH_MAX + 1);
> ...
> ret = get_tmp_
On 9/26/22 12:05, Vladimir Sementsov-Ogievskiy wrote:
[+ Den]
On 9/25/22 16:53, luzhipeng wrote:
From: lu zhipeng
Prevent the NBD socket stuck all the time, So
set timeout.
Signed-off-by: lu zhipeng
---
nbd/client.c | 8
1 file changed, 8 insertions(+)
diff --git a/nbd/client.c
On 9/26/22 12:32, Emanuele Giuseppe Esposito wrote:
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
On 9/18/22 20:12, Emanuele Giuseppe Esposito wrote:
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -911,7 +911,6 @@ static void run_block_job(BlockJob *job, Error
**errp)
AioContext *aio_context = block_job_get_aio_context(job);
int ret = 0;
- aio_context_acquire(aio_context);
jo
On 25/09/2022 13.29, Bin Meng wrote:
From: Bin Meng
This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_file_open_tmp() for a portable implementation.
Signed-off-by: Bin Meng
---
Changes in v3:
- Split to a separate patch
- Ensure g_autofree variable is
On 9/26/22 12:32, Emanuele Giuseppe Esposito wrote:
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
On 9/26/22 12:32, Emanuele Giuseppe Esposito wrote:
These public functions are not used anywhere, thus can be dropped.
Also, since this is the final job API that doesn't use AioContext
lock and replaces it with job_lock, adjust all remaining function
documentation to clearly specify if the job lo
On 9/26/22 14:34, Denis V. Lunev wrote:
On 9/26/22 12:05, Vladimir Sementsov-Ogievskiy wrote:
[+ Den]
On 9/25/22 16:53, luzhipeng wrote:
From: lu zhipeng
Prevent the NBD socket stuck all the time, So
set timeout.
Signed-off-by: lu zhipeng
---
nbd/client.c | 8
1 file changed, 8
Previously, the yielding in v9fs_reopen_fid and put_fid could result
in other parts of the code modifying the fid table. This would
invalidate the hash table iterator, causing misbehaviour.
Now we ensure that we complete the iteration before yielding, so that
the iterator remains valid throughout
On Sun, Sep 25, 2022 at 3:49 PM Bin Meng wrote:
> From: Bin Meng
>
> This case was written to use hardcoded /tmp directory for temporary
> files. Update to use g_file_open_tmp() for a portable implementation.
>
> Signed-off-by: Bin Meng
>
Reviewed-by: Marc-André Lureau
> ---
>
> Changes in
On Sun, Sep 25, 2022 at 3:56 PM Bin Meng wrote:
> From: Bin Meng
>
> This case was written to use hardcoded /tmp directory for temporary
> files. Update to use g_file_open_tmp() for a portable implementation.
>
> Signed-off-by: Bin Meng
>
Reviewed-by: Marc-André Lureau
> ---
>
> Changes in
On Sun, Sep 25, 2022 at 4:09 PM Bin Meng wrote:
> From: Bin Meng
>
> This case was written to use hardcoded /tmp directory for temporary
> files. Update to use g_file_open_tmp() for a portable implementation.
>
> Signed-off-by: Bin Meng
>
Reviewed-by: Marc-André Lureau
> ---
>
> Changes in
On Mon, Sep 26, 2022 at 6:13 PM Markus Armbruster wrote:
>
> Bin Meng writes:
>
> > From: Bin Meng
> >
> > At present there are two callers of get_tmp_filename() and they are
> > inconsistent.
> >
> > One does:
> >
> > /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
> >
On Montag, 26. September 2022 14:42:06 CEST Linus Heckemann wrote:
> Previously, the yielding in v9fs_reopen_fid and put_fid could result
> in other parts of the code modifying the fid table. This would
> invalidate the hash table iterator, causing misbehaviour.
>
> Now we ensure that we complete
On 25/09/2022 13.30, Bin Meng wrote:
From: Bin Meng
These test cases uses "blkdebug:path/to/config:path/to/image" for
testing. On Windows, absolute file paths contain the delimiter ':'
which causes the blkdebug filename parser fail to parse filenames.
Signed-off-by: Bin Meng
Reviewed-by: Marc
Hi Thomas,
On Tue, Sep 27, 2022 at 12:20 AM Thomas Huth wrote:
>
> On 25/09/2022 13.30, Bin Meng wrote:
> > From: Bin Meng
> >
> > These test cases uses "blkdebug:path/to/config:path/to/image" for
> > testing. On Windows, absolute file paths contain the delimiter ':'
> > which causes the blkdebu
在 2022/9/26 20:44, Vladimir Sementsov-Ogievskiy 写道:
On 9/26/22 14:34, Denis V. Lunev wrote:
On 9/26/22 12:05, Vladimir Sementsov-Ogievskiy wrote:
[+ Den]
On 9/25/22 16:53, luzhipeng wrote:
From: lu zhipeng
Prevent the NBD socket stuck all the time, So
set timeout.
Signed-off-by: lu zhip
Bin Meng writes:
> On Mon, Sep 26, 2022 at 6:13 PM Markus Armbruster wrote:
>>
>> Bin Meng writes:
>>
>> > From: Bin Meng
>> >
>> > At present there are two callers of get_tmp_filename() and they are
>> > inconsistent.
>> >
>> > One does:
>> >
>> > /* TODO: extra byte is a hack to ensure M
在 2022/9/26 20:44, Vladimir Sementsov-Ogievskiy 写道:
On 9/26/22 14:34, Denis V. Lunev wrote:
On 9/26/22 12:05, Vladimir Sementsov-Ogievskiy wrote:
[+ Den]
On 9/25/22 16:53, luzhipeng wrote:
From: lu zhipeng
Prevent the NBD socket stuck all the time, So
set timeout.
Signed-off-by: lu zhip
Hi Markus,
On Tue, Sep 27, 2022 at 2:22 PM Markus Armbruster wrote:
>
> Bin Meng writes:
>
> > On Mon, Sep 26, 2022 at 6:13 PM Markus Armbruster wrote:
> >>
> >> Bin Meng writes:
> >>
> >> > From: Bin Meng
> >> >
> >> > At present there are two callers of get_tmp_filename() and they are
> >>
45 matches
Mail list logo
