diff mbox

[02/15] colo-compare: implement the process of checkpoint

Message ID 58F4A12C.5070404@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhanghailiang April 17, 2017, 11:04 a.m. UTC
Hi Jason,

On 2017/4/14 14:38, Jason Wang wrote:
>
> On 2017年04月14日 14:22, Hailiang Zhang wrote:
>> Hi Jason,
>>
>> On 2017/4/14 13:57, Jason Wang wrote:
>>> On 2017年02月22日 17:31, Zhang Chen wrote:
>>>> On 02/22/2017 11:42 AM, zhanghailiang wrote:
>>>>> While do checkpoint, we need to flush all the unhandled packets,
>>>>> By using the filter notifier mechanism, we can easily to notify
>>>>> every compare object to do this process, which runs inside
>>>>> of compare threads as a coroutine.
>>>> Hi~ Jason and Hailiang.
>>>>
>>>> I will send a patch set later about colo-compare notify mechanism for
>>>> Xen like this patch.
>>>> I want to add a new chardev socket way in colo-comapre connect to Xen
>>>> colo, for notify
>>>> checkpoint or failover, Because We have no choice to use this way
>>>> communicate with Xen codes.
>>>> That's means we will have two notify mechanism.
>>>> What do you think about this?
>>>>
>>>>
>>>> Thanks
>>>> Zhang Chen
>>> I was thinking the possibility of using similar way to for colo compare.
>>> E.g can we use socket? This can saves duplicated codes more or less.
>> Since there are too many sockets used by filter and COLO, (Two unix
>> sockets and two
>>   tcp sockets for each vNIC), I don't want to introduce more ;) , but
>> i'm not sure if it is
>> possible to make it more flexible and optional, abstract these
>> duplicated codes,
>> pass the opened fd (No matter eventfd or socket fd ) as parameter, for
>> example.
>> Is this way acceptable ?
>>
>> Thanks,
>> Hailiang
> Yes, that's kind of what I want. We don't want to use two message
> format. Passing a opened fd need management support, we still need a
> fallback if there's no management on top. For qemu/kvm, we can do all
> stuffs transparent to the cli by e.g socketpair() or others, but the key
> is to have a unified message format.

After a deeper investigation, i think we can re-use most codes, since there is no
existing way to notify xen (no ?), we still needs notify chardev socket (Be used to notify xen, it is optional.)
(http://patchwork.ozlabs.org/patch/733431/ "COLO-compare: Add Xen notify chardev socket handler frame")

Besides, there is an existing qmp comand 'xen-colo-do-checkpoint', we can re-use it to notify
colo-compare objects and other filter objects to do checkpoint, for the opposite direction, we use
the notify chardev socket (Only for xen).

So the codes will be like:

How about this scenario ?

> Thoughts?
>
> Thanks
>
>>> Thanks
>>>
>>>
>>> .
>>>
>>
>
> .
>

Comments

Zhang Chen April 18, 2017, 1:32 a.m. UTC | #1
On 04/17/2017 07:04 PM, Hailiang Zhang wrote:
> Hi Jason,
>
> On 2017/4/14 14:38, Jason Wang wrote:
>>
>> On 2017年04月14日 14:22, Hailiang Zhang wrote:
>>> Hi Jason,
>>>
>>> On 2017/4/14 13:57, Jason Wang wrote:
>>>> On 2017年02月22日 17:31, Zhang Chen wrote:
>>>>> On 02/22/2017 11:42 AM, zhanghailiang wrote:
>>>>>> While do checkpoint, we need to flush all the unhandled packets,
>>>>>> By using the filter notifier mechanism, we can easily to notify
>>>>>> every compare object to do this process, which runs inside
>>>>>> of compare threads as a coroutine.
>>>>> Hi~ Jason and Hailiang.
>>>>>
>>>>> I will send a patch set later about colo-compare notify mechanism for
>>>>> Xen like this patch.
>>>>> I want to add a new chardev socket way in colo-comapre connect to Xen
>>>>> colo, for notify
>>>>> checkpoint or failover, Because We have no choice to use this way
>>>>> communicate with Xen codes.
>>>>> That's means we will have two notify mechanism.
>>>>> What do you think about this?
>>>>>
>>>>>
>>>>> Thanks
>>>>> Zhang Chen
>>>> I was thinking the possibility of using similar way to for colo 
>>>> compare.
>>>> E.g can we use socket? This can saves duplicated codes more or less.
>>> Since there are too many sockets used by filter and COLO, (Two unix
>>> sockets and two
>>>   tcp sockets for each vNIC), I don't want to introduce more ;) , but
>>> i'm not sure if it is
>>> possible to make it more flexible and optional, abstract these
>>> duplicated codes,
>>> pass the opened fd (No matter eventfd or socket fd ) as parameter, for
>>> example.
>>> Is this way acceptable ?
>>>
>>> Thanks,
>>> Hailiang
>> Yes, that's kind of what I want. We don't want to use two message
>> format. Passing a opened fd need management support, we still need a
>> fallback if there's no management on top. For qemu/kvm, we can do all
>> stuffs transparent to the cli by e.g socketpair() or others, but the key
>> is to have a unified message format.
>
> After a deeper investigation, i think we can re-use most codes, since 
> there is no
> existing way to notify xen (no ?), we still needs notify chardev 
> socket (Be used to notify xen, it is optional.)
> (http://patchwork.ozlabs.org/patch/733431/ "COLO-compare: Add Xen 
> notify chardev socket handler frame")
>
> Besides, there is an existing qmp comand 'xen-colo-do-checkpoint', we 
> can re-use it to notify
> colo-compare objects and other filter objects to do checkpoint, for 
> the opposite direction, we use
> the notify chardev socket (Only for xen).
>
> So the codes will be like:
> diff --git a/migration/colo.c b/migration/colo.c
> index 91da936..813c281 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -224,7 +224,19 @@ ReplicationStatus 
> *qmp_query_xen_replication_status(Error **errp)
>
>  void qmp_xen_colo_do_checkpoint(Error **errp)
>  {
> +    Error *local_err = NULL;
> +
>      replication_do_checkpoint_all(errp);
> +    /* Notify colo-compare and other filters to do checkpoint */
> +    colo_notify_compares_event(NULL, COLO_CHECKPOINT, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +    colo_notify_filters_event(COLO_CHECKPOINT, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +    }
>  }
>
>  static void colo_send_message(QEMUFile *f, COLOMessage msg,
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 24e13f0..de975c5 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -391,6 +391,9 @@ static void colo_compare_inconsistent_notify(void)
>  {
>      notifier_list_notify(&colo_compare_notifiers,
>                  migrate_get_current());
> +    if (s->notify_dev) {
> +       /* Do something, notify remote side through notify dev */
> +    }
>  }
>
>  void colo_compare_register_notifier(Notifier *notify)
>
> How about this scenario ?

I agree this way, maybe rename qmp_xen_colo_do_checkpoint()
to qmp_remote_colo_do_checkpoint() is more generic.

Thanks
Zhang Chen

>
>> Thoughts?
>>
>> Thanks
>>
>>>> Thanks
>>>>
>>>>
>>>> .
>>>>
>>>
>>
>> .
>>
>
>
>
>
> .
>
Jason Wang April 18, 2017, 3:55 a.m. UTC | #2
On 2017年04月17日 19:04, Hailiang Zhang wrote:
> Hi Jason,
>
> On 2017/4/14 14:38, Jason Wang wrote:
>>
>> On 2017年04月14日 14:22, Hailiang Zhang wrote:
>>> Hi Jason,
>>>
>>> On 2017/4/14 13:57, Jason Wang wrote:
>>>> On 2017年02月22日 17:31, Zhang Chen wrote:
>>>>> On 02/22/2017 11:42 AM, zhanghailiang wrote:
>>>>>> While do checkpoint, we need to flush all the unhandled packets,
>>>>>> By using the filter notifier mechanism, we can easily to notify
>>>>>> every compare object to do this process, which runs inside
>>>>>> of compare threads as a coroutine.
>>>>> Hi~ Jason and Hailiang.
>>>>>
>>>>> I will send a patch set later about colo-compare notify mechanism for
>>>>> Xen like this patch.
>>>>> I want to add a new chardev socket way in colo-comapre connect to Xen
>>>>> colo, for notify
>>>>> checkpoint or failover, Because We have no choice to use this way
>>>>> communicate with Xen codes.
>>>>> That's means we will have two notify mechanism.
>>>>> What do you think about this?
>>>>>
>>>>>
>>>>> Thanks
>>>>> Zhang Chen
>>>> I was thinking the possibility of using similar way to for colo 
>>>> compare.
>>>> E.g can we use socket? This can saves duplicated codes more or less.
>>> Since there are too many sockets used by filter and COLO, (Two unix
>>> sockets and two
>>>   tcp sockets for each vNIC), I don't want to introduce more ;) , but
>>> i'm not sure if it is
>>> possible to make it more flexible and optional, abstract these
>>> duplicated codes,
>>> pass the opened fd (No matter eventfd or socket fd ) as parameter, for
>>> example.
>>> Is this way acceptable ?
>>>
>>> Thanks,
>>> Hailiang
>> Yes, that's kind of what I want. We don't want to use two message
>> format. Passing a opened fd need management support, we still need a
>> fallback if there's no management on top. For qemu/kvm, we can do all
>> stuffs transparent to the cli by e.g socketpair() or others, but the key
>> is to have a unified message format.
>
> After a deeper investigation, i think we can re-use most codes, since 
> there is no
> existing way to notify xen (no ?), we still needs notify chardev 
> socket (Be used to notify xen, it is optional.)
> (http://patchwork.ozlabs.org/patch/733431/ "COLO-compare: Add Xen 
> notify chardev socket handler frame")

Yes and actually you can use this for bi-directional communication. The 
only differences is the implementation of comparing.

>
> Besides, there is an existing qmp comand 'xen-colo-do-checkpoint', 

I don't see this in master?

> we can re-use it to notify
> colo-compare objects and other filter objects to do checkpoint, for 
> the opposite direction, we use
> the notify chardev socket (Only for xen).

Just want to make sure I understand the design, who will trigger this 
command? Management?

Can we just use the socket?

>
> So the codes will be like:
> diff --git a/migration/colo.c b/migration/colo.c
> index 91da936..813c281 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -224,7 +224,19 @@ ReplicationStatus 
> *qmp_query_xen_replication_status(Error **errp)
>
>  void qmp_xen_colo_do_checkpoint(Error **errp)
>  {
> +    Error *local_err = NULL;
> +
>      replication_do_checkpoint_all(errp);
> +    /* Notify colo-compare and other filters to do checkpoint */
> +    colo_notify_compares_event(NULL, COLO_CHECKPOINT, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +    colo_notify_filters_event(COLO_CHECKPOINT, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +    }
>  }
>
>  static void colo_send_message(QEMUFile *f, COLOMessage msg,
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 24e13f0..de975c5 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -391,6 +391,9 @@ static void colo_compare_inconsistent_notify(void)
>  {
>      notifier_list_notify(&colo_compare_notifiers,
>                  migrate_get_current());
> +    if (s->notify_dev) {
> +       /* Do something, notify remote side through notify dev */
> +    }
>  }
>
>  void colo_compare_register_notifier(Notifier *notify)
>
> How about this scenario ?

See my reply above, and we need unify the message format too. Raw string 
is ok but we'd better have something like TLV or others.

Thanks

>
>> Thoughts?
>>
>> Thanks
>>
>>>> Thanks
>>>>
>>>>
>>>> .
>>>>
>>>
>>
>> .
>>
>
>
>
Zhanghailiang April 18, 2017, 6:58 a.m. UTC | #3
On 2017/4/18 11:55, Jason Wang wrote:
>
> On 2017年04月17日 19:04, Hailiang Zhang wrote:
>> Hi Jason,
>>
>> On 2017/4/14 14:38, Jason Wang wrote:
>>> On 2017年04月14日 14:22, Hailiang Zhang wrote:
>>>> Hi Jason,
>>>>
>>>> On 2017/4/14 13:57, Jason Wang wrote:
>>>>> On 2017年02月22日 17:31, Zhang Chen wrote:
>>>>>> On 02/22/2017 11:42 AM, zhanghailiang wrote:
>>>>>>> While do checkpoint, we need to flush all the unhandled packets,
>>>>>>> By using the filter notifier mechanism, we can easily to notify
>>>>>>> every compare object to do this process, which runs inside
>>>>>>> of compare threads as a coroutine.
>>>>>> Hi~ Jason and Hailiang.
>>>>>>
>>>>>> I will send a patch set later about colo-compare notify mechanism for
>>>>>> Xen like this patch.
>>>>>> I want to add a new chardev socket way in colo-comapre connect to Xen
>>>>>> colo, for notify
>>>>>> checkpoint or failover, Because We have no choice to use this way
>>>>>> communicate with Xen codes.
>>>>>> That's means we will have two notify mechanism.
>>>>>> What do you think about this?
>>>>>>
>>>>>>
>>>>>> Thanks
>>>>>> Zhang Chen
>>>>> I was thinking the possibility of using similar way to for colo
>>>>> compare.
>>>>> E.g can we use socket? This can saves duplicated codes more or less.
>>>> Since there are too many sockets used by filter and COLO, (Two unix
>>>> sockets and two
>>>>    tcp sockets for each vNIC), I don't want to introduce more ;) , but
>>>> i'm not sure if it is
>>>> possible to make it more flexible and optional, abstract these
>>>> duplicated codes,
>>>> pass the opened fd (No matter eventfd or socket fd ) as parameter, for
>>>> example.
>>>> Is this way acceptable ?
>>>>
>>>> Thanks,
>>>> Hailiang
>>> Yes, that's kind of what I want. We don't want to use two message
>>> format. Passing a opened fd need management support, we still need a
>>> fallback if there's no management on top. For qemu/kvm, we can do all
>>> stuffs transparent to the cli by e.g socketpair() or others, but the key
>>> is to have a unified message format.
>> After a deeper investigation, i think we can re-use most codes, since
>> there is no
>> existing way to notify xen (no ?), we still needs notify chardev
>> socket (Be used to notify xen, it is optional.)
>> (http://patchwork.ozlabs.org/patch/733431/ "COLO-compare: Add Xen
>> notify chardev socket handler frame")
> Yes and actually you can use this for bi-directional communication. The
> only differences is the implementation of comparing.
>
>> Besides, there is an existing qmp comand 'xen-colo-do-checkpoint',
> I don't see this in master?

Er, it has been merged already, please see migration/colo.c, void qmp_xen_colo_do_checkpoint(Error **errp);

>> we can re-use it to notify
>> colo-compare objects and other filter objects to do checkpoint, for
>> the opposite direction, we use
>> the notify chardev socket (Only for xen).
> Just want to make sure I understand the design, who will trigger this
> command? Management?

The command will be issued by XEN (xc_save ?), the original existing xen-colo-do-checkpoint
command now only be used to notify block replication to do checkpoint, we can re-use it for filters too.

> Can we just use the socket?

I don't quite understand ...
Just as the codes showed bellow, in this scenario,
XEN notifies colo-compare and fiters do checkpoint by using qmp command,
and colo-compare notifies XEN about net inconsistency event by using the new socket.

>> So the codes will be like:
>> diff --git a/migration/colo.c b/migration/colo.c
>> index 91da936..813c281 100644
>> --- a/migration/colo.c
>> +++ b/migration/colo.c
>> @@ -224,7 +224,19 @@ ReplicationStatus
>> *qmp_query_xen_replication_status(Error **errp)
>>
>>   void qmp_xen_colo_do_checkpoint(Error **errp)
>>   {
>> +    Error *local_err = NULL;
>> +
>>       replication_do_checkpoint_all(errp);
>> +    /* Notify colo-compare and other filters to do checkpoint */
>> +    colo_notify_compares_event(NULL, COLO_CHECKPOINT, &local_err);
>> +    if (local_err) {
>> +        error_propagate(errp, local_err);
>> +        return;
>> +    }
>> +    colo_notify_filters_event(COLO_CHECKPOINT, &local_err);
>> +    if (local_err) {
>> +        error_propagate(errp, local_err);
>> +    }
>>   }
>>
>>   static void colo_send_message(QEMUFile *f, COLOMessage msg,
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index 24e13f0..de975c5 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -391,6 +391,9 @@ static void colo_compare_inconsistent_notify(void)
>>   {
>>       notifier_list_notify(&colo_compare_notifiers,
>>                   migrate_get_current());

KVM will use this notifier/callback way, and in this way, we can avoid the redundant socket.

>> +    if (s->notify_dev) {
>> +       /* Do something, notify remote side through notify dev */
>> +    }
>>   }

If we have a notify socket configured, we will send the message about net inconsistent event.

>>
>>   void colo_compare_register_notifier(Notifier *notify)
>>
>> How about this scenario ?
> See my reply above, and we need unify the message format too. Raw string
> is ok but we'd better have something like TLV or others.

Agreed, we need it to be more standard.

Thanks,
Hailiang

> Thanks
>
>>> Thoughts?
>>>
>>> Thanks
>>>
>>>>> Thanks
>>>>>
>>>>>
>>>>> .
>>>>>
>>> .
>>>
>>
>>
>
> .
>
Jason Wang April 20, 2017, 5:15 a.m. UTC | #4
On 2017年04月18日 14:58, Hailiang Zhang wrote:
> On 2017/4/18 11:55, Jason Wang wrote:
>>
>> On 2017年04月17日 19:04, Hailiang Zhang wrote:
>>> Hi Jason,
>>>
>>> On 2017/4/14 14:38, Jason Wang wrote:
>>>> On 2017年04月14日 14:22, Hailiang Zhang wrote:
>>>>> Hi Jason,
>>>>>
>>>>> On 2017/4/14 13:57, Jason Wang wrote:
>>>>>> On 2017年02月22日 17:31, Zhang Chen wrote:
>>>>>>> On 02/22/2017 11:42 AM, zhanghailiang wrote:
>>>>>>>> While do checkpoint, we need to flush all the unhandled packets,
>>>>>>>> By using the filter notifier mechanism, we can easily to notify
>>>>>>>> every compare object to do this process, which runs inside
>>>>>>>> of compare threads as a coroutine.
>>>>>>> Hi~ Jason and Hailiang.
>>>>>>>
>>>>>>> I will send a patch set later about colo-compare notify 
>>>>>>> mechanism for
>>>>>>> Xen like this patch.
>>>>>>> I want to add a new chardev socket way in colo-comapre connect 
>>>>>>> to Xen
>>>>>>> colo, for notify
>>>>>>> checkpoint or failover, Because We have no choice to use this way
>>>>>>> communicate with Xen codes.
>>>>>>> That's means we will have two notify mechanism.
>>>>>>> What do you think about this?
>>>>>>>
>>>>>>>
>>>>>>> Thanks
>>>>>>> Zhang Chen
>>>>>> I was thinking the possibility of using similar way to for colo
>>>>>> compare.
>>>>>> E.g can we use socket? This can saves duplicated codes more or less.
>>>>> Since there are too many sockets used by filter and COLO, (Two unix
>>>>> sockets and two
>>>>>    tcp sockets for each vNIC), I don't want to introduce more ;) , 
>>>>> but
>>>>> i'm not sure if it is
>>>>> possible to make it more flexible and optional, abstract these
>>>>> duplicated codes,
>>>>> pass the opened fd (No matter eventfd or socket fd ) as parameter, 
>>>>> for
>>>>> example.
>>>>> Is this way acceptable ?
>>>>>
>>>>> Thanks,
>>>>> Hailiang
>>>> Yes, that's kind of what I want. We don't want to use two message
>>>> format. Passing a opened fd need management support, we still need a
>>>> fallback if there's no management on top. For qemu/kvm, we can do all
>>>> stuffs transparent to the cli by e.g socketpair() or others, but 
>>>> the key
>>>> is to have a unified message format.
>>> After a deeper investigation, i think we can re-use most codes, since
>>> there is no
>>> existing way to notify xen (no ?), we still needs notify chardev
>>> socket (Be used to notify xen, it is optional.)
>>> (http://patchwork.ozlabs.org/patch/733431/ "COLO-compare: Add Xen
>>> notify chardev socket handler frame")
>> Yes and actually you can use this for bi-directional communication. The
>> only differences is the implementation of comparing.
>>
>>> Besides, there is an existing qmp comand 'xen-colo-do-checkpoint',
>> I don't see this in master?
>
> Er, it has been merged already, please see migration/colo.c, void 
> qmp_xen_colo_do_checkpoint(Error **errp);

Aha, I see. Thanks.

>
>>> we can re-use it to notify
>>> colo-compare objects and other filter objects to do checkpoint, for
>>> the opposite direction, we use
>>> the notify chardev socket (Only for xen).
>> Just want to make sure I understand the design, who will trigger this
>> command? Management?
>
> The command will be issued by XEN (xc_save ?), the original existing 
> xen-colo-do-checkpoint
> command now only be used to notify block replication to do checkpoint, 
> we can re-use it for filters too.

So it was called by management. For KVM case, we probably don't need 
this since the comparing thread are under control of qemu.

>
>> Can we just use the socket?
>
> I don't quite understand ...
> Just as the codes showed bellow, in this scenario,
> XEN notifies colo-compare and fiters do checkpoint by using qmp command,

Yes, that's just what I mean. Technically Xen can use socket to do this too.

Thanks

> and colo-compare notifies XEN about net inconsistency event by using 
> the new socket.
>
>>> So the codes will be like:
>>> diff --git a/migration/colo.c b/migration/colo.c
>>> index 91da936..813c281 100644
>>> --- a/migration/colo.c
>>> +++ b/migration/colo.c
>>> @@ -224,7 +224,19 @@ ReplicationStatus
>>> *qmp_query_xen_replication_status(Error **errp)
>>>
>>>   void qmp_xen_colo_do_checkpoint(Error **errp)
>>>   {
>>> +    Error *local_err = NULL;
>>> +
>>>       replication_do_checkpoint_all(errp);
>>> +    /* Notify colo-compare and other filters to do checkpoint */
>>> +    colo_notify_compares_event(NULL, COLO_CHECKPOINT, &local_err);
>>> +    if (local_err) {
>>> +        error_propagate(errp, local_err);
>>> +        return;
>>> +    }
>>> +    colo_notify_filters_event(COLO_CHECKPOINT, &local_err);
>>> +    if (local_err) {
>>> +        error_propagate(errp, local_err);
>>> +    }
>>>   }
>>>
>>>   static void colo_send_message(QEMUFile *f, COLOMessage msg,
>>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>>> index 24e13f0..de975c5 100644
>>> --- a/net/colo-compare.c
>>> +++ b/net/colo-compare.c
>>> @@ -391,6 +391,9 @@ static void colo_compare_inconsistent_notify(void)
>>>   {
>>>       notifier_list_notify(&colo_compare_notifiers,
>>>                   migrate_get_current());
>
> KVM will use this notifier/callback way, and in this way, we can avoid 
> the redundant socket.
>
>>> +    if (s->notify_dev) {
>>> +       /* Do something, notify remote side through notify dev */
>>> +    }
>>>   }
>
> If we have a notify socket configured, we will send the message about 
> net inconsistent event.
>
>>>
>>>   void colo_compare_register_notifier(Notifier *notify)
>>>
>>> How about this scenario ?
>> See my reply above, and we need unify the message format too. Raw string
>> is ok but we'd better have something like TLV or others.
>
> Agreed, we need it to be more standard.
>
> Thanks,
> Hailiang
>
>> Thanks
>>
>>>> Thoughts?
>>>>
>>>> Thanks
>>>>
>>>>>> Thanks
>>>>>>
>>>>>>
>>>>>> .
>>>>>>
>>>> .
>>>>
>>>
>>>
>>
>> .
>>
>
>
Zhanghailiang April 21, 2017, 8:10 a.m. UTC | #5
On 2017/4/20 13:15, Jason Wang wrote:
>
> On 2017年04月18日 14:58, Hailiang Zhang wrote:
>> On 2017/4/18 11:55, Jason Wang wrote:
>>> On 2017年04月17日 19:04, Hailiang Zhang wrote:
>>>> Hi Jason,
>>>>
>>>> On 2017/4/14 14:38, Jason Wang wrote:
>>>>> On 2017年04月14日 14:22, Hailiang Zhang wrote:
>>>>>> Hi Jason,
>>>>>>
>>>>>> On 2017/4/14 13:57, Jason Wang wrote:
>>>>>>> On 2017年02月22日 17:31, Zhang Chen wrote:
>>>>>>>> On 02/22/2017 11:42 AM, zhanghailiang wrote:
>>>>>>>>> While do checkpoint, we need to flush all the unhandled packets,
>>>>>>>>> By using the filter notifier mechanism, we can easily to notify
>>>>>>>>> every compare object to do this process, which runs inside
>>>>>>>>> of compare threads as a coroutine.
>>>>>>>> Hi~ Jason and Hailiang.
>>>>>>>>
>>>>>>>> I will send a patch set later about colo-compare notify
>>>>>>>> mechanism for
>>>>>>>> Xen like this patch.
>>>>>>>> I want to add a new chardev socket way in colo-comapre connect
>>>>>>>> to Xen
>>>>>>>> colo, for notify
>>>>>>>> checkpoint or failover, Because We have no choice to use this way
>>>>>>>> communicate with Xen codes.
>>>>>>>> That's means we will have two notify mechanism.
>>>>>>>> What do you think about this?
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Zhang Chen
>>>>>>> I was thinking the possibility of using similar way to for colo
>>>>>>> compare.
>>>>>>> E.g can we use socket? This can saves duplicated codes more or less.
>>>>>> Since there are too many sockets used by filter and COLO, (Two unix
>>>>>> sockets and two
>>>>>>     tcp sockets for each vNIC), I don't want to introduce more ;) ,
>>>>>> but
>>>>>> i'm not sure if it is
>>>>>> possible to make it more flexible and optional, abstract these
>>>>>> duplicated codes,
>>>>>> pass the opened fd (No matter eventfd or socket fd ) as parameter,
>>>>>> for
>>>>>> example.
>>>>>> Is this way acceptable ?
>>>>>>
>>>>>> Thanks,
>>>>>> Hailiang
>>>>> Yes, that's kind of what I want. We don't want to use two message
>>>>> format. Passing a opened fd need management support, we still need a
>>>>> fallback if there's no management on top. For qemu/kvm, we can do all
>>>>> stuffs transparent to the cli by e.g socketpair() or others, but
>>>>> the key
>>>>> is to have a unified message format.
>>>> After a deeper investigation, i think we can re-use most codes, since
>>>> there is no
>>>> existing way to notify xen (no ?), we still needs notify chardev
>>>> socket (Be used to notify xen, it is optional.)
>>>> (http://patchwork.ozlabs.org/patch/733431/ "COLO-compare: Add Xen
>>>> notify chardev socket handler frame")
>>> Yes and actually you can use this for bi-directional communication. The
>>> only differences is the implementation of comparing.
>>>
>>>> Besides, there is an existing qmp comand 'xen-colo-do-checkpoint',
>>> I don't see this in master?
>> Er, it has been merged already, please see migration/colo.c, void
>> qmp_xen_colo_do_checkpoint(Error **errp);
> Aha, I see. Thanks.

;)

>>>> we can re-use it to notify
>>>> colo-compare objects and other filter objects to do checkpoint, for
>>>> the opposite direction, we use
>>>> the notify chardev socket (Only for xen).
>>> Just want to make sure I understand the design, who will trigger this
>>> command? Management?
>> The command will be issued by XEN (xc_save ?), the original existing
>> xen-colo-do-checkpoint
>> command now only be used to notify block replication to do checkpoint,
>> we can re-use it for filters too.
> So it was called by management. For KVM case, we probably don't need
> this since the comparing thread are under control of qemu.

Yes, you are right.

>>> Can we just use the socket?
>> I don't quite understand ...
>> Just as the codes showed bellow, in this scenario,
>> XEN notifies colo-compare and fiters do checkpoint by using qmp command,
> Yes, that's just what I mean. Technically Xen can use socket to do this too.

Yes, great, since we have come to an agreement on the scenario, I'll update this series.

Thanks,
Hailiang.

> Thanks
>
>> and colo-compare notifies XEN about net inconsistency event by using
>> the new socket.
>>
>>>> So the codes will be like:
>>>> diff --git a/migration/colo.c b/migration/colo.c
>>>> index 91da936..813c281 100644
>>>> --- a/migration/colo.c
>>>> +++ b/migration/colo.c
>>>> @@ -224,7 +224,19 @@ ReplicationStatus
>>>> *qmp_query_xen_replication_status(Error **errp)
>>>>
>>>>    void qmp_xen_colo_do_checkpoint(Error **errp)
>>>>    {
>>>> +    Error *local_err = NULL;
>>>> +
>>>>        replication_do_checkpoint_all(errp);
>>>> +    /* Notify colo-compare and other filters to do checkpoint */
>>>> +    colo_notify_compares_event(NULL, COLO_CHECKPOINT, &local_err);
>>>> +    if (local_err) {
>>>> +        error_propagate(errp, local_err);
>>>> +        return;
>>>> +    }
>>>> +    colo_notify_filters_event(COLO_CHECKPOINT, &local_err);
>>>> +    if (local_err) {
>>>> +        error_propagate(errp, local_err);
>>>> +    }
>>>>    }
>>>>
>>>>    static void colo_send_message(QEMUFile *f, COLOMessage msg,
>>>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>>>> index 24e13f0..de975c5 100644
>>>> --- a/net/colo-compare.c
>>>> +++ b/net/colo-compare.c
>>>> @@ -391,6 +391,9 @@ static void colo_compare_inconsistent_notify(void)
>>>>    {
>>>>        notifier_list_notify(&colo_compare_notifiers,
>>>>                    migrate_get_current());
>> KVM will use this notifier/callback way, and in this way, we can avoid
>> the redundant socket.
>>
>>>> +    if (s->notify_dev) {
>>>> +       /* Do something, notify remote side through notify dev */
>>>> +    }
>>>>    }
>> If we have a notify socket configured, we will send the message about
>> net inconsistent event.
>>
>>>>    void colo_compare_register_notifier(Notifier *notify)
>>>>
>>>> How about this scenario ?
>>> See my reply above, and we need unify the message format too. Raw string
>>> is ok but we'd better have something like TLV or others.
>> Agreed, we need it to be more standard.
>>
>> Thanks,
>> Hailiang
>>
>>> Thanks
>>>
>>>>> Thoughts?
>>>>>
>>>>> Thanks
>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>>>
>>>>>>> .
>>>>>>>
>>>>> .
>>>>>
>>>>
>>> .
>>>
>>
>
> .
>
diff mbox

Patch

diff --git a/migration/colo.c b/migration/colo.c
index 91da936..813c281 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -224,7 +224,19 @@  ReplicationStatus *qmp_query_xen_replication_status(Error **errp)

  void qmp_xen_colo_do_checkpoint(Error **errp)
  {
+    Error *local_err = NULL;
+
      replication_do_checkpoint_all(errp);
+    /* Notify colo-compare and other filters to do checkpoint */
+    colo_notify_compares_event(NULL, COLO_CHECKPOINT, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    colo_notify_filters_event(COLO_CHECKPOINT, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+    }
  }

  static void colo_send_message(QEMUFile *f, COLOMessage msg,
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 24e13f0..de975c5 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -391,6 +391,9 @@  static void colo_compare_inconsistent_notify(void)
  {
      notifier_list_notify(&colo_compare_notifiers,
                  migrate_get_current());
+    if (s->notify_dev) {
+       /* Do something, notify remote side through notify dev */
+    }
  }

  void colo_compare_register_notifier(Notifier *notify)