Re: [PATCH v3 6/7] 9p/trans_virtio: support larger msize values

On Montag, 22. November 2021 23:35:18 CET Dominique Martinet wrote:

Christian Schoenebeck wrote on Mon, Nov 22, 2021 at 02:32:23PM +0100:

quoted

I "think" this could be used for all 9p message types except for the
listed
former three, but I'll review the 9p specs more carefully before v4. For
Tread and Twrite we already received the requested size, which just
leaves Treaddir, which is however indeed tricky, because I don't think we
have any info how many directory entries we could expect.

count in Treaddir is a number of bytes, not a number of entries -- so
it's perfect for this as well :)

Yes it is in bytes, but it's currently always simply msize - P9_READDIRHDRSZ:
https://github.com/torvalds/linux/blob/5d9f4cf36721aba199975a9be7863a3ff5cd4b59/fs/9p/vfs_dir.c#L159

As my planned milestone for this series is max. 4 MB msize, it might be okay
for now, but it is something to keep in mind and should be checked whether it
will slow down things.

On the long term, when msize >4MB is supported, this definitely must be
addressed.

quoted

A simple compile time constant (e.g. one macro) could be used instead of
this function. If you prefer a constant instead, I could go for it in v4
of course. For this 9p client I would recommend a function though, simply
because this code has already seen some authors come and go over the
years, so it might be worth the redundant code for future safety. But
I'll adapt to what others think.

In this case a fallback constant seems simpler than a big switch like
you've done, but honestly I'm not fussy at this point -- if you work on
this you have the right to decide this kind of things in my opinion.

My worry with the snippet you listed is that you need to enumerate all
calls again, so if someday the protocol is extended it'll be a place to
forget adding new calls (although compiler warnings help with that),
whereas a fallback constant will always work as long as it's small
messages.

But really, as long as it's not horrible I'll take it :)

Maybe I can find a compromise. :)

Best regards,
Christian Schoenebeck