Re: [Qemu-devel] [PATCH 4/4] hw/9pfs: fix P9_STATS_GEN handling

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> writes:

> Currently we fail getattr request altogether if we can't read
> P9_STATS_GEN for some reason. It breaks valid use cases:
>
> E.g let's assume we have non-readable directory with execution bit set
> on host and we export it to client over 9p On host we can chdir into
> directory, but not open directory on read and list content.
>
> But if client will try to call getattr (as part of chdir(2)) for the
> directory it will fail with -EACCES. It happens because we try to open
> the directory on read to call ioctl(FS_IOC_GETVERSION), it fails and we
> return the error code to client.
>
> It's excessive. The solution is to make P9_STATS_GEN failure non-fatal
> for getattr request. Just don't set P9_STATS_GEN flag in result mask on
> failure.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>


Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

> ---
>  hw/9pfs/cofile.c    |  4 ----
>  hw/9pfs/virtio-9p.c | 12 ++++++++++--
>  2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
> index 194c1306c665..2efebf35710f 100644
> --- a/hw/9pfs/cofile.c
> +++ b/hw/9pfs/cofile.c
> @@ -38,10 +38,6 @@ int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t st_mode,
>              });
>          v9fs_path_unlock(s);
>      }
> -    /* The ioctl may not be supported depending on the path */
> -    if (err == -ENOTTY) {
> -        err = 0;
> -    }
>      return err;
>  }
>
> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
> index 8cbb8ae32a03..83e4e9398347 100644
> --- a/hw/9pfs/virtio-9p.c
> +++ b/hw/9pfs/virtio-9p.c
> @@ -1080,10 +1080,18 @@ static void v9fs_getattr(void *opaque)
>      /*  fill st_gen if requested and supported by underlying fs */
>      if (request_mask & P9_STATS_GEN) {
>          retval = v9fs_co_st_gen(pdu, &fidp->path, stbuf.st_mode, &v9stat_dotl);
> -        if (retval < 0) {
> +        switch (retval) {
> +        case 0:
> +            /* we have valid st_gen: update result mask */
> +            v9stat_dotl.st_result_mask |= P9_STATS_GEN;
> +            break;
> +        case -EINTR:
> +            /* request cancelled, e.g. by Tflush */
>              goto out;
> +        default:
> +            /* failed to get st_gen: not fatal, ignore */
> +            break;
>          }
> -        v9stat_dotl.st_result_mask |= P9_STATS_GEN;
>      }
>      retval = pdu_marshal(pdu, offset, "A", &v9stat_dotl);
>      if (retval < 0) {
> -- 
> 1.8.5.2