[Qemu-devel] Question nbd_refresh_filename()

[Qemu-devel] Question nbd_refresh_filename()

[Markus Armbruster]

BlockdevOptionsNbd has a member SocketAddress, and nbd_config() doesn't
restrict variants.  Thus, all four SOCKET_ADDRESS_KIND_ can occur.

Now have a look at nbd_refresh_filename().  s->saddr->type is If
SOCKET_ADDRESS_KIND_VSOCK or SOCKET_ADDRESS_KIND_FD, then @host, @port
and @path all remain null, and bs->exact_filename[] is not touched.

Does this work as intended?

Re: [Qemu-devel] Question nbd_refresh_filename()

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 871 bytes --]

On Thu, Mar 23, 2017 at 05:48:48PM +0100, Markus Armbruster wrote:
> BlockdevOptionsNbd has a member SocketAddress, and nbd_config() doesn't
> restrict variants.  Thus, all four SOCKET_ADDRESS_KIND_ can occur.
> 
> Now have a look at nbd_refresh_filename().  s->saddr->type is If
> SOCKET_ADDRESS_KIND_VSOCK or SOCKET_ADDRESS_KIND_FD, then @host, @port
> and @path all remain null, and bs->exact_filename[] is not touched.
> 
> Does this work as intended?

NDB over AF_VSOCK has not been tested.  I would expect it to fail
earlier than nbd_refresh_filename().  It's certainly possible to come up
with the host=,port= for AF_VSOCK.

Not sure about file descriptor passing.  Either we could getsockaddr()
and print out the host=,port= or path= (depending on the address
family), or we could use some syntax for referring to the file
descriptor.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] Question nbd_refresh_filename()

[Kevin Wolf]

Am 24.03.2017 um 16:47 hat Stefan Hajnoczi geschrieben:
> On Thu, Mar 23, 2017 at 05:48:48PM +0100, Markus Armbruster wrote:
> > BlockdevOptionsNbd has a member SocketAddress, and nbd_config() doesn't
> > restrict variants.  Thus, all four SOCKET_ADDRESS_KIND_ can occur.
> > 
> > Now have a look at nbd_refresh_filename().  s->saddr->type is If
> > SOCKET_ADDRESS_KIND_VSOCK or SOCKET_ADDRESS_KIND_FD, then @host, @port
> > and @path all remain null, and bs->exact_filename[] is not touched.
> > 
> > Does this work as intended?
> 
> NDB over AF_VSOCK has not been tested.  I would expect it to fail
> earlier than nbd_refresh_filename().

Why would that be? Do the socket creation helper functions not support
vsock yet?

> It's certainly possible to come up with the host=,port= for AF_VSOCK.
> 
> Not sure about file descriptor passing.  Either we could getsockaddr()
> and print out the host=,port= or path= (depending on the address
> family), or we could use some syntax for referring to the file
> descriptor.

The point of .bdrv_refresh_filename is to provide a "nicer" filename for
humans, but it still needs to be a valid input. We do accept URLs as a
convenience feature for NBD connections over TCP and Unix domain
sockets, so we create a filename for those. For everything else, we
require individual options rather than a URL and should therefore not
create a filename. The block layer creates a json: description then.

Kevin

Re: [Qemu-devel] Question nbd_refresh_filename()

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1400 bytes --]

On Fri, Mar 24, 2017 at 06:48:44PM +0100, Kevin Wolf wrote:
> Am 24.03.2017 um 16:47 hat Stefan Hajnoczi geschrieben:
> > On Thu, Mar 23, 2017 at 05:48:48PM +0100, Markus Armbruster wrote:
> > > BlockdevOptionsNbd has a member SocketAddress, and nbd_config() doesn't
> > > restrict variants.  Thus, all four SOCKET_ADDRESS_KIND_ can occur.
> > > 
> > > Now have a look at nbd_refresh_filename().  s->saddr->type is If
> > > SOCKET_ADDRESS_KIND_VSOCK or SOCKET_ADDRESS_KIND_FD, then @host, @port
> > > and @path all remain null, and bs->exact_filename[] is not touched.
> > > 
> > > Does this work as intended?
> > 
> > NDB over AF_VSOCK has not been tested.  I would expect it to fail
> > earlier than nbd_refresh_filename().
> 
> Why would that be? Do the socket creation helper functions not support
> vsock yet?

You are right.  The following creates an AF_VSOCK connection:

  -drive if=virtio,file.driver=nbd,file.server.type=vsock,file.server.data.cid=3,file.server.data.port=1234

There is no pretty filename or URL representation that block/nbd.c
parses today.  The user must specify the full file.server
VsockSocketAddress options.

I think nbd_refresh_filename() is fine from a vsock point of view.  If
we'd like to encourage NBD-over-vsock in the future we could add code to
for nbd+vsock:// but in the meantime we don't need to fill in
bs->exact_filename[].

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]