[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 4/9] block: treat BDRV_REQ_ALLOCATE as serial
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [PATCH v7 4/9] block: treat BDRV_REQ_ALLOCATE as serialising |
Date: |
Thu, 01 Feb 2018 15:01:42 +0100 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Wed 31 Jan 2018 06:11:27 PM CET, Anton Nefedov wrote:
> On 31/1/2018 6:11 PM, Alberto Garcia wrote:
>> On Thu 18 Jan 2018 06:49:02 PM CET, Anton Nefedov wrote:
>>
>>> -static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest
>>> *self)
>>> +static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest
>>> *self,
>>> + bool nowait)
>>
>> It's a bit confusing to have a function called wait_foo() with a
>> parameter that says "don't wait"...
>>
>> How about
>>
>> check_serialising_requests(BdrvTrackedRequest *self, bool wait)
>>
>
> I think it might be more important to emphasize in the name that the
> function _might_ wait.
>
> i.e. it feels worse to read
> check_serialising_requests(req, true);
> when one needs to follow the function to find out that it might yield.
>
> Personally I'd vote for
>
> static int check_or_wait_serialising_requests(
> BdrvTrackedRequest *self, bool wait) {}
>
> and maybe even:
>
> static int check_serialising_requests(BdrvTrackedRequest *self) {
> return check_or_wait_serialising_requests(self, false);
>
> static int wait_serialising_requests(BdrvTrackedRequest *self) {
> return check_or_wait_serialising_requests(self, true);
> }
You're right. Either approach works for me though, also keeping the
current solution, wait_serialising_requests(req, true).
Berto
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v7 4/9] block: treat BDRV_REQ_ALLOCATE as serialising,
Alberto Garcia <=