Re: [RFC PATCH v2 24/25] job.h: split function pointers in JobDriver

[Emanuele Giuseppe Esposito]

On 07/10/2021 16:54, Stefan Hajnoczi wrote:
> On Tue, Oct 05, 2021 at 10:32:14AM -0400, Emanuele Giuseppe Esposito wrote:
> > The job API will be handled separately in another serie.
> >
> > Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> > ---
> >   include/qemu/job.h | 31 +++++++++++++++++++++++++++++++
> >   1 file changed, 31 insertions(+)
> >
> > diff --git a/include/qemu/job.h b/include/qemu/job.h
> > index 41162ed494..c236c43026 100644
> > --- a/include/qemu/job.h
> > +++ b/include/qemu/job.h
> > @@ -169,12 +169,21 @@ typedef struct Job {
> >    * Callbacks and other information about a Job driver.
> >    */
> >   struct JobDriver {
> > +
> > +    /* Fields initialized in struct definition and never changed. */
> > +
> >       /** Derived Job struct size */
> >       size_t instance_size;
> >   
> >       /** Enum describing the operation */
> >       JobType job_type;
> >   
> > +    /*
> > +     * I/O API functions. These functions are thread-safe, and therefore
> > +     * can run in any thread as long as they have called
> > +     * aio_context_acquire/release().
> > +     */
>
> This comment described the block layer well but job.h is more generic. I
> don't think these functions are necessarily thread-safe (they shouldn't
> be invoked from multiple threads at the same time). It's just that they
> are run without regard to the BQL and may run in an arbitrary thread.

Ok, in this instance I will change it to:

/*
 * Functions run without regard to the BQL and may run in any
 * arbitrary thread.
 */

Thank you,
Emanuele

> Other than that:
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>