[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Nbd] [PULL 23/28] nbd: always query export list in fix
From: |
Alex Bligh |
Subject: |
Re: [Qemu-devel] [Nbd] [PULL 23/28] nbd: always query export list in fixed new style protocol |
Date: |
Tue, 17 May 2016 16:22:06 +0100 |
Eric, Richard,
On 17 May 2016, at 16:09, Eric Blake <address@hidden> wrote:
> [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).
nbdkit is non-compliant in that case. Support of NBD_OPT_LIST is
compulsory, even if you support it by returning a nameless export
(or default). Moreover support of export names is compulsory
(even if you have a single fixed one called "" or "default").
This is assuming nbdkit supports 'fixed newstyle'; if nbdkit merely
supports 'newstyle' negotiation, then we know qemu won't connect
to it as modern qemu only supports servers that support 'fixed newstyle'
I believe.
> Perhaps nbdkit should implement NBD_OPT_LIST returning just "" (the
> default name) as its only list entry?
Or "default".
> 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.
Sure, but NBD_OPT_INFO/GO is still an experimental protocol extension.
>> 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.
Whilst that is fair enough, NBD_OPT_LIST is a mandatory part of the
specification. If a server doesn't implement mandatory parts of
the specification, then bad things will happen.
My interpretation of NBD_OPT_LIST failing would be 'this server
doesn't have anything it can export'.
>> 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.
I can't actually see the issue here. It explains what needs to be
implemented by the server, and that includes NBD_OPT_LIST. Very
happy to add some clarity, but I'm not sure where it's needed.
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
- Re: [Qemu-devel] [PULL 23/28] nbd: always query export list in fixed new style protocol, Richard W.M. Jones, 2016/05/17
- Re: [Qemu-devel] [PULL 23/28] nbd: always query export list in fixed new style protocol, Eric Blake, 2016/05/17
- Re: [Qemu-devel] [Nbd] [PULL 23/28] nbd: always query export list in fixed new style protocol,
Alex Bligh <=
- Re: [Qemu-devel] [Nbd] [PULL 23/28] nbd: always query export list in fixed new style protocol, Eric Blake, 2016/05/17
- Re: [Qemu-devel] [Nbd] [PULL 23/28] nbd: always query export list in fixed new style protocol, Richard W.M. Jones, 2016/05/17
- Re: [Qemu-devel] [Nbd] [PULL 23/28] nbd: always query export list in fixed new style protocol, Eric Blake, 2016/05/17
- Re: [Qemu-devel] [Nbd] [PULL 23/28] nbd: always query export list in fixed new style protocol, Richard W.M. Jones, 2016/05/17
- Re: [Qemu-devel] [Nbd] [PULL 23/28] nbd: always query export list in fixed new style protocol, Eric Blake, 2016/05/17
- Re: [Qemu-devel] [Nbd] [PULL 23/28] nbd: always query export list in fixed new style protocol, Alex Bligh, 2016/05/17
- Re: [Qemu-devel] [Nbd] [PULL 23/28] nbd: always query export list in fixed new style protocol, Richard W.M. Jones, 2016/05/17
- Re: [Qemu-devel] [Nbd] [PULL 23/28] nbd: always query export list in fixed new style protocol, Eric Blake, 2016/05/17
- Re: [Qemu-devel] [Nbd] [PULL 23/28] nbd: always query export list in fixed new style protocol, Richard W.M. Jones, 2016/05/17
- Re: [Qemu-devel] [Nbd] [PULL 23/28] nbd: always query export list in fixed new style protocol, Eric Blake, 2016/05/17