archive mirror
 help / color / mirror / Atom feed
From: Dominique Martinet <>
To: Christian Schoenebeck <>
Cc: Nikolay Kichukov <>,,,
	Eric Van Hensbergen <>,
	Latchesar Ionkov <>, Greg Kurz <>,
	Vivek Goyal <>
Subject: Re: [PATCH v3 6/7] 9p/trans_virtio: support larger msize values
Date: Tue, 23 Nov 2021 07:35:18 +0900	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <1797352.eH9cFvQebf@silver>

Christian Schoenebeck wrote on Mon, Nov 22, 2021 at 02:32:23PM +0100:
> 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 :)

> 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

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


  reply	other threads:[~2021-11-23  3:29 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-22 16:00 [PATCH v3 0/7] net/9p: remove msize limit in virtio transport Christian Schoenebeck
2021-09-22 16:00 ` [PATCH v3 1/7] net/9p: show error message if user 'msize' cannot be satisfied Christian Schoenebeck
2021-09-22 16:00 ` [PATCH v3 2/7] 9p/trans_virtio: separate allocation of scatter gather list Christian Schoenebeck
2021-09-22 16:00 ` [PATCH v3 3/7] 9p/trans_virtio: turn amount of sg lists into runtime info Christian Schoenebeck
2021-09-22 16:00 ` [PATCH v3 4/7] 9p/trans_virtio: introduce struct virtqueue_sg Christian Schoenebeck
2021-09-22 16:00 ` [PATCH v3 5/7] net/9p: add trans_maxsize to struct p9_client Christian Schoenebeck
2021-09-22 16:00 ` [PATCH v3 6/7] 9p/trans_virtio: support larger msize values Christian Schoenebeck
2021-11-20 11:20   ` Nikolay Kichukov
2021-11-20 11:45     ` Dominique Martinet
2021-11-20 14:46       ` Christian Schoenebeck
2021-11-20 21:28         ` Nikolay Kichukov
2021-11-20 23:02         ` Dominique Martinet
2021-11-21 16:57           ` Christian Schoenebeck
2021-11-21 22:12             ` Dominique Martinet
2021-11-22 13:32               ` Christian Schoenebeck
2021-11-22 22:35                 ` Dominique Martinet [this message]
2021-11-24 12:22                   ` Christian Schoenebeck
2021-09-22 16:00 ` [PATCH v3 7/7] 9p/trans_virtio: resize sg lists to whatever is possible Christian Schoenebeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).