qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Nbd] [PATCH] Further tidy-up on block status


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-devel] [Nbd] [PATCH] Further tidy-up on block status
Date: Thu, 1 Mar 2018 12:54:15 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

Hi,

Sorry too for long delay.

28.02.2018 16:08, Wouter Verhelst wrote:
Hi,

Sorry, I forgot to reply to this earlier.

On Fri, Feb 16, 2018 at 10:10:59AM -0600, Eric Blake wrote:
On 02/16/2018 07:53 AM, Vladimir Sementsov-Ogievskiy wrote:
Good idea. But it would be tricky thing to maintain backward
compatibility with published versions of virtuozzo product. And finally
our implementation would be more complex because of this simplification.
Hm. Finally, you suggested several changes (including already merged
56c77720 :( ). Suggestions are logical. But if they will be accepted, we
(Virtuozzo) will have to invent tricky hard-to-maintain code, to
distinguish by third factors our already published versions.
Hrm. I wasn't aware you'd already published those versions; if you had
told us, we could've reviewed the spec at that point and either merge it
or update it to incorporate whatever changes you think seem like they
are necessary.

Note that the section "Experimental extensions" contains the following
wording:

     In addition to the normative elements of the specification set out
     herein, various experimental non-normative extensions have been
     proposed. These may not be implemented in any known server or
     client, and are subject to change at any point. A full
     implementation may require changes to the specifications, or cause
     the specifications to be withdrawn altogether.

     [...]

     Implementors of these extensions are strongly suggested to contact
     the mailinglist in order to help fine-tune the specifications before
     committing to a particular implementation.

i.e., what's in an "extension-" branch can be described as "we're
thinking about it and it will probably happen as written, but we're not
entirely sure yet". The idea behind this is that we don't want to limit
ourselves to things that haven't been implemented (but that the fact of
"implementing something" causes it to get written into stone, sortof).

This is different from how most standards bodies work, I'm sure, but it
seemed to work so far, and I wish you had let us know that the work you
were doing was about to be reaching customers. Anyway.

Oh, it's my big mistake, I understand now. Discussion was (and continues to be=) too long, so we couldn't wait, but I of course should have announce it somehow, or use bigger experimental constants for the protocol.


[...]
Also, the Virtuozzo product hasn't been mentioned on the NBD list, and while
you are preparing patches to the public qemu based on the Virtuozzo product,
I'm not sure if there is a public repository for the existing Virtuozzo
product out in the wild.  Is your product using NBD_OPT_LIST_META_CONTEXT,
or _just_ NBD_OPT_SET_META_CONTEXT?
+1. What's published currently?

No, we don't have public repo. But our implementation is similar (in protocol) with sent patches. My first published implementation was written one year ago, in Feb 2017. I'm afraid I've missed later changes of the spec, will check. [hm, it's not very easy to do..]

_LIST_ is implemented in server, but only _SET_ is used in client.


---- checking for changes ----

Comparing 3118b21df (in extension-blockstatus, committed in Jan 2017) with current state of extension-blockstatus, don't see any significant changes for blockstatus, except command number 5->3, which we decided to rollback (thanks for this).



[...]
Or, we can revert the change in commit 56c77720, and keep
NBD_REPLY_TYPE_BLOCK_STATUS at 5 (it leaves a hole in the NBD_REPLY_TYPE
numbering, where 3 and 4 might be filled in by other future extensions, or
permanently skipped).  This works IF there are no OTHER incompatible changes
made to the rest of the block status extension as part of promoting it to
current (where we still haven't finished that debate, given my question on
whether 32-bit lengths and colon-separated namespace:leaf in a single string
is the best representation).

So, I'd like some feedback from Alex or Wouter on which alternatives seem
nicest at this point.
I'm thinking that reverting at least the number change seems like a good
idea. If Vladimir can shed some light on what's been published so far,
we can see what damage has been done and try to limit further damage.

It makes no sense to ignore that the spec has been implemented; the
whole point of writing a spec is so that third parties can implement it
and be compatible. If we ignore that just because there was a
misunderstanding, then I think we're throwing away the kid with the
bathwater.

Since I don't think a gap in numbers is that much of a problem, I'm
happy reverting the renumbering part of 56c77720 and keep it at 5. I'd
also like to know what it is exactly that Virtuozzo implemented, so we
can update the extension-blockstatus (if necessary) and then merge that
to master.

However, for future reference, Vladimir, I would prefer it if you could
give us a heads-up if you're getting close to releasing something to
the public that's still in an experimental branch, so that we can make
updates if necessary and merge it to master.

Thanks,



--
Best regards,
Vladimir




reply via email to

[Prev in Thread] Current Thread [Next in Thread]