Re: [Qemu-devel] [PATCH V2 4/5] COLO-compare: Add colo-compare remote notify support

[Zhang, Chen]

> -----Original Message-----
> From: Jason Wang [mailto:address@hidden]
> Sent: Thursday, June 27, 2019 9:58 AM
> To: Zhang, Chen <address@hidden>; Li Zhijian <address@hidden>;
> Dr. David Alan Gilbert <address@hidden>; Juan Quintela
> <address@hidden>; zhanghailiang <address@hidden>;
> qemu-dev <address@hidden>; Stefano Stabellini
> <address@hidden>; Paul Durrant <address@hidden>
> Cc: Zhang Chen <address@hidden>
> Subject: Re: [Qemu-devel] [PATCH V2 4/5] COLO-compare: Add colo-compare
> remote notify support
> 
> 
> On 2019/6/10 上午12:44, Zhang Chen wrote:
> > From: Zhang Chen <address@hidden>
> >
> > This patch make colo-compare can send message to remote COLO frame(Xen)
> when occur checkpoint.
> >
> > Signed-off-by: Zhang Chen <address@hidden>
> > ---
> >   net/colo-compare.c | 54 +++++++++++++++++++++++++++++++++++++--------
> -
> >   1 file changed, 44 insertions(+), 10 deletions(-)
> >
> > diff --git a/net/colo-compare.c b/net/colo-compare.c index
> > 16285f4a96..516b651ecd 100644
> > --- a/net/colo-compare.c
> > +++ b/net/colo-compare.c
> > @@ -120,11 +120,6 @@ enum {
> >       SECONDARY_IN,
> >   };
> >
> > -static void colo_compare_inconsistency_notify(void)
> > -{
> > -    notifier_list_notify(&colo_compare_notifiers,
> > -                migrate_get_current());
> > -}
> >
> >   static int compare_chr_send(CompareState *s,
> >                               const uint8_t *buf, @@ -132,6 +127,27 @@
> > static int compare_chr_send(CompareState *s,
> >                               uint32_t vnet_hdr_len,
> >                               bool notify_remote_frame);
> >
> > +static void notify_remote_frame(CompareState *s) {
> > +    char msg[] = "DO_CHECKPOINT";
> > +    int ret = 0;
> > +
> > +    ret = compare_chr_send(s, (uint8_t *)msg, strlen(msg), 0, true);
> > +    if (ret < 0) {
> > +        error_report("Notify Xen COLO-frame failed");
> > +    }
> > +}
> > +
> > +static void colo_compare_inconsistency_notify(CompareState *s) {
> > +    if (s->notify_dev) {
> > +        notify_remote_frame(s);
> > +    } else {
> > +        notifier_list_notify(&colo_compare_notifiers,
> > +                             migrate_get_current());
> > +    }
> > +}
> > +
> >   static gint seq_sorter(Packet *a, Packet *b, gpointer data)
> >   {
> >       struct tcp_hdr *atcp, *btcp;
> > @@ -435,7 +451,7 @@ sec:
> >           qemu_hexdump((char *)spkt->data, stderr,
> >                        "colo-compare spkt", spkt->size);
> >
> > -        colo_compare_inconsistency_notify();
> > +        colo_compare_inconsistency_notify(s);
> >       }
> >   }
> >
> > @@ -577,7 +593,7 @@ void colo_compare_unregister_notifier(Notifier
> *notify)
> >   }
> >
> >   static int colo_old_packet_check_one_conn(Connection *conn,
> > -                                           void *user_data)
> > +                                          CompareState *s)
> >   {
> >       GList *result = NULL;
> >       int64_t check_time = REGULAR_PACKET_CHECK_MS; @@ -588,7 +604,7
> > @@ static int colo_old_packet_check_one_conn(Connection *conn,
> >
> >       if (result) {
> >           /* Do checkpoint will flush old packet */
> > -        colo_compare_inconsistency_notify();
> > +        colo_compare_inconsistency_notify(s);
> >           return 0;
> >       }
> >
> > @@ -608,7 +624,7 @@ static void colo_old_packet_check(void *opaque)
> >        * If we find one old packet, stop finding job and notify
> >        * COLO frame do checkpoint.
> >        */
> > -    g_queue_find_custom(&s->conn_list, NULL,
> > +    g_queue_find_custom(&s->conn_list, s,
> >                           (GCompareFunc)colo_old_packet_check_one_conn);
> >   }
> >
> > @@ -637,7 +653,8 @@ static void colo_compare_packet(CompareState *s,
> Connection *conn,
> >                */
> >               trace_colo_compare_main("packet different");
> >               g_queue_push_head(&conn->primary_list, pkt);
> > -            colo_compare_inconsistency_notify();
> > +
> > +            colo_compare_inconsistency_notify(s);
> >               break;
> >           }
> >       }
> > @@ -989,7 +1006,24 @@ static void
> > compare_sec_rs_finalize(SocketReadState *sec_rs)
> >
> >   static void compare_notify_rs_finalize(SocketReadState *notify_rs)
> >   {
> > +    CompareState *s = container_of(notify_rs, CompareState,
> > + notify_rs);
> > +
> >       /* Get Xen colo-frame's notify and handle the message */
> > +    char *data = g_memdup(notify_rs->buf, notify_rs->packet_len);
> > +    char msg[] = "COLO_COMPARE_GET_XEN_INIT";
> > +    int ret;
> > +
> > +    if (!strcmp(data, "COLO_USERSPACE_PROXY_INIT")) {
> > +        ret = compare_chr_send(s, (uint8_t *)msg, strlen(msg), 0, true);
> > +        if (ret < 0) {
> > +            error_report("Notify Xen COLO-frame INIT failed");
> > +        }
> > +    }
> > +
> > +    if (!strcmp(data, "COLO_CHECKPOINT")) {
> > +        /* colo-compare do checkpoint, flush pri packet and remove sec 
> > packet
> */
> > +        g_queue_foreach(&s->conn_list, colo_flush_packets, s);
> > +    }
> >   }
> 
> 
> This protocol looks too simple, is this accepted by Xen?

Yes, that Xen patch from long time ago....
Currently, Our target is make Xen COLO can running, and we can optimize the 
protocol in the future.

Related Xen code:
https://github.com/xen-project/xen/blob/master/tools/libxl/libxl_colo_proxy.c


Thanks
Zhang Chen

> 
> Thanks
> 
> 
> >
> >   /*