[RFC,0/2] Split padded I/O vectors exceeding IOV_MAX

Message

Hanna Czenczek March 15, 2023, 12:13 p.m. UTC

Hi,

We accept I/O vectors with up to 1024 (IOV_MAX) elements from guests.
When a guest request does not conform to the underlying storage's
alignment requirements, we pad it with head and/or tail buffers as
necessary, which are simply appended to the I/O vector.

As of 4c002cef, we (sensibly) check that such-padded vectors do not then
exceed IOV_MAX.  However, there seems to be no sensible error handling;
instead, the guest simply receives an I/O error.

That’s a problem, because it submitted a perfectly sensible I/O request
(which adhered to the alignment the guest device has announced), but
receives an error back.  That shouldn’t happen.

I think we need to handle such excess lengths internally, but I’m not
sure how exactly.  What I can think of is either breaking up the request
into two (which seems like it might cause side effects) or to join up to
three vector elements into one, using a bounce buffer.  The latter
seemed simpler to me, so this RFC does that.  Still, it’s an RFC because
I don’t know whether that’s the ideal solution.


Hanna Czenczek (2):
  block: Split padded I/O vectors exceeding IOV_MAX
  iotests/iov-padding: New test

 block/io.c                               | 139 ++++++++++++++++++++++-
 util/iov.c                               |   4 -
 tests/qemu-iotests/tests/iov-padding     |  85 ++++++++++++++
 tests/qemu-iotests/tests/iov-padding.out |  59 ++++++++++
 4 files changed, 277 insertions(+), 10 deletions(-)
 create mode 100755 tests/qemu-iotests/tests/iov-padding
 create mode 100644 tests/qemu-iotests/tests/iov-padding.out

Comments

Stefan Hajnoczi March 15, 2023, 3:29 p.m. UTC | #1

On Wed, Mar 15, 2023 at 01:13:28PM +0100, Hanna Czenczek wrote:
> Hi,
> 
> We accept I/O vectors with up to 1024 (IOV_MAX) elements from guests.
> When a guest request does not conform to the underlying storage's
> alignment requirements, we pad it with head and/or tail buffers as
> necessary, which are simply appended to the I/O vector.
> 
> As of 4c002cef, we (sensibly) check that such-padded vectors do not then
> exceed IOV_MAX.  However, there seems to be no sensible error handling;
> instead, the guest simply receives an I/O error.
> 
> That???s a problem, because it submitted a perfectly sensible I/O request

Looks like there is an encoding issue. I get 3 question marks instead of
an apostrophe. lore.kernel.org also renders mojibake:
https://lore.kernel.org/qemu-devel/[e-mail]/

Stefan

Hanna Czenczek March 15, 2023, 4:05 p.m. UTC | #2

On 15.03.23 16:29, Stefan Hajnoczi wrote:
> On Wed, Mar 15, 2023 at 01:13:28PM +0100, Hanna Czenczek wrote:
>> Hi,
>>
>> We accept I/O vectors with up to 1024 (IOV_MAX) elements from guests.
>> When a guest request does not conform to the underlying storage's
>> alignment requirements, we pad it with head and/or tail buffers as
>> necessary, which are simply appended to the I/O vector.
>>
>> As of 4c002cef, we (sensibly) check that such-padded vectors do not then
>> exceed IOV_MAX.  However, there seems to be no sensible error handling;
>> instead, the guest simply receives an I/O error.
>>
>> That???s a problem, because it submitted a perfectly sensible I/O request
> Looks like there is an encoding issue. I get 3 question marks instead of
> an apostrophe. lore.kernel.org also renders mojibake:
> https://lore.kernel.org/qemu-devel/[e-mail]/

Hm, yeah, no “charset=UTF-8” in that mail’s Content-type...  I think 
that’s because it just uses what’s in the patch file and sends it, and I 
needed to force it to be format.headers="Content-type: text/plain" in 
.gitconfig at some point because of some Mimecast thing.  I hope putting 
format.headers="Content-type: text/plain; charset=UTF-8" will fix that 
for the future.  Thanks for telling me!

Hanna