[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v6 6/7] block/nbd-client: nbd reconnect
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH v6 6/7] block/nbd-client: nbd reconnect |
Date: |
Mon, 10 Jun 2019 09:33:34 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 |
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
Re: [Qemu-block] [PATCH v6 6/7] block/nbd-client: nbd reconnect, Vladimir Sementsov-Ogievskiy, 2019/06/07
Re: [Qemu-block] [PATCH v6 6/7] block/nbd-client: nbd reconnect, Vladimir Sementsov-Ogievskiy, 2019/06/10