Libguestfs - Aug 2018 - Re: [PATCH nbdkit] protocol: Implement NBD_OPT_GO.

On Sat, Aug 4, 2018 at 10:58 PM Nir Soffer <nsoffer@redhat.com> wrote:
> On Sat, Aug 4, 2018 at 4:04 PM Richard W.M. Jones <rjones@redhat.com>
> wrote:
>
>> This is only lightly tested (against just qemu NBD client), and the
>> code might be structured a little better as the
>> _negotiate_handshake_newstyle_options function has now grown to be
>> huge.  Anyway works for me.
>>
>
> Works for my python nbd client:
>
> $ rm -f /tmp/nbd.sock && src/nbdkit -f
> plugins/file/.libs/nbdkit-file-plugin.so file=test.raw -e export -U
> /tmp/nbd.sock
>
> $ python -c "import logging; logging.basicConfig(level=logging.DEBUG);
> from ovirt_imageio_common import nbd; c =
nbd.Client('/tmp/nbd.sock',
> 'export'); c.write(1024**2, 'it works'); print
c.read(1024**2, 8)"
> INFO:nbd:Connecting to '/tmp/nbd.sock' 'export'
> DEBUG:nbd:Received server flags: 3
> DEBUG:nbd:Sending client flags: 1:
> DEBUG:nbd:Sending option:
'IHAVEOPT\x00\x00\x00\x07\x00\x00\x00\x0c' data:
> bytearray(b'\x00\x00\x00\x06export\x00\x00')
> DEBUG:nbd:Received reply [magic=3e889045565a9 option=7 type=3 len=12]
> DEBUG:nbd:Received export info [size=1073741824 flags=109]
> DEBUG:nbd:Received reply [magic=3e889045565a9 option=7 type=1 len=0]
> INFO:nbd:Ready for transmission
> it works
>
But we have a bug - server configure to allow access to "export",
but allow access to the default empty "" export.

$ python -c "import logging; logging.basicConfig(level=logging.DEBUG); from
ovirt_imageio_common import nbd; c = nbd.Client('/tmp/nbd.sock',
'')"
INFO:nbd:Connecting to '/tmp/nbd.sock' ''
DEBUG:nbd:Received server flags: 3
DEBUG:nbd:Sending client flags: 1:
DEBUG:nbd:Sending option: 'IHAVEOPT\x00\x00\x00\x07\x00\x00\x00\x06'
data:
bytearray(b'\x00\x00\x00\x00\x00\x00')
DEBUG:nbd:Received reply [magic=3e889045565a9 option=7 type=3 len=12]
DEBUG:nbd:Received export info [size=6442450944 flags=109]
DEBUG:nbd:Received reply [magic=3e889045565a9 option=7 type=1 len=0]
INFO:nbd:Ready for transmission

Same with qemu-nbd:

$ qemu-nbd -k /tmp/nbd.sock -t -f raw test.raw --export-name export
--cache=none --aio=native --discard=unmap --detect-zeroes=unmap

$ python -c "import logging; logging.basicConfig(level=logging.DEBUG); from
ovirt_imageio_common import nbd; c = nbd.Client('/tmp/nbd.sock',
'')"
INFO:nbd:Connecting to '/tmp/nbd.sock' ''
DEBUG:nbd:Received server flags: 3
DEBUG:nbd:Sending client flags: 1:
DEBUG:nbd:Sending option: 'IHAVEOPT\x00\x00\x00\x07\x00\x00\x00\x06'
data:
bytearray(b'\x00\x00\x00\x00\x00\x00')
DEBUG:nbd:Received reply [magic=3e889045565a9 option=7 type=80000006 len=21]
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "ovirt_imageio_common/nbd.py", line 126, in __init__
    self._newstyle_handshake(export_name)
  File "ovirt_imageio_common/nbd.py", line 181, in _newstyle_handshake
    self._receive_go_reply()
  File "ovirt_imageio_common/nbd.py", line 206, in _receive_go_reply
    .format(ERROR_REPLY[reply], message))
ovirt_imageio_common.nbd.Error: The requested export is not available
[message=export '' not present])

On Sun, Aug 05, 2018 at 12:11:04AM +0300, Nir Soffer
wrote:
> But we have a bug - server configure to allow access to "export",
> but allow access to the default empty "" export.
nbdkit ignores the export name you pass in and always has done:

https://github.com/libguestfs/nbdkit/blob/a2c4e88af503310892ebce6da3ac478beb47888a/src/connections.c#L626

The -e option on the command line is only for advertizing an export
name to clients.  It is otherwise unused & unenforced.

One day we'll do something useful with it but we're not sure what.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

On 08/06/2018 02:54 AM, Richard W.M. Jones wrote:
> On Sun, Aug 05, 2018 at 12:11:04AM +0300, Nir Soffer wrote:
>> But we have a bug - server configure to allow access to
"export",
>> but allow access to the default empty "" export.
> 
> nbdkit ignores the export name you pass in and always has done:
> 
>
https://github.com/libguestfs/nbdkit/blob/a2c4e88af503310892ebce6da3ac478beb47888a/src/connections.c#L626
> 
> The -e option on the command line is only for advertizing an export
> name to clients.  It is otherwise unused & unenforced.
Agreed - it is not necessarily a bug for the server to permit the client 
to connect to different name(s) than what the server was told to advertise.
> 
> One day we'll do something useful with it but we're not sure what.
We might eventually reject clients that don't request the same export 
name as what was advertised, but that doesn't make the current behavior 
buggy (it only means that current clients shouldn't rely on a mismatched 
name always working).

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