qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 3/7] block: add max_hw_transfer to BlockLimits


From: Max Reitz
Subject: Re: [PATCH v3 3/7] block: add max_hw_transfer to BlockLimits
Date: Wed, 16 Jun 2021 15:46:40 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1

On 16.06.21 15:18, Paolo Bonzini wrote:
On 15/06/21 18:18, Max Reitz wrote:
  }
+/* Returns the maximum hardware transfer length, in bytes; guaranteed nonzero */
+uint64_t blk_get_max_hw_transfer(BlockBackend *blk)
+{
+    BlockDriverState *bs = blk_bs(blk);
+    uint64_t max = INT_MAX;
+
+    if (bs) {
+        max = MIN_NON_ZERO(bs->bl.max_hw_transfer, bs->bl.max_transfer);
+    }
+    return max;

Both `max_hw_transfer` and `max_transfer` can be 0, so this could return 0, contrary to what the comment above promises.

Should `max` be initialized to 0 with a `MIN_NON_ZERO(max, INT_MAX)` here (like `blk_get_max_transfer()` does it)?

Yes, something to that effect.

(As for the rest, I think aligning to the request alignment makes sense, but then again we don’t do that for max_transfer either, so... this at least wouldn’t be a new bug.

Ok, will do.  I will also add a new patch to align max_transfer to the request alignment.

Regarding the comment, checkpatch complains about it, so it should be fixed so that /* is on its own line.

That makes it different from every other comment in block_int.h though.  Is it okay to fix all of them in a follow-up?

The reason it’s different is that the comment style in question was added to checkpatch only relatively recently. I can’t speak for others, but I’m a simple person. I just do what makes checkpatch happy. :)

Given that the checkpatch complaint is only a warning, I think it’s OK to keep the comment as it is here, and perhaps optionally fix all comments in block_int.h in a follow-up. I don’t think we need to fix existing comments, but, well, it wouldn’t be wrong.

Max




reply via email to

[Prev in Thread] Current Thread [Next in Thread]