RE: [PATCH V4 5/7] net/colo-compare: Move data structure and define to .h file.

["Zhang, Chen" <chen.zhang@intel.com>]

> -----Original Message-----
> From: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Sent: Wednesday, March 24, 2021 7:02 PM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Jason Wang <jasowang@redhat.com>; qemu-dev <qemu-
> devel@nongnu.org>; Eric Blake <eblake@redhat.com>; Markus Armbruster
> <armbru@redhat.com>; Li Zhijian <lizhijian@cn.fujitsu.com>; Zhang Chen
> <zhangckid@gmail.com>; Lukas Straub <lukasstraub2@web.de>
> Subject: Re: [PATCH V4 5/7] net/colo-compare: Move data structure and
> define to .h file.
> 
> * Zhang Chen (chen.zhang@intel.com) wrote:
> > Make other modules can reuse COLO code.
> >
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> >  net/colo-compare.c | 106
> > ---------------------------------------------
> >  net/colo-compare.h | 106
> > +++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 106 insertions(+), 106 deletions(-)
> >
> 
> <snip>
> 
> > diff --git a/net/colo-compare.h b/net/colo-compare.h index
> > 22ddd512e2..2a9dcac0a7 100644
> > --- a/net/colo-compare.h
> > +++ b/net/colo-compare.h
> > @@ -17,6 +17,112 @@
> >  #ifndef QEMU_COLO_COMPARE_H
> >  #define QEMU_COLO_COMPARE_H
> >
> > +#include "net/net.h"
> > +#include "chardev/char-fe.h"
> > +#include "migration/colo.h"
> > +#include "migration/migration.h"
> > +#include "sysemu/iothread.h"
> > +#include "colo.h"
> > +
> > +#define TYPE_COLO_COMPARE "colo-compare"
> > +typedef struct CompareState CompareState;
> > +DECLARE_INSTANCE_CHECKER(CompareState, COLO_COMPARE,
> > +                         TYPE_COLO_COMPARE)
> > +
> > +#define COMPARE_READ_LEN_MAX NET_BUFSIZE #define
> MAX_QUEUE_SIZE 1024
> 
> ^^^^
> 
> > +#define COLO_COMPARE_FREE_PRIMARY     0x01
> > +#define COLO_COMPARE_FREE_SECONDARY   0x02
> > +
> > +#define REGULAR_PACKET_CHECK_MS 1000
> > +#define DEFAULT_TIME_OUT_MS 3000
> > +
> > +typedef struct SendCo {
> > +    Coroutine *co;
> > +    struct CompareState *s;
> > +    CharBackend *chr;
> > +    GQueue send_list;
> > +    bool notify_remote_frame;
> > +    bool done;
> > +    int ret;
> > +} SendCo;
> 
> ^^^^^
> > +typedef struct SendEntry {
> > +    uint32_t size;
> > +    uint32_t vnet_hdr_len;
> > +    uint8_t *buf;
> > +} SendEntry;
> 
> ^^^^^
> 
> > +/*
> > + *  + CompareState ++
> > + *  |               |
> > + *  +---------------+   +---------------+         +---------------+
> > + *  |   conn list   + - >      conn     + ------- >      conn     + -- > ......
> > + *  +---------------+   +---------------+         +---------------+
> > + *  |               |     |           |             |          |
> > + *  +---------------+ +---v----+  +---v----+    +---v----+ +---v----+
> > + *                    |primary |  |secondary    |primary | |secondary
> > + *                    |packet  |  |packet  +    |packet  | |packet  +
> > + *                    +--------+  +--------+    +--------+ +--------+
> > + *                        |           |             |          |
> > + *                    +---v----+  +---v----+    +---v----+ +---v----+
> > + *                    |primary |  |secondary    |primary | |secondary
> > + *                    |packet  |  |packet  +    |packet  | |packet  +
> > + *                    +--------+  +--------+    +--------+ +--------+
> > + *                        |           |             |          |
> > + *                    +---v----+  +---v----+    +---v----+ +---v----+
> > + *                    |primary |  |secondary    |primary | |secondary
> > + *                    |packet  |  |packet  +    |packet  | |packet  +
> > + *                    +--------+  +--------+    +--------+ +--------+
> > + */
> > +struct CompareState {
> 
>  ^^^^^
> 
> For a header, these are too generic names - they need to have Colo in them;
> e.g. MAX_COLOQUEUE_SIZE and COLOSendEntry etc

Thanks Dave, I will fix it in next version.

Thanks
Chen

> 
> Dave
> 
> > +    Object parent;
> > +
> > +    char *pri_indev;
> > +    char *sec_indev;
> > +    char *outdev;
> > +    char *notify_dev;
> > +    CharBackend chr_pri_in;
> > +    CharBackend chr_sec_in;
> > +    CharBackend chr_out;
> > +    CharBackend chr_notify_dev;
> > +    SocketReadState pri_rs;
> > +    SocketReadState sec_rs;
> > +    SocketReadState notify_rs;
> > +    SendCo out_sendco;
> > +    SendCo notify_sendco;
> > +    bool vnet_hdr;
> > +    uint64_t compare_timeout;
> > +    uint32_t expired_scan_cycle;
> > +
> > +    /*
> > +     * Record the connection that through the NIC
> > +     * Element type: Connection
> > +     */
> > +    GQueue conn_list;
> > +    /* Record the connection without repetition */
> > +    GHashTable *connection_track_table;
> > +
> > +    IOThread *iothread;
> > +    GMainContext *worker_context;
> > +    QEMUTimer *packet_check_timer;
> > +
> > +    QEMUBH *event_bh;
> > +    enum colo_event event;
> > +
> > +    QTAILQ_ENTRY(CompareState) next;
> > +};
> > +
> > +typedef struct CompareClass {
> > +    ObjectClass parent_class;
> > +} CompareClass;
> > +
> > +enum {
> > +    PRIMARY_IN = 0,
> > +    SECONDARY_IN,
> > +};
> > +
> >  void colo_notify_compares_event(void *opaque, int event, Error
> > **errp);  void colo_compare_register_notifier(Notifier *notify);  void
> > colo_compare_unregister_notifier(Notifier *notify);
> > --
> > 2.25.1
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK