* [PATCH 1/4] migration/ram: only possible bit set in invalid_flags is RAM_SAVE_FLAG_COMPRESS_PAGE
2019-10-05 22:05 [PATCH 0/4] migration: trivial cleanup and refine Wei Yang
@ 2019-10-05 22:05 ` Wei Yang
2019-10-08 17:38 ` Dr. David Alan Gilbert
2019-10-05 22:05 ` [PATCH 2/4] migration/postcopy: fix typo in mark_postcopy_blocktime_begin's comment Wei Yang
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Wei Yang @ 2019-10-05 22:05 UTC (permalink / raw)
To: quintela, dgilbert; +Cc: qemu-devel, Wei Yang
The only possible bit set in invalid_flags is
RAM_SAVE_FLAG_COMPRESS_PAGE at the beginning of function
ram_load_precopy(), which means it is not necessary to do
another check for RAM_SAVE_FLAG_COMPRESS_PAGE bit.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/ram.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 31051935c8..769d3f6454 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4263,10 +4263,7 @@ static int ram_load_precopy(QEMUFile *f)
addr &= TARGET_PAGE_MASK;
if (flags & invalid_flags) {
- if (flags & invalid_flags & RAM_SAVE_FLAG_COMPRESS_PAGE) {
- error_report("Received an unexpected compressed page");
- }
-
+ error_report("Received an unexpected compressed page");
ret = -EINVAL;
break;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] migration/ram: only possible bit set in invalid_flags is RAM_SAVE_FLAG_COMPRESS_PAGE
2019-10-05 22:05 ` [PATCH 1/4] migration/ram: only possible bit set in invalid_flags is RAM_SAVE_FLAG_COMPRESS_PAGE Wei Yang
@ 2019-10-08 17:38 ` Dr. David Alan Gilbert
2019-10-09 1:11 ` Wei Yang
0 siblings, 1 reply; 11+ messages in thread
From: Dr. David Alan Gilbert @ 2019-10-08 17:38 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, quintela
* Wei Yang (richardw.yang@linux.intel.com) wrote:
> The only possible bit set in invalid_flags is
> RAM_SAVE_FLAG_COMPRESS_PAGE at the beginning of function
> ram_load_precopy(), which means it is not necessary to do
> another check for RAM_SAVE_FLAG_COMPRESS_PAGE bit.
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
> migration/ram.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 31051935c8..769d3f6454 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -4263,10 +4263,7 @@ static int ram_load_precopy(QEMUFile *f)
> addr &= TARGET_PAGE_MASK;
>
> if (flags & invalid_flags) {
> - if (flags & invalid_flags & RAM_SAVE_FLAG_COMPRESS_PAGE) {
> - error_report("Received an unexpected compressed page");
> - }
> -
> + error_report("Received an unexpected compressed page");
> ret = -EINVAL;
I'd rather keep this one; I think Juan's idea is that we might make
other flags illegal here and then it's easy to add to invalid_flags at
the top.
Dave
> break;
> }
> --
> 2.17.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] migration/ram: only possible bit set in invalid_flags is RAM_SAVE_FLAG_COMPRESS_PAGE
2019-10-08 17:38 ` Dr. David Alan Gilbert
@ 2019-10-09 1:11 ` Wei Yang
0 siblings, 0 replies; 11+ messages in thread
From: Wei Yang @ 2019-10-09 1:11 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: qemu-devel, Wei Yang, quintela
On Tue, Oct 08, 2019 at 06:38:25PM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.yang@linux.intel.com) wrote:
>> The only possible bit set in invalid_flags is
>> RAM_SAVE_FLAG_COMPRESS_PAGE at the beginning of function
>> ram_load_precopy(), which means it is not necessary to do
>> another check for RAM_SAVE_FLAG_COMPRESS_PAGE bit.
>>
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>> ---
>> migration/ram.c | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 31051935c8..769d3f6454 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -4263,10 +4263,7 @@ static int ram_load_precopy(QEMUFile *f)
>> addr &= TARGET_PAGE_MASK;
>>
>> if (flags & invalid_flags) {
>> - if (flags & invalid_flags & RAM_SAVE_FLAG_COMPRESS_PAGE) {
>> - error_report("Received an unexpected compressed page");
>> - }
>> -
>> + error_report("Received an unexpected compressed page");
>> ret = -EINVAL;
>
>I'd rather keep this one; I think Juan's idea is that we might make
>other flags illegal here and then it's easy to add to invalid_flags at
>the top.
>
Reasonable.
>Dave
>> break;
>> }
>> --
>> 2.17.1
>>
>--
>Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] migration/postcopy: fix typo in mark_postcopy_blocktime_begin's comment
2019-10-05 22:05 [PATCH 0/4] migration: trivial cleanup and refine Wei Yang
2019-10-05 22:05 ` [PATCH 1/4] migration/ram: only possible bit set in invalid_flags is RAM_SAVE_FLAG_COMPRESS_PAGE Wei Yang
@ 2019-10-05 22:05 ` Wei Yang
2019-10-07 10:08 ` Philippe Mathieu-Daudé
2019-10-05 22:05 ` [PATCH 3/4] migration: pass in_postcopy instead of check state again Wei Yang
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Wei Yang @ 2019-10-05 22:05 UTC (permalink / raw)
To: quintela, dgilbert; +Cc: qemu-devel, Wei Yang
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/postcopy-ram.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index d2bdd21ae3..a394c7c3a6 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -768,9 +768,11 @@ static void mark_postcopy_blocktime_begin(uintptr_t addr, uint32_t ptid,
atomic_xchg(&dc->page_fault_vcpu_time[cpu], low_time_offset);
atomic_xchg(&dc->vcpu_addr[cpu], addr);
- /* check it here, not at the begining of the function,
- * due to, check could accur early than bitmap_set in
- * qemu_ufd_copy_ioctl */
+ /*
+ * check it here, not at the beginning of the function,
+ * due to, check could occur early than bitmap_set in
+ * qemu_ufd_copy_ioctl
+ */
already_received = ramblock_recv_bitmap_test(rb, (void *)addr);
if (already_received) {
atomic_xchg(&dc->vcpu_addr[cpu], 0);
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] migration/postcopy: fix typo in mark_postcopy_blocktime_begin's comment
2019-10-05 22:05 ` [PATCH 2/4] migration/postcopy: fix typo in mark_postcopy_blocktime_begin's comment Wei Yang
@ 2019-10-07 10:08 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-10-07 10:08 UTC (permalink / raw)
To: Wei Yang, quintela, dgilbert; +Cc: qemu-devel
On 10/6/19 12:05 AM, Wei Yang wrote:
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
> migration/postcopy-ram.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index d2bdd21ae3..a394c7c3a6 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -768,9 +768,11 @@ static void mark_postcopy_blocktime_begin(uintptr_t addr, uint32_t ptid,
> atomic_xchg(&dc->page_fault_vcpu_time[cpu], low_time_offset);
> atomic_xchg(&dc->vcpu_addr[cpu], addr);
>
> - /* check it here, not at the begining of the function,
> - * due to, check could accur early than bitmap_set in
> - * qemu_ufd_copy_ioctl */
> + /*
> + * check it here, not at the beginning of the function,
> + * due to, check could occur early than bitmap_set in
> + * qemu_ufd_copy_ioctl
> + */
> already_received = ramblock_recv_bitmap_test(rb, (void *)addr);
> if (already_received) {
> atomic_xchg(&dc->vcpu_addr[cpu], 0);
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] migration: pass in_postcopy instead of check state again
2019-10-05 22:05 [PATCH 0/4] migration: trivial cleanup and refine Wei Yang
2019-10-05 22:05 ` [PATCH 1/4] migration/ram: only possible bit set in invalid_flags is RAM_SAVE_FLAG_COMPRESS_PAGE Wei Yang
2019-10-05 22:05 ` [PATCH 2/4] migration/postcopy: fix typo in mark_postcopy_blocktime_begin's comment Wei Yang
@ 2019-10-05 22:05 ` Wei Yang
2019-10-08 17:57 ` Dr. David Alan Gilbert
2019-10-05 22:05 ` [PATCH 4/4] migration: report SaveStateEntry id and name on failure Wei Yang
2019-10-11 13:32 ` [PATCH 0/4] migration: trivial cleanup and refine Dr. David Alan Gilbert
4 siblings, 1 reply; 11+ messages in thread
From: Wei Yang @ 2019-10-05 22:05 UTC (permalink / raw)
To: quintela, dgilbert; +Cc: qemu-devel, Wei Yang
Not necessary to do the check again.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/migration.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index c8eaa85867..56031305e3 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3148,8 +3148,7 @@ static MigIterateState migration_iteration_run(MigrationState *s)
return MIG_ITERATE_SKIP;
}
/* Just another iteration step */
- qemu_savevm_state_iterate(s->to_dst_file,
- s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE);
+ qemu_savevm_state_iterate(s->to_dst_file, in_postcopy);
} else {
trace_migration_thread_low_pending(pending_size);
migration_completion(s);
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] migration: pass in_postcopy instead of check state again
2019-10-05 22:05 ` [PATCH 3/4] migration: pass in_postcopy instead of check state again Wei Yang
@ 2019-10-08 17:57 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 11+ messages in thread
From: Dr. David Alan Gilbert @ 2019-10-08 17:57 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, quintela
* Wei Yang (richardw.yang@linux.intel.com) wrote:
> Not necessary to do the check again.
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/migration.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index c8eaa85867..56031305e3 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3148,8 +3148,7 @@ static MigIterateState migration_iteration_run(MigrationState *s)
> return MIG_ITERATE_SKIP;
> }
> /* Just another iteration step */
> - qemu_savevm_state_iterate(s->to_dst_file,
> - s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE);
> + qemu_savevm_state_iterate(s->to_dst_file, in_postcopy);
> } else {
> trace_migration_thread_low_pending(pending_size);
> migration_completion(s);
> --
> 2.17.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] migration: report SaveStateEntry id and name on failure
2019-10-05 22:05 [PATCH 0/4] migration: trivial cleanup and refine Wei Yang
` (2 preceding siblings ...)
2019-10-05 22:05 ` [PATCH 3/4] migration: pass in_postcopy instead of check state again Wei Yang
@ 2019-10-05 22:05 ` Wei Yang
2019-10-07 10:08 ` Philippe Mathieu-Daudé
2019-10-11 13:32 ` [PATCH 0/4] migration: trivial cleanup and refine Dr. David Alan Gilbert
4 siblings, 1 reply; 11+ messages in thread
From: Wei Yang @ 2019-10-05 22:05 UTC (permalink / raw)
To: quintela, dgilbert; +Cc: qemu-devel, Wei Yang
This provides helpful information on which entry failed.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/savevm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/migration/savevm.c b/migration/savevm.c
index 9f0122583d..feb757de79 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1215,6 +1215,8 @@ int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy)
save_section_footer(f, se);
if (ret < 0) {
+ error_report("failed to save SaveStateEntry with id(name): %d(%s)",
+ se->section_id, se->idstr);
qemu_file_set_error(f, ret);
}
if (ret <= 0) {
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] migration: report SaveStateEntry id and name on failure
2019-10-05 22:05 ` [PATCH 4/4] migration: report SaveStateEntry id and name on failure Wei Yang
@ 2019-10-07 10:08 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-10-07 10:08 UTC (permalink / raw)
To: Wei Yang, quintela, dgilbert; +Cc: qemu-devel
On 10/6/19 12:05 AM, Wei Yang wrote:
> This provides helpful information on which entry failed.
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
> migration/savevm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 9f0122583d..feb757de79 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1215,6 +1215,8 @@ int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy)
> save_section_footer(f, se);
>
> if (ret < 0) {
> + error_report("failed to save SaveStateEntry with id(name): %d(%s)",
> + se->section_id, se->idstr);
> qemu_file_set_error(f, ret);
> }
> if (ret <= 0) {
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] migration: trivial cleanup and refine
2019-10-05 22:05 [PATCH 0/4] migration: trivial cleanup and refine Wei Yang
` (3 preceding siblings ...)
2019-10-05 22:05 ` [PATCH 4/4] migration: report SaveStateEntry id and name on failure Wei Yang
@ 2019-10-11 13:32 ` Dr. David Alan Gilbert
4 siblings, 0 replies; 11+ messages in thread
From: Dr. David Alan Gilbert @ 2019-10-11 13:32 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, quintela
* Wei Yang (richardw.yang@linux.intel.com) wrote:
> This patch set contains several cleanup and refine for migration.
>
> simplify some calculation
> reuse the result
> fix typo in comment
> provide error message when failed
2,3,4 queued
>
> Wei Yang (4):
> migration/ram: only possible bit set in invalid_flags is
> RAM_SAVE_FLAG_COMPRESS_PAGE
> migration/postcopy: fix typo in mark_postcopy_blocktime_begin's
> comment
> migration: pass in_postcopy instead of check state again
> migration: report SaveStateEntry id and name on failure
>
> migration/migration.c | 3 +--
> migration/postcopy-ram.c | 8 +++++---
> migration/ram.c | 5 +----
> migration/savevm.c | 2 ++
> 4 files changed, 9 insertions(+), 9 deletions(-)
>
> --
> 2.17.1
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 11+ messages in thread