diff mbox

[COLO-Frame,(Base),v21,05/17] COLO: Establish a new communicating path for COLO

Message ID 1476792613-11712-6-git-send-email-zhang.zhanghailiang@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhanghailiang Oct. 18, 2016, 12:10 p.m. UTC
This new communication path will be used for returning messages
from Secondary side to Primary side.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
v13:
- Remove useless error report
v12:
- Add Reviewed-by tag
v11:
- Rebase master to use qemu_file_get_return_path() for opening return path
v10:
- fix the the error log (Dave's suggestion).
---
 migration/colo.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Amit Shah Oct. 26, 2016, 5:06 a.m. UTC | #1
On (Tue) 18 Oct 2016 [20:10:01], zhanghailiang wrote:
> This new communication path will be used for returning messages
> from Secondary side to Primary side.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Amit Shah <amit.shah@redhat.com>

> @@ -63,8 +75,24 @@ void *colo_process_incoming_thread(void *opaque)
>      migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
>                        MIGRATION_STATUS_COLO);
>  
> +    mis->to_src_file = qemu_file_get_return_path(mis->from_src_file);
> +    if (!mis->to_src_file) {
> +        error_report("COLO incoming thread: Open QEMUFile to_src_file failed");
> +        goto out;
> +    }
> +    /*
> +     * Note: We set the fd to unblocked in migration incoming coroutine,
> +     * But here we are in the COLO incoming thread, so it is ok to set the
> +     * fd back to blocked.
> +     */
> +    qemu_file_set_blocking(mis->from_src_file, true);

Why does it need to be blocking?

		Amit
Zhanghailiang Oct. 26, 2016, 2:05 p.m. UTC | #2
On 2016/10/26 13:06, Amit Shah wrote:
> On (Tue) 18 Oct 2016 [20:10:01], zhanghailiang wrote:
>> This new communication path will be used for returning messages
>> from Secondary side to Primary side.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> Reviewed-by: Amit Shah <amit.shah@redhat.com>
>
>> @@ -63,8 +75,24 @@ void *colo_process_incoming_thread(void *opaque)
>>       migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
>>                         MIGRATION_STATUS_COLO);
>>
>> +    mis->to_src_file = qemu_file_get_return_path(mis->from_src_file);
>> +    if (!mis->to_src_file) {
>> +        error_report("COLO incoming thread: Open QEMUFile to_src_file failed");
>> +        goto out;
>> +    }
>> +    /*
>> +     * Note: We set the fd to unblocked in migration incoming coroutine,
>> +     * But here we are in the COLO incoming thread, so it is ok to set the
>> +     * fd back to blocked.
>> +     */
>> +    qemu_file_set_blocking(mis->from_src_file, true);
>
> Why does it need to be blocking?
>

Because, the communication/action between Primary side and Secondary side should be
sequential. Just as postcopy does. :)

Thanks,
hailiang

> 		Amit
>
> .
>
Amit Shah Oct. 27, 2016, 3:57 a.m. UTC | #3
On (Wed) 26 Oct 2016 [22:05:22], Hailiang Zhang wrote:
> On 2016/10/26 13:06, Amit Shah wrote:
> >On (Tue) 18 Oct 2016 [20:10:01], zhanghailiang wrote:
> >>This new communication path will be used for returning messages
> >>from Secondary side to Primary side.
> >>
> >>Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> >>Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> >>Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >
> >Reviewed-by: Amit Shah <amit.shah@redhat.com>
> >
> >>@@ -63,8 +75,24 @@ void *colo_process_incoming_thread(void *opaque)
> >>      migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
> >>                        MIGRATION_STATUS_COLO);
> >>
> >>+    mis->to_src_file = qemu_file_get_return_path(mis->from_src_file);
> >>+    if (!mis->to_src_file) {
> >>+        error_report("COLO incoming thread: Open QEMUFile to_src_file failed");
> >>+        goto out;
> >>+    }
> >>+    /*
> >>+     * Note: We set the fd to unblocked in migration incoming coroutine,
> >>+     * But here we are in the COLO incoming thread, so it is ok to set the
> >>+     * fd back to blocked.
> >>+     */
> >>+    qemu_file_set_blocking(mis->from_src_file, true);
> >
> >Why does it need to be blocking?
> >
> 
> Because, the communication/action between Primary side and Secondary side should be
> sequential. Just as postcopy does. :)

Yea - I mean please include that in the comment too so it's obvious
why it's done.


		Amit
Zhanghailiang Oct. 27, 2016, 6:06 a.m. UTC | #4
On 2016/10/27 11:57, Amit Shah wrote:
> On (Wed) 26 Oct 2016 [22:05:22], Hailiang Zhang wrote:
>> On 2016/10/26 13:06, Amit Shah wrote:
>>> On (Tue) 18 Oct 2016 [20:10:01], zhanghailiang wrote:
>>>> This new communication path will be used for returning messages
>>> >from Secondary side to Primary side.
>>>>
>>>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>>>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>>
>>> Reviewed-by: Amit Shah <amit.shah@redhat.com>
>>>
>>>> @@ -63,8 +75,24 @@ void *colo_process_incoming_thread(void *opaque)
>>>>       migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
>>>>                         MIGRATION_STATUS_COLO);
>>>>
>>>> +    mis->to_src_file = qemu_file_get_return_path(mis->from_src_file);
>>>> +    if (!mis->to_src_file) {
>>>> +        error_report("COLO incoming thread: Open QEMUFile to_src_file failed");
>>>> +        goto out;
>>>> +    }
>>>> +    /*
>>>> +     * Note: We set the fd to unblocked in migration incoming coroutine,
>>>> +     * But here we are in the COLO incoming thread, so it is ok to set the
>>>> +     * fd back to blocked.
>>>> +     */
>>>> +    qemu_file_set_blocking(mis->from_src_file, true);
>>>
>>> Why does it need to be blocking?
>>>
>>
>> Because, the communication/action between Primary side and Secondary side should be
>> sequential. Just as postcopy does. :)
>
> Yea - I mean please include that in the comment too so it's obvious
> why it's done.
>

OK, i will add this in next version, thanks.

>
> 		Amit
>
> .
>
diff mbox

Patch

diff --git a/migration/colo.c b/migration/colo.c
index 968cd51..7c5769b 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -14,6 +14,7 @@ 
 #include "sysemu/sysemu.h"
 #include "migration/colo.h"
 #include "trace.h"
+#include "qemu/error-report.h"
 
 bool colo_supported(void)
 {
@@ -36,6 +37,12 @@  bool migration_incoming_in_colo_state(void)
 
 static void colo_process_checkpoint(MigrationState *s)
 {
+    s->rp_state.from_dst_file = qemu_file_get_return_path(s->to_dst_file);
+    if (!s->rp_state.from_dst_file) {
+        error_report("Open QEMUFile from_dst_file failed");
+        goto out;
+    }
+
     qemu_mutex_lock_iothread();
     vm_start();
     qemu_mutex_unlock_iothread();
@@ -43,8 +50,13 @@  static void colo_process_checkpoint(MigrationState *s)
 
     /* TODO: COLO checkpoint savevm loop */
 
+out:
     migrate_set_state(&s->state, MIGRATION_STATUS_COLO,
                       MIGRATION_STATUS_COMPLETED);
+
+    if (s->rp_state.from_dst_file) {
+        qemu_fclose(s->rp_state.from_dst_file);
+    }
 }
 
 void migrate_start_colo_process(MigrationState *s)
@@ -63,8 +75,24 @@  void *colo_process_incoming_thread(void *opaque)
     migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
                       MIGRATION_STATUS_COLO);
 
+    mis->to_src_file = qemu_file_get_return_path(mis->from_src_file);
+    if (!mis->to_src_file) {
+        error_report("COLO incoming thread: Open QEMUFile to_src_file failed");
+        goto out;
+    }
+    /*
+     * Note: We set the fd to unblocked in migration incoming coroutine,
+     * But here we are in the COLO incoming thread, so it is ok to set the
+     * fd back to blocked.
+     */
+    qemu_file_set_blocking(mis->from_src_file, true);
+
     /* TODO: COLO checkpoint restore loop */
 
+out:
+    if (mis->to_src_file) {
+        qemu_fclose(mis->to_src_file);
+    }
     migration_incoming_exit_colo();
 
     return NULL;