On Thu, Mar 03, 2022 at 02:14:34PM -0600, Eric Blake wrote: > On Thu, Mar 03, 2022 at 04:03:20PM +0000, Daniel P. Berrangé wrote: > > In > > > > commit a71d597b989fd701b923f09b3c20ac4fcaa55e81 > > Author: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > > Date: Thu Jun 10 13:08:00 2021 +0300 > > > > block/nbd: reuse nbd_co_do_establish_connection() in nbd_open() > > > > the use of the 'hostname' field from the BDRVNBDState struct was > > lost, and 'nbd_connect' just hardcoded it to match the IP socket > > address. This was a harmless bug at the time since we block use > > with anything other than IP sockets. > > > > Shortly though, We want to allow the caller to override the hostname > > s/We/we/ > > > used in the TLS certificate checks. This is to allow for TLS > > when doing port forwarding or tunneling. Thus we need to reinstate > > the passing along of the 'hostname'. > > > > Signed-off-by: Daniel P. Berrangé <berra...@redhat.com> > > --- > > block/nbd.c | 7 ++++--- > > include/block/nbd.h | 3 ++- > > nbd/client-connection.c | 12 +++++++++--- > > 3 files changed, 15 insertions(+), 7 deletions(-) > > Nice - this a great step towards fixing a longstanding annoyance of > mine that libnbd and nbdkit support TLS over Unix sockets, but qemu > didn't.
> > diff --git a/include/block/nbd.h b/include/block/nbd.h > > index 78d101b774..a98eb665da 100644 > > --- a/include/block/nbd.h > > +++ b/include/block/nbd.h > > @@ -415,7 +415,8 @@ NBDClientConnection *nbd_client_connection_new(const > > SocketAddress *saddr, > > bool do_negotiation, > > const char *export_name, > > const char *x_dirty_bitmap, > > - QCryptoTLSCreds *tlscreds); > > + QCryptoTLSCreds *tlscreds, > > + const char *tlshostname); > > We already have a lot of parameters; does it make sense to bundle > tlshostname into the QCryptoTLSCreds struct at all? But that would > change the QAPI (or maybe you do it later in the series), it is not a > show-stopper to this patch. The credentials object is something that can be used for multiple connections. The TLS hostname override meanwhile is specific to a single connection. Thus it would not be appropriate to store the TLS hostname in the credentials struct. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|