On 3/4/24 22:30, Fabiano Rosas wrote:
> Cédric Le Goater <clg@redhat.com> writes:
>
>> This will prepare ground for futur changes adding an Error** argument
>> to the save_setup() handler. We need to make sure that on failure,
>> ram_save_setup() sets a new error.
>>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>> migration/ram.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 45a00b45edd429ef0568adce09b7459883e00167..dbd04d8ee2167881007c836a6bbc79c1aced72d0 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -2937,12 +2937,14 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>> int ret;
>>
>> if (compress_threads_save_setup()) {
>> + error_report("%s: failed to start compress threads", __func__);
>> return -1;
>> }
>>
>> /* migration has already setup the bitmap, reuse it. */
>> if (!migration_in_colo_state()) {
>> if (ram_init_all(rsp) != 0) {
>> + error_report("%s: failed to setup RAM for migration", __func__);
>> compress_threads_save_cleanup();
>> return -1;
>> }
>> @@ -2969,12 +2971,14 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>>
>> ret = rdma_registration_start(f, RAM_CONTROL_SETUP);
>> if (ret < 0) {
>> + error_report("%s: failed to start RDMA registration", __func__);
>> qemu_file_set_error(f, ret);
>> return ret;
>> }
>>
>> ret = rdma_registration_stop(f, RAM_CONTROL_SETUP);
>> if (ret < 0) {
>> + error_report("%s: failed to stop RDMA registration", __func__);
>> qemu_file_set_error(f, ret);
>> return ret;
>> }
>> @@ -2986,6 +2990,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>> ret = multifd_send_sync_main();
>> bql_lock();
>> if (ret < 0) {
>> + error_report("%s: multifd synchronization failed", __func__);
>> return ret;
>> }
>>
>> @@ -2994,7 +2999,11 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>> }
>>
>> qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
>> - return qemu_fflush(f);
>> + ret = qemu_fflush(f);
>> + if (ret) {
>> + error_report("%s failed : %s", __func__, strerror(ret));
>
> Here it should be -ret
>
> The qemu_fflush function returns the QEMUFile error (f->last_error) and
> that is expected to be a -errno value.
oh yes. I should have know that since qemu_file_set_error() takes a -errno.
> I started making sure all callers of qemu_file_set_error() use a
> negative value, but got lost in the vmstate code and never finished:
>
> https://lore.kernel.org/r/20230706195201.18595-1-farosas@suse.de
Thanks for the pointer.
Thanks,
C.