| Message ID | 20240509033106.1321880-2-lizhijian@fujitsu.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] migration/colo: Minor fix for colo error message | expand |
On Thu, May 09, 2024 at 11:31:05AM +0800, Li Zhijian via wrote: > Currently, it always returns 0, no need to check the return value at all. > In addition, enter colo coroutine only if migration_incoming_colo_enabled() > is true. > Once the destination side enters the COLO* state, the COLO process will > take over the remaining processes until COLO exits. > > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> Reviewed-by: Peter Xu <peterx@redhat.com>
> -----Original Message----- > From: Li Zhijian <lizhijian@fujitsu.com> > Sent: Thursday, May 9, 2024 11:31 AM > To: Peter Xu <peterx@redhat.com>; Fabiano Rosas <farosas@suse.de> > Cc: Zhang, Hailiang <zhanghailiang@xfusion.com>; qemu- > devel@nongnu.org; Zhang, Chen <chen.zhang@intel.com>; Li Zhijian > <lizhijian@fujitsu.com> > Subject: [PATCH 2/3] migration/colo: make colo_incoming_co() return void > > Currently, it always returns 0, no need to check the return value at all. > In addition, enter colo coroutine only if migration_incoming_colo_enabled() is > true. > Once the destination side enters the COLO* state, the COLO process will take > over the remaining processes until COLO exits. > > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> Reviewed-by: Zhang Chen <chen.zhang@intel.com> > --- > migration/colo.c | 9 ++------- > migration/migration.c | 6 +++--- > 2 files changed, 5 insertions(+), 10 deletions(-) > > diff --git a/migration/colo.c b/migration/colo.c index 5600a43d78..991806c06a > 100644 > --- a/migration/colo.c > +++ b/migration/colo.c > @@ -929,16 +929,13 @@ out: > return NULL; > } > > -int coroutine_fn colo_incoming_co(void) > +void coroutine_fn colo_incoming_co(void) > { > MigrationIncomingState *mis = migration_incoming_get_current(); > QemuThread th; > > assert(bql_locked()); > - > - if (!migration_incoming_colo_enabled()) { > - return 0; > - } > + assert(migration_incoming_colo_enabled()); > > qemu_thread_create(&th, "COLO incoming", > colo_process_incoming_thread, > mis, QEMU_THREAD_JOINABLE); @@ -954,6 +951,4 @@ int > coroutine_fn colo_incoming_co(void) > > /* We hold the global BQL, so it is safe here */ > colo_release_ram_cache(); > - > - return 0; > } > diff --git a/migration/migration.c b/migration/migration.c index > b4a09c561c..6dc1f3bab4 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -789,9 +789,9 @@ process_incoming_migration_co(void *opaque) > goto fail; > } > > - if (colo_incoming_co() < 0) { > - error_setg(&local_err, "colo incoming failed"); > - goto fail; > + if (migration_incoming_colo_enabled()) { > + /* yield until COLO exit */ > + colo_incoming_co(); > } > > migration_bh_schedule(process_incoming_migration_bh, mis); > -- > 2.31.1
Li Zhijian via <qemu-devel@nongnu.org> writes: > Currently, it always returns 0, no need to check the return value at all. > In addition, enter colo coroutine only if migration_incoming_colo_enabled() > is true. > Once the destination side enters the COLO* state, the COLO process will > take over the remaining processes until COLO exits. > > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> > --- > migration/colo.c | 9 ++------- > migration/migration.c | 6 +++--- > 2 files changed, 5 insertions(+), 10 deletions(-) > > diff --git a/migration/colo.c b/migration/colo.c > index 5600a43d78..991806c06a 100644 > --- a/migration/colo.c > +++ b/migration/colo.c > @@ -929,16 +929,13 @@ out: > return NULL; > } > > -int coroutine_fn colo_incoming_co(void) > +void coroutine_fn colo_incoming_co(void) > { > MigrationIncomingState *mis = migration_incoming_get_current(); > QemuThread th; > > assert(bql_locked()); > - > - if (!migration_incoming_colo_enabled()) { > - return 0; > - } > + assert(migration_incoming_colo_enabled()); FAILED: libcommon.fa.p/migration_colo.c.o /usr/bin/gcc-13 ... ../migration/colo.c ../migration/colo.c:930:19: error: conflicting types for ‘colo_incoming_co’; have ‘void(void)’ 930 | void coroutine_fn colo_incoming_co(void) | ^~~~~~~~~~~~~~~~ In file included from ../migration/colo.c:20: ... qemu/include/migration/colo.h:52:18: note: previous declaration of ‘colo_incoming_co’ with type ‘int(void)’ 52 | int coroutine_fn colo_incoming_co(void); | ^~~~~~~~~~~~~~~~
On 16/05/2024 03:04, Fabiano Rosas wrote: > Li Zhijian via <qemu-devel@nongnu.org> writes: > >> Currently, it always returns 0, no need to check the return value at all. >> In addition, enter colo coroutine only if migration_incoming_colo_enabled() >> is true. >> Once the destination side enters the COLO* state, the COLO process will >> take over the remaining processes until COLO exits. >> >> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> >> --- >> migration/colo.c | 9 ++------- >> migration/migration.c | 6 +++--- >> 2 files changed, 5 insertions(+), 10 deletions(-) >> >> diff --git a/migration/colo.c b/migration/colo.c >> index 5600a43d78..991806c06a 100644 >> --- a/migration/colo.c >> +++ b/migration/colo.c >> @@ -929,16 +929,13 @@ out: >> return NULL; >> } >> >> -int coroutine_fn colo_incoming_co(void) >> +void coroutine_fn colo_incoming_co(void) >> { >> MigrationIncomingState *mis = migration_incoming_get_current(); >> QemuThread th; >> >> assert(bql_locked()); >> - >> - if (!migration_incoming_colo_enabled()) { >> - return 0; >> - } >> + assert(migration_incoming_colo_enabled()); > > FAILED: libcommon.fa.p/migration_colo.c.o > /usr/bin/gcc-13 ... ../migration/colo.c > ../migration/colo.c:930:19: error: conflicting types for ‘colo_incoming_co’; have ‘void(void)’ > 930 | void coroutine_fn colo_incoming_co(void) > | ^~~~~~~~~~~~~~~~ > In file included from ../migration/colo.c:20: > ... qemu/include/migration/colo.h:52:18: note: previous declaration of ‘colo_incoming_co’ with type ‘int(void)’ > 52 | int coroutine_fn colo_incoming_co(void); My fault, will fix it soon Thanks Zhijian > | ^~~~~~~~~~~~~~~~
diff --git a/migration/colo.c b/migration/colo.c index 5600a43d78..991806c06a 100644 --- a/migration/colo.c +++ b/migration/colo.c @@ -929,16 +929,13 @@ out: return NULL; } -int coroutine_fn colo_incoming_co(void) +void coroutine_fn colo_incoming_co(void) { MigrationIncomingState *mis = migration_incoming_get_current(); QemuThread th; assert(bql_locked()); - - if (!migration_incoming_colo_enabled()) { - return 0; - } + assert(migration_incoming_colo_enabled()); qemu_thread_create(&th, "COLO incoming", colo_process_incoming_thread, mis, QEMU_THREAD_JOINABLE); @@ -954,6 +951,4 @@ int coroutine_fn colo_incoming_co(void) /* We hold the global BQL, so it is safe here */ colo_release_ram_cache(); - - return 0; } diff --git a/migration/migration.c b/migration/migration.c index b4a09c561c..6dc1f3bab4 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -789,9 +789,9 @@ process_incoming_migration_co(void *opaque) goto fail; } - if (colo_incoming_co() < 0) { - error_setg(&local_err, "colo incoming failed"); - goto fail; + if (migration_incoming_colo_enabled()) { + /* yield until COLO exit */ + colo_incoming_co(); } migration_bh_schedule(process_incoming_migration_bh, mis);
Currently, it always returns 0, no need to check the return value at all. In addition, enter colo coroutine only if migration_incoming_colo_enabled() is true. Once the destination side enters the COLO* state, the COLO process will take over the remaining processes until COLO exits. Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> --- migration/colo.c | 9 ++------- migration/migration.c | 6 +++--- 2 files changed, 5 insertions(+), 10 deletions(-)