Re: [Qemu-devel] [PATCH V2 0/3] Introduce COLO-compare

[Zhang Chen]

On 03/30/2016 08:05 PM, Dr. David Alan Gilbert wrote:
> * Zhang Chen (address@hidden) wrote:
> > COLO-compare is a part of COLO project. It is used
> > to compare the network package to help COLO decide
> > whether to do checkpoint.
> Hi Zhang Chen,
>    I've put comments on the individual patches, but some more general things:
>
>    1) Please add a coment giving the example of the command line for the primary
>      and secondary use of this module - it helps make it easier to understand 
> the patches.

Yes, have reply in
Re: [Qemu-devel] [PATCH V2 1/3] colo-compare: introduce colo compare 
initlization

>    2) There's no tracing in here - please add some; I found when I tried to get
>      COLO working I needed to use lots of tracing and debugging to understand 
> the
>      packet flow.

I will add some trace in next.

>    3) Add comments; e.g. for each function say which thread is using it and 
> where
>       the packets are coming from; e.g.
>          called from the main thread on the primary for packets arriving over 
> the socket
>          from the secondary.
>
>       There's just so many packets going in so many directions it would make it
>       easier to follow.

OK~

>    4) A more fundamental problem is what happens if the secondary never sends 
> anything
>       on the socket, the result is you end up running until the end of the long 
> COLO
>       checkpoint without triggering a discompare - in my world I added a 
> timeout (400ms)
>       for an unmatched packet from the primary, where if no matching packet was 
> received
>       a checkpoint would be triggered.

OK,I will do this in futrue

>    5) I see the packet comparison is still the simple memcmpy that you had in 
> December;
>       are you planning on doing anything more complicated; you must be seing 
> most packets
>       miscompare?

Yes, this is just a compare-frame RFC, We will continue to improve it in 
the future

Thanks
Zhang Chen


> You can see my current world at; 
> https://github.com/orbitfp7/qemu/commits/orbit-wp4-colo-mar16
> which has my basic TCP comparison (it's only tracking incoming connections) and 
> I know it's
> not complete either.  It mostly works OK, although I've got an occasional seg
> (which makes me wonder if I need to add the conn_list_lock I see you added).  
> I'm also
> not doing any TCP reassembly which is probably needed.
>
> Dave
>      
> > v2:
> >   - add jhash.h
> >
> > v1:
> >   - initial patch
> >
> >
> > Zhang Chen (3):
> >    colo-compare: introduce colo compare initlization
> >    colo-compare: track connection and enqueue packet
> >    colo-compare: introduce packet comparison thread
> >
> >   include/qemu/jhash.h |  59 ++++
> >   net/Makefile.objs    |   1 +
> >   net/colo-compare.c   | 782 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >   vl.c                 |   3 +-
> >   4 files changed, 844 insertions(+), 1 deletion(-)
> >   create mode 100644 include/qemu/jhash.h
> >   create mode 100644 net/colo-compare.c
> >
> > --
> > 1.9.1
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK
>
>
> .

--
Thanks
zhangchen