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 <[email protected]> >> --- >> 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
