From: Markus Armbruster <armbru@redhat.com>
References: <1544701071-2922-1-git-send-email-liq3ea@gmail.com>
Date: Thu, 13 Dec 2018 13:38:20 +0100
In-Reply-To: <1544701071-2922-1-git-send-email-liq3ea@gmail.com> (Li Qiang's
	message of "Thu, 13 Dec 2018 03:37:51 -0800")
Message-ID: <87k1kda9fn.fsf@dusky.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v2] util: check the return value of fcntl
 in qemu_set_{block, nonblock}
To: Li Qiang <liq3ea@gmail.com>
Cc: peter.maydell@linaro.org, marcandre.lureau@redhat.com, pbonzini@redhat.com, berrange@redhat.com, qemu-devel@nongnu.org

Li Qiang <liq3ea@gmail.com> writes:

> Also add diagnostics info in 'qemu_set_cloexec' so that we can know
> what happen when error occurs.
>
> Signed-off-by: Li Qiang <liq3ea@gmail.com>
> ---
> Change since v1: add diagnostics info
>  
>  util/oslib-posix.c | 37 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index c1bee2a581..14cbef1e35 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -38,6 +38,7 @@
>  #include <libgen.h>
>  #include <sys/signal.h>
>  #include "qemu/cutils.h"
> +#include "qemu/error-report.h"
>  
>  #ifdef CONFIG_LINUX
>  #include <sys/syscall.h>
> @@ -233,14 +234,32 @@ void qemu_set_block(int fd)
>  {
>      int f;
>      f = fcntl(fd, F_GETFL);
> -    fcntl(fd, F_SETFL, f & ~O_NONBLOCK);
> +    if (f < 0) {
> +        error_report("Unable to get file status flag on fd %d: %s(errno=%d)",
> +                    fd, strerror(errno), errno);
> +        abort();
> +    }
> +    if (fcntl(fd, F_SETFL, f & ~O_NONBLOCK) < 0) {
> +        error_report("Unable to set blocking flag on fd %d: %s(errno=%d)",
> +                    fd, strerror(errno), errno);
> +        abort();
> +    }
>  }

If this is a programming error, i.e. one that can happen only when the
function gets misused, then abort() is appropriate, but error_report()
is putting lipstick on the pig.

If it's not a programming error, but something outside QEMU misbehaves
(user error, network outage, ...), then error_report() is appropriate,
but abort() is not.  exit(1) then.

[...]