Re: [Qemu-devel] [PATCH 02/13] 9p: Avoid warning if FS_IOC_GETVERSION is not defined

[Greg Kurz <groug@kaod.org>]

On Sat, 26 May 2018 01:23:04 -0400
keno@juliacomputing.com wrote:

> From: Keno Fischer <keno@alumni.harvard.edu>
> 
> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> ---
>  hw/9pfs/9p-local.c | 39 +++++++++++++++++++--------------------
>  1 file changed, 19 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index f6c7526..7592f8d 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -1375,10 +1375,10 @@ static int local_unlinkat(FsContext *ctx, V9fsPath *dir,
>      return ret;
>  }
>  
> +#ifdef FS_IOC_GETVERSION
>  static int local_ioc_getversion(FsContext *ctx, V9fsPath *path,
>                                  mode_t st_mode, uint64_t *st_gen)
>  {
> -#ifdef FS_IOC_GETVERSION
>      int err;
>      V9fsFidOpenState fid_open;
>  
> @@ -1397,15 +1397,11 @@ static int local_ioc_getversion(FsContext *ctx, V9fsPath *path,
>      err = ioctl(fid_open.fd, FS_IOC_GETVERSION, st_gen);
>      local_close(ctx, &fid_open);
>      return err;
> -#else
> -    errno = ENOTTY;
> -    return -1;
> -#endif
>  }
> +#endif
>  
>  static int local_init(FsContext *ctx, Error **errp)
>  {
> -    struct statfs stbuf;
>      LocalData *data = g_malloc(sizeof(*data));
>  
>      data->mountfd = open(ctx->fs_root, O_DIRECTORY | O_RDONLY);
> @@ -1415,20 +1411,23 @@ static int local_init(FsContext *ctx, Error **errp)
>      }
>  
>  #ifdef FS_IOC_GETVERSION
> -    /*
> -     * use ioc_getversion only if the ioctl is definied
> -     */
> -    if (fstatfs(data->mountfd, &stbuf) < 0) {
> -        close_preserve_errno(data->mountfd);

Hmm... I now realize that this path doesn't set errp, which means that
we could possibly fail the device realization without reporting any
error to the caller. Could you please fix this in a preparatory patch ?

> -        goto err;
> -    }
> -    switch (stbuf.f_type) {
> -    case EXT2_SUPER_MAGIC:
> -    case BTRFS_SUPER_MAGIC:
> -    case REISERFS_SUPER_MAGIC:
> -    case XFS_SUPER_MAGIC:
> -        ctx->exops.get_st_gen = local_ioc_getversion;
> -        break;
> +    {
> +        struct statfs stbuf;
> +        /*
> +        * use ioc_getversion only if the ioctl is definied
> +        */
> +        if (fstatfs(data->mountfd, &stbuf) < 0) {
> +            close_preserve_errno(data->mountfd);
> +            goto err;
> +        }
> +        switch (stbuf.f_type) {
> +        case EXT2_SUPER_MAGIC:
> +        case BTRFS_SUPER_MAGIC:
> +        case REISERFS_SUPER_MAGIC:
> +        case XFS_SUPER_MAGIC:
> +            ctx->exops.get_st_gen = local_ioc_getversion;
> +            break;
> +        }

Please move this to a separate local_ioc_getversion_init() function, that would
be empty if FS_IOC_GETVERSION is not defined.

>      }
>  #endif
>