From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37646) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VgX02-0000Gm-2g for qemu-devel@nongnu.org; Wed, 13 Nov 2013 04:44:46 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VgWzt-00079H-2u for qemu-devel@nongnu.org; Wed, 13 Nov 2013 04:44:38 -0500 Received: from mail-ea0-x234.google.com ([2a00:1450:4013:c01::234]:44689) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VgWzs-000791-RW for qemu-devel@nongnu.org; Wed, 13 Nov 2013 04:44:29 -0500 Received: by mail-ea0-f180.google.com with SMTP id b11so94588eae.25 for ; Wed, 13 Nov 2013 01:44:27 -0800 (PST) Sender: Paolo Bonzini Message-ID: <528349F7.1060700@redhat.com> Date: Wed, 13 Nov 2013 10:44:23 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1384271389-20716-1-git-send-email-pbonzini@redhat.com> <1384271389-20716-10-git-send-email-pbonzini@redhat.com> <93FBFFE9-D6A4-4540-AEFB-1D4CE250E61F@kamp.de> In-Reply-To: <93FBFFE9-D6A4-4540-AEFB-1D4CE250E61F@kamp.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 09/11] raw-posix: implement write_zeroes with MAY_UNMAP for block devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Lieven Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com Il 13/11/2013 07:29, Peter Lieven ha scritto: > Wouldn't it be good to add bdi->can_write_zeroes_with_unmap here as well? We do: > + bdi->unallocated_blocks_are_zero = s->discard_zeroes; > + bdi->can_write_zeroes_with_unmap = s->discard_zeroes; > This would automatically avoid full allocation when converting something to a host device > supporting BLKDISCARDZEROES. Yes, that's (part of) the point of this patch. Regarding the question you posed in the previous patch: > does BLKDISCARDZEROES ioctl guarantee that a device is > zero initialized or does it just guarantee that a discard may not > fail and that it reads as zeroes afterwards? Only the latter. ".bdrv_has_zero_init" is only present in the bdrv_file BlockDriver. Paolo > Peter > > Am 12.11.2013 um 16:49 schrieb Paolo Bonzini : > >> See the next commit for the description of the Linux kernel problem >> that is worked around in raw_open_common. >> >> Signed-off-by: Paolo Bonzini >> --- >> block/raw-posix.c | 38 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 38 insertions(+) >> >> diff --git a/block/raw-posix.c b/block/raw-posix.c >> index 830e109..5cb46f1 100644 >> --- a/block/raw-posix.c >> +++ b/block/raw-posix.c >> @@ -335,6 +335,23 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, >> if (S_ISREG(st.st_mode)) { >> s->discard_zeroes = true; >> } >> +#ifdef BLKDISCARDZEROES >> + if (S_ISBLK(st.st_mode)) { >> + unsigned int arg; >> + if (ioctl(s->fd, BLKDISCARDZEROES, &arg) == 0 && arg) { >> + s->discard_zeroes = true; >> + } >> + } >> +#endif >> +#ifdef CONFIG_LINUX >> + /* On Linux 3.10, BLKDISCARD leaves stale data in the page cache. Do >> + * not rely on the contents of discarded blocks unless using O_DIRECT. >> + */ >> + if (!(bs->open_flags & BDRV_O_NOCACHE)) { >> + s->discard_zeroes = false; >> + } >> +#endif >> + } >> >> #ifdef CONFIG_XFS >> if (platform_test_xfs_fd(s->fd)) { >> @@ -1587,6 +1604,26 @@ static coroutine_fn BlockDriverAIOCB *hdev_aio_discard(BlockDriverState *bs, >> cb, opaque, QEMU_AIO_DISCARD|QEMU_AIO_BLKDEV); >> } >> >> +static coroutine_fn int hdev_co_write_zeroes(BlockDriverState *bs, >> + int64_t sector_num, int nb_sectors, BdrvRequestFlags flags) >> +{ >> + BDRVRawState *s = bs->opaque; >> + int rc; >> + >> + rc = fd_open(bs); >> + if (rc < 0) { >> + return rc; >> + } >> + if (!(flags & BDRV_REQ_MAY_UNMAP)) { >> + return -ENOTSUP; >> + } >> + if (!s->discard_zeroes) { >> + return -ENOTSUP; >> + } >> + return paio_submit_co(bs, s->fd, sector_num, NULL, nb_sectors, >> + QEMU_AIO_DISCARD|QEMU_AIO_BLKDEV); >> +} >> + >> static int hdev_create(const char *filename, QEMUOptionParameter *options, >> Error **errp) >> { >> @@ -1639,6 +1676,7 @@ static BlockDriver bdrv_host_device = { >> .bdrv_reopen_abort = raw_reopen_abort, >> .bdrv_create = hdev_create, >> .create_options = raw_create_options, >> + .bdrv_co_write_zeroes = hdev_co_write_zeroes, >> >> .bdrv_aio_readv = raw_aio_readv, >> .bdrv_aio_writev = raw_aio_writev, >> -- >> 1.8.4.2 >> >> > > >