[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V2 2/3] colo-compare: track connection and enque
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH V2 2/3] colo-compare: track connection and enqueue packet |
Date: |
Thu, 31 Mar 2016 09:47:16 +0100 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
* Li Zhijian (address@hidden) wrote:
>
>
> On 03/30/2016 06:36 PM, Dr. David Alan Gilbert wrote:
> >* Zhang Chen (address@hidden) wrote:
...
> >>+static inline void colo_flush_connection(void *opaque, void *user_data)
> >>+{
> >
> >Is this used?
> Yes, it isn't used currently.
> Actually this is needed after compare module is integrated to COLO frame.
OK, leave it out for the moment; I think some compilers complain when
you compile with an unused static.
Dave
>
>
> >
> >>+ Connection *conn = opaque;
> >>+ Packet *pkt = NULL;
> >>+
> >>+ qemu_mutex_lock(&conn->list_lock);
> >>+ while (!g_queue_is_empty(&conn->primary_list)) {
> >>+ pkt = g_queue_pop_head(&conn->primary_list);
> >>+ compare_chr_send(pkt->s->chr_out, pkt->data, pkt->size);
> >>+ /* FIXME: destroy pkt ?*/
> >>+ }
> >>+ while (!g_queue_is_empty(&conn->secondary_list)) {
> >>+ pkt = g_queue_pop_head(&conn->secondary_list);
> >>+ packet_destroy(pkt, NULL);
> >>+ }
> >>+ qemu_mutex_unlock(&conn->list_lock);
> >>+}
> >>+
> >> static int compare_chr_send(CharDriverState *out, const uint8_t *buf, int
> >> size)
> >> {
> >> int ret = 0;
> >>@@ -142,8 +431,10 @@ static void compare_pri_chr_in(void *opaque, const
> >>uint8_t *buf, int size)
> >>
> >> ret = compare_chr_fill_rstate(&s->pri_rs, buf, size);
> >> if (ret == 1) {
> >>- /* FIXME: enqueue to primary packet list */
> >>- compare_chr_send(s->chr_out, buf, size);
> >>+ if (packet_enqueue(s, PRIMARY_IN)) {
> >>+ error_report("primary: unsupported packet in");
> >
> >Is this for non-IP packets? If so you don't want an error_report - because
> >non-IP are
> >quite common; a trace would be useful giving the packet type etc
> Agree. And further more, IMO release the packet to client is not always
> correct for all non-IP
> but at current stage, this looks fine.
>
> Thanks
> Li Zhijian
>
> >
> >>+ compare_chr_send(s->chr_out, buf, size);
> >>+ }
> >> } else if (ret == -1) {
> >> qemu_chr_add_handlers(s->chr_pri_in, NULL, NULL, NULL, NULL);
> >> }
> >>@@ -156,7 +447,9 @@ static void compare_sec_chr_in(void *opaque, const
> >>uint8_t *buf, int size)
> >>
> >> ret = compare_chr_fill_rstate(&s->sec_rs, buf, size);
> >> if (ret == 1) {
> >>- /* TODO: enqueue to secondary packet list*/
> >>+ if (packet_enqueue(s, SECONDARY_IN)) {
> >>+ error_report("secondary: unsupported packet in");
> >>+ }
> >> } else if (ret == -1) {
> >> qemu_chr_add_handlers(s->chr_sec_in, NULL, NULL, NULL, NULL);
> >> }
> >>@@ -210,6 +503,7 @@ static void compare_set_outdev(Object *obj, const char
> >>*value, Error **errp)
> >> static void colo_compare_complete(UserCreatable *uc, Error **errp)
> >> {
> >> CompareState *s = COLO_COMPARE(uc);
> >>+ struct sysinfo si;
> >>
> >> if (!s->pri_indev || !s->sec_indev || !s->outdev) {
> >> error_setg(errp, "colo compare needs 'primary_in' ,"
> >>@@ -255,6 +549,29 @@ static void colo_compare_complete(UserCreatable *uc,
> >>Error **errp)
> >>
> >> QTAILQ_INSERT_TAIL(&net_compares, s, next);
> >>
> >>+ g_queue_init(&s->conn_list);
> >>+ qemu_mutex_init(&s->conn_list_lock);
> >>+
> >>+ s->hashtable_size = 0;
> >>+ /*
> >>+ * Idea from kernel tcp.c: use 1/16384 of memory. On i386: 32MB
> >>+ * machine has 512 buckets. >= 1GB machines have 16384 buckets.
> >>+ */
> >>+ sysinfo(&si);
> >>+ hashtable_max_size = si.totalram / 16384;
> >>+ if (si.totalram > (1024 * 1024 * 1024 / PAGE_SIZE)) {
> >>+ hashtable_max_size = 16384;
> >>+ }
> >>+ if (hashtable_max_size < 32) {
> >>+ hashtable_max_size = 32;
> >>+ }
> >>+ hashtable_max_size = hashtable_max_size * 8; /* default factor = 8 */
> >
> >Make this a lot simpler; just pick a size and if it's a problem then we'll
> >worry
> >about it later, or make it an option on the filter if you want it changeable.
> >
> >>+ s->connection_track_table = g_hash_table_new_full(connection_key_hash,
> >>+ connection_key_equal,
> >>+ g_free,
> >>+ connection_destroy);
> >>+
> >> return;
> >>
> >> out:
> >>@@ -297,6 +614,7 @@ static void colo_compare_class_finalize(ObjectClass
> >>*oc, void *data)
> >> if (!QTAILQ_EMPTY(&net_compares)) {
> >> QTAILQ_REMOVE(&net_compares, s, next);
> >> }
> >>+ qemu_mutex_destroy(&s->conn_list_lock);
> >> }
> >>
> >> static void colo_compare_init(Object *obj)
> >>--
> >>1.9.1
> >>
> >>
> >>
> >--
> >Dr. David Alan Gilbert / address@hidden / Manchester, UK
> >
> >
> >.
> >
>
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
[Qemu-devel] [PATCH V2 3/3] colo-compare: introduce packet comparison thread, Zhang Chen, 2016/03/30
Re: [Qemu-devel] [PATCH V2 0/3] Introduce COLO-compare, Dr. David Alan Gilbert, 2016/03/30