| Message ID | 20230918144206.560120-18-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration/rdma: Error handling fixes | expand |
Markus Armbruster <armbru@redhat.com> writes: > Hiding return statements in macros is a bad idea. Use a function > instead, and open code the return part. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Fabiano Rosas <farosas@suse.de>
On 18/09/2023 22:41, Markus Armbruster wrote: > Hiding return statements in macros is a bad idea. Use a function > instead, and open code the return part. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Li Zhijian <lizhijian@fujitsu.com> > --- > migration/rdma.c | 43 +++++++++++++++++++++++++++---------------- > 1 file changed, 27 insertions(+), 16 deletions(-) > > diff --git a/migration/rdma.c b/migration/rdma.c > index 30e6dff875..be66f53489 100644 > --- a/migration/rdma.c > +++ b/migration/rdma.c > @@ -85,18 +85,6 @@ > */ > static uint32_t known_capabilities = RDMA_CAPABILITY_PIN_ALL; > > -#define CHECK_ERROR_STATE() \ > - do { \ > - if (rdma->error_state) { \ > - if (!rdma->error_reported) { \ > - error_report("RDMA is in an error state waiting migration" \ > - " to abort!"); \ > - rdma->error_reported = true; \ > - } \ > - return rdma->error_state; \ > - } \ > - } while (0) > - > /* > * A work request ID is 64-bits and we split up these bits > * into 3 parts: > @@ -451,6 +439,16 @@ typedef struct QEMU_PACKED { > uint64_t chunks; /* how many sequential chunks to register */ > } RDMARegister; > > +static int check_error_state(RDMAContext *rdma) > +{ > + if (rdma->error_state && !rdma->error_reported) { > + error_report("RDMA is in an error state waiting migration" > + " to abort!"); > + rdma->error_reported = true; > + } > + return rdma->error_state; > +} > + > static void register_to_network(RDMAContext *rdma, RDMARegister *reg) > { > RDMALocalBlock *local_block; > @@ -3219,7 +3217,10 @@ static size_t qemu_rdma_save_page(QEMUFile *f, > return -EIO; > } > > - CHECK_ERROR_STATE(); > + ret = check_error_state(rdma); > + if (ret) { > + return ret; > + } > > qemu_fflush(f); > > @@ -3535,7 +3536,10 @@ static int qemu_rdma_registration_handle(QEMUFile *f) > return -EIO; > } > > - CHECK_ERROR_STATE(); > + ret = check_error_state(rdma); > + if (ret) { > + return ret; > + } > > local = &rdma->local_ram_blocks; > do { > @@ -3839,6 +3843,7 @@ static int qemu_rdma_registration_start(QEMUFile *f, > { > QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f)); > RDMAContext *rdma; > + int ret; > > if (migration_in_postcopy()) { > return 0; > @@ -3850,7 +3855,10 @@ static int qemu_rdma_registration_start(QEMUFile *f, > return -EIO; > } > > - CHECK_ERROR_STATE(); > + ret = check_error_state(rdma); > + if (ret) { > + return ret; > + } > > trace_qemu_rdma_registration_start(flags); > qemu_put_be64(f, RAM_SAVE_FLAG_HOOK); > @@ -3881,7 +3889,10 @@ static int qemu_rdma_registration_stop(QEMUFile *f, > return -EIO; > } > > - CHECK_ERROR_STATE(); > + ret = check_error_state(rdma); > + if (ret) { > + return ret; > + } > > qemu_fflush(f); > ret = qemu_rdma_drain_cq(f, rdma);
diff --git a/migration/rdma.c b/migration/rdma.c index 30e6dff875..be66f53489 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -85,18 +85,6 @@ */ static uint32_t known_capabilities = RDMA_CAPABILITY_PIN_ALL; -#define CHECK_ERROR_STATE() \ - do { \ - if (rdma->error_state) { \ - if (!rdma->error_reported) { \ - error_report("RDMA is in an error state waiting migration" \ - " to abort!"); \ - rdma->error_reported = true; \ - } \ - return rdma->error_state; \ - } \ - } while (0) - /* * A work request ID is 64-bits and we split up these bits * into 3 parts: @@ -451,6 +439,16 @@ typedef struct QEMU_PACKED { uint64_t chunks; /* how many sequential chunks to register */ } RDMARegister; +static int check_error_state(RDMAContext *rdma) +{ + if (rdma->error_state && !rdma->error_reported) { + error_report("RDMA is in an error state waiting migration" + " to abort!"); + rdma->error_reported = true; + } + return rdma->error_state; +} + static void register_to_network(RDMAContext *rdma, RDMARegister *reg) { RDMALocalBlock *local_block; @@ -3219,7 +3217,10 @@ static size_t qemu_rdma_save_page(QEMUFile *f, return -EIO; } - CHECK_ERROR_STATE(); + ret = check_error_state(rdma); + if (ret) { + return ret; + } qemu_fflush(f); @@ -3535,7 +3536,10 @@ static int qemu_rdma_registration_handle(QEMUFile *f) return -EIO; } - CHECK_ERROR_STATE(); + ret = check_error_state(rdma); + if (ret) { + return ret; + } local = &rdma->local_ram_blocks; do { @@ -3839,6 +3843,7 @@ static int qemu_rdma_registration_start(QEMUFile *f, { QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f)); RDMAContext *rdma; + int ret; if (migration_in_postcopy()) { return 0; @@ -3850,7 +3855,10 @@ static int qemu_rdma_registration_start(QEMUFile *f, return -EIO; } - CHECK_ERROR_STATE(); + ret = check_error_state(rdma); + if (ret) { + return ret; + } trace_qemu_rdma_registration_start(flags); qemu_put_be64(f, RAM_SAVE_FLAG_HOOK); @@ -3881,7 +3889,10 @@ static int qemu_rdma_registration_stop(QEMUFile *f, return -EIO; } - CHECK_ERROR_STATE(); + ret = check_error_state(rdma); + if (ret) { + return ret; + } qemu_fflush(f); ret = qemu_rdma_drain_cq(f, rdma);
Hiding return statements in macros is a bad idea. Use a function instead, and open code the return part. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- migration/rdma.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-)