[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] util: fix virFileOpenAs return value and resulting error logs

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.



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]