Re: [Qemu-devel] [PATCH v2] util: check the return value of fcntl in qemu_set_{block, nonblock}

[Markus Armbruster]

Li Qiang <address@hidden> writes:

> Also add diagnostics info in 'qemu_set_cloexec' so that we can know
> what happen when error occurs.
>
> Signed-off-by: Li Qiang <address@hidden>
> ---
> 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.

[...]