From 9c53d369e5903375a2e3358f739be77dcb8dae49 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Mon, 15 May 2023 21:57:08 +0200 Subject: [PATCH] migration/rdma: Simplify the function that saves a page When we sent a page through QEMUFile hooks (RDMA) there are three posiblities: - We are not using RDMA. return RAM_SAVE_CONTROL_DELAYED and control_save_page() returns false to let anything else to proceed. - There is one error but we are using RDMA. Then we return a negative value, control_save_page() needs to return true. - Everything goes well and RDMA start the sent of the page asynchronously. It returns RAM_SAVE_CONTROL_DELAYED and we need to return 1 for ram_save_page_legacy. Clear? I know, I know, the interface is as bad as it gets. I think that now it is a bit clearer, but this needs to be done some other way. Reviewed-by: Leonardo Bras Signed-off-by: Juan Quintela Message-Id: <20230515195709.63843-16-quintela@redhat.com> --- migration/qemu-file.c | 12 ++++++------ migration/qemu-file.h | 14 ++++++-------- migration/ram.c | 10 +++------- migration/rdma.c | 19 +++---------------- 4 files changed, 18 insertions(+), 37 deletions(-) diff --git a/migration/qemu-file.c b/migration/qemu-file.c index 5c43fa34e7..5e8207dae4 100644 --- a/migration/qemu-file.c +++ b/migration/qemu-file.c @@ -322,14 +322,14 @@ void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data) } } -size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset, - ram_addr_t offset, size_t size, - uint64_t *bytes_sent) +int ram_control_save_page(QEMUFile *f, ram_addr_t block_offset, + ram_addr_t offset, size_t size) { if (f->hooks && f->hooks->save_page) { - int ret = f->hooks->save_page(f, block_offset, - offset, size, bytes_sent); - + int ret = f->hooks->save_page(f, block_offset, offset, size); + /* + * RAM_SAVE_CONTROL_* are negative values + */ if (ret != RAM_SAVE_CONTROL_DELAYED && ret != RAM_SAVE_CONTROL_NOT_SUPP) { if (ret < 0) { diff --git a/migration/qemu-file.h b/migration/qemu-file.h index 57b00c8562..03e718c264 100644 --- a/migration/qemu-file.h +++ b/migration/qemu-file.h @@ -49,11 +49,10 @@ typedef int (QEMURamHookFunc)(QEMUFile *f, uint64_t flags, void *data); * This function allows override of where the RAM page * is saved (such as RDMA, for example.) */ -typedef size_t (QEMURamSaveFunc)(QEMUFile *f, - ram_addr_t block_offset, - ram_addr_t offset, - size_t size, - uint64_t *bytes_sent); +typedef int (QEMURamSaveFunc)(QEMUFile *f, + ram_addr_t block_offset, + ram_addr_t offset, + size_t size); typedef struct QEMUFileHooks { QEMURamHookFunc *before_ram_iterate; @@ -142,9 +141,8 @@ void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data); #define RAM_SAVE_CONTROL_NOT_SUPP -1000 #define RAM_SAVE_CONTROL_DELAYED -2000 -size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset, - ram_addr_t offset, size_t size, - uint64_t *bytes_sent); +int ram_control_save_page(QEMUFile *f, ram_addr_t block_offset, + ram_addr_t offset, size_t size); QIOChannel *qemu_file_get_ioc(QEMUFile *file); #endif diff --git a/migration/ram.c b/migration/ram.c index c6238f7a8b..99cff591a3 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1186,23 +1186,19 @@ static int save_zero_page(PageSearchStatus *pss, QEMUFile *f, RAMBlock *block, static bool control_save_page(PageSearchStatus *pss, RAMBlock *block, ram_addr_t offset, int *pages) { - uint64_t bytes_xmit = 0; int ret; - *pages = -1; ret = ram_control_save_page(pss->pss_channel, block->offset, offset, - TARGET_PAGE_SIZE, &bytes_xmit); + TARGET_PAGE_SIZE); if (ret == RAM_SAVE_CONTROL_NOT_SUPP) { return false; } - if (bytes_xmit) { - *pages = 1; - } - if (ret == RAM_SAVE_CONTROL_DELAYED) { + *pages = 1; return true; } + *pages = ret; return true; } diff --git a/migration/rdma.c b/migration/rdma.c index 9007261b5c..5748c9045b 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -3240,13 +3240,12 @@ qio_channel_rdma_shutdown(QIOChannel *ioc, * * @size : Number of bytes to transfer * - * @bytes_sent : User-specificed pointer to indicate how many bytes were + * @pages_sent : User-specificed pointer to indicate how many pages were * sent. Usually, this will not be more than a few bytes of * the protocol because most transfers are sent asynchronously. */ -static size_t qemu_rdma_save_page(QEMUFile *f, - ram_addr_t block_offset, ram_addr_t offset, - size_t size, uint64_t *bytes_sent) +static int qemu_rdma_save_page(QEMUFile *f, ram_addr_t block_offset, + ram_addr_t offset, size_t size) { QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f)); RDMAContext *rdma; @@ -3278,18 +3277,6 @@ static size_t qemu_rdma_save_page(QEMUFile *f, goto err; } - /* - * We always return 1 bytes because the RDMA - * protocol is completely asynchronous. We do not yet know - * whether an identified chunk is zero or not because we're - * waiting for other pages to potentially be merged with - * the current chunk. So, we have to call qemu_update_position() - * later on when the actual write occurs. - */ - if (bytes_sent) { - *bytes_sent = 1; - } - /* * Drain the Completion Queue if possible, but do not block, * just poll.