On 10/27/21 10:29 AM, Christian Schoenebeck wrote:
On Mittwoch, 27. Oktober 2021 18:48:10 CEST Philippe Mathieu-Daudé wrote:
On 10/27/21 18:21, Christian Schoenebeck wrote:
On Mittwoch, 27. Oktober 2021 17:36:03 CEST Philippe Mathieu-Daudé wrote:
Hi Christian,
On 10/27/21 16:05, Christian Schoenebeck wrote:
On Mittwoch, 27. Oktober 2021 15:18:33 CEST Christian Schoenebeck wrote:
The following changes since commit
931ce30859176f0f7daac6bac255dae5eb21284e:
Merge remote-tracking branch
'remotes/dagrh/tags/pull-virtiofs-20211026'
into staging (2021-10-26 07:38:41 -0700)
are available in the Git repository at:
https://github.com/cschoenebeck/qemu.git tags/pull-9p-20211027
for you to fetch changes up to
7e985780aaab93d2c5be9b62d8d386568dfb071e:
9pfs: use P9Array in v9fs_walk() (2021-10-27 14:45:22 +0200)
----------------------------------------------------------------
9pfs: performance fix and cleanup
* First patch fixes suboptimal I/O performance on guest due to
previously
incorrect block size being transmitted to 9p client.
* Subsequent patches are cleanup ones intended to reduce code
complexity.
----------------------------------------------------------------
Christian Schoenebeck (8):
9pfs: fix wrong I/O block size in Rgetattr
9pfs: deduplicate iounit code
9pfs: simplify blksize_to_iounit()
9pfs: introduce P9Array
fsdev/p9array.h: check scalar type in P9ARRAY_NEW()
9pfs: make V9fsString usable via P9Array API
9pfs: make V9fsPath usable via P9Array API
9pfs: use P9Array in v9fs_walk()
fsdev/9p-marshal.c | 2 +
fsdev/9p-marshal.h | 3 +
fsdev/file-op-9p.h | 2 +
fsdev/p9array.h | 160
+++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/9pfs/9p.c
70 +++++++++++++----------
5 files changed, 208 insertions(+), 29 deletions(-)
create mode 100644 fsdev/p9array.h
Regarding last 5 patches: Daniel raised a concern that not using
g_autoptr
would deviate from current QEMU coding patterns:
https://lists.gnu.org/archive/html/qemu-devel/2021-10/msg00081.html
Unfortunately I saw no way to address his concern without adding
unnecessary code complexity, so I decided to make this a 9p local type
(QArray -> P9Array) for now, which can easily be replaced in future
(e.g.
when there will be something appropriate on glib side).
Hmm various patches aren't reviewed yet... In particular
patch #5 has a Suggested-by tag without Reviewed-by, this
looks odd.
See https://wiki.qemu.org/Contribute/SubmitAPullRequest:
Don't send pull requests for code that hasn't passed review.
A pull request says these patches are ready to go into QEMU now,
so they must have passed the standard code review processes. In
particular if you've corrected issues in one round of code review,
you need to send your fixed patch series as normal to the list;
you can't put it in a pull request until it's gone through.
(Extremely trivial fixes may be OK to just fix in passing, but
if in doubt err on the side of not.)
There are in general exactly two persons adding their RBs to 9p patches,
which is either Greg or me, and Greg made it already clear that he barely
has time for anything above trivial set.
So what do you suggest? You want to participate and review 9p patches?
Well I am a bit surprised...
$ git log --oneline \
--grep='Reviewed-by: Philippe Mathieu-Daudé' -- hw/9pfs/ | wc -l
18
I also reviewed patch #3 if this pull request...
Now I see you posted this 4 times in 2 months, so indeed eventual
reviewers had plenty of time to look at your patches.
Note I haven't said I'd NAck your pull request, I noticed your own
concern wrt Daniel comment, so I looked at the patch and realized
it was not reviewed, and simply said this is this is odd.
Regards,
Phil.
Philippe, of course I understand why this looks odd to you, and yes you
reviewed that particular patch. But the situation on the 9p front is like this
for >2 years now: people quickly come by to nack patches, but even after
having addressed their concerns they barely add their RBs afterwards. That
means I am currently forced to send out PRs without RBs once in a while.
In know the feeling -- it takes quite some prodding to get tcg patches reviewed, and I
have also sent out PRs that are incompletely reviewed.
I see that patch 5 was something I suggested myself, which I then failed to review
afterward. In recompense, I have reviewed the whole patch set:
Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
I think the P9Array is fairly clever, and I do prefer it over glib ugliness. I can
imagine only C++ being an improvement over what you've created.
Rather than force you to re-create the PR, I'll simply apply this along with the S-o-b, to
the merge object.
r~
