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

[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

[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: 

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

[Fotis Xenakis]

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?

Τη Τετάρτη, 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: mounting ramfs at /
> VFS: mounting devfs at /dev
> net: initializing - done
> vga: Add VGA device instance
> eth0: ethernet address: 52:54:00:12:34:56
> virtio-blk: Add blk device instances 0 as vblk0, devsize=6470656
> random: virtio-rng registered as a source.
> virtio-fs: Detected device with tag: [myfs] and num_queues: 1
> virtio-fs: Detected DAX window with length 67108864
> virtio-fs: Add device instance 0 as [virtiofs1]
> random: intel drng, rdrand registered as a source.
> random:  initialized
> VFS: unmounting /dev
> VFS: mounting rofs at /rofs
> VFS: mounting devfs at /dev
> VFS: mounting procfs at /proc
> VFS: 

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

[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: mounting ramfs at /
VFS: mounting devfs at /dev
net: initializing - done
vga: Add VGA device instance
eth0: ethernet address: 52:54:00:12:34:56
virtio-blk: Add blk device instances 0 as vblk0, devsize=6470656
random: virtio-rng registered as a source.
virtio-fs: Detected device with tag: [myfs] and num_queues: 1
virtio-fs: Detected DAX window with length 67108864
virtio-fs: Add device instance 0 as [virtiofs1]
random: intel drng, rdrand registered as a source.
random:  initialized
VFS: unmounting /dev
VFS: mounting rofs at /rofs
VFS: mounting devfs at /dev
VFS: mounting procfs at /proc
VFS: mounting sysfs at /sys
VFS: mounting ramfs at /tmp
VFS: mounting virtiofs at /virtiofs
[virtiofs] Failed to initialize fuse filesystem!
failed to mount virtiofs, error = Protocol error
[I/43 dhcp]: Broadcasting DHCPDISCOVER message with xid: [1603537588]
[I/43 dhcp]: Waiting for IP...
[I/55 dhcp]: Received DHCPOFFER message from DHCP server: 192.168.122.1 
regarding offerred IP address: 192.168.122.15
[I/55 dhcp]: Broadcasting DHCPREQUEST message with xid: [1603537588] to 
SELECT offered IP: 192.168.122.15
[I/55 dhcp]: Received DHCPACK message from DHCP server: 192.168.122.1 
regarding offerred IP address: 192.168.122.15
[I/55 dhcp]: Server acknowledged IP 192.168.122.15 for interface eth0 with 
time to lease in seconds: 86400
eth0: 192.168.122.15
[I/55 dhcp]: Configuring eth0: ip 192.168.122.15 subnet mask 255.255.255.0 
gateway 192.168.122.1 MTU 1500
Booted up in 140.48 ms
Cmdline: /virtiofs/hello
Failed to load object: /virtiofs/hello. Powering off.

# and from virtiofsd [7426562093843] [ID: 0008] INIT: 7.27
[7426562097664] [ID: 0008] flags=0x
[7426562100498] [ID: 0008] max_readahead=0x1000
[7426562104503] [ID: 0008] fuse: unsupported protocol version: 7.27
[7426562119457] [ID: 0008]unique: 1, error: -71 (Protocol error), 
outsize: 16
[7426562125006] [ID: 0008] virtio_send_msg: elem 0: with 2 in desc of 
length 80
[7426577096593] [ID: 0001] virtio_loop: Got VU event

This happens when I use stock QEMU 5.0 (just released a couple of days ago, 
which seems to have not DAX support yet) and qemu version from 
https://gitlab.com/virtio-fs/qemu/-/commits/virtio-dev (see virtio-dev 
branch).

I had to bump the version to 31 and then it works. Could you please 
investigate?

Waldek
 

>   
>  /** The node ID of the root inode */ 
>  #define FUSE_ROOT_ID 1 
> @@ -236,14 +222,10 @@ struct fuse_file_lock { 
>   * FOPEN_DIRECT_IO: bypass page cache for this open file 
>   * FOPEN_KEEP_CACHE: don't invalidate the data cache on open 
>   * FOPEN_NONSEEKABLE: the file is not seekable 
> - * FOPEN_CACHE_DIR: allow caching this directory 
> - * FOPEN_STREAM: the file is stream-like (no file position at all) 
>   */ 
>  #define FOPEN_DIRECT_IO(1 << 0) 
>  #define