On 03.04.2017 21:19, Philippe Mathieu-Daudé wrote: > Hi Max, > > On 04/03/2017 01:09 PM, Max Reitz wrote: >> This patch adds two new parameters to the preallocate() function so we >> will be able to use it not just for preallocating a new image but also >> for preallocated image growth. >> >> The offset parameter allows the caller to specify a virtual offset from >> which to start preallocating. For newly created images this is always 0, >> but for preallocating growth this will be the old image length. >> >> The new_length parameter specifies the supposed new length of the image >> (basically the "end offset" for preallocation). During image truncation, >> bdrv_getlength() will return the old image length so we cannot rely on >> its return value then. >> >> Signed-off-by: Max Reitz <mre...@redhat.com> >> --- >> block/qcow2.c | 16 +++++++++++----- >> 1 file changed, 11 insertions(+), 5 deletions(-) >> >> diff --git a/block/qcow2.c b/block/qcow2.c >> index 9fd220ec34..163d51ec65 100644 >> --- a/block/qcow2.c >> +++ b/block/qcow2.c >> @@ -2034,17 +2034,23 @@ static int >> qcow2_change_backing_file(BlockDriverState *bs, >> return qcow2_update_header(bs); >> } >> >> -static int preallocate(BlockDriverState *bs) >> +/** >> + * Preallocates metadata structures for data clusters between @offset >> (in the >> + * guest disk) and @new_length (which is thus generally the new guest >> disk >> + * size). >> + * >> + * Returns: 0 on success, -errno on failure. >> + */ >> +static int preallocate(BlockDriverState *bs, int64_t offset, int64_t >> new_length) >> { >> uint64_t bytes; >> - uint64_t offset; >> uint64_t host_offset = 0; >> unsigned int cur_bytes; >> int ret; >> QCowL2Meta *meta; >> >> - bytes = bdrv_getlength(bs); >> - offset = 0; > > why use `int64_t`? is it ok to have a negative offset? > preallocate a negative length sound weird... even `bytes` is unsigned.
Right. That's actually a very good question. I can't remember why I chose them to be signed. Generally, it won't be an issue because the QEMU block layer will generally not permit files to be longer than 2^63 (e.g. bdrv_co_pwritev() and bdrv_co_preadv() take int64_t offsets). But you're right, there is no reason for those parameters to be signed here. I'll make them unsigned, thanks! Max >> + assert(offset <= new_length); >> + bytes = new_length - offset; >> >> while (bytes) { >> cur_bytes = MIN(bytes, INT_MAX); >> @@ -2314,7 +2320,7 @@ static int qcow2_create2(const char *filename, >> int64_t total_size, >> if (prealloc != PREALLOC_MODE_OFF) { >> BDRVQcow2State *s = blk_bs(blk)->opaque; >> qemu_co_mutex_lock(&s->lock); >> - ret = preallocate(blk_bs(blk)); >> + ret = preallocate(blk_bs(blk), 0, total_size); >> qemu_co_mutex_unlock(&s->lock); >> if (ret < 0) { >> error_setg_errno(errp, -ret, "Could not preallocate >> metadata"); >>
signature.asc
Description: OpenPGP digital signature