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);
}
- waited = wait_serialising_requests(req);
+ waited = wait_serialising_requests(req, flags & BDRV_REQ_ALLOCATE);
+ if (waited && flags & BDRV_REQ_ALLOCATE) {
+ return -EAGAIN;
+ }
I find this more readable (even if not strictly necessary):
if (waited && (flags & BDRV_REQ_ALLOCATE)) {
Done!
None of my two comments are blockers, though, so
Reviewed-by: Alberto Garcia <be...@igalia.com>
Berto
