Re: [Qemu-devel] [PATCH] 9p: Be robust against paths without FS_IOC_GETVERSION

[Aneesh Kumar K.V]

Gabriel de Perthuis <address@hidden> writes:

> Le 10/05/2013 05:30, Aneesh Kumar K.V a écrit :
>> Gabriel de Perthuis <address@hidden> writes:
>> 
>>> The current implementation checked for supported filesystems at mount
>>> time, but actual support depends on the path.  Don't error out when
>>> finding unversioned paths.
>> 
>> Can you elaborate this a bit ?
>
> ioctl support generally depends on the mount point.
>
> You have a check that verifies the filesystem at the root of the source side
> is ext4 or something else whitelisted to have generation support, but that's
> not sufficient, the source may contain other mountpoints that don't support
> the getversion ioctl.  When a path gives you ENOTTY, that means the ioctl
> is not supported.

May be we can update the commit message to be more clear. ie, currently
the st_gen support is decided based on whether export file system
support ioc_getversion ioctl. But we could have other file system mounted 
further down
in the exported path that doesn't support ioc_getversion. So instead of
failing stat request, we do the same we did for the export path. If we
find the ioctl not supported we silently ignore the error. This results in us
returning st_gen value 0 which should be ok 

Reviewed-by: Aneesh Kumar K.V <address@hidden>

>
>>> This fix allows booting a linux kernel with the same / filesystem as the
>>> host; otherwise the boot fails when mounting devtmpfs.
>>>
>>> Signed-off-by: Gabriel de Perthuis <address@hidden>
>>> ---
>>>  hw/9pfs/cofile.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
>>> index 2efebf3..194c130 100644
>>> --- a/hw/9pfs/cofile.c
>>> +++ b/hw/9pfs/cofile.c
>>> @@ -36,10 +36,14 @@ int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t 
>>> st_mode,
>>>                      err = -errno;
>>>                  }
>>>              });
>>>          v9fs_path_unlock(s);
>>>      }
>>> +    /* The ioctl may not be supported depending on the path */
>>> +    if (err == -ENOTTY) {
>>> +        err = 0;
>>> +    }
>> 
>> So you don't want to consider -ENOTTY as an error ? why ?
>
> ENOTTY means the ioctl is not supported.
> That should be handled the same as if the source-side root didn't have a
> whitelisted filesystem.  That means we need to return 0 in both cases.
>
>>>      return err;
>>>  }
>>>
>>>  int v9fs_co_lstat(V9fsPDU *pdu, V9fsPath *path, struct stat *stbuf)
>>>  {
>>> -- 
>> 
>> -aneesh