[libvirt] [PATCH v2 22/38] daemon: Introduce virNetServerProgramSendStreamSkip

[John Ferlan]

On 05/11/2017 02:36 AM, Michal Privoznik wrote:
> On 05/10/2017 01:53 PM, Michal Privoznik wrote:
>> On 05/05/2017 05:26 PM, John Ferlan wrote:
>>>
>>>
>>> On 04/20/2017 06:01 AM, Michal Privoznik wrote:
>>>> This is just a helper function that takes in a length value,
>>>> encodes it into XDR and sends to client.
>>>
>>> would be adjusted w/ @flags arg....
>>>
>>>>
>>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>>> ---
>>>>  src/libvirt_remote.syms       |  1 +
>>>>  src/rpc/virnetserverprogram.c | 33 +++++++++++++++++++++++++++++++++
>>>>  src/rpc/virnetserverprogram.h |  7 +++++++
>>>>  3 files changed, 41 insertions(+)
>>>>
>>>> diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
>>>> index ca1f3ac..29dceab 100644
>>>> --- a/src/libvirt_remote.syms
>>>> +++ b/src/libvirt_remote.syms
>>>> @@ -178,6 +178,7 @@ virNetServerProgramNew;
>>>>  virNetServerProgramSendReplyError;
>>>>  virNetServerProgramSendStreamData;
>>>>  virNetServerProgramSendStreamError;
>>>> +virNetServerProgramSendStreamSkip;
>>>>  virNetServerProgramUnknownError;
>>>>  
>>>>  
>>>> diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c
>>>> index d1597f4..6d84056 100644
>>>> --- a/src/rpc/virnetserverprogram.c
>>>> +++ b/src/rpc/virnetserverprogram.c
>>>> @@ -548,6 +548,39 @@ int virNetServerProgramSendStreamData(virNetServerProgramPtr prog,
>>>>  }
>>>>  
>>>>  
>>>> +int virNetServerProgramSendStreamSkip(virNetServerProgramPtr prog,
>>>> +                                      virNetServerClientPtr client,
>>>> +                                      virNetMessagePtr msg,
>>>> +                                      int procedure,
>>>> +                                      unsigned int serial,
>>>> +                                      unsigned long long length)
>>>
>>> Doesn't follow the newer style
>>>
>>> int
>>> vir...(args...)
>>>
>>>
>>> Of course now it starts dawning on me... if the functions change to
>>> SetSkip and GetSkip - then I'd assume that has impact for the RPC
>>> nomenclature too. Of course seeing "SetSkip" in a name for RPC would
>>> make things even more clear (unless of course you're the one that's been
>>> working on this code for a long time and already have the names burned
>>> into your memory).
>>
>> Exactly. For me Skip and HoleSize have clear meaning. So your suggestion
>> is to have: SetSkip and GetSkip? Well, I don't like it that much but if
>> that's the only thing that should prevent this from merging ...
>>
>> Also, as I've said earlier, I'm gonna send v2 without any name change
>> for the time being. As you've correctly noticed, a lot of functions, RPC
>> calls, and other stuff have their name derived from current naming.
>> Therefore changing that would be a non-trivial amount of work and
>> therefore I'd like to do it just once. After we have a clear agreement
>> on the naming.
> 
> For some unknown reason I failed to see the obvious.
> virStreamSendHole()   <-- for public APIs
> virStreamRecvHole()
> 
> VIR_NET_STREAM_HOLE   <-- for RPC packet
> 
> What do you think?
> 
> Michal
> 

Seems reasonable - it'll be (hopefully) obvious that the two are related
and should cause no "self inflicted pain" to figure out which is a send
and which is a receive that I kept tripping over with Skip and HoleSize.

John

FWIW: At one point during reading the code I think I also had the hey
why not just use the @flags argument for Send/Recv APIs, but I recall
ruling it out since @data is checked for NULL and of course that would
"overload" those API's.