On 27.05.2014 21:13, Eric Blake wrote:
On 05/27/2014 02:22 AM, Chen Fan wrote:
From: Hu Tao <hu...@cn.fujitsu.com>
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 <hu...@cn.fujitsu.com>
---
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.
Actually, these are not only candidates, but either *need* to use
DIV_ROUND_UP or "options->value.n + BDRV_SECTOR_SIZE - 1" to be correct.
Without "- 1", they are not rounding up, but rounding up and sometimes
adding BDRV_SECTOR_SIZE on top of that.
Max
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.
