On Fri, Jan 17, 2014 at 04:25:14PM +0800, Stefan Hajnoczi wrote: > On Fri, Dec 27, 2013 at 11:05:53AM +0800, Hu Tao wrote: > > This patch adds a new option preallocation for raw format, and implements > > full preallocation. > > > > Signed-off-by: Hu Tao <hu...@cn.fujitsu.com> > > --- > > block/raw-posix.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 46 insertions(+) > > > > diff --git a/block/raw-posix.c b/block/raw-posix.c > > index 6f6b8c1..a722d27 100644 > > --- a/block/raw-posix.c > > +++ b/block/raw-posix.c > > @@ -1160,17 +1160,52 @@ static int64_t > > raw_get_allocated_file_size(BlockDriverState *bs) > > return (int64_t)st.st_blocks * 512; > > } > > > > +#ifdef __linux__ > > +static int raw_preallocate2(int fd, int64_t offset, int64_t length) > > Why is this function called raw_preallocate2()? Please choose a > descriptive name.
Sure. > > > +{ > > + int ret = -1; > > + > > + ret = fallocate(fd, 0, offset, length); > > + if (ret < 0) { > > + ret = -errno; > > + } > > + > > + /* fallback to posix_fallocate() if fallocate() is not supported */ > > + if (ret == -ENOSYS || ret == -EOPNOTSUPP) { > > + ret = posix_fallocate(fd, offset, length); > > From the posix_fallocate(3) man page: > "posix_fallocate() returns zero on success, or an error number on > failure. Note that errno is not set." > > You need to negate the error number: > if (ret > 0) { > ret = -ret; /* posix_fallocate(3) returns a positive errno */ > } Sure. > > > + } > > + > > + return ret; > > +} > > +#else > > +static int raw_preallocate2(int fd, int64_t offset, int64_t length) > > +{ > > + return posix_fallocate(fd, offset, length); > > Same error number problem as above. > > > @@ -1185,6 +1220,12 @@ static int raw_create(const char *filename, > > QEMUOptionParameter *options, > > result = -errno; > > error_setg_errno(errp, -result, "Could not resize file"); > > } > > + if (prealloc == PREALLOC_MODE_FULL) { > > + result = raw_preallocate2(fd, 0, total_size); > > What if ftruncate() failed? We should not try to fallocate. Please add > a proper error return path. Sure. > > > + if (result != 0) > > + error_setg_errno(errp, -result, > > + "Could not preallocate data for the new > > file"); > > WARNING: braces {} are necessary for all arms of this statement > > Please run scripts/checkpatch.pl before submitting patches, it checks > QEMU coding style. Thanks!