[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V9 2/4] raw, qcow2: don't convert file size to s
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH V9 2/4] raw, qcow2: don't convert file size to sector size |
Date: |
Tue, 27 May 2014 13:13:06 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 |
On 05/27/2014 02:22 AM, Chen Fan wrote:
> From: Hu Tao <address@hidden>
>
> 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 <address@hidden>
> ---
> 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
[Qemu-devel] [PATCH V9 2/4] raw, qcow2: don't convert file size to sector size, Chen Fan, 2014/05/27
- Re: [Qemu-devel] [PATCH V9 2/4] raw, qcow2: don't convert file size to sector size,
Eric Blake <=
[Qemu-devel] [PATCH V9 4/4] qcow2: Add full image preallocation option, Chen Fan, 2014/05/27