qemu-devel
[Top][All Lists]
Advanced

[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: Lukas Straub
Subject: Re: [PATCH V6 4/6] net/colo-compare: Move data structure and define to .h file.
Date: Mon, 17 May 2021 22:03:40 +0200

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.

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.

> 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);



-- 

Attachment: pgpfem46mFxVg.pgp
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]