Re: [osv-dev] Re: [PATCH 3/6] virtio-fs: update fuse protocol header

2020-04-30 Thread Fotis Xenakis 

Τη Παρασκευή, 1 Μαΐου 2020 - 1:31:00 π.μ. UTC+3, ο χρήστης Waldek Kozaczuk 
έγραψε:
>
>
>
> On Thu, Apr 30, 2020 at 6:19 PM Fotis Xenakis  > wrote:
>
>> Indeed, QEMU 5.0 does not support DAX and the virtiofsd in QEMU 5.0 won't 
>> accept any version other than 7.31 as I see here 
>> ,
>>  
>> thus the mount fails.
>> Both on the QEMU and the Linux side, DAX is not close to upstreaming yet. 
>> Although it seems no longer marked as "experimental" here 
>> , I think it's still under 
>> development (*not* verified with the devs) and that's the source for 
>> some instability.
>>
>> To summarize:
>>
>>- Upstream QEMU 5.0 includes stable virtio-fs support, with the basic 
>>feature set. It negotiates *FUSE 7.31* (latest in upstream Linux).
>>- Downstream virtio-fs QEMU  
>>currently contains:
>>   - The default (thus recommended in the docs 
>>   ) virtio-fs 
>>    branch. This 
>>   negotiates *FUSE 7.27* and supports DAX. This is the one I have 
>>   based my patches upon, because it is the most stable *with DAX 
>>   support*.
>>   - The development branches, virtio-dev 
>>    and 
>>   virtio-fs-dev 
>>    (don't 
>>   know what distinguishes them TBH). They both negotiate *FUSE 7.31* 
>>   and support DAX (with changed protocol details). These iterate 
>> quickly, so 
>>   I haven't used them.
>>
>> I hadn't anticipated this hard constraint upstream, which poses a 
>> problem, since I guess we want to be compatible with it.
>> My plan is to reach out to the virtio-fs devs, asking for the status of 
>> DAX in the dev branches. If they deem it stabilized, I will probably try to 
>> go with those, offering upstream compatibility *and* DAX.
>> Otherwise, we could have a hybrid approach, compatible with upstream for 
>> the stable features, but following the more stale "virtio-fs" downstream 
>> branch as far as DAX is concerned.
>> What do you think?
>>
> I am not sure I 100% understand what you are proposing. Adding some kind 
> of negotiating logic on OSv side that will be able to deal with both 27 and 
> 31 and "advertise" accordingly? Can we simply send 31 if there is no DAX 
> window detected in the driver layer and 27 otherwise?
>

> I guess for we could just keep this header per 31 and 
> add FUSE_SETUPMAPPING AND FUSE_REMOVEMAPPING to our header, no?
>
This is the "hybrid" approach I was thinking of above and the one I will go 
with for now.
Also, I will contact the virtio-fs devs for insight on how the project will 
evolve in the near future.

>
> Meanwhile I will rollback this particular patch to make OSv work with with 
> stock qemu and virtiofs. 
>
Absolutely, this makes sense. 

>
>> Τη Τετάρτη, 29 Απριλίου 2020 - 7:48:02 μ.μ. UTC+3, ο χρήστης Waldek 
>> Kozaczuk έγραψε:
>>>
>>> On Monday, April 20, 2020 at 5:04:27 PM UTC-4, Fotis Xenakis wrote:

 Copy from virtiofsd @ 32006c66f2578af4121d7effaccae4aa4fa12e46. This 
 includes the definitions for FUSE_SETUPMAPPING AND FUSE_REMOVEMAPPING. 

 Signed-off-by: Fotis Xenakis  
 --- 
  fs/virtiofs/fuse_kernel.h | 82 ++- 
  1 file changed, 38 insertions(+), 44 deletions(-) 

 diff --git a/fs/virtiofs/fuse_kernel.h b/fs/virtiofs/fuse_kernel.h 
 index 018a00a2..ce46046a 100644 
 --- a/fs/virtiofs/fuse_kernel.h 
 +++ b/fs/virtiofs/fuse_kernel.h 
 @@ -44,7 +44,6 @@ 
   *  - add lock_owner field to fuse_setattr_in, fuse_read_in and 
 fuse_write_in 
   *  - add blksize field to fuse_attr 
   *  - add file flags field to fuse_read_in and fuse_write_in 
 - *  - Add ATIME_NOW and MTIME_NOW flags to fuse_setattr_in 
   * 
   * 7.10 
   *  - add nonseekable open flag 
 @@ -55,7 +54,7 @@ 
   *  - add POLL message and NOTIFY_POLL notification 
   * 
   * 7.12 
 - *  - add umask flag to input argument of create, mknod and mkdir 
 + *  - add umask flag to input argument of open, mknod and mkdir 
   *  - add notification messages for invalidation of inodes and 
   *directory entries 
   * 
 @@ -120,19 +119,6 @@ 
   * 
   *  7.28 
   *  - add FUSE_COPY_FILE_RANGE 
 - *  - add FOPEN_CACHE_DIR 
 - *  - add FUSE_MAX_PAGES, add max_pages to init_out 
 - *  - add FUSE_CACHE_SYMLINKS 
 - * 
 - *  7.29 
 - *  - add FUSE_NO_OPENDIR_SUPPORT flag 
 - * 
 - *  7.30 
 - *  - add FUSE_EXPLICIT_INVAL_DATA 
 - *  - add FUSE_IOCTL_COMPAT_X32 
 - * 
 - *  7.31 
 - *  - add FUSE_WRITE_KILL_PRIV

Re: [osv-dev] Re: [PATCH 3/6] virtio-fs: update fuse protocol header

2020-04-30 Thread Waldek Kozaczuk 
On Thu, Apr 30, 2020 at 6:19 PM Fotis Xenakis  wrote:

> Indeed, QEMU 5.0 does not support DAX and the virtiofsd in QEMU 5.0 won't
> accept any version other than 7.31 as I see here
> ,
> thus the mount fails.
> Both on the QEMU and the Linux side, DAX is not close to upstreaming yet.
> Although it seems no longer marked as "experimental" here
> , I think it's still under
> development (*not* verified with the devs) and that's the source for some
> instability.
>
> To summarize:
>
>- Upstream QEMU 5.0 includes stable virtio-fs support, with the basic
>feature set. It negotiates *FUSE 7.31* (latest in upstream Linux).
>- Downstream virtio-fs QEMU 
>currently contains:
>   - The default (thus recommended in the docs
>   ) virtio-fs
>    branch. This
>   negotiates *FUSE 7.27* and supports DAX. This is the one I have
>   based my patches upon, because it is the most stable *with DAX
>   support*.
>   - The development branches, virtio-dev
>    and
>   virtio-fs-dev
>    (don't
>   know what distinguishes them TBH). They both negotiate *FUSE 7.31*
>   and support DAX (with changed protocol details). These iterate quickly, 
> so
>   I haven't used them.
>
> I hadn't anticipated this hard constraint upstream, which poses a problem,
> since I guess we want to be compatible with it.
> My plan is to reach out to the virtio-fs devs, asking for the status of
> DAX in the dev branches. If they deem it stabilized, I will probably try to
> go with those, offering upstream compatibility *and* DAX.
> Otherwise, we could have a hybrid approach, compatible with upstream for
> the stable features, but following the more stale "virtio-fs" downstream
> branch as far as DAX is concerned.
> What do you think?
>
I am not sure I 100% understand what you are proposing. Adding some kind of
negotiating logic on OSv side that will be able to deal with both 27 and 31
and "advertise" accordingly? Can we simply send 31 if there is no DAX
window detected in the driver layer and 27 otherwise?

I guess for we could just keep this header per 31 and add FUSE_SETUPMAPPING
AND FUSE_REMOVEMAPPING to our header, no?

Meanwhile I will rollback this particular patch to make OSv work with with
stock qemu and virtiofs.

>
> Τη Τετάρτη, 29 Απριλίου 2020 - 7:48:02 μ.μ. UTC+3, ο χρήστης Waldek
> Kozaczuk έγραψε:
>>
>> On Monday, April 20, 2020 at 5:04:27 PM UTC-4, Fotis Xenakis wrote:
>>>
>>> Copy from virtiofsd @ 32006c66f2578af4121d7effaccae4aa4fa12e46. This
>>> includes the definitions for FUSE_SETUPMAPPING AND FUSE_REMOVEMAPPING.
>>>
>>> Signed-off-by: Fotis Xenakis 
>>> ---
>>>  fs/virtiofs/fuse_kernel.h | 82 ++-
>>>  1 file changed, 38 insertions(+), 44 deletions(-)
>>>
>>> diff --git a/fs/virtiofs/fuse_kernel.h b/fs/virtiofs/fuse_kernel.h
>>> index 018a00a2..ce46046a 100644
>>> --- a/fs/virtiofs/fuse_kernel.h
>>> +++ b/fs/virtiofs/fuse_kernel.h
>>> @@ -44,7 +44,6 @@
>>>   *  - add lock_owner field to fuse_setattr_in, fuse_read_in and
>>> fuse_write_in
>>>   *  - add blksize field to fuse_attr
>>>   *  - add file flags field to fuse_read_in and fuse_write_in
>>> - *  - Add ATIME_NOW and MTIME_NOW flags to fuse_setattr_in
>>>   *
>>>   * 7.10
>>>   *  - add nonseekable open flag
>>> @@ -55,7 +54,7 @@
>>>   *  - add POLL message and NOTIFY_POLL notification
>>>   *
>>>   * 7.12
>>> - *  - add umask flag to input argument of create, mknod and mkdir
>>> + *  - add umask flag to input argument of open, mknod and mkdir
>>>   *  - add notification messages for invalidation of inodes and
>>>   *directory entries
>>>   *
>>> @@ -120,19 +119,6 @@
>>>   *
>>>   *  7.28
>>>   *  - add FUSE_COPY_FILE_RANGE
>>> - *  - add FOPEN_CACHE_DIR
>>> - *  - add FUSE_MAX_PAGES, add max_pages to init_out
>>> - *  - add FUSE_CACHE_SYMLINKS
>>> - *
>>> - *  7.29
>>> - *  - add FUSE_NO_OPENDIR_SUPPORT flag
>>> - *
>>> - *  7.30
>>> - *  - add FUSE_EXPLICIT_INVAL_DATA
>>> - *  - add FUSE_IOCTL_COMPAT_X32
>>> - *
>>> - *  7.31
>>> - *  - add FUSE_WRITE_KILL_PRIV flag
>>>   */
>>>
>>>  #ifndef _LINUX_FUSE_H
>>> @@ -168,7 +154,7 @@
>>>  #define FUSE_KERNEL_VERSION 7
>>>
>>>  /** Minor version number of this interface */
>>> -#define FUSE_KERNEL_MINOR_VERSION 31
>>> +#define FUSE_KERNEL_MINOR_VERSION 27
>>>
>> I have applied this patch but when I started testing your later patches
>> that enable DAX logic I would get error messages about the wrong protocol
>> version:
>>
>> OSv v0.54.0-179-g2f92fc91
>> 4 CPUs detected
>> Firmware vendor: SeaBIOS
>> bsd: initializing - done
>> VFS: