[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 0/8] 9p queue 2021-10-27
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PULL 0/8] 9p queue 2021-10-27 |
Date: |
Wed, 27 Oct 2021 20:11:33 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 |
Hi Richard,
On 10/27/21 19:29, 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:
>>>> On 10/27/21 16:05, Christian Schoenebeck wrote:
>>>>> 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.
>
> The mentioned 5 patches look like overkill on first sight, but they are just
> intended as preparatory ones. I actually should fix a protocol implementation
> deficit in "Twalk" request handling, and that in turn means I will have to
> add
> complexity to function v9fs_walk(). But before even daring to do so, I should
> get rid of as much of complexity as possible. Because we already had a bunch
> of issues in that function before. I believe these are trivial 5 patches. But
> I can also accompany them with test cases if somebody is worried.
>
> Greg, I could also drop them now, but the general issue will retain: Reality
> is that I am the only person working on 9p right now and a fact that I cannot
> change. The rest is for other people to decide.
To be explicit, I just asked clarifications to Christian. I understand
the 9pfs subsystem situation, and I am not against (Nacking) this pull
request being merged.
Thanks both,
Phil.
- [PULL 4/8] 9pfs: introduce P9Array, (continued)
- [PULL 4/8] 9pfs: introduce P9Array, Christian Schoenebeck, 2021/10/27
- [PULL 5/8] fsdev/p9array.h: check scalar type in P9ARRAY_NEW(), Christian Schoenebeck, 2021/10/27
- [PULL 7/8] 9pfs: make V9fsPath usable via P9Array API, Christian Schoenebeck, 2021/10/27
- [PULL 6/8] 9pfs: make V9fsString usable via P9Array API, Christian Schoenebeck, 2021/10/27
- [PULL 8/8] 9pfs: use P9Array in v9fs_walk(), Christian Schoenebeck, 2021/10/27
- Re: [PULL 0/8] 9p queue 2021-10-27, Christian Schoenebeck, 2021/10/27
- Re: [PULL 0/8] 9p queue 2021-10-27, Philippe Mathieu-Daudé, 2021/10/27
- Re: [PULL 0/8] 9p queue 2021-10-27, Christian Schoenebeck, 2021/10/27
- Re: [PULL 0/8] 9p queue 2021-10-27, Philippe Mathieu-Daudé, 2021/10/27
- Re: [PULL 0/8] 9p queue 2021-10-27, Christian Schoenebeck, 2021/10/27
- Re: [PULL 0/8] 9p queue 2021-10-27,
Philippe Mathieu-Daudé <=
- Re: [PULL 0/8] 9p queue 2021-10-27, Richard Henderson, 2021/10/27
- Re: [PULL 0/8] 9p queue 2021-10-27, Christian Schoenebeck, 2021/10/28
Re: [PULL 0/8] 9p queue 2021-10-27, Greg Kurz, 2021/10/27
Re: [PULL 0/8] 9p queue 2021-10-27, Richard Henderson, 2021/10/27