[Libguestfs] [PATCH nbdkit] server: Deprecate the -e/--exportname parameter.

[Eric Blake]

On 7/22/20 4:02 AM, Richard W.M. Jones wrote:

>> The more I think about it, the more I disagree with disabling this.
>> Or, put another way, I think that the _only_ time -e makes sense is
>> _when_ you are using --run.  Consider:
>>
>> nbdkit -U - -e foo info --run 'nbdsh -u $uri -c "print(h.pread(3, 0))"'
>> nbdkit -U - -e bar info --run 'nbdsh -u $uri -c "print(h.pread(3, 0))"'
>>
>> Of course, you can accomplish the same with:
>>
>> nbdkit -U - info --run 'nbdsh -u nbd+unix:///foo\?socket=$unixsocket \
>>    -c "print(h.pread(3, 0))"'
>>
>> but that's a lot more painful to write.
>>
>> We _don't_ need to advertise 'foo' or 'bar' over NBD_OPT_INFO (at
>> least, not for plugins that don't implement the forthcoming
>> .export_list callback), but _do_ need a way for the captive
>> application (nbdsh in this case) to know _which_ export the captive
>> should connect to.
>>
>> And, if we reinstate _just_ that usage of -e,...
>>
>>>> (4) I have temporarily disabled tests/test-long-name.sh.  This is
>>>>       still a valid test, but we will have to rewrite it to use
>>>>       (probably) sh or eval plugins once we have an implementation of
>>>>       list_exports.
>>>> ---
>>
>> ...we may not have to disable this test after all.
> 
> I feel for the end user this is all pretty confusing and requires them
> to have some insight into what the plugin is expecting and even
> understanding of the protocol.
> 
> Perhaps we should have a rule something like this:
> 
>   - plugins should serve default content on the "" export
> 
>   - unless they implement .list_exports, in which case the
>     first export returned is the default export

That could be expensive, reading the entire list and throwing away all 
but the first. I'm now leaning towards two callbacks:

.list_exports: compute all exports to advertise
.default_export: compute the name to be returned for NBD_OPT_INFO("") 
and to use by default as $exportname in --run

Then, any plugin that wants to advertise a different default export name 
supplies .default_export, and if .default_export is missing, we supply 
"" on their behalf.  We could also translate a client request for "" 
into .default_export before calling .open (v3) or populating 
nbdkit_export_name (v2) (I could see that mattering for the file plugin 
with a mode that serves all files in a directory, since stat("") fails 
but providing a default filename as the largest file in the directory 
could be useful).

> 
> Then we could implement --run transparently by having it set
> $exportname/$uri to the first entry in .list_exports or "" if
> .list_exports is not implemented.
> 
> Here's another idea: Is it possible that if the --run script itself
> overrides exportname it could be used by $uri.  Not sure if there's
> some kind of delayed shell variable expansion that would make this
> possible:
> 
>    nbdkit ... --run 'exportname=foo; nbdsh -u $uri ...'

Not as written, but we _can_ do the following.  Right now, --run creates 
a shell fragment, which prepends this code in front of the user's:

uri=...
nbd=...
...
exportname=...

we could instead prepend:

set_export() {
   uri=...
   nbd=...
   ...
   exportname=$1
}
set_export ...

where that last line uses .default_export (defaulting to "" as usual). 
The user can now run:

nbdkit ... --run 'set_export foo; nbdsh -u "$uri" ... '

Oh, and I just realized, because $uri can contain ?, we really should 
scrub all uses in the tree to always write it as "$uri" rather than 
unquoted, just to set a good example.  (It's unlikely that anyone would 
ever have a subdirectory named 'nbd+unix:' in their current working 
directory to the point that the ? would trigger unintended globbing, but 
better safe than sorry...)

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