On Tue, Feb 11, 2014 at 05:04:09PM +0800, Fam Zheng wrote: > On Tue, 02/11 15:07, 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 | 43 +++++++++++++++++++++++++++++++++++-------- > > 1 file changed, 35 insertions(+), 8 deletions(-) > > > > diff --git a/block/raw-posix.c b/block/raw-posix.c > > index 01fb41a..1961b74 100644 > > --- a/block/raw-posix.c > > +++ b/block/raw-posix.c > > @@ -1229,11 +1229,22 @@ static int raw_create(const char *filename, > > QEMUOptionParameter *options, > > int fd; > > int result = 0; > > int64_t total_size = 0; > > + PreallocMode prealloc = PREALLOC_MODE_OFF; > > > > /* Read out options */ > > while (options && options->name) { > > if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > > total_size = options->value.n & BDRV_SECTOR_MASK; > > + } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) { > > + if (!options->value.s || !strcmp(options->value.s, "off")) { > > + prealloc = PREALLOC_MODE_OFF; > > + } else if (!strcmp(options->value.s, "full")) { > > + prealloc = PREALLOC_MODE_FULL; > > + } else { > > + error_setg(errp, "Invalid preallocation mode: '%s'", > > + options->value.s); > > + return -EINVAL; > > + } > > } > > options++; > > } > > @@ -1243,16 +1254,27 @@ static int raw_create(const char *filename, > > QEMUOptionParameter *options, > > if (fd < 0) { > > result = -errno; > > error_setg_errno(errp, -result, "Could not create file"); > > - } else { > > - if (ftruncate(fd, total_size) != 0) { > > - result = -errno; > > - error_setg_errno(errp, -result, "Could not resize file"); > > - } > > - if (qemu_close(fd) != 0) { > > - result = -errno; > > - error_setg_errno(errp, -result, "Could not close the new > > file"); > > + goto out; > > + } > > + if (ftruncate(fd, total_size) != 0) { > > + result = -errno; > > + error_setg_errno(errp, -result, "Could not resize file"); > > + goto out_close; > > + } > > + if (prealloc == PREALLOC_MODE_FULL) { > > + /* posix_fallocate() doesn't set errno. */ > > + result = -posix_fallocate(fd, 0, total_size); > > + if (result != 0) { > > + error_setg_errno(errp, -result, > > + "Could not preallocate data for the new > > file"); > > } > > } > > +out_close: > > + if (qemu_close(fd) != 0) { > > + result = -errno; > > + error_setg_errno(errp, -result, "Could not close the new file"); > > If errp is already set because ftruncate or posix_ftruncate failed, and > qemu_close() fails too, the call to error_setg_errno() will abort on failing > the assertion of (*errp == NULL).
The original code also has the problem. This brings a general problem that two error_setg_errno() could be called on the same errp. In general the first error is better to reveal to user. > > You could either embed the two failures in one error message, or pick the > urgent one and drop the other. > > Thanks, > Fam > > > + } > > +out: > > return result; > > } > > > > @@ -1403,6 +1425,11 @@ static QEMUOptionParameter raw_create_options[] = { > > .type = OPT_SIZE, > > .help = "Virtual disk size" > > }, > > + { > > + .name = BLOCK_OPT_PREALLOC, > > + .type = OPT_STRING, > > + .help = "Preallocation mode (allowed values: off, full)" > > + }, > > { NULL } > > }; > > > > -- > > 1.8.0 > >