Re: [Qemu-devel] [PULL 23/28] nbd: always query export list in fixed new style protocol

[Eric Blake]

[adding nbd-list]

On 05/17/2016 03:53 AM, Richard W.M. Jones wrote:
> On Tue, Feb 16, 2016 at 05:34:41PM +0100, Paolo Bonzini wrote:
>> From: "Daniel P. Berrange" <address@hidden>
>>
>> With the new style protocol, the NBD client will currenetly
>> send NBD_OPT_EXPORT_NAME as the first (and indeed only)
>> option it wants. The problem is that the NBD protocol spec
>> does not allow for returning an error message with the
>> NBD_OPT_EXPORT_NAME option. So if the server mandates use
>> of TLS, the client will simply see an immediate connection
>> close after issuing NBD_OPT_EXPORT_NAME which is not user
>> friendly.
>>

> I just bisected qemu 2.6 to find out where it breaks interop with
> nbdkit, and it is this commit.
> 
> nbdkit implements the newstyle protocol, but doesn't implement export
> names (yet, maybe in future).  It processes the NBD_OPT_EXPORT_NAME
> option, but ignores the export name and carries on regardless.
> 
> nbdkit's implemention of NBD_OPT_LIST returns an error, because there
> is no such thing as a list of export names supported (in effect nbdkit
> allows any export name).

Perhaps nbdkit should implement NBD_OPT_LIST returning just "" (the
default name) as its only list entry?

And at some point, nbdkit should probably implement NBD_OPT_GO (which is
a nicer replacement to NBD_OPT_EXPORT_NAME; I've got patches pending for
qemu to implement that).

In fact, if NBD_OPT_GO is supported, then my patches to qemu won't even
try NBD_OPT_LIST.

> 
> Therefore I don't believe the assumption made here -- that you can
> list export names and choose them on the client side -- is a valid
> one.

Qemu is not choosing an export name, so much as proving whether any
OTHER errors can be detected (such as export name not present, or TLS
required).  It _should_ be gracefully ignoring NBD_OPT_LIST errors if
such errors don't definitively prove that the export will not succeed,
but I guess this is not happening.  I'll see if I can tweak the qemu
logic to be more forgiving of nbdkit's current behavior.

> 
> Naturally the protocol document
> (https://github.com/yoe/nbd/blob/master/doc/proto.md) isn't clear on
> this case.

You're right that we may also want to tweak the NBD protocol to make
this interoperability point obvious.

> 
> To test qemu against nbdkit you can do this in the nbdkit sources:
> 
>   make
>   make check TESTS=test-newstyle \
>     LIBGUESTFS_HV=/path/to/qemu/x86_64-softmmu/qemu-system-x86_64 \
>     LIBGUESTFS_DEBUG=1 LIBGUESTFS_TRACE=1
> 
> Rich.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature