We need to change the full chain to pass the Error parameter.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/migration.c | 10 +++++-----
migration/migration.h | 2 +-
migration/ram.c | 2 +-
migration/ram.h | 2 +-
migration/rdma.c | 2 +-
5 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 5a56bd0c91..cf6cec5fb6 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -518,11 +518,11 @@ fail:
exit(EXIT_FAILURE);
}
-static void migration_incoming_setup(QEMUFile *f)
+static void migration_incoming_setup(QEMUFile *f, Error **errp)
{
MigrationIncomingState *mis = migration_incoming_get_current();
- if (multifd_load_setup() != 0) {
+ if (multifd_load_setup(errp) != 0) {
/* We haven't been able to create multifd threads
nothing better to do */
exit(EXIT_FAILURE);
@@ -572,13 +572,13 @@ static bool postcopy_try_recover(QEMUFile *f)
return false;
}
-void migration_fd_process_incoming(QEMUFile *f)
+void migration_fd_process_incoming(QEMUFile *f, Error **errp)
{
if (postcopy_try_recover(f)) {
return;
}
- migration_incoming_setup(f);
+ migration_incoming_setup(f, errp);
migration_incoming_process();
}
@@ -596,7 +596,7 @@ void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp)
return;
}
- migration_incoming_setup(f);
+ migration_incoming_setup(f, errp);
/*
* Common migration only needs one channel, so we can start
diff --git a/migration/migration.h b/migration/migration.h
index 79b3dda146..545f283ae7 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -265,7 +265,7 @@ struct MigrationState
void migrate_set_state(int *state, int old_state, int new_state);
-void migration_fd_process_incoming(QEMUFile *f);
+void migration_fd_process_incoming(QEMUFile *f, Error **errp);
void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp);
void migration_incoming_process(void);
diff --git a/migration/ram.c b/migration/ram.c
index 1f364cc23d..fcf50e648a 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1421,7 +1421,7 @@ static void *multifd_recv_thread(void *opaque)
return NULL;
}
-int multifd_load_setup(void)
+int multifd_load_setup(Error **errp)
{
int thread_count;
uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
diff --git a/migration/ram.h b/migration/ram.h
index da22a417ea..42be471d52 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -43,7 +43,7 @@ uint64_t ram_bytes_total(void);
int multifd_save_setup(Error **errp);
void multifd_save_cleanup(void);
-int multifd_load_setup(void);
+int multifd_load_setup(Error **errp);
int multifd_load_cleanup(Error **errp);
bool multifd_recv_all_channels_created(void);
bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp);
diff --git a/migration/rdma.c b/migration/rdma.c
index e241dcb992..2379b8345b 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -4004,7 +4004,7 @@ static void rdma_accept_incoming_migration(void *opaque)
}
rdma->migration_started_on_destination = 1;
- migration_fd_process_incoming(f);
+ migration_fd_process_incoming(f, errp);
}
void rdma_start_incoming_migration(const char *host_port, Error **errp)
--
2.23.0
* Juan Quintela (quintela@redhat.com) wrote:
> We need to change the full chain to pass the Error parameter.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> migration/migration.c | 10 +++++-----
> migration/migration.h | 2 +-
> migration/ram.c | 2 +-
> migration/ram.h | 2 +-
> migration/rdma.c | 2 +-
> 5 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 5a56bd0c91..cf6cec5fb6 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -518,11 +518,11 @@ fail:
> exit(EXIT_FAILURE);
> }
>
> -static void migration_incoming_setup(QEMUFile *f)
> +static void migration_incoming_setup(QEMUFile *f, Error **errp)
> {
> MigrationIncomingState *mis = migration_incoming_get_current();
>
> - if (multifd_load_setup() != 0) {
> + if (multifd_load_setup(errp) != 0) {
> /* We haven't been able to create multifd threads
> nothing better to do */
But if you're taking an errp and the load fails, don't you want to
report the error before you exit? (with an error_get_pretty or
something?)
> exit(EXIT_FAILURE);
> @@ -572,13 +572,13 @@ static bool postcopy_try_recover(QEMUFile *f)
> return false;
> }
>
> -void migration_fd_process_incoming(QEMUFile *f)
> +void migration_fd_process_incoming(QEMUFile *f, Error **errp)
> {
> if (postcopy_try_recover(f)) {
> return;
> }
>
> - migration_incoming_setup(f);
> + migration_incoming_setup(f, errp);
> migration_incoming_process();
and if you're making incoming_setup able to fail, don't you need
to.... hmm, skip the incoming_process?
> }
>
> @@ -596,7 +596,7 @@ void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp)
> return;
> }
>
> - migration_incoming_setup(f);
> + migration_incoming_setup(f, errp);
Don't you need to make that use a local_err and propagate, like in the
other half of the if/else?
> /*
> * Common migration only needs one channel, so we can start
> diff --git a/migration/migration.h b/migration/migration.h
> index 79b3dda146..545f283ae7 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -265,7 +265,7 @@ struct MigrationState
>
> void migrate_set_state(int *state, int old_state, int new_state);
>
> -void migration_fd_process_incoming(QEMUFile *f);
> +void migration_fd_process_incoming(QEMUFile *f, Error **errp);
> void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp);
> void migration_incoming_process(void);
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 1f364cc23d..fcf50e648a 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1421,7 +1421,7 @@ static void *multifd_recv_thread(void *opaque)
> return NULL;
> }
>
> -int multifd_load_setup(void)
> +int multifd_load_setup(Error **errp)
> {
> int thread_count;
> uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
> diff --git a/migration/ram.h b/migration/ram.h
> index da22a417ea..42be471d52 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -43,7 +43,7 @@ uint64_t ram_bytes_total(void);
>
> int multifd_save_setup(Error **errp);
> void multifd_save_cleanup(void);
> -int multifd_load_setup(void);
> +int multifd_load_setup(Error **errp);
> int multifd_load_cleanup(Error **errp);
> bool multifd_recv_all_channels_created(void);
> bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp);
> diff --git a/migration/rdma.c b/migration/rdma.c
> index e241dcb992..2379b8345b 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -4004,7 +4004,7 @@ static void rdma_accept_incoming_migration(void *opaque)
> }
>
> rdma->migration_started_on_destination = 1;
> - migration_fd_process_incoming(f);
> + migration_fd_process_incoming(f, errp);
Heck, the errp handling in rdma_accept_incoming_migration is very very
broken; I don't see how the errp ever gets reported or freed.
(But that's an existing problem)
> }
>
> void rdma_start_incoming_migration(const char *host_port, Error **errp)
> --
> 2.23.0
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> We need to change the full chain to pass the Error parameter.
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>> migration/migration.c | 10 +++++-----
>> migration/migration.h | 2 +-
>> migration/ram.c | 2 +-
>> migration/ram.h | 2 +-
>> migration/rdma.c | 2 +-
>> 5 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 5a56bd0c91..cf6cec5fb6 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -518,11 +518,11 @@ fail:
>> exit(EXIT_FAILURE);
>> }
>>
>> -static void migration_incoming_setup(QEMUFile *f)
>> +static void migration_incoming_setup(QEMUFile *f, Error **errp)
>> {
>> MigrationIncomingState *mis = migration_incoming_get_current();
>>
>> - if (multifd_load_setup() != 0) {
>> + if (multifd_load_setup(errp) != 0) {
>> /* We haven't been able to create multifd threads
>> nothing better to do */
>
> But if you're taking an errp and the load fails, don't you want to
> report the error before you exit? (with an error_get_pretty or
> something?)
error_report_err() that is.
>
>> exit(EXIT_FAILURE);
>> @@ -572,13 +572,13 @@ static bool postcopy_try_recover(QEMUFile *f)
>> return false;
>> }
>>
>> -void migration_fd_process_incoming(QEMUFile *f)
>> +void migration_fd_process_incoming(QEMUFile *f, Error **errp)
>> {
>> if (postcopy_try_recover(f)) {
>> return;
>> }
>>
>> - migration_incoming_setup(f);
>> + migration_incoming_setup(f, errp);
>> migration_incoming_process();
>
> and if you're making incoming_setup able to fail, don't you need
> to.... hmm, skip the incoming_process?
Changing it to test for errors, thanks.
>> }
>>
>> @@ -596,7 +596,7 @@ void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp)
>> return;
>> }
>>
>> - migration_incoming_setup(f);
>> + migration_incoming_setup(f, errp);
>
> Don't you need to make that use a local_err and propagate, like in the
> other half of the if/else?
Changed the whole business. It appears that each time that I use an
Error ** I have to relearn how to use it. Changed all places to use a
local error and propagate/error_report_err() as apropiate.
>> rdma->migration_started_on_destination = 1;
>> - migration_fd_process_incoming(f);
>> + migration_fd_process_incoming(f, errp);
>
> Heck, the errp handling in rdma_accept_incoming_migration is very very
> broken; I don't see how the errp ever gets reported or freed.
> (But that's an existing problem)
Leaving this for next series.
Thanks, Juan.
© 2016 - 2026 Red Hat, Inc.