It's nice to see it pick up more functionality (readdir sure was 
necessary). My next goal is booting from virtio-fs (probably taking an 
approach similar to ROFS), hoping that will come together within the next 
couple of weeks.

The omission of d_off is not much of an issue, just something that striked 
me while reading other readdir implementations. From the manpage: "*The  
value  returned in d_off is the same as would be returned by calling 
telldir(3) at the current position in the directory stream.*" Since our 
telldir implementation returns the value of file.f_offset 
(https://github.com/cloudius-systems/osv/blob/a16e5c9c0ef11217be374824587e05225fbfde61/fs/vfs/vfs_syscalls.cc#L508),
 
I guess that making sure that's also returned in d_off is a safe choice. I 
shall get round to sending a patch for this soon!

Στις Τετάρτη, 22 Ιουλίου 2020 στις 3:57:58 π.μ. UTC+3, ο χρήστης 
[email protected] έγραψε:

> Fotis,
>
> Thanks for these patches - it makes virtiofs almost fully functional.
>
> Regarding the d_off issue in other filesystems, I would welcome your patch 
> to fix it. What exact symptom is caused by this incorrect implementation of 
> d_off?
>
> Now according to this - 
> https://man7.org/linux/man-pages/man3/readdir.3.html - d_off is not 
> something that userspace should necessarily rely on and I am not sure how 
> easy it will be to fix it for each other FS. 
>
> Waldek
>
>
> On Sun, Jul 19, 2020 at 5:58 PM Fotis Xenakis <[email protected]> 
> wrote:
>
>> This set of patches adds readdir() support for virtio-fs: the first
>> patch contains minor fixes, while the second and third lay the ground
>> work for the fourth which actually implements the readdir functionality.
>>
>> While working on this, I noticed that the readdir implementations for
>> other OSv filesystems (e.g. ROFS, devfs, pseudofs) don't set the d_off
>> field of the dirent struct they return (this patch _does_ set it). Is
>> there a reason behind this pattern? If so, I will be glad to align this
>> implementation with the rest.
>>
>> As always, any feedback is more than welcome.
>>
>> Fotis Xenakis (4):
>>   virtio-fs: fix allocation failure condition
>>   virtio-fs: expose FUSE reply length
>>   virtio-fs: use FUSE_OPENDIR and FUSE_RELEASEDIR
>>   virtio-fs: implement readdir
>>
>>  fs/virtiofs/virtiofs_dax.cc    |   4 +-
>>  fs/virtiofs/virtiofs_i.hh      |   8 ++-
>>  fs/virtiofs/virtiofs_vfsops.cc |  16 +++--
>>  fs/virtiofs/virtiofs_vnops.cc  | 103 ++++++++++++++++++++++++++++-----
>>  4 files changed, 106 insertions(+), 25 deletions(-)
>>
>> -- 
>> 2.27.0
>>
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "OSv Development" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to [email protected].
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/osv-dev/AM0PR03MB62928C209CFA5D9AF2F1731BA67A0%40AM0PR03MB6292.eurprd03.prod.outlook.com
>> .
>>
>

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/ddc8862e-45fe-4bd7-9e39-0c6cced22d7dn%40googlegroups.com.

Reply via email to