Re: [PULL 0/8] 9p queue 2021-10-27

[Philippe Mathieu-Daudé]

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.)