Eduardo Habkost <ehabk...@redhat.com> wrote: > Summary of the problem: > > - qemu_fclose() calls qemu_fflush() > - Writes done by qemu_fflush() can fail > - Those errors are lost after qemu_fclose() returns > > So, this series change qemu_fclose() to return last_error. But to do that we > need to make sure all involve code use the -errno convention, hence the large > series. > > > Michael, probably this will conflict with your ongoing work. I don't want to > delay other work, so I can rebase my patches if needed. This is just a RFC. > > Juan, maybe you were already working on this. But as I was already fixing this > code while auditing the migration handling, I thought it was interesting to > send this for review anyway. I hope I didn't duplicate any work. > > > This is still completely untested, I am just using this series as a way to > report the issue and get comments so I know I am going through the right path. > > > Detailed description of the changes: > > Small cleanups: > > - Always use qemu_file_set_error() to set last_error (patch 1) > - Add return value documentation to QEMUFileCloseFunc (patch 2) > > Actual qemu_fclose() behavior changes are done in 3 steps: > > - First step: fix qemu_fclose() callers: > - exec_close() > - Fixed to check for negative values, not -1 (patch 3) > - Note: exec_close() is changed in two steps: first on the qemu_fclose() > calling code, then on the return value code > - migrate_fd_cleanup > - Fixed to: > - check qemu_fclose() return value for <0 (patch 4) > - return -errno, not just -1 (patch 4) > - Callers: > - migrate_fd_completed: > - Error checking is done properly, already. > - migrate_fd_error: > - It ignores migrated_fd_cleanup() return value. > - migrate_fd_cancel: > - It ignores migrated_fd_cleanup() return value. > - exec_accept_incoming_migration(): no return value check (yet) > - fd_accept_incoming_migration(): no return value check (yet) > - tcp_accept_incoming_migration(): no return value check (yet) > - unix_accept_incoming_migration(): no return value check (yet) > - do_savevm(): no return value check (yet) > - load_vmstate(): no return value check (yet) > > - Second step: change qemu_fclose() to return last_error (patch 5) > - Made sure to return unchanged (positive) success value on success > (required by exec_close()) > > - Third step: change qemu_fclose() implementations (QEMUFileCloseFunc): > - stdio_fclose > - Fixed to return -errno (patch 6) > - stdio_pclose > - Fixed to return -errno (patch 7) > - buffered_close > - Implemented through QEMUFileBuffered.close: > - Only implementation is migrate_fd_close(), that calls the following, > through MigrationState.close: > - exec_close(): > - fixed to return original error value, not -1 (patch 8) > - fd_close > - Fixed to return -errno on close() errors. (patch 9) > - tcp_close > - Fixed to return -errno on close() errors. (patch 10) > - unix_close > - Fixed to return -errno on close() errors. (patch 11) > - socket_close > - No system call is made, returns always 0. > - bdrv_fclose > - No system call is made, returns always 0. > > > > Eduardo Habkost (11): > savevm: use qemu_file_set_error() instead of setting last_error > directly > QEMUFileCloseFunc: add return value documentation > exec_close(): accept any negative value as qemu_fclose() error > migrate_fd_cleanup: accept any negative qemu_fclose() value as error > qemu_fclose: return last_error if set > stdio_pclose: return -errno on error > stdio_fclose: return -errno on errors > exec_close(): return -errno on errors > fd_close(): check for close() errors too > tcp_close(): check for close() errors too > unix_close(): check for close() errors too > > hw/hw.h | 8 ++++++- > migration-exec.c | 9 ++----- > migration-fd.c | 6 +++- > migration-tcp.c | 6 +++- > migration-unix.c | 6 +++- > migration.c | 4 +-- > savevm.c | 61 > ++++++++++++++++++++++++++++++++++++++++++++++-------- > 7 files changed, 75 insertions(+), 25 deletions(-)
ACK series
