On 05/27/2014 02:22 AM, Chen Fan wrote: > From: Hu Tao <[email protected]> > > and avoid converting it back later. And round up file size to nearest > sector.
The fact that you started a sentence with "And" makes me wonder if this should be two patches. > > Signed-off-by: Hu Tao <[email protected]> > --- > block/qcow2.c | 8 ++++---- > block/raw-posix.c | 5 +++-- > block/raw-win32.c | 5 +++-- > 3 files changed, 10 insertions(+), 8 deletions(-) > > @@ -1779,7 +1779,7 @@ static int qcow2_create(const char *filename, > QEMUOptionParameter *options, > /* Read out options */ > while (options && options->name) { > if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - sectors = options->value.n / 512; > + size = (options->value.n + BDRV_SECTOR_SIZE) & BDRV_SECTOR_MASK; Probably better to use DIV_ROUND_UP for this. Also, I have mentioned several times that it appears that the qcow2 file format is perfectly capable of representing a file size that is not a sector or cluster multiple; it's just that the rest of the code base is probably not well prepared to deal with that. While I think that rounding up instead of truncating is the RIGHT thing to do (and therefore your patch is fixing a valid bug), I wonder if it is further worth trying to represent the EXACT size requested by the user. There's probably a lot more followups. If nothing else, I think that this change (which corresponds to the sentence starting with "And" in your commit message) deserves to be in its own patch, and accompanied by a testsuite addition to prove we don't regress (whether we allow exact size or always round up, the testsuite should at least prove that 'qemu-img create -f qcow2 foo.img 1234' should not truncate to a 1024 byte file, but should be at least 1234 bytes visible to the guest). > +++ b/block/raw-posix.c > @@ -1252,7 +1252,8 @@ static int raw_create(const char *filename, > QEMUOptionParameter *options, > /* Read out options */ > while (options && options->name) { > if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - total_size = options->value.n / BDRV_SECTOR_SIZE; > + total_size = (options->value.n + BDRV_SECTOR_SIZE) & > + BDRV_SECTOR_MASK; Another candidate for DIV_ROUND_UP > +++ b/block/raw-win32.c > @@ -489,7 +489,8 @@ static int raw_create(const char *filename, > QEMUOptionParameter *options, > /* Read out options */ > while (options && options->name) { > if (!strcmp(options->name, BLOCK_OPT_SIZE)) { > - total_size = options->value.n / 512; > + total_size = (options->value.n + BDRV_SECTOR_SIZE) & > + BDRV_SECTOR_MASK; And again. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
