* [PATCH] migration: Optimization about wait-unplug migration state
@ 2020-02-04 5:08 Keqian Zhu
2020-02-04 9:14 ` Juan Quintela
2020-02-05 14:40 ` Jens Freimann
0 siblings, 2 replies; 5+ messages in thread
From: Keqian Zhu @ 2020-02-04 5:08 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Dr. David Alan Gilbert, qemu-arm, wanghaibin.wang,
jfreimann, Keqian Zhu
qemu_savevm_nr_failover_devices() is originally designed to
get the number of failover devices, but it actually returns
the number of "unplug-pending" failover devices now. Moreover,
what drives migration state to wait-unplug should be the number
of "unplug-pending" failover devices, not all failover devices.
We can also notice that qemu_savevm_state_guest_unplug_pending()
and qemu_savevm_nr_failover_devices() is equivalent almost (from
the code view). So the latter is incorrect semantically and
useless, just delete it.
In the qemu_savevm_state_guest_unplug_pending(), once hit a
unplug-pending failover device, then it can return true right
now to save cpu time.
Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
---
Cc: jfreimann@redhat.com
Cc: Juan Quintela <quintela@redhat.com>
Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
---
migration/migration.c | 2 +-
migration/savevm.c | 24 +++---------------------
migration/savevm.h | 1 -
3 files changed, 4 insertions(+), 23 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 3a21a4686c..deedc968cf 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3333,7 +3333,7 @@ static void *migration_thread(void *opaque)
qemu_savevm_state_setup(s->to_dst_file);
- if (qemu_savevm_nr_failover_devices()) {
+ if (qemu_savevm_state_guest_unplug_pending()) {
migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
MIGRATION_STATUS_WAIT_UNPLUG);
diff --git a/migration/savevm.c b/migration/savevm.c
index f19cb9ec7a..1d4220ece8 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1140,36 +1140,18 @@ void qemu_savevm_state_header(QEMUFile *f)
}
}
-int qemu_savevm_nr_failover_devices(void)
+bool qemu_savevm_state_guest_unplug_pending(void)
{
SaveStateEntry *se;
- int n = 0;
QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
if (se->vmsd && se->vmsd->dev_unplug_pending &&
se->vmsd->dev_unplug_pending(se->opaque)) {
- n++;
- }
- }
-
- return n;
-}
-
-bool qemu_savevm_state_guest_unplug_pending(void)
-{
- SaveStateEntry *se;
- int n = 0;
-
- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
- if (!se->vmsd || !se->vmsd->dev_unplug_pending) {
- continue;
- }
- if (se->vmsd->dev_unplug_pending(se->opaque)) {
- n++;
+ return true;
}
}
- return n > 0;
+ return false;
}
void qemu_savevm_state_setup(QEMUFile *f)
diff --git a/migration/savevm.h b/migration/savevm.h
index c42b9c80ee..ba64a7e271 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -31,7 +31,6 @@
bool qemu_savevm_state_blocked(Error **errp);
void qemu_savevm_state_setup(QEMUFile *f);
-int qemu_savevm_nr_failover_devices(void);
bool qemu_savevm_state_guest_unplug_pending(void);
int qemu_savevm_state_resume_prepare(MigrationState *s);
void qemu_savevm_state_header(QEMUFile *f);
--
2.19.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] migration: Optimization about wait-unplug migration state
2020-02-04 5:08 [PATCH] migration: Optimization about wait-unplug migration state Keqian Zhu
@ 2020-02-04 9:14 ` Juan Quintela
2020-02-10 6:14 ` zhukeqian
2020-02-05 14:40 ` Jens Freimann
1 sibling, 1 reply; 5+ messages in thread
From: Juan Quintela @ 2020-02-04 9:14 UTC (permalink / raw)
To: Keqian Zhu
Cc: jfreimann, wanghaibin.wang, qemu-arm, qemu-devel,
Dr. David Alan Gilbert
Keqian Zhu <zhukeqian1@huawei.com> wrote:
> qemu_savevm_nr_failover_devices() is originally designed to
> get the number of failover devices, but it actually returns
> the number of "unplug-pending" failover devices now. Moreover,
> what drives migration state to wait-unplug should be the number
> of "unplug-pending" failover devices, not all failover devices.
>
> We can also notice that qemu_savevm_state_guest_unplug_pending()
> and qemu_savevm_nr_failover_devices() is equivalent almost (from
> the code view). So the latter is incorrect semantically and
> useless, just delete it.
>
> In the qemu_savevm_state_guest_unplug_pending(), once hit a
> unplug-pending failover device, then it can return true right
> now to save cpu time.
>
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
For my reading you are right:
- 1st function naming is not right
- 2nd function achieves exactly the same effect
I will wait until Jens says anything, but then I will queue it.
Thanks, Juan.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] migration: Optimization about wait-unplug migration state
2020-02-04 5:08 [PATCH] migration: Optimization about wait-unplug migration state Keqian Zhu
2020-02-04 9:14 ` Juan Quintela
@ 2020-02-05 14:40 ` Jens Freimann
2020-02-10 6:14 ` zhukeqian
1 sibling, 1 reply; 5+ messages in thread
From: Jens Freimann @ 2020-02-05 14:40 UTC (permalink / raw)
To: Keqian Zhu
Cc: wanghaibin.wang, qemu-arm, qemu-devel, Dr. David Alan Gilbert,
Juan Quintela
On Tue, Feb 04, 2020 at 01:08:41PM +0800, Keqian Zhu wrote:
>qemu_savevm_nr_failover_devices() is originally designed to
>get the number of failover devices, but it actually returns
>the number of "unplug-pending" failover devices now. Moreover,
>what drives migration state to wait-unplug should be the number
>of "unplug-pending" failover devices, not all failover devices.
>
>We can also notice that qemu_savevm_state_guest_unplug_pending()
>and qemu_savevm_nr_failover_devices() is equivalent almost (from
>the code view). So the latter is incorrect semantically and
>useless, just delete it.
>
>In the qemu_savevm_state_guest_unplug_pending(), once hit a
>unplug-pending failover device, then it can return true right
>now to save cpu time.
>
>Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
>---
>Cc: jfreimann@redhat.com
>Cc: Juan Quintela <quintela@redhat.com>
>Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>---
> migration/migration.c | 2 +-
> migration/savevm.c | 24 +++---------------------
> migration/savevm.h | 1 -
> 3 files changed, 4 insertions(+), 23 deletions(-)
>
>diff --git a/migration/migration.c b/migration/migration.c
>index 3a21a4686c..deedc968cf 100644
>--- a/migration/migration.c
>+++ b/migration/migration.c
>@@ -3333,7 +3333,7 @@ static void *migration_thread(void *opaque)
>
> qemu_savevm_state_setup(s->to_dst_file);
>
>- if (qemu_savevm_nr_failover_devices()) {
>+ if (qemu_savevm_state_guest_unplug_pending()) {
> migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> MIGRATION_STATUS_WAIT_UNPLUG);
>
>diff --git a/migration/savevm.c b/migration/savevm.c
>index f19cb9ec7a..1d4220ece8 100644
>--- a/migration/savevm.c
>+++ b/migration/savevm.c
>@@ -1140,36 +1140,18 @@ void qemu_savevm_state_header(QEMUFile *f)
> }
> }
>
>-int qemu_savevm_nr_failover_devices(void)
>+bool qemu_savevm_state_guest_unplug_pending(void)
> {
> SaveStateEntry *se;
>- int n = 0;
>
> QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> if (se->vmsd && se->vmsd->dev_unplug_pending &&
> se->vmsd->dev_unplug_pending(se->opaque)) {
>- n++;
>- }
>- }
>-
>- return n;
>-}
>-
>-bool qemu_savevm_state_guest_unplug_pending(void)
>-{
>- SaveStateEntry *se;
>- int n = 0;
>-
>- QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>- if (!se->vmsd || !se->vmsd->dev_unplug_pending) {
>- continue;
>- }
>- if (se->vmsd->dev_unplug_pending(se->opaque)) {
>- n++;
>+ return true;
> }
> }
>
>- return n > 0;
>+ return false;
> }
>
> void qemu_savevm_state_setup(QEMUFile *f)
>diff --git a/migration/savevm.h b/migration/savevm.h
>index c42b9c80ee..ba64a7e271 100644
>--- a/migration/savevm.h
>+++ b/migration/savevm.h
>@@ -31,7 +31,6 @@
>
> bool qemu_savevm_state_blocked(Error **errp);
> void qemu_savevm_state_setup(QEMUFile *f);
>-int qemu_savevm_nr_failover_devices(void);
> bool qemu_savevm_state_guest_unplug_pending(void);
> int qemu_savevm_state_resume_prepare(MigrationState *s);
> void qemu_savevm_state_header(QEMUFile *f);
>--
>2.19.1
Looks good to me. I tested it and it still works, so
Tested-by: Jens Freimann <jfreimann@redhat.com>
Reviewed-by: Jens Freimann <jfreimann@redhat.com>
regards
Jens
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] migration: Optimization about wait-unplug migration state
2020-02-04 9:14 ` Juan Quintela
@ 2020-02-10 6:14 ` zhukeqian
0 siblings, 0 replies; 5+ messages in thread
From: zhukeqian @ 2020-02-10 6:14 UTC (permalink / raw)
To: quintela
Cc: jfreimann, wanghaibin.wang, qemu-arm, qemu-devel,
Dr. David Alan Gilbert
On 2020/2/4 17:14, Juan Quintela wrote:
> Keqian Zhu <zhukeqian1@huawei.com> wrote:
>> qemu_savevm_nr_failover_devices() is originally designed to
>> get the number of failover devices, but it actually returns
>> the number of "unplug-pending" failover devices now. Moreover,
>> what drives migration state to wait-unplug should be the number
>> of "unplug-pending" failover devices, not all failover devices.
>>
>> We can also notice that qemu_savevm_state_guest_unplug_pending()
>> and qemu_savevm_nr_failover_devices() is equivalent almost (from
>> the code view). So the latter is incorrect semantically and
>> useless, just delete it.
>>
>> In the qemu_savevm_state_guest_unplug_pending(), once hit a
>> unplug-pending failover device, then it can return true right
>> now to save cpu time.
>>
>> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
>
> For my reading you are right:
> - 1st function naming is not right
> - 2nd function achieves exactly the same effect
>
> I will wait until Jens says anything, but then I will queue it.
>
> Thanks, Juan.
>
>
> .
>
Thanks,
Keqian.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] migration: Optimization about wait-unplug migration state
2020-02-05 14:40 ` Jens Freimann
@ 2020-02-10 6:14 ` zhukeqian
0 siblings, 0 replies; 5+ messages in thread
From: zhukeqian @ 2020-02-10 6:14 UTC (permalink / raw)
To: Jens Freimann
Cc: wanghaibin.wang, qemu-arm, qemu-devel, Dr. David Alan Gilbert,
Juan Quintela
On 2020/2/5 22:40, Jens Freimann wrote:
> On Tue, Feb 04, 2020 at 01:08:41PM +0800, Keqian Zhu wrote:
>> qemu_savevm_nr_failover_devices() is originally designed to
>> get the number of failover devices, but it actually returns
>> the number of "unplug-pending" failover devices now. Moreover,
>> what drives migration state to wait-unplug should be the number
>> of "unplug-pending" failover devices, not all failover devices.
>>
>> We can also notice that qemu_savevm_state_guest_unplug_pending()
>> and qemu_savevm_nr_failover_devices() is equivalent almost (from
>> the code view). So the latter is incorrect semantically and
>> useless, just delete it.
>>
>> In the qemu_savevm_state_guest_unplug_pending(), once hit a
>> unplug-pending failover device, then it can return true right
>> now to save cpu time.
>>
>> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
>> ---
>> Cc: jfreimann@redhat.com
>> Cc: Juan Quintela <quintela@redhat.com>
>> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> ---
>> migration/migration.c | 2 +-
>> migration/savevm.c | 24 +++---------------------
>> migration/savevm.h | 1 -
>> 3 files changed, 4 insertions(+), 23 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 3a21a4686c..deedc968cf 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -3333,7 +3333,7 @@ static void *migration_thread(void *opaque)
>>
>> qemu_savevm_state_setup(s->to_dst_file);
>>
>> - if (qemu_savevm_nr_failover_devices()) {
>> + if (qemu_savevm_state_guest_unplug_pending()) {
>> migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
>> MIGRATION_STATUS_WAIT_UNPLUG);
>>
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index f19cb9ec7a..1d4220ece8 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -1140,36 +1140,18 @@ void qemu_savevm_state_header(QEMUFile *f)
>> }
>> }
>>
>> -int qemu_savevm_nr_failover_devices(void)
>> +bool qemu_savevm_state_guest_unplug_pending(void)
>> {
>> SaveStateEntry *se;
>> - int n = 0;
>>
>> QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> if (se->vmsd && se->vmsd->dev_unplug_pending &&
>> se->vmsd->dev_unplug_pending(se->opaque)) {
>> - n++;
>> - }
>> - }
>> -
>> - return n;
>> -}
>> -
>> -bool qemu_savevm_state_guest_unplug_pending(void)
>> -{
>> - SaveStateEntry *se;
>> - int n = 0;
>> -
>> - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> - if (!se->vmsd || !se->vmsd->dev_unplug_pending) {
>> - continue;
>> - }
>> - if (se->vmsd->dev_unplug_pending(se->opaque)) {
>> - n++;
>> + return true;
>> }
>> }
>>
>> - return n > 0;
>> + return false;
>> }
>>
>> void qemu_savevm_state_setup(QEMUFile *f)
>> diff --git a/migration/savevm.h b/migration/savevm.h
>> index c42b9c80ee..ba64a7e271 100644
>> --- a/migration/savevm.h
>> +++ b/migration/savevm.h
>> @@ -31,7 +31,6 @@
>>
>> bool qemu_savevm_state_blocked(Error **errp);
>> void qemu_savevm_state_setup(QEMUFile *f);
>> -int qemu_savevm_nr_failover_devices(void);
>> bool qemu_savevm_state_guest_unplug_pending(void);
>> int qemu_savevm_state_resume_prepare(MigrationState *s);
>> void qemu_savevm_state_header(QEMUFile *f);
>> --
>> 2.19.1
>
> Looks good to me. I tested it and it still works, so
> Tested-by: Jens Freimann <jfreimann@redhat.com>
> Reviewed-by: Jens Freimann <jfreimann@redhat.com>
> regards
> Jens
>
>
> .
>
Thanks,
Keqian.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-02-10 6:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-04 5:08 [PATCH] migration: Optimization about wait-unplug migration state Keqian Zhu
2020-02-04 9:14 ` Juan Quintela
2020-02-10 6:14 ` zhukeqian
2020-02-05 14:40 ` Jens Freimann
2020-02-10 6:14 ` zhukeqian
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).