[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH] block: fix write with zero flag se
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH] block: fix write with zero flag set and iovector provided |
Date: |
Thu, 01 Feb 2018 16:19:38 +0100 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Thu 01 Feb 2018 03:40:51 PM CET, Eric Blake wrote:
>>> --- a/block/io.c
>>> +++ b/block/io.c
>>> @@ -1701,7 +1701,7 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
>>> */
>>> tracked_request_begin(&req, bs, offset, bytes, BDRV_TRACKED_WRITE);
>>>
>>> - if (!qiov) {
>>> + if (flags & BDRV_REQ_ZERO_WRITE) {
>>> ret = bdrv_co_do_zero_pwritev(child, offset, bytes, flags, &req);
>>
>> So now, the flag rules, but we assert that !qiov (so it would only break
>> a caller that passed the flag but used qiov, which you argued shouldn't
>> exist).
>
> Sorry, I hit send too soon. I'm asking if we should have
> assert(!qiov) right before calling bdrv_co_do_zero_pwritev (it would
> break a caller that passed the flag and qiov, but you were arguing
> that such callers previously misbehaved, so we don't want such
> callers).
Those callers do exist as a matter of fact: bdrv_rw_co_entry() always
passes a qiov to bdrv_co_pwritev() regardless of the flags (the request
size is actually taken from the very qiov).
bdrv_pwrite_zeroes() is one example:
$ qemu-img create -f qcow2 base.img 100M
$ qemu-img create -f qcow2 -b base.img active.img
$ qemu-io -c 'write -z 0 128k' -f qcow2 active.img
$ qemu-img amend -o compat=0.10 active.img
It even uses an iovec with iov_base = NULL but iov_len != 0, which looks
like an abuse of the data structure.
Berto