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

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(a)redhat.com&gt; >> --- >> 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. Also, now that we seem to be using Reviewed-by tags, should I include those in my commit messages for v2? I mean, you've reviewed patches 00-30. And you'll definitely deserve the tag. But what I'm asking is if I should put it there already for v2 or before pushing the patches. To extend my thinking further, review is not the same as ACK. So Reviewed-by should include all the reviewers even though just one of them gave ACK? Or if a patch is respin in version X, should it include all the past reviewers? So many questions and not many answers. Michal