* [PATCH 1/4] net/colo-compare.c: Fix compare_timeout format issue
2020-09-18 9:21 [PATCH 0/4] Several optimization and bugfix for COLO compare Zhang Chen
@ 2020-09-18 9:22 ` Zhang Chen
2020-09-22 6:14 ` Li Zhijian
2020-09-18 9:22 ` [PATCH 2/4] net/colo-compare.c: Change the timer clock type Zhang Chen
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Zhang Chen @ 2020-09-18 9:22 UTC (permalink / raw)
To: Jason Wang, qemu-dev; +Cc: Zhang Chen, Li Zhijian, Zhang Chen
From: Zhang Chen <chen.zhang@intel.com>
This parameter need compare with the return of qemu_clock_get_ms(),
it is uinit64_t. So we need fix this issue here.
Reported-by: Derek Su <dereksu@qnap.com>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
net/colo-compare.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 3a45d64175..7cba573dae 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -120,7 +120,7 @@ struct CompareState {
SendCo out_sendco;
SendCo notify_sendco;
bool vnet_hdr;
- uint32_t compare_timeout;
+ uint64_t compare_timeout;
uint32_t expired_scan_cycle;
/*
@@ -1081,9 +1081,9 @@ static void compare_get_timeout(Object *obj, Visitor *v,
Error **errp)
{
CompareState *s = COLO_COMPARE(obj);
- uint32_t value = s->compare_timeout;
+ uint64_t value = s->compare_timeout;
- visit_type_uint32(v, name, &value, errp);
+ visit_type_uint64(v, name, &value, errp);
}
static void compare_set_timeout(Object *obj, Visitor *v,
@@ -1146,9 +1146,9 @@ static void set_max_queue_size(Object *obj, Visitor *v,
Error **errp)
{
Error *local_err = NULL;
- uint32_t value;
+ uint64_t value;
- visit_type_uint32(v, name, &value, &local_err);
+ visit_type_uint64(v, name, &value, &local_err);
if (local_err) {
goto out;
}
@@ -1396,7 +1396,7 @@ static void colo_compare_init(Object *obj)
object_property_add_str(obj, "notify_dev",
compare_get_notify_dev, compare_set_notify_dev);
- object_property_add(obj, "compare_timeout", "uint32",
+ object_property_add(obj, "compare_timeout", "uint64",
compare_get_timeout,
compare_set_timeout, NULL, NULL);
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 1/4] net/colo-compare.c: Fix compare_timeout format issue
2020-09-18 9:22 ` [PATCH 1/4] net/colo-compare.c: Fix compare_timeout format issue Zhang Chen
@ 2020-09-22 6:14 ` Li Zhijian
0 siblings, 0 replies; 10+ messages in thread
From: Li Zhijian @ 2020-09-22 6:14 UTC (permalink / raw)
To: Zhang Chen, Jason Wang, qemu-dev; +Cc: Zhang Chen
On 9/18/20 5:22 PM, Zhang Chen wrote:
> From: Zhang Chen <chen.zhang@intel.com>
>
> This parameter need compare with the return of qemu_clock_get_ms(),
> it is uinit64_t. So we need fix this issue here.
>
> Reported-by: Derek Su <dereksu@qnap.com>
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> ---
> net/colo-compare.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 3a45d64175..7cba573dae 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -120,7 +120,7 @@ struct CompareState {
> SendCo out_sendco;
> SendCo notify_sendco;
> bool vnet_hdr;
> - uint32_t compare_timeout;
> + uint64_t compare_timeout;
> uint32_t expired_scan_cycle;
>
> /*
> @@ -1081,9 +1081,9 @@ static void compare_get_timeout(Object *obj, Visitor *v,
> Error **errp)
> {
> CompareState *s = COLO_COMPARE(obj);
> - uint32_t value = s->compare_timeout;
> + uint64_t value = s->compare_timeout;
>
> - visit_type_uint32(v, name, &value, errp);
> + visit_type_uint64(v, name, &value, errp);
> }
>
> static void compare_set_timeout(Object *obj, Visitor *v,
> @@ -1146,9 +1146,9 @@ static void set_max_queue_size(Object *obj, Visitor *v,
> Error **errp)
> {
> Error *local_err = NULL;
> - uint32_t value;
> + uint64_t value;
>
> - visit_type_uint32(v, name, &value, &local_err);
> + visit_type_uint64(v, name, &value, &local_err);
> if (local_err) {
> goto out;
> }
> @@ -1396,7 +1396,7 @@ static void colo_compare_init(Object *obj)
> object_property_add_str(obj, "notify_dev",
> compare_get_notify_dev, compare_set_notify_dev);
>
> - object_property_add(obj, "compare_timeout", "uint32",
> + object_property_add(obj, "compare_timeout", "uint64",
> compare_get_timeout,
> compare_set_timeout, NULL, NULL);
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/4] net/colo-compare.c: Change the timer clock type
2020-09-18 9:21 [PATCH 0/4] Several optimization and bugfix for COLO compare Zhang Chen
2020-09-18 9:22 ` [PATCH 1/4] net/colo-compare.c: Fix compare_timeout format issue Zhang Chen
@ 2020-09-18 9:22 ` Zhang Chen
2020-09-22 6:18 ` Li Zhijian
2020-09-18 9:22 ` [PATCH 3/4] net/colo-compare.c: Add secondary old packet detection Zhang Chen
2020-09-18 9:22 ` [PATCH 4/4] net/colo-compare.c: Increase default queued packet scan frequency Zhang Chen
3 siblings, 1 reply; 10+ messages in thread
From: Zhang Chen @ 2020-09-18 9:22 UTC (permalink / raw)
To: Jason Wang, qemu-dev; +Cc: Zhang Chen, Li Zhijian, Zhang Chen
From: Zhang Chen <chen.zhang@intel.com>
The virtual clock only runs during the emulation. It stops
when the virtual machine is stopped.
The host clock should be used for device models that emulate accurate
real time sources. It will continue to run when the virtual machine
is suspended. COLO need to know the host time here.
Reported-by: Derek Su <dereksu@qnap.com>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
net/colo-compare.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 7cba573dae..3b72309d08 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -905,7 +905,7 @@ static void check_old_packet_regular(void *opaque)
/* if have old packet we will notify checkpoint */
colo_old_packet_check(s);
- timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+ timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_HOST) +
s->expired_scan_cycle);
}
@@ -939,10 +939,10 @@ static void colo_compare_timer_init(CompareState *s)
{
AioContext *ctx = iothread_get_aio_context(s->iothread);
- s->packet_check_timer = aio_timer_new(ctx, QEMU_CLOCK_VIRTUAL,
+ s->packet_check_timer = aio_timer_new(ctx, QEMU_CLOCK_HOST,
SCALE_MS, check_old_packet_regular,
s);
- timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+ timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_HOST) +
s->expired_scan_cycle);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/4] net/colo-compare.c: Change the timer clock type
2020-09-18 9:22 ` [PATCH 2/4] net/colo-compare.c: Change the timer clock type Zhang Chen
@ 2020-09-22 6:18 ` Li Zhijian
0 siblings, 0 replies; 10+ messages in thread
From: Li Zhijian @ 2020-09-22 6:18 UTC (permalink / raw)
To: Zhang Chen, Jason Wang, qemu-dev; +Cc: Zhang Chen
On 9/18/20 5:22 PM, Zhang Chen wrote:
> From: Zhang Chen <chen.zhang@intel.com>
>
> The virtual clock only runs during the emulation. It stops
> when the virtual machine is stopped.
> The host clock should be used for device models that emulate accurate
> real time sources. It will continue to run when the virtual machine
> is suspended. COLO need to know the host time here.
>
> Reported-by: Derek Su <dereksu@qnap.com>
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> ---
> net/colo-compare.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 7cba573dae..3b72309d08 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -905,7 +905,7 @@ static void check_old_packet_regular(void *opaque)
>
> /* if have old packet we will notify checkpoint */
> colo_old_packet_check(s);
> - timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
> + timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_HOST) +
> s->expired_scan_cycle);
> }
>
> @@ -939,10 +939,10 @@ static void colo_compare_timer_init(CompareState *s)
> {
> AioContext *ctx = iothread_get_aio_context(s->iothread);
>
> - s->packet_check_timer = aio_timer_new(ctx, QEMU_CLOCK_VIRTUAL,
> + s->packet_check_timer = aio_timer_new(ctx, QEMU_CLOCK_HOST,
> SCALE_MS, check_old_packet_regular,
> s);
> - timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
> + timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_HOST) +
> s->expired_scan_cycle);
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] net/colo-compare.c: Add secondary old packet detection
2020-09-18 9:21 [PATCH 0/4] Several optimization and bugfix for COLO compare Zhang Chen
2020-09-18 9:22 ` [PATCH 1/4] net/colo-compare.c: Fix compare_timeout format issue Zhang Chen
2020-09-18 9:22 ` [PATCH 2/4] net/colo-compare.c: Change the timer clock type Zhang Chen
@ 2020-09-18 9:22 ` Zhang Chen
2020-09-22 6:46 ` Li Zhijian
2020-09-18 9:22 ` [PATCH 4/4] net/colo-compare.c: Increase default queued packet scan frequency Zhang Chen
3 siblings, 1 reply; 10+ messages in thread
From: Zhang Chen @ 2020-09-18 9:22 UTC (permalink / raw)
To: Jason Wang, qemu-dev; +Cc: Zhang Chen, Li Zhijian, Zhang Chen
From: Zhang Chen <chen.zhang@intel.com>
Detect queued secondary packet to sync VM state in time.
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
net/colo-compare.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 3b72309d08..f7271b976f 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -641,19 +641,26 @@ void colo_compare_unregister_notifier(Notifier *notify)
static int colo_old_packet_check_one_conn(Connection *conn,
CompareState *s)
{
- GList *result = NULL;
-
- result = g_queue_find_custom(&conn->primary_list,
- &s->compare_timeout,
- (GCompareFunc)colo_old_packet_check_one);
+ if (!g_queue_is_empty(&conn->primary_list)) {
+ if (g_queue_find_custom(&conn->primary_list,
+ &s->compare_timeout,
+ (GCompareFunc)colo_old_packet_check_one))
+ goto out;
+ }
- if (result) {
- /* Do checkpoint will flush old packet */
- colo_compare_inconsistency_notify(s);
- return 0;
+ if (!g_queue_is_empty(&conn->secondary_list)) {
+ if (g_queue_find_custom(&conn->secondary_list,
+ &s->compare_timeout,
+ (GCompareFunc)colo_old_packet_check_one))
+ goto out;
}
return 1;
+
+out:
+ /* Do checkpoint will flush old packet */
+ colo_compare_inconsistency_notify(s);
+ return 0;
}
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/4] net/colo-compare.c: Add secondary old packet detection
2020-09-18 9:22 ` [PATCH 3/4] net/colo-compare.c: Add secondary old packet detection Zhang Chen
@ 2020-09-22 6:46 ` Li Zhijian
2020-09-23 6:47 ` Zhang, Chen
0 siblings, 1 reply; 10+ messages in thread
From: Li Zhijian @ 2020-09-22 6:46 UTC (permalink / raw)
To: Zhang Chen, Jason Wang, qemu-dev; +Cc: Zhang Chen
On 9/18/20 5:22 PM, Zhang Chen wrote:
> From: Zhang Chen <chen.zhang@intel.com>
>
> Detect queued secondary packet to sync VM state in time.
>
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> ---
> net/colo-compare.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 3b72309d08..f7271b976f 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -641,19 +641,26 @@ void colo_compare_unregister_notifier(Notifier *notify)
> static int colo_old_packet_check_one_conn(Connection *conn,
> CompareState *s)
> {
> - GList *result = NULL;
> -
> - result = g_queue_find_custom(&conn->primary_list,
> - &s->compare_timeout,
> - (GCompareFunc)colo_old_packet_check_one);
> + if (!g_queue_is_empty(&conn->primary_list)) {
Looks we don't need to check is_empty
> + if (g_queue_find_custom(&conn->primary_list,
> + &s->compare_timeout,
> + (GCompareFunc)colo_old_packet_check_one))
> + goto out;
> + }
>
> - if (result) {
> - /* Do checkpoint will flush old packet */
> - colo_compare_inconsistency_notify(s);
> - return 0;
> + if (!g_queue_is_empty(&conn->secondary_list)) {
Ditto
Thanks
> + if (g_queue_find_custom(&conn->secondary_list,
> + &s->compare_timeout,
> + (GCompareFunc)colo_old_packet_check_one))
> + goto out;
> }
>
> return 1;
> +
> +out:
> + /* Do checkpoint will flush old packet */
> + colo_compare_inconsistency_notify(s);
> + return 0;
> }
>
> /*
^ permalink raw reply [flat|nested] 10+ messages in thread* RE: [PATCH 3/4] net/colo-compare.c: Add secondary old packet detection
2020-09-22 6:46 ` Li Zhijian
@ 2020-09-23 6:47 ` Zhang, Chen
2020-09-24 2:35 ` Li Zhijian
0 siblings, 1 reply; 10+ messages in thread
From: Zhang, Chen @ 2020-09-23 6:47 UTC (permalink / raw)
To: Li Zhijian, Jason Wang, qemu-dev; +Cc: Zhang Chen
> -----Original Message-----
> From: Li Zhijian <lizhijian@cn.fujitsu.com>
> Sent: Tuesday, September 22, 2020 2:47 PM
> To: Zhang, Chen <chen.zhang@intel.com>; Jason Wang
> <jasowang@redhat.com>; qemu-dev <qemu-devel@nongnu.org>
> Cc: Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH 3/4] net/colo-compare.c: Add secondary old packet
> detection
>
>
>
> On 9/18/20 5:22 PM, Zhang Chen wrote:
> > From: Zhang Chen <chen.zhang@intel.com>
> >
> > Detect queued secondary packet to sync VM state in time.
> >
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> > net/colo-compare.c | 25 ++++++++++++++++---------
> > 1 file changed, 16 insertions(+), 9 deletions(-)
> >
> > diff --git a/net/colo-compare.c b/net/colo-compare.c index
> > 3b72309d08..f7271b976f 100644
> > --- a/net/colo-compare.c
> > +++ b/net/colo-compare.c
> > @@ -641,19 +641,26 @@ void colo_compare_unregister_notifier(Notifier
> *notify)
> > static int colo_old_packet_check_one_conn(Connection *conn,
> > CompareState *s)
> > {
> > - GList *result = NULL;
> > -
> > - result = g_queue_find_custom(&conn->primary_list,
> > - &s->compare_timeout,
> > - (GCompareFunc)colo_old_packet_check_one);
> > + if (!g_queue_is_empty(&conn->primary_list)) {
> Looks we don't need to check is_empty
Re-checked glib code, it just checked the queue rather than inside content.
Maybe check empty before that will benefit performance.
Thanks
Zhang Chen
>
> > + if (g_queue_find_custom(&conn->primary_list,
> > + &s->compare_timeout,
> > + (GCompareFunc)colo_old_packet_check_one))
> > + goto out;
> > + }
> >
> > - if (result) {
> > - /* Do checkpoint will flush old packet */
> > - colo_compare_inconsistency_notify(s);
> > - return 0;
> > + if (!g_queue_is_empty(&conn->secondary_list)) {
> Ditto
>
> Thanks
> > + if (g_queue_find_custom(&conn->secondary_list,
> > + &s->compare_timeout,
> > + (GCompareFunc)colo_old_packet_check_one))
> > + goto out;
> > }
> >
> > return 1;
> > +
> > +out:
> > + /* Do checkpoint will flush old packet */
> > + colo_compare_inconsistency_notify(s);
> > + return 0;
> > }
> >
> > /*
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 3/4] net/colo-compare.c: Add secondary old packet detection
2020-09-23 6:47 ` Zhang, Chen
@ 2020-09-24 2:35 ` Li Zhijian
0 siblings, 0 replies; 10+ messages in thread
From: Li Zhijian @ 2020-09-24 2:35 UTC (permalink / raw)
To: Zhang, Chen, Jason Wang, qemu-dev; +Cc: Zhang Chen
On 9/23/20 2:47 PM, Zhang, Chen wrote:
>
>> -----Original Message-----
>> From: Li Zhijian <lizhijian@cn.fujitsu.com>
>> Sent: Tuesday, September 22, 2020 2:47 PM
>> To: Zhang, Chen <chen.zhang@intel.com>; Jason Wang
>> <jasowang@redhat.com>; qemu-dev <qemu-devel@nongnu.org>
>> Cc: Zhang Chen <zhangckid@gmail.com>
>> Subject: Re: [PATCH 3/4] net/colo-compare.c: Add secondary old packet
>> detection
>>
>>
>>
>> On 9/18/20 5:22 PM, Zhang Chen wrote:
>>> From: Zhang Chen <chen.zhang@intel.com>
>>>
>>> Detect queued secondary packet to sync VM state in time.
>>>
>>> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
>>> ---
>>> net/colo-compare.c | 25 ++++++++++++++++---------
>>> 1 file changed, 16 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/net/colo-compare.c b/net/colo-compare.c index
>>> 3b72309d08..f7271b976f 100644
>>> --- a/net/colo-compare.c
>>> +++ b/net/colo-compare.c
>>> @@ -641,19 +641,26 @@ void colo_compare_unregister_notifier(Notifier
>> *notify)
>>> static int colo_old_packet_check_one_conn(Connection *conn,
>>> CompareState *s)
>>> {
>>> - GList *result = NULL;
>>> -
>>> - result = g_queue_find_custom(&conn->primary_list,
>>> - &s->compare_timeout,
>>> - (GCompareFunc)colo_old_packet_check_one);
>>> + if (!g_queue_is_empty(&conn->primary_list)) {
>> Looks we don't need to check is_empty
> Re-checked glib code, it just checked the queue rather than inside content.
> Maybe check empty before that will benefit performance.
Yeah, you are right
Reviewed-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Thank
>
> Thanks
> Zhang Chen
>
>>> + if (g_queue_find_custom(&conn->primary_list,
>>> + &s->compare_timeout,
>>> + (GCompareFunc)colo_old_packet_check_one))
>>> + goto out;
>>> + }
>>>
>>> - if (result) {
>>> - /* Do checkpoint will flush old packet */
>>> - colo_compare_inconsistency_notify(s);
>>> - return 0;
>>> + if (!g_queue_is_empty(&conn->secondary_list)) {
>> Ditto
>>
>> Thanks
>>> + if (g_queue_find_custom(&conn->secondary_list,
>>> + &s->compare_timeout,
>>> + (GCompareFunc)colo_old_packet_check_one))
>>> + goto out;
>>> }
>>>
>>> return 1;
>>> +
>>> +out:
>>> + /* Do checkpoint will flush old packet */
>>> + colo_compare_inconsistency_notify(s);
>>> + return 0;
>>> }
>>>
>>> /*
>>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] net/colo-compare.c: Increase default queued packet scan frequency
2020-09-18 9:21 [PATCH 0/4] Several optimization and bugfix for COLO compare Zhang Chen
` (2 preceding siblings ...)
2020-09-18 9:22 ` [PATCH 3/4] net/colo-compare.c: Add secondary old packet detection Zhang Chen
@ 2020-09-18 9:22 ` Zhang Chen
3 siblings, 0 replies; 10+ messages in thread
From: Zhang Chen @ 2020-09-18 9:22 UTC (permalink / raw)
To: Jason Wang, qemu-dev; +Cc: Zhang Chen, Li Zhijian, Zhang Chen
From: Zhang Chen <chen.zhang@intel.com>
In my test, use this default parameter looks better.
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
net/colo-compare.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index f7271b976f..6df6a7e66f 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -52,7 +52,7 @@ static NotifierList colo_compare_notifiers =
#define COLO_COMPARE_FREE_PRIMARY 0x01
#define COLO_COMPARE_FREE_SECONDARY 0x02
-#define REGULAR_PACKET_CHECK_MS 3000
+#define REGULAR_PACKET_CHECK_MS 1000
#define DEFAULT_TIME_OUT_MS 3000
/* #define DEBUG_COLO_PACKETS */
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread