* [Qemu-devel] [PATCH 1/4] migration: multifd_save_setup always return 0
2019-05-28 1:46 [Qemu-devel] [PATCH 0/4] Multifd Cleanup Wei Yang
@ 2019-05-28 1:47 ` Wei Yang
2019-05-28 8:11 ` Juan Quintela
2019-05-28 1:47 ` [Qemu-devel] [PATCH 2/4] migration/ram.c: use same type in MultiFDPages_t to define offsest Wei Yang
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Wei Yang @ 2019-05-28 1:47 UTC (permalink / raw)
To: qemu-devel; +Cc: Wei Yang, dgilbert, quintela
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/migration.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index d0a0f68f11..3aae4f2734 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3336,12 +3336,7 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
return;
}
- if (multifd_save_setup() != 0) {
- migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
- MIGRATION_STATUS_FAILED);
- migrate_fd_cleanup(s);
- return;
- }
+ multifd_save_setup();
qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
QEMU_THREAD_JOINABLE);
s->migration_thread_running = true;
--
2.19.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] migration: multifd_save_setup always return 0
2019-05-28 1:47 ` [Qemu-devel] [PATCH 1/4] migration: multifd_save_setup always return 0 Wei Yang
@ 2019-05-28 8:11 ` Juan Quintela
2019-05-29 0:34 ` Wei Yang
0 siblings, 1 reply; 15+ messages in thread
From: Juan Quintela @ 2019-05-28 8:11 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, dgilbert
Wei Yang <richardw.yang@linux.intel.com> wrote:
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
> migration/migration.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index d0a0f68f11..3aae4f2734 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3336,12 +3336,7 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
> return;
> }
>
> - if (multifd_save_setup() != 0) {
> - migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> - MIGRATION_STATUS_FAILED);
> - migrate_fd_cleanup(s);
> - return;
> - }
> + multifd_save_setup();
> qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
> QEMU_THREAD_JOINABLE);
> s->migration_thread_running = true;
Nack.
On the compression patches that are on list, multifd_save_setup()
returns -1 if there is a problem when we allocate a zbuff.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] migration: multifd_save_setup always return 0
2019-05-28 8:11 ` Juan Quintela
@ 2019-05-29 0:34 ` Wei Yang
2019-05-29 8:22 ` Juan Quintela
2019-05-29 8:22 ` Juan Quintela
0 siblings, 2 replies; 15+ messages in thread
From: Wei Yang @ 2019-05-29 0:34 UTC (permalink / raw)
To: Juan Quintela; +Cc: Wei Yang, dgilbert, qemu-devel
On Tue, May 28, 2019 at 10:11:11AM +0200, Juan Quintela wrote:
>Wei Yang <richardw.yang@linux.intel.com> wrote:
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>> ---
>> migration/migration.c | 7 +------
>> 1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index d0a0f68f11..3aae4f2734 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -3336,12 +3336,7 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
>> return;
>> }
>>
>> - if (multifd_save_setup() != 0) {
>> - migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
>> - MIGRATION_STATUS_FAILED);
>> - migrate_fd_cleanup(s);
>> - return;
>> - }
>> + multifd_save_setup();
>> qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
>> QEMU_THREAD_JOINABLE);
>> s->migration_thread_running = true;
>
>Nack.
>
>On the compression patches that are on list, multifd_save_setup()
>returns -1 if there is a problem when we allocate a zbuff.
You mean there are some patches in mail list?
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] migration: multifd_save_setup always return 0
2019-05-29 0:34 ` Wei Yang
@ 2019-05-29 8:22 ` Juan Quintela
2019-05-29 8:22 ` Juan Quintela
1 sibling, 0 replies; 15+ messages in thread
From: Juan Quintela @ 2019-05-29 8:22 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, dgilbert
Wei Yang <richardw.yang@linux.intel.com> wrote:
> On Tue, May 28, 2019 at 10:11:11AM +0200, Juan Quintela wrote:
>>Wei Yang <richardw.yang@linux.intel.com> wrote:
>>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>>> ---
>>> migration/migration.c | 7 +------
>>> 1 file changed, 1 insertion(+), 6 deletions(-)
>>>
>>> diff --git a/migration/migration.c b/migration/migration.c
>>> index d0a0f68f11..3aae4f2734 100644
>>> --- a/migration/migration.c
>>> +++ b/migration/migration.c
>>> @@ -3336,12 +3336,7 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
>>> return;
>>> }
>>>
>>> - if (multifd_save_setup() != 0) {
>>> - migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
>>> - MIGRATION_STATUS_FAILED);
>>> - migrate_fd_cleanup(s);
>>> - return;
>>> - }
>>> + multifd_save_setup();
>>> qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
>>> QEMU_THREAD_JOINABLE);
>>> s->migration_thread_running = true;
>>
>>Nack.
>>
>>On the compression patches that are on list, multifd_save_setup()
>>returns -1 if there is a problem when we allocate a zbuff.
>
> You mean there are some patches in mail list?
Yeap.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] migration: multifd_save_setup always return 0
2019-05-29 0:34 ` Wei Yang
2019-05-29 8:22 ` Juan Quintela
@ 2019-05-29 8:22 ` Juan Quintela
1 sibling, 0 replies; 15+ messages in thread
From: Juan Quintela @ 2019-05-29 8:22 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, dgilbert
Wei Yang <richardw.yang@linux.intel.com> wrote:
> On Tue, May 28, 2019 at 10:11:11AM +0200, Juan Quintela wrote:
>>Wei Yang <richardw.yang@linux.intel.com> wrote:
>>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>>> ---
>>> migration/migration.c | 7 +------
>>> 1 file changed, 1 insertion(+), 6 deletions(-)
>>>
>>> diff --git a/migration/migration.c b/migration/migration.c
>>> index d0a0f68f11..3aae4f2734 100644
>>> --- a/migration/migration.c
>>> +++ b/migration/migration.c
>>> @@ -3336,12 +3336,7 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
>>> return;
>>> }
>>>
>>> - if (multifd_save_setup() != 0) {
>>> - migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
>>> - MIGRATION_STATUS_FAILED);
>>> - migrate_fd_cleanup(s);
>>> - return;
>>> - }
>>> + multifd_save_setup();
>>> qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
>>> QEMU_THREAD_JOINABLE);
>>> s->migration_thread_running = true;
>>
>>Nack.
>>
>>On the compression patches that are on list, multifd_save_setup()
>>returns -1 if there is a problem when we allocate a zbuff.
>
> You mean there are some patches in mail list?
Yeap.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 2/4] migration/ram.c: use same type in MultiFDPages_t to define offsest
2019-05-28 1:46 [Qemu-devel] [PATCH 0/4] Multifd Cleanup Wei Yang
2019-05-28 1:47 ` [Qemu-devel] [PATCH 1/4] migration: multifd_save_setup always return 0 Wei Yang
@ 2019-05-28 1:47 ` Wei Yang
2019-05-28 8:12 ` Juan Quintela
2019-05-28 1:47 ` [Qemu-devel] [PATCH 3/4] migration/ram.c: MultiFDSendParams.sem_sync is not really used Wei Yang
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Wei Yang @ 2019-05-28 1:47 UTC (permalink / raw)
To: qemu-devel; +Cc: Wei Yang, dgilbert, quintela
MultiFDPacket_t.offset is allocated to store MultiFDPages_t.offset.
It would be better to use the same type.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/ram.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/migration/ram.c b/migration/ram.c
index 4c60869226..dcf4c54eb5 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -607,7 +607,7 @@ typedef struct {
uint64_t packet_num;
uint64_t unused[4]; /* Reserved for future use */
char ramblock[256];
- uint64_t offset[];
+ ram_addr_t offset[];
} __attribute__((packed)) MultiFDPacket_t;
typedef struct {
--
2.19.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] migration/ram.c: use same type in MultiFDPages_t to define offsest
2019-05-28 1:47 ` [Qemu-devel] [PATCH 2/4] migration/ram.c: use same type in MultiFDPages_t to define offsest Wei Yang
@ 2019-05-28 8:12 ` Juan Quintela
2019-05-29 0:34 ` Wei Yang
0 siblings, 1 reply; 15+ messages in thread
From: Juan Quintela @ 2019-05-28 8:12 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, dgilbert
Wei Yang <richardw.yang@linux.intel.com> wrote:
> MultiFDPacket_t.offset is allocated to store MultiFDPages_t.offset.
>
> It would be better to use the same type.
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
> ---
> migration/ram.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 4c60869226..dcf4c54eb5 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -607,7 +607,7 @@ typedef struct {
> uint64_t packet_num;
> uint64_t unused[4]; /* Reserved for future use */
> char ramblock[256];
> - uint64_t offset[];
> + ram_addr_t offset[];
> } __attribute__((packed)) MultiFDPacket_t;
>
> typedef struct {
This needs a comment, but it is on purpose. We want that the value on
the wire to be the same for any architecture. (Migration stream is
supposed to be architecture independent). ram_addr_t is architecture
dependent.
Later, Juan.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] migration/ram.c: use same type in MultiFDPages_t to define offsest
2019-05-28 8:12 ` Juan Quintela
@ 2019-05-29 0:34 ` Wei Yang
0 siblings, 0 replies; 15+ messages in thread
From: Wei Yang @ 2019-05-29 0:34 UTC (permalink / raw)
To: Juan Quintela; +Cc: Wei Yang, dgilbert, qemu-devel
On Tue, May 28, 2019 at 10:12:39AM +0200, Juan Quintela wrote:
>Wei Yang <richardw.yang@linux.intel.com> wrote:
>> MultiFDPacket_t.offset is allocated to store MultiFDPages_t.offset.
>>
>> It would be better to use the same type.
>>
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>> ---
>> migration/ram.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 4c60869226..dcf4c54eb5 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -607,7 +607,7 @@ typedef struct {
>> uint64_t packet_num;
>> uint64_t unused[4]; /* Reserved for future use */
>> char ramblock[256];
>> - uint64_t offset[];
>> + ram_addr_t offset[];
>> } __attribute__((packed)) MultiFDPacket_t;
>>
>> typedef struct {
>
>This needs a comment, but it is on purpose. We want that the value on
>the wire to be the same for any architecture. (Migration stream is
>supposed to be architecture independent). ram_addr_t is architecture
>dependent.
>
Sounds reasonable.
>Later, Juan.
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 3/4] migration/ram.c: MultiFDSendParams.sem_sync is not really used
2019-05-28 1:46 [Qemu-devel] [PATCH 0/4] Multifd Cleanup Wei Yang
2019-05-28 1:47 ` [Qemu-devel] [PATCH 1/4] migration: multifd_save_setup always return 0 Wei Yang
2019-05-28 1:47 ` [Qemu-devel] [PATCH 2/4] migration/ram.c: use same type in MultiFDPages_t to define offsest Wei Yang
@ 2019-05-28 1:47 ` Wei Yang
2019-05-28 8:16 ` Juan Quintela
2019-05-28 1:47 ` [Qemu-devel] [PATCH 4/4] migration/ram.c: multifd_send_state->count " Wei Yang
2019-06-04 2:25 ` [Qemu-devel] [PATCH 0/4] Multifd Cleanup Wei Yang
4 siblings, 1 reply; 15+ messages in thread
From: Wei Yang @ 2019-05-28 1:47 UTC (permalink / raw)
To: qemu-devel; +Cc: Wei Yang, dgilbert, quintela
Besides init and destroy, MultiFDSendParams.sem_sync is not really used.
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/ram.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index dcf4c54eb5..5d31f7bd4c 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -661,8 +661,6 @@ typedef struct {
uint64_t num_packets;
/* pages sent through this channel */
uint64_t num_pages;
- /* syncs main thread and channels */
- QemuSemaphore sem_sync;
} MultiFDSendParams;
typedef struct {
@@ -1027,7 +1025,6 @@ void multifd_save_cleanup(void)
p->c = NULL;
qemu_mutex_destroy(&p->mutex);
qemu_sem_destroy(&p->sem);
- qemu_sem_destroy(&p->sem_sync);
g_free(p->name);
p->name = NULL;
multifd_pages_clear(p->pages);
@@ -1201,7 +1198,6 @@ int multifd_save_setup(void)
qemu_mutex_init(&p->mutex);
qemu_sem_init(&p->sem, 0);
- qemu_sem_init(&p->sem_sync, 0);
p->quit = false;
p->pending_job = 0;
p->id = i;
--
2.19.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] migration/ram.c: MultiFDSendParams.sem_sync is not really used
2019-05-28 1:47 ` [Qemu-devel] [PATCH 3/4] migration/ram.c: MultiFDSendParams.sem_sync is not really used Wei Yang
@ 2019-05-28 8:16 ` Juan Quintela
2019-05-29 0:40 ` Wei Yang
0 siblings, 1 reply; 15+ messages in thread
From: Juan Quintela @ 2019-05-28 8:16 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, dgilbert
Wei Yang <richardw.yang@linux.intel.com> wrote:
> Besides init and destroy, MultiFDSendParams.sem_sync is not really used.
>
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
I mad SendParamas and RecvParams identical, but they are different. You
are right.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] migration/ram.c: MultiFDSendParams.sem_sync is not really used
2019-05-28 8:16 ` Juan Quintela
@ 2019-05-29 0:40 ` Wei Yang
0 siblings, 0 replies; 15+ messages in thread
From: Wei Yang @ 2019-05-29 0:40 UTC (permalink / raw)
To: Juan Quintela; +Cc: Wei Yang, dgilbert, qemu-devel
On Tue, May 28, 2019 at 10:16:06AM +0200, Juan Quintela wrote:
>Wei Yang <richardw.yang@linux.intel.com> wrote:
>> Besides init and destroy, MultiFDSendParams.sem_sync is not really used.
>>
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>
>Reviewed-by: Juan Quintela <quintela@redhat.com>
>
>I mad SendParamas and RecvParams identical, but they are different. You
>are right.
Thanks.
BTW, I found some interesting thing about multifd_send_state->channels_ready.
By checking the value of this semaphore, it grows far beyond what we really
have.
For example, we have default 2 channels which means
multifd_send_state->channels_ready's value is no more than 2. But the actual
value could go to more than 30.
The behavior sounds not right.
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 4/4] migration/ram.c: multifd_send_state->count is not really used
2019-05-28 1:46 [Qemu-devel] [PATCH 0/4] Multifd Cleanup Wei Yang
` (2 preceding siblings ...)
2019-05-28 1:47 ` [Qemu-devel] [PATCH 3/4] migration/ram.c: MultiFDSendParams.sem_sync is not really used Wei Yang
@ 2019-05-28 1:47 ` Wei Yang
2019-05-28 8:19 ` Juan Quintela
2019-06-04 2:25 ` [Qemu-devel] [PATCH 0/4] Multifd Cleanup Wei Yang
4 siblings, 1 reply; 15+ messages in thread
From: Wei Yang @ 2019-05-28 1:47 UTC (permalink / raw)
To: qemu-devel; +Cc: Wei Yang, dgilbert, quintela
Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
migration/ram.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 5d31f7bd4c..c9a9f7489b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -892,8 +892,6 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
struct {
MultiFDSendParams *params;
- /* number of created threads */
- int count;
/* array of pages to sent */
MultiFDPages_t *pages;
/* syncs main thread and channels */
@@ -1171,8 +1169,6 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
p->running = true;
qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
QEMU_THREAD_JOINABLE);
-
- atomic_inc(&multifd_send_state->count);
}
}
@@ -1188,7 +1184,6 @@ int multifd_save_setup(void)
thread_count = migrate_multifd_channels();
multifd_send_state = g_malloc0(sizeof(*multifd_send_state));
multifd_send_state->params = g_new0(MultiFDSendParams, thread_count);
- atomic_set(&multifd_send_state->count, 0);
multifd_send_state->pages = multifd_pages_init(page_count);
qemu_sem_init(&multifd_send_state->sem_sync, 0);
qemu_sem_init(&multifd_send_state->channels_ready, 0);
--
2.19.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 0/4] Multifd Cleanup
2019-05-28 1:46 [Qemu-devel] [PATCH 0/4] Multifd Cleanup Wei Yang
` (3 preceding siblings ...)
2019-05-28 1:47 ` [Qemu-devel] [PATCH 4/4] migration/ram.c: multifd_send_state->count " Wei Yang
@ 2019-06-04 2:25 ` Wei Yang
4 siblings, 0 replies; 15+ messages in thread
From: Wei Yang @ 2019-06-04 2:25 UTC (permalink / raw)
To: Wei Yang; +Cc: qemu-devel, dgilbert, quintela
Hi, David
Do you prefer I resend 3/4 or it is ok for you to pick up here directly?
On Tue, May 28, 2019 at 09:46:59AM +0800, Wei Yang wrote:
>Just found several small places for unused variables.
>
>Wei Yang (4):
> migration: multifd_save_setup always return 0
> migration/ram.c: use same type in MultiFDPages_t to define offsest
> migration/ram.c: MultiFDSendParams.sem_sync is not really used
> migration/ram.c: multifd_send_state->count is not really used
>
> migration/migration.c | 7 +------
> migration/ram.c | 11 +----------
> 2 files changed, 2 insertions(+), 16 deletions(-)
>
>--
>2.19.1
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 15+ messages in thread