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

[Wouter Verhelst]

On Wed, Dec 14, 2016 at 08:55:56PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 14.12.2016 20:36, Alex Bligh wrote:
> >> On 14 Dec 2016, at 17:05, Vladimir Sementsov-Ogievskiy <address@hidden> 
> >> wrote:
> >>> However, this raises another question. Wouter deliberately made the
> >>> query format freeform so that you could e.g. set a context like:
> >>>
> >>>     backup:modtime>201612081034
> >>>
> >>> which might (in theory) return a list of blocks which are newer than
> >>> the given timestamp. It would clearly be impossible to return all such
> >>> contexts. I wonder if we should carve out an exception here.
> >>>
> >>>
> >> Interesting. Even query 'backup:modtime>*' would be a problem, not only 
> >> empty query list.
> >>
> >> Actually, we do not need to 'list contexts', as it is about management, 
> >> not about data transfer. We only need a way to check, that particular 
> >> query selects all needed contexts and no others. Just to be sure that we 
> >> are know, what we will select by NBD_OPT_SET_META_CONTEXT with _same_ 
> >> query.
> >>
> >> So, I suggest just to say, that _LIST_ can return error if too much 
> >> contexts are selected. And same should be done for _SET_. And it should be 
> >> documented that this result of query (list or error) should be equal for 
> >> these two commands.
> > (two CCs that always bounce removed)
> >
> > Hmmm... Well in the '*' case, it's up to the namespace as to how it parses 
> > things, so '*' could be prohibited entirely or mean 'return the first 20 
> > matching' or similar. So that's less of a problem. And 'set all' doesn't 
> > exist (unlike 'list all').
> >
> > I think in the LIST case we should allow the server to omit contexts under 
> > certain circumstances, and allow SET to return ETOOBIG.
> >
> 
> We can't prohibit '*' as query string as implementation defined. And we 
> shouldn't (I think) define its meaning. Opposite, we can define, that 
> query must not select more than 20 contexts, but I'm not sure that this 
> is good.

No, I don't think so, either.

I can see how "list all contexts" might be difficult to implement, in
the example of the dynamic contexts that I pointed out earlier. We could
muddle the spec further to fix that (hypothetical) problem, but I'm not
sure that's worth it.

Instead, I would suggest that the spec leave it up to the namespace spec
to define what gets listed when. The namespace should still give some
indication that a particular *type* of context is available, though;
e.g., _LIST_ could return 'backup:snapshot-diff{*}' as well as a list of
'backup:snapshot{snapshot-X}' contexts, where the latter are all
available snapshots, and the 'snapshot-diff' bit implies that the whole
cartesian product of all possible diffs between snapshots is possible in
a format of 'backup:snapshot-diff{snapshot-X:snapshot-Y}', or some such
(you get the idea).

The spec currently says that a server SHOULD allow choosing exports by
simply naming them, but doesn't make that a MUST; and it also says that
the query format is defined by the namespace. If we simply clarify that
the output of _LIST_ doesn't necessarily need to be a full list of all
that's available (that it may be symbolic in the case of namespaces that
allow things to be dynamically defined), we should be good.

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12