Re: [Qemu-devel] [PATCH v6 6/7] block/nbd-client: nbd reconnect

[Vladimir Sementsov-Ogievskiy]

07.06.2019 6:17, Eric Blake wrote:
>> +typedef struct NBDConnection {
>> +    BlockDriverState *bs;
>> +    SocketAddress *saddr;
>> +    const char *export;
>> +    QCryptoTLSCreds *tlscreds;
>> +    const char *hostname;
>> +    const char *x_dirty_bitmap;
>> +} NBDConnection;
> Can we put this type in a header, and use it instead of passing lots of
> individual parameters to nbd_client_connect()?  Probably as a separate
> pre-requisite cleanup patch.
> 


Hmm, and then, include it into BDRVNBDState? I don't remember why didn't do
it, and now I don't see any reason for it. We store this information anyway
for the whole life of the driver..

So, if I'm going to refactor it, I have a question: is there a reason for
layered BDRVNBDState:

typedef struct BDRVNBDState {
     NBDClientSession client;

     /* For nbd_refresh_filename() */
     SocketAddress *saddr;
     char *export, *tlscredsid;
} BDRVNBDState;

and use nbd_get_client_session everywhere instead of simple converting void 
*opaque
to State pointer like in qcow2 for example?

The only reason I can imagine is not to share the whole BDRVNBDState, and keep
things we are using only in nbd.c to be available only in nbd.c.. But I've put
saddr and export to NBDConnection to be used in nbd-client.c anyway. So, I think
it's a good moment to flatten and share BDRVNBDState and drop 
nbd_get_client_session.

-- 
Best regards,
Vladimir