qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 6/7] block/raw-posix: call plain fallocate in ha


From: Denis V. Lunev
Subject: Re: [Qemu-devel] [PATCH 6/7] block/raw-posix: call plain fallocate in handle_aiocb_write_zeroes
Date: Tue, 27 Jan 2015 21:19:13 +0300
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.4.0

On 27/01/15 20:57, Max Reitz wrote:
On 2015-01-27 at 08:51, Denis V. Lunev wrote:
There is a possibility that we are extending our image and thus writing
zeroes beyond the end of the file. In this case we do not need to care
about the hole to make sure that there is no data in the file under
this offset (pre-condition to fallocate(0) to work). We could simply call
fallocate(0).

This improves the performance of writing zeroes even on really old
platforms which do not have even FALLOC_FL_PUNCH_HOLE.

Signed-off-by: Denis V. Lunev <address@hidden>
CC: Kevin Wolf <address@hidden>
CC: Stefan Hajnoczi <address@hidden>
CC: Peter Lieven <address@hidden>
CC: Fam Zheng <address@hidden>
---
  block/raw-posix.c | 10 ++++++++--
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index c039bef..fa05239 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -60,7 +60,7 @@
#define FS_NOCOW_FL 0x00800000 /* Do not cow file */
  #endif
  #endif
-#if defined(CONFIG_FALLOCATE_PUNCH_HOLE) || defined(CONFIG_FALLOCATE_ZERO_RANGE)
+#ifdef CONFIG_FALLOCATE

This change doesn't seem right; CONFIG_FALLOCATE is set if posix_fallocate() is available, not for the Linux-specific fallocate() from linux/falloc.h.


here is a check for fallocate and posix_fallocate in configure script

# check for fallocate
fallocate=no
cat > $TMPC << EOF
#include <fcntl.h>

int main(void)
{
    fallocate(0, 0, 0, 0);
    return 0;
}
EOF
if compile_prog "" "" ; then
  fallocate=yes
fi
...
# check for posix_fallocate
posix_fallocate=no
cat > $TMPC << EOF
#include <fcntl.h>

int main(void)
{
    posix_fallocate(0, 0, 0);
    return 0;
}
EOF
if compile_prog "" "" ; then
    posix_fallocate=yes
fi
...
if test "$fallocate" = "yes" ; then
  echo "CONFIG_FALLOCATE=y" >> $config_host_mak
fi
...
if test "$posix_fallocate" = "yes" ; then
  echo "CONFIG_POSIX_FALLOCATE=y" >> $config_host_mak
fi

Thus my check looks correct to me.

  #include <linux/falloc.h>
  #endif
  #if defined (__FreeBSD__) || defined(__FreeBSD_kernel__)
@@ -902,7 +902,7 @@ static int translate_err(int err)
      return err;
  }
-#if defined(CONFIG_FALLOCATE_PUNCH_HOLE) || defined(CONFIG_FALLOCATE_ZERO_RANGE)
+#ifdef CONFIG_FALLOCATE

Same here.

  static int do_fallocate(int fd, int mode, off_t offset, off_t len)
  {
      do {
@@ -981,6 +981,12 @@ static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData *aiocb)
      }
  #endif
  +#ifdef CONFIG_FALLOCATE
+ if (aiocb->aio_offset >= aiocb->bs->total_sectors << BDRV_SECTOR_BITS) { + return do_fallocate(s->fd, 0, aiocb->aio_offset, aiocb->aio_nbytes);
+    }
+#endif
+

This seems fine though, but as I've asked in patch 5: Do we want to have a "has_fallocate"?

Other than that, this is the first usage of bs->total_sectors in this file; raw_co_get_block_status() does a similar check, but it uses bdrv_getlength() instead. If bs->total_sectors is correct, bdrv_getlength() will actually do nothing but return bs->total_sectors * BDRV_SECTOR_SIZE; it will only do more (that is, update bs->total_sectors) if it is not correct to use bs->total_sectors (and I feel like it may not be correct because BlockDriver.has_variable_length is true).

Max

ok, will do



reply via email to

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