[Qemu-devel] [PATCH] block-vpc: Don't silently create smaller image than requested

[Qemu-devel] [PATCH] block-vpc: Don't silently create smaller image than requested

[Kevin Wolf]

The algorithm from the VHD specification for CHS calculation silently limits
images to 127 GB which may confuse a user who requested a larger image. Better
output an error message and abort.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block-vpc.c |    9 ++++++---
 qemu-img.c  |    4 ++++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/block-vpc.c b/block-vpc.c
index 7ae5202..d915d20 100644
--- a/block-vpc.c
+++ b/block-vpc.c
@@ -433,14 +433,16 @@ static int vpc_write(BlockDriverState *bs, int64_t sector_num,
  *
  * Note that the geometry doesn't always exactly match total_sectors but
  * may round it down.
+ *
+ * Returns 0 on success, -EFBIG if the size is larger than 127 GB
  */
-static void calculate_geometry(int64_t total_sectors, uint16_t* cyls,
+static int calculate_geometry(int64_t total_sectors, uint16_t* cyls,
     uint8_t* heads, uint8_t* secs_per_cyl)
 {
     uint32_t cyls_times_heads;
 
     if (total_sectors > 65535 * 16 * 255)
-        total_sectors = 65535 * 16 * 255;
+        return -EFBIG;
 
     if (total_sectors > 65535 * 16 * 63) {
         *secs_per_cyl = 255;
@@ -493,7 +495,8 @@ static int vpc_create(const char *filename, int64_t total_sectors,
         return -EIO;
 
     // Calculate matching total_size and geometry
-    calculate_geometry(total_sectors, &cyls, &heads, &secs_per_cyl);
+    if (calculate_geometry(total_sectors, &cyls, &heads, &secs_per_cyl))
+        return -EFBIG;
     total_sectors = (int64_t) cyls * heads * secs_per_cyl;
 
     // Prepare the Hard Disk Footer
diff --git a/qemu-img.c b/qemu-img.c
index 913ad34..ccf4a6f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -306,6 +306,8 @@ static int img_create(int argc, char **argv)
     if (ret < 0) {
         if (ret == -ENOTSUP) {
             error("Formatting or formatting option not supported for file format '%s'", fmt);
+        } else if (ret == -EFBIG) {
+            error("The image size is too large for file format '%s'", fmt);
         } else {
             error("Error while formatting");
         }
@@ -494,6 +496,8 @@ static int img_convert(int argc, char **argv)
     if (ret < 0) {
         if (ret == -ENOTSUP) {
             error("Formatting not supported for file format '%s'", out_fmt);
+        } else if (ret == -EFBIG) {
+            error("The image size is too large for file format '%s'", out_fmt);
         } else {
             error("Error while formatting '%s'", out_filename);
         }
-- 
1.6.0.6

Re: [Qemu-devel] [PATCH] block-vpc: Don't silently create smaller image than requested

[Aurelien Jarno]

On Wed, Apr 08, 2009 at 04:49:05PM +0200, Kevin Wolf wrote:
> The algorithm from the VHD specification for CHS calculation silently limits
> images to 127 GB which may confuse a user who requested a larger image. Better
> output an error message and abort.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block-vpc.c |    9 ++++++---
>  qemu-img.c  |    4 ++++
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/block-vpc.c b/block-vpc.c
> index 7ae5202..d915d20 100644
> --- a/block-vpc.c
> +++ b/block-vpc.c
> @@ -433,14 +433,16 @@ static int vpc_write(BlockDriverState *bs, int64_t sector_num,
>   *
>   * Note that the geometry doesn't always exactly match total_sectors but
>   * may round it down.
> + *
> + * Returns 0 on success, -EFBIG if the size is larger than 127 GB
>   */

Actually 0 is never returned, leading to a warning:
  CC    block-vpc.o
block-vpc.c: In function ‘calculate_geometry’:
block-vpc.c:475: warning: control reaches end of non-void function

> -static void calculate_geometry(int64_t total_sectors, uint16_t* cyls,
> +static int calculate_geometry(int64_t total_sectors, uint16_t* cyls,
>      uint8_t* heads, uint8_t* secs_per_cyl)
>  {
>      uint32_t cyls_times_heads;
>  
>      if (total_sectors > 65535 * 16 * 255)
> -        total_sectors = 65535 * 16 * 255;
> +        return -EFBIG;
>  
>      if (total_sectors > 65535 * 16 * 63) {
>          *secs_per_cyl = 255;
> @@ -493,7 +495,8 @@ static int vpc_create(const char *filename, int64_t total_sectors,
>          return -EIO;
>  
>      // Calculate matching total_size and geometry
> -    calculate_geometry(total_sectors, &cyls, &heads, &secs_per_cyl);
> +    if (calculate_geometry(total_sectors, &cyls, &heads, &secs_per_cyl))
> +        return -EFBIG;
>      total_sectors = (int64_t) cyls * heads * secs_per_cyl;
>  
>      // Prepare the Hard Disk Footer
> diff --git a/qemu-img.c b/qemu-img.c
> index 913ad34..ccf4a6f 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -306,6 +306,8 @@ static int img_create(int argc, char **argv)
>      if (ret < 0) {
>          if (ret == -ENOTSUP) {
>              error("Formatting or formatting option not supported for file format '%s'", fmt);
> +        } else if (ret == -EFBIG) {
> +            error("The image size is too large for file format '%s'", fmt);
>          } else {
>              error("Error while formatting");
>          }
> @@ -494,6 +496,8 @@ static int img_convert(int argc, char **argv)
>      if (ret < 0) {
>          if (ret == -ENOTSUP) {
>              error("Formatting not supported for file format '%s'", out_fmt);
> +        } else if (ret == -EFBIG) {
> +            error("The image size is too large for file format '%s'", out_fmt);
>          } else {
>              error("Error while formatting '%s'", out_filename);
>          }
> -- 
> 1.6.0.6
> 
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH v2] block-vpc: Don't silently create smaller image than requested

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 1292 bytes --]

Aurelien Jarno schrieb:
> On Wed, Apr 08, 2009 at 04:49:05PM +0200, Kevin Wolf wrote:
>> The algorithm from the VHD specification for CHS calculation silently limits
>> images to 127 GB which may confuse a user who requested a larger image. Better
>> output an error message and abort.
>>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>  block-vpc.c |    9 ++++++---
>>  qemu-img.c  |    4 ++++
>>  2 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/block-vpc.c b/block-vpc.c
>> index 7ae5202..d915d20 100644
>> --- a/block-vpc.c
>> +++ b/block-vpc.c
>> @@ -433,14 +433,16 @@ static int vpc_write(BlockDriverState *bs, int64_t sector_num,
>>   *
>>   * Note that the geometry doesn't always exactly match total_sectors but
>>   * may round it down.
>> + *
>> + * Returns 0 on success, -EFBIG if the size is larger than 127 GB
>>   */
> 
> Actually 0 is never returned, leading to a warning:
>   CC    block-vpc.o
> block-vpc.c: In function ‘calculate_geometry’:
> block-vpc.c:475: warning: control reaches end of non-void function

Oops, you're right. I'm pretty sure that it compiled fine when I tested 
it. Maybe forgot to commit or something.

Please find attached a new version which actually contains a return 0.

Kevin

[-- Attachment #2: 0001-block-vpc-Don-t-silently-create-smaller-image-than.patch --]
[-- Type: text/plain, Size: 3078 bytes --]

From 9d612c70ee232443337c79825afd754aad1db878 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Wed, 8 Apr 2009 15:44:04 +0200
Subject: [PATCH] block-vpc: Don't silently create smaller image than requested

The algorithm from the VHD specification for CHS calculation silently limits
images to 127 GB which may confuse a user who requested a larger image. Better
output an error message and abort.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block-vpc.c |   11 ++++++++---
 qemu-img.c  |    4 ++++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/block-vpc.c b/block-vpc.c
index 7ae5202..71a171d 100644
--- a/block-vpc.c
+++ b/block-vpc.c
@@ -433,14 +433,16 @@ static int vpc_write(BlockDriverState *bs, int64_t sector_num,
  *
  * Note that the geometry doesn't always exactly match total_sectors but
  * may round it down.
+ *
+ * Returns 0 on success, -EFBIG if the size is larger than 127 GB
  */
-static void calculate_geometry(int64_t total_sectors, uint16_t* cyls,
+static int calculate_geometry(int64_t total_sectors, uint16_t* cyls,
     uint8_t* heads, uint8_t* secs_per_cyl)
 {
     uint32_t cyls_times_heads;
 
     if (total_sectors > 65535 * 16 * 255)
-        total_sectors = 65535 * 16 * 255;
+        return -EFBIG;
 
     if (total_sectors > 65535 * 16 * 63) {
         *secs_per_cyl = 255;
@@ -470,6 +472,8 @@ static void calculate_geometry(int64_t total_sectors, uint16_t* cyls,
     // Note: Rounding up deviates from the Virtual PC behaviour
     // However, we need this to avoid truncating images in qemu-img convert
     *cyls = (cyls_times_heads + *heads - 1) / *heads;
+
+    return 0;
 }
 
 static int vpc_create(const char *filename, int64_t total_sectors,
@@ -493,7 +497,8 @@ static int vpc_create(const char *filename, int64_t total_sectors,
         return -EIO;
 
     // Calculate matching total_size and geometry
-    calculate_geometry(total_sectors, &cyls, &heads, &secs_per_cyl);
+    if (calculate_geometry(total_sectors, &cyls, &heads, &secs_per_cyl))
+        return -EFBIG;
     total_sectors = (int64_t) cyls * heads * secs_per_cyl;
 
     // Prepare the Hard Disk Footer
diff --git a/qemu-img.c b/qemu-img.c
index 913ad34..ccf4a6f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -306,6 +306,8 @@ static int img_create(int argc, char **argv)
     if (ret < 0) {
         if (ret == -ENOTSUP) {
             error("Formatting or formatting option not supported for file format '%s'", fmt);
+        } else if (ret == -EFBIG) {
+            error("The image size is too large for file format '%s'", fmt);
         } else {
             error("Error while formatting");
         }
@@ -494,6 +496,8 @@ static int img_convert(int argc, char **argv)
     if (ret < 0) {
         if (ret == -ENOTSUP) {
             error("Formatting not supported for file format '%s'", out_fmt);
+        } else if (ret == -EFBIG) {
+            error("The image size is too large for file format '%s'", out_fmt);
         } else {
             error("Error while formatting '%s'", out_filename);
         }
-- 
1.6.0.6

Re: [Qemu-devel] [PATCH v2] block-vpc: Don't silently create smaller image than requested

[Aurelien Jarno]

On Thu, Apr 09, 2009 at 09:43:22AM +0200, Kevin Wolf wrote:
> Aurelien Jarno schrieb:
>> On Wed, Apr 08, 2009 at 04:49:05PM +0200, Kevin Wolf wrote:
>>> The algorithm from the VHD specification for CHS calculation silently limits
>>> images to 127 GB which may confuse a user who requested a larger image. Better
>>> output an error message and abort.
>>>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>> ---
>>>  block-vpc.c |    9 ++++++---
>>>  qemu-img.c  |    4 ++++
>>>  2 files changed, 10 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/block-vpc.c b/block-vpc.c
>>> index 7ae5202..d915d20 100644
>>> --- a/block-vpc.c
>>> +++ b/block-vpc.c
>>> @@ -433,14 +433,16 @@ static int vpc_write(BlockDriverState *bs, int64_t sector_num,
>>>   *
>>>   * Note that the geometry doesn't always exactly match total_sectors but
>>>   * may round it down.
>>> + *
>>> + * Returns 0 on success, -EFBIG if the size is larger than 127 GB
>>>   */
>>
>> Actually 0 is never returned, leading to a warning:
>>   CC    block-vpc.o
>> block-vpc.c: In function ‘calculate_geometry’:
>> block-vpc.c:475: warning: control reaches end of non-void function
>
> Oops, you're right. I'm pretty sure that it compiled fine when I tested  
> it. Maybe forgot to commit or something.
>
> Please find attached a new version which actually contains a return 0.

Thanks, applied.

> Kevin

> From 9d612c70ee232443337c79825afd754aad1db878 Mon Sep 17 00:00:00 2001
> From: Kevin Wolf <kwolf@redhat.com>
> Date: Wed, 8 Apr 2009 15:44:04 +0200
> Subject: [PATCH] block-vpc: Don't silently create smaller image than requested
> 
> The algorithm from the VHD specification for CHS calculation silently limits
> images to 127 GB which may confuse a user who requested a larger image. Better
> output an error message and abort.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block-vpc.c |   11 ++++++++---
>  qemu-img.c  |    4 ++++
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/block-vpc.c b/block-vpc.c
> index 7ae5202..71a171d 100644
> --- a/block-vpc.c
> +++ b/block-vpc.c
> @@ -433,14 +433,16 @@ static int vpc_write(BlockDriverState *bs, int64_t sector_num,
>   *
>   * Note that the geometry doesn't always exactly match total_sectors but
>   * may round it down.
> + *
> + * Returns 0 on success, -EFBIG if the size is larger than 127 GB
>   */
> -static void calculate_geometry(int64_t total_sectors, uint16_t* cyls,
> +static int calculate_geometry(int64_t total_sectors, uint16_t* cyls,
>      uint8_t* heads, uint8_t* secs_per_cyl)
>  {
>      uint32_t cyls_times_heads;
>  
>      if (total_sectors > 65535 * 16 * 255)
> -        total_sectors = 65535 * 16 * 255;
> +        return -EFBIG;
>  
>      if (total_sectors > 65535 * 16 * 63) {
>          *secs_per_cyl = 255;
> @@ -470,6 +472,8 @@ static void calculate_geometry(int64_t total_sectors, uint16_t* cyls,
>      // Note: Rounding up deviates from the Virtual PC behaviour
>      // However, we need this to avoid truncating images in qemu-img convert
>      *cyls = (cyls_times_heads + *heads - 1) / *heads;
> +
> +    return 0;
>  }
>  
>  static int vpc_create(const char *filename, int64_t total_sectors,
> @@ -493,7 +497,8 @@ static int vpc_create(const char *filename, int64_t total_sectors,
>          return -EIO;
>  
>      // Calculate matching total_size and geometry
> -    calculate_geometry(total_sectors, &cyls, &heads, &secs_per_cyl);
> +    if (calculate_geometry(total_sectors, &cyls, &heads, &secs_per_cyl))
> +        return -EFBIG;
>      total_sectors = (int64_t) cyls * heads * secs_per_cyl;
>  
>      // Prepare the Hard Disk Footer
> diff --git a/qemu-img.c b/qemu-img.c
> index 913ad34..ccf4a6f 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -306,6 +306,8 @@ static int img_create(int argc, char **argv)
>      if (ret < 0) {
>          if (ret == -ENOTSUP) {
>              error("Formatting or formatting option not supported for file format '%s'", fmt);
> +        } else if (ret == -EFBIG) {
> +            error("The image size is too large for file format '%s'", fmt);
>          } else {
>              error("Error while formatting");
>          }
> @@ -494,6 +496,8 @@ static int img_convert(int argc, char **argv)
>      if (ret < 0) {
>          if (ret == -ENOTSUP) {
>              error("Formatting not supported for file format '%s'", out_fmt);
> +        } else if (ret == -EFBIG) {
> +            error("The image size is too large for file format '%s'", out_fmt);
>          } else {
>              error("Error while formatting '%s'", out_filename);
>          }
> -- 
> 1.6.0.6
> 


-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH] block-vpc: Don't silently create smaller image than requested

[Anthony Liguori]

Kevin Wolf wrote:
> The algorithm from the VHD specification for CHS calculation silently limits
> images to 127 GB which may confuse a user who requested a larger image. Better
> output an error message and abort.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>   
Applied to trunk and stable.  Thanks.

-- 
Regards,

Anthony Liguori