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

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

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]