[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WR
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server |
Date: |
Tue, 19 Jul 2016 21:47:18 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 07/19/2016 09:34 PM, Fam Zheng wrote:
> On Tue, 07/19 17:45, Paolo Bonzini wrote:
>>
>>
>> On 19/07/2016 17:28, Eric Blake wrote:
>>>> If I'm reading the NBD proto.md correctly, this is not enough if
>>>> NBD_CMD_FLAG_NO_HOLE is specified. We probably need to use a zeroed buffer
>>>> with
>>>> blk_pwrite, or pass a new flag (BDRV_RED_NO_HOLE) to blk_pwrite_zeroes to
>>>> enforce the bdrv_driver_pwritev() branch in bdrv_co_do_pwrite_zeroes().
>>
>> I agree with Eric's interpretation. It's a bit weird to have the
>> direction inverted, but I'm not sure I see the ambiguity. Can you explain?
>
> Write zeroes _means_ "punch hole" on a raw file.
>
> In block/raw-posix.c:handle_aiocb_write_zeroes():
>> #ifdef CONFIG_FALLOCATE_PUNCH_HOLE
>> if (s->has_discard && s->has_fallocate) {
>> int ret = do_fallocate(s->fd,
>> FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
>> aiocb->aio_offset, aiocb->aio_nbytes);
>> if (ret == 0) {
>> ret = do_fallocate(s->fd, 0, aiocb->aio_offset,
>> aiocb->aio_nbytes);
That is just implementation: punch a hole, BUT THEN reallocate it back,
so that in the end, the file is still not sparse in that region. Or am
I reading it wrong?
But the implementation under the hood is not visible to the guest - as
long as the end result is that a guest requesting NO_HOLE ends up with a
non-sparse file, and the data reads back as all 0, the client doesn't
care whether the zeros were written byte-by-byte or sped up by punching
a hole then reallocating.
>
> And unmap is translated to "punch hole", too.
>
> In block/raw-posix.c:handle_aiocb_discard():
>> #ifdef CONFIG_FALLOCATE_PUNCH_HOLE
>> ret = do_fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
>> aiocb->aio_offset, aiocb->aio_nbytes);
>> #endif
No, this call is different - it punches a hole, then stops. There is no
followup do_fallocate(,0,,) to reallocate, so the file remains sparse.
>
> So I agree that NBD_CMD_FLAG_NO_HOLE is a poorly named flag, because there is
> always going to be a hole event if it's set.
If we are punching holes even when BDRV_REQ_MAY_UNMAP is not set, that
seems like we have a bug in qemu (unless we are immediately then
reallocating so that there is no resulting hole).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-block] [Qemu-devel] [PATCH v5 06/14] nbd: Send message along with server NBD_REP_ERR errors, (continued)
- [Qemu-block] [PATCH v5 03/14] nbd: Limit nbdflags to 16 bits, Eric Blake, 2016/07/19
- [Qemu-block] [PATCH v5 09/14] nbd: Let client skip portions of server reply, Eric Blake, 2016/07/19
- [Qemu-block] [PATCH v5 08/14] nbd: Let server know when client gives up negotiation, Eric Blake, 2016/07/19
- [Qemu-block] [PATCH v5 04/14] nbd: Treat flags vs. command type as separate fields, Eric Blake, 2016/07/19
- [Qemu-block] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server, Eric Blake, 2016/07/19
- Re: [Qemu-block] [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server, Fam Zheng, 2016/07/19
- Re: [Qemu-block] [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server, Eric Blake, 2016/07/19
- Re: [Qemu-block] [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server, Paolo Bonzini, 2016/07/19
- Re: [Qemu-block] [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server, Fam Zheng, 2016/07/19
- Re: [Qemu-block] [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server,
Eric Blake <=
- Re: [Qemu-block] [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server, Fam Zheng, 2016/07/20
- Re: [Qemu-block] [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server, Paolo Bonzini, 2016/07/20
- Re: [Qemu-block] [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server, Fam Zheng, 2016/07/20
- Re: [Qemu-block] [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server, Paolo Bonzini, 2016/07/20
- Re: [Qemu-block] [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server, Fam Zheng, 2016/07/20
- [Qemu-block] semantics of FIEMAP without FIEMAP_FLAG_SYNC (was Re: [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server), Paolo Bonzini, 2016/07/20
- Re: [Qemu-block] semantics of FIEMAP without FIEMAP_FLAG_SYNC (was Re: [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server), Dave Chinner, 2016/07/20
- Re: [Qemu-block] semantics of FIEMAP without FIEMAP_FLAG_SYNC (was Re: [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server), Niels de Vos, 2016/07/20
- Re: [Qemu-block] semantics of FIEMAP without FIEMAP_FLAG_SYNC (was Re: [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server), Dave Chinner, 2016/07/21
- Re: [Qemu-block] semantics of FIEMAP without FIEMAP_FLAG_SYNC (was Re: [Qemu-devel] [PATCH v5 13/14] nbd: Implement NBD_CMD_WRITE_ZEROES on server), Pádraig Brady, 2016/07/21