[libvirt] [PATCH] Log an error when we fail to set the COW attribute

John Ferlan jferlan at redhat.com
Thu Jul 17 11:54:06 UTC 2014



On 07/17/2014 06:22 AM, Ján Tomko wrote:
> Coverity complains about the return value of ioctl not being checked.
> 
> Even though we carry on when this fails (just like qemu-img does),
> we can log an error.
> ---
>  src/storage/storage_backend.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 5e7aa3c..b8b89ca 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -464,9 +464,14 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
>           * The FS_IOC_SETFLAGS ioctl return value will be ignored since any
>           * failure of this operation should not block the left work.
>           */
> -        if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0) {
> +        if (ioctl(fd, FS_IOC_GETFLAGS, &attr) < 0) {
> +            virReportSystemError(errno, "%s", _("Failed to get fs flags"));
> +        } else {
>              attr |= FS_NOCOW_FL;
> -            ioctl(fd, FS_IOC_SETFLAGS, &attr);
> +            if (ioctl(fd, FS_IOC_SETFLAGS, &attr) < 0) {
> +                virReportSystemError(errno, "%s",
> +                                     _("Failed to set NOCOW flag"));
> +            }
>          }
>  #endif
>      }
> 

Looks like you were already looking at the failure when my Coverity scan
ran... Should have checked if there already was a patch before sending
my response in Chunyan Liu's original series.

In any case, seems better to me to at least log the error whether or not
the code wants to "block the left work" (any chance to clean that
comment up a bit :-) - hopefully there's not too much "right work" left
either).

ACK,

John




More information about the libvir-list mailing list