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

[Eric Blake]

On 6/10/19 8:29 AM, Vladimir Sementsov-Ogievskiy wrote:

>> 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.
>>
> 
> And if we are here, what is exact purpose of splitting  nbd-client.c from 
> nbd.c?

I see no strong reasons to keep the separation. If a single large file
is easier to maintain than an arbitrary split between two files, I'm
open to the idea of a patch that undoes the split.


> or need of defining driver callbacks in header file, instead of keeping them 
> together
> with driver struct definition and static (like other block drivers)...
> 
> 
> And names of these two files always confused me.. nbd.c is nbd client block 
> driver, and
> driver is defined here.. But we have nbd-client.c which defines nbd client 
> driver too.
> And we also have nbd/client.c (OK, this split I understand of course, but it 
> increases
> confusion anyway).

I'm also toying with the idea of writing block/nbd.c to utilize the
relatively-new libnbd library, to see if there are any differences in
behavior by offloading NBD access to a 3rd-party library.  But that's
further down the road.

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

signature.asc
Description: OpenPGP digital signature