On 09.05.2013 21:02, Laine Stump wrote:
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=851411 https://bugzilla.redhat.com/show_bug.cgi?id=955500
The first problem was that virFileOpenAs was returning fd (-1) in one of the error cases rather than ret (-errno), so the caller thought that the error was EPERM rather than ENOENT.
The second problem was that some log messages in the general purpose qemuOpenFile() function would always say "Failed to create" even if the caller hadn't included O_CREAT (i.e. they were trying to open an existing file).
This fixes virFileOpenAs to jup down to the error return (which returns ret instead of fd) in the previously incorrect failure case of virFileOpenAs(), removes all error logging from virFileOpenAs() (since the callers report it), and modifies qemuOpenFile to appropriately use "open" or "create" in its log messages.
NB: I seriously considered removing logging from all callers of virFileOpenAs(), but there is at least one case where the callers doesn't want virFileOpenAs() to log any errors, because it's just going to try again (qemuOpenFile(). We can't simply make a silent variation of virFileOpenAs() though, because qemuOpenFile() can't make the decision about whether or not it wants to retry until after virFileOpenAs() has already returned an error code.
Likewise, I also considered changing virFileOpenAs() to return -1 with errno set on return, and may still do that, but only as a separate patch, as it obscures the intent of this patch too much. --- src/libxl/libxl_driver.c | 6 ++--- src/qemu/qemu_driver.c | 55 ++++++++++++++++++++++--------------------- src/storage/storage_backend.c | 2 +- src/util/virstoragefile.c | 4 ++-- src/util/virutil.c | 35 ++++++++------------------- 5 files changed, 44 insertions(+), 58 deletions(-)
Nice, a cleanup and bugfix in one package. ACK Michal