qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 11/32] block/nbd: drop thr->state


From: Eric Blake
Subject: Re: [PATCH v4 11/32] block/nbd: drop thr->state
Date: Fri, 11 Jun 2021 09:25:44 -0500
User-agent: NeoMutt/20210205

On Thu, Jun 10, 2021 at 01:07:41PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We don't need all these states. The code refactored to use two boolean
> variables looks simpler.
> 
> While moving the comment in nbd_co_establish_connection() rework it to
> give better information. Also, we are going to move the connection code
> to separate file and mentioning drained section would be confusing.
> 
> Improve also the comment in NBDConnectThread, while dropping removed
> state names from it.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/nbd.c | 139 +++++++++++++++++-----------------------------------
>  1 file changed, 44 insertions(+), 95 deletions(-)
> 

>  typedef struct NBDConnectThread {
>      /* Initialization constants */
>      SocketAddress *saddr; /* address to connect to */
>  
> +    QemuMutex mutex;
> +
>      /*
> -     * Result of last attempt. Valid in FAIL and SUCCESS states.
> -     * If you want to steal error, don't forget to set pointer to NULL.
> +     * @sioc and @err present a result of connection attempt.
> +     * While running is true, they are used only by thread, mutex locking is 
> not
> +     * needed. When thread is finished nbd_co_establish_connection steal 
> these
> +     * pointers under mutex.

@sioc and @err represent a connection attempt.  While running is true,
they are only used by the connection thread, and mutex locking is not
needed.  Once the thread finishes, nbd_co_establish_connection then
steals these pointers under mutex.

>       */
>      QIOChannelSocket *sioc;
>      Error *err;
>  
> -    QemuMutex mutex;
> -    /* All further fields are protected by mutex */
> -    NBDConnectThreadState state; /* current state of the thread */
> +    /* All further fields are accessed only under mutex */
> +    bool running; /* thread is running now */
> +    bool detached; /* thread is detached and should cleanup the state */
>

Okay, I'm understanding this better than I did in v3.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org




reply via email to

[Prev in Thread] Current Thread [Next in Thread]