[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 00/11] qemu_fclose() error handling fixes
From: |
Michael Roth |
Subject: |
Re: [Qemu-devel] [RFC PATCH 00/11] qemu_fclose() error handling fixes |
Date: |
Fri, 04 Nov 2011 11:32:10 -0500 |
On Tue, 1 Nov 2011 17:20:19 -0200, Eduardo Habkost <address@hidden> 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.
No worries, my stuff is probably a ways off whereas this is likely a
candidate for 1.0 or a follow-up stable revision. Plus I've gotten pretty used
to working through QEMUFile-related merge conflicts as of late :)
Still reviewing, but series looks good so far (other than 6)
>
> 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(-)
>
> --
> 1.7.3.2
>
--
Sincerely,
Mike Roth
IBM Linux Technology Center
- [Qemu-devel] [RFC PATCH 09/11] fd_close(): check for close() errors too, (continued)
[Qemu-devel] [RFC PATCH 06/11] stdio_pclose: return -errno on error, Eduardo Habkost, 2011/11/01
Re: [Qemu-devel] [RFC PATCH 00/11] qemu_fclose() error handling fixes, Juan Quintela, 2011/11/02
Re: [Qemu-devel] [RFC PATCH 00/11] qemu_fclose() error handling fixes,
Michael Roth <=