Re: [libvirt] [PATCH v3 28/31] gendispatch: Introduce @sparseflag for our calls
On 05/17/2017 05:32 PM, John Ferlan wrote:
On 05/16/2017 10:04 AM, Michal Privoznik wrote:
> Now, not all APIs are going to support sparse streams. To some it
> makes no sense at all, e.g. virDomainOpenConsole() or
> virDomainOpenChannel(). To others, we will need a special flag to
> indicate that client wants to enable sparse streams. Instead of
> having to write RPC dispatchers by hand we can just annotate in
> our .x files that a certain flag to certain RPC call enables this
> feature. For instance:
>
> /**
> * @generate: both
> * @readstream: 1
> * @sparseflag: VIR_SPARSE_STREAM
> * @acl: storage_vol:data_read
> */
> REMOTE_PROC_DOMAIN_SOME_API = XXX,
>
> Therefore, whenever client calls virDomainSomeAPI(..,
> VIR_SPARSE_STREAM); daemon will mark that down and send stream
> skips when possible.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/rpc/gendispatch.pl | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
Still a bit of black magic to me ;-), but I see that it works...
Yes. So the problem this patch fixes is the following: nearly all APIs
we have result in RPC call. Except a few ones. Unfortunately,
virStreamNew() is one of them. Therefore, if one side creates a stream
object, the other side has no knowledge of that. We fix this problem by
creating server side stream object in the dispatch function, e.g.
remoteDispatchStorageVolDownload. If something fails, we unref the
object and we're back to the starting position.
However, when creating the stream object we have to know if it is going
to be sparse or not (trying to set it afterwards would complicate the
code needlessly). And that's exactly what this patch does. It allows us
to say "this flag for this RPC call enables sparse stream". Therefore,
the generated code of dispatcher functions can then check if the flag is
set, and if so create sparse stream.
This all could have been avoided if virStreamNew() was RPC call. Then we
could:
virStreamNew(conn, SPARSE);
virStorageVolDownload();
without introducing special flags for each API that wants to utilize
sparse stream. Because sparseness is really an attribute of the stream,
not StorageVol or whatever API. Well, maybe next time.
Michal