[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH V6 4/6] net/colo-compare: Move data structure and define to .
|
From: |
Zhang, Chen |
|
Subject: |
RE: [PATCH V6 4/6] net/colo-compare: Move data structure and define to .h file. |
|
Date: |
Thu, 20 May 2021 01:50:14 +0000 |
> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Tuesday, May 18, 2021 4:04 AM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Jason Wang <jasowang@redhat.com>; qemu-dev <qemu-
> devel@nongnu.org>; Eric Blake <eblake@redhat.com>; Dr. David Alan
> Gilbert <dgilbert@redhat.com>; Markus Armbruster <armbru@redhat.com>;
> Daniel P. Berrangé <berrange@redhat.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Li Zhijian <lizhijian@cn.fujitsu.com>; Zhang Chen
> <zhangckid@gmail.com>
> Subject: Re: [PATCH V6 4/6] net/colo-compare: Move data structure and
> define to .h file.
>
> On Tue, 20 Apr 2021 23:15:35 +0800
> Zhang Chen <chen.zhang@intel.com> wrote:
>
> > Rename structure with COLO index and move it to .h file, It make other
> > modules can reuse COLO code.
>
> Hi,
> There are some definitions that don't need to be moved into the header,
> more comments below.
>
OK.
> In general I think the new passthrough feature can be exclusive to colo-
> compare for now and that everything can remain there. If other net filters
> implement the feature, we can still move it outside of colo-compare later.
>
Agree, This patch move some code to colo-compare.h, it still in colo-compare.
Thanks
Chen
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> > net/colo-compare.c | 134
> > +++++----------------------------------------
> > net/colo-compare.h | 106 +++++++++++++++++++++++++++++++++++
> > 2 files changed, 120 insertions(+), 120 deletions(-)
> >
> > diff --git a/net/colo-compare.c b/net/colo-compare.c index
> > 9d1ad99941..b51b1437ef 100644
> > --- a/net/colo-compare.c
> > +++ b/net/colo-compare.c
> > @@ -17,44 +17,24 @@
> > #include "qemu/error-report.h"
> > #include "trace.h"
> > #include "qapi/error.h"
> > -#include "net/net.h"
> > #include "net/eth.h"
> > #include "qom/object_interfaces.h"
> > #include "qemu/iov.h"
> > #include "qom/object.h"
> > #include "net/queue.h"
> > -#include "chardev/char-fe.h"
> > #include "qemu/sockets.h"
> > -#include "colo.h"
> > -#include "sysemu/iothread.h"
> > #include "net/colo-compare.h"
> > -#include "migration/colo.h"
> > -#include "migration/migration.h"
> > #include "util.h"
> >
> > #include "block/aio-wait.h"
> > #include "qemu/coroutine.h"
> >
> > -#define TYPE_COLO_COMPARE "colo-compare"
> > -typedef struct CompareState CompareState;
> > -DECLARE_INSTANCE_CHECKER(CompareState, COLO_COMPARE,
> > - TYPE_COLO_COMPARE)
> > -
> > static QTAILQ_HEAD(, CompareState) net_compares =
> > QTAILQ_HEAD_INITIALIZER(net_compares);
> >
> > static NotifierList colo_compare_notifiers =
> > NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
> >
> > -#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
> > -
>
> These 6 defines should stay here.
>
> > /* #define DEBUG_COLO_PACKETS */
> >
> > static QemuMutex colo_compare_mutex;
> > @@ -64,92 +44,6 @@ static QemuCond event_complete_cond; static int
> > event_unhandled_count; static uint32_t max_queue_size;
> >
> > -/*
> > - * + 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 +
> > - * +--------+ +--------+ +--------+ +--------+
> > - */
> > -
> > -typedef struct SendCo {
> > - Coroutine *co;
> > - struct CompareState *s;
> > - CharBackend *chr;
> > - GQueue send_list;
> > - bool notify_remote_frame;
> > - bool done;
> > - int ret;
> > -} SendCo;
>
> This struct should stay here.
>
> > -typedef struct SendEntry {
> > - uint32_t size;
> > - uint32_t vnet_hdr_len;
> > - uint8_t *buf;
> > -} SendEntry;
>
> This struct should stay here.
>
> > -struct CompareState {
> > - 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,
> > -};
>
> The enum should stay here.
>
> > static const char *colo_mode[] = {
> > [PRIMARY_IN] = "primary",
> > [SECONDARY_IN] = "secondary",
> > @@ -737,19 +631,19 @@ static void colo_compare_connection(void
> > *opaque, void *user_data)
> >
> > static void coroutine_fn _compare_chr_send(void *opaque) {
> > - SendCo *sendco = opaque;
> > + COLOSendCo *sendco = opaque;
> > CompareState *s = sendco->s;
> > int ret = 0;
> >
> > while (!g_queue_is_empty(&sendco->send_list)) {
> > - SendEntry *entry = g_queue_pop_tail(&sendco->send_list);
> > + COLOSendEntry *entry = g_queue_pop_tail(&sendco->send_list);
> > uint32_t len = htonl(entry->size);
> >
> > ret = qemu_chr_fe_write_all(sendco->chr, (uint8_t *)&len,
> > sizeof(len));
> >
> > if (ret != sizeof(len)) {
> > g_free(entry->buf);
> > - g_slice_free(SendEntry, entry);
> > + g_slice_free(COLOSendEntry, entry);
> > goto err;
> > }
> >
> > @@ -766,7 +660,7 @@ static void coroutine_fn _compare_chr_send(void
> > *opaque)
> >
> > if (ret != sizeof(len)) {
> > g_free(entry->buf);
> > - g_slice_free(SendEntry, entry);
> > + g_slice_free(COLOSendEntry, entry);
> > goto err;
> > }
> > }
> > @@ -777,12 +671,12 @@ static void coroutine_fn
> _compare_chr_send(void
> > *opaque)
> >
> > if (ret != entry->size) {
> > g_free(entry->buf);
> > - g_slice_free(SendEntry, entry);
> > + g_slice_free(COLOSendEntry, entry);
> > goto err;
> > }
> >
> > g_free(entry->buf);
> > - g_slice_free(SendEntry, entry);
> > + g_slice_free(COLOSendEntry, entry);
> > }
> >
> > sendco->ret = 0;
> > @@ -790,9 +684,9 @@ static void coroutine_fn _compare_chr_send(void
> > *opaque)
> >
> > err:
> > while (!g_queue_is_empty(&sendco->send_list)) {
> > - SendEntry *entry = g_queue_pop_tail(&sendco->send_list);
> > + COLOSendEntry *entry = g_queue_pop_tail(&sendco->send_list);
> > g_free(entry->buf);
> > - g_slice_free(SendEntry, entry);
> > + g_slice_free(COLOSendEntry, entry);
> > }
> > sendco->ret = ret < 0 ? ret : -EIO;
> > out:
> > @@ -808,8 +702,8 @@ static int compare_chr_send(CompareState *s,
> > bool notify_remote_frame,
> > bool zero_copy) {
> > - SendCo *sendco;
> > - SendEntry *entry;
> > + COLOSendCo *sendco;
> > + COLOSendEntry *entry;
> >
> > if (notify_remote_frame) {
> > sendco = &s->notify_sendco;
> > @@ -821,7 +715,7 @@ static int compare_chr_send(CompareState *s,
> > return 0;
> > }
> >
> > - entry = g_slice_new(SendEntry);
> > + entry = g_slice_new(COLOSendEntry);
> > entry->size = size;
> > entry->vnet_hdr_len = vnet_hdr_len;
> > if (zero_copy) {
> > @@ -1274,17 +1168,17 @@ static void
> > colo_compare_complete(UserCreatable *uc, Error **errp)
> >
> > if (!s->compare_timeout) {
> > /* Set default value to 3000 MS */
> > - s->compare_timeout = DEFAULT_TIME_OUT_MS;
> > + s->compare_timeout = COLO_DEFAULT_TIME_OUT_MS;
> > }
> >
> > if (!s->expired_scan_cycle) {
> > /* Set default value to 3000 MS */
> > - s->expired_scan_cycle = REGULAR_PACKET_CHECK_MS;
> > + s->expired_scan_cycle = COLO_REGULAR_PACKET_CHECK_MS;
> > }
> >
> > if (!max_queue_size) {
> > /* Set default queue size to 1024 */
> > - max_queue_size = MAX_QUEUE_SIZE;
> > + max_queue_size = MAX_COLO_QUEUE_SIZE;
> > }
> >
> > if (find_and_check_chardev(&chr, s->pri_indev, errp) || diff
> > --git a/net/colo-compare.h b/net/colo-compare.h index
> > 22ddd512e2..ab649c9dbe 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_COLO_QUEUE_SIZE
> > +1024
> > +
> > +#define COLO_COMPARE_FREE_PRIMARY 0x01
> > +#define COLO_COMPARE_FREE_SECONDARY 0x02
> > +
> > +#define COLO_REGULAR_PACKET_CHECK_MS 1000 #define
> > +COLO_DEFAULT_TIME_OUT_MS 3000
> > +
> > +typedef struct COLOSendCo {
> > + Coroutine *co;
> > + struct CompareState *s;
> > + CharBackend *chr;
> > + GQueue send_list;
> > + bool notify_remote_frame;
> > + bool done;
> > + int ret;
> > +} COLOSendCo;
> > +
> > +typedef struct COLOSendEntry {
> > + uint32_t size;
> > + uint32_t vnet_hdr_len;
> > + uint8_t *buf;
> > +} COLOSendEntry;
> > +
> > +/*
> > + * + 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 {
> > + 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;
> > + COLOSendCo out_sendco;
> > + COLOSendCo 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);
>
>
>
> --