Re: [PATCH v3 14/33] nbd: move connection code from block/nbd to nbd/client-connection

[Vladimir Sementsov-Ogievskiy]

28.04.2021 11:14, Vladimir Sementsov-Ogievskiy wrote:
> > > +struct NBDClientConnection {
> > > +    /* Initialization constants */
> > > +    SocketAddress *saddr; /* address to connect to */
> > > +
> > > +    /*
> > > +     * Result of last attempt. Valid in FAIL and SUCCESS states.
> > > +     * If you want to steal error, don't forget to set pointer to NULL.
> > > +     */
> > > +    QIOChannelSocket *sioc;
> > > +    Error *err;
> >
> > These two are also manipulated under the mutex.  Consider also updating
> > the comment: both these pointers are to be "stolen" by the caller, with
> > the former being valid when the connection succeeds and the latter
> > otherwise.
>
> Hmm. I should move mutex and "All further" comment above these two fields.
>
> Ok, I'll think on updating the comment (probably as an additional patch, to 
> keep this as a simple movement). I don't like to document that they are stolen 
> by caller(). For me it sounds like caller is user of the interface. And caller 
> of nbd_co_establish_connection() doesn't stole anything: the structure is 
> private now..

Finally, I decided to improve the comment as part of "[PATCH v3 08/33] block/nbd: drop 
thr->state" commit, as "FAIL and SUCCESS states" string becomes outdated when we 
drop these states.

--
Best regards,
Vladimir