[Qemu-devel] [PATCH 0/2] qcow2: Avoid making the L1 table too big

[Qemu-devel] [PATCH 0/2] qcow2: Avoid making the L1 table too big

[Max Reitz]

See https://bugs.launchpad.net/qemu/+bug/1592590 for a bug report.

Reproducer:

$ ./qemu-img create -f qcow2 test.qcow2 1M
Formatting 'test.qcow2', fmt=qcow2 size=1048576 encryption=off
cluster_size=65536 lazy_refcounts=off refcount_bits=16
$ ./qemu-img resize test.qcow2 100000T
Image resized.
$ ./qemu-img info test.qcow2
qemu-img: Could not open 'test.qcow2': Active L1 table too large

After this series:

$ ./qemu-img resize test.qcow2 100000T
qemu-img: Error resizing image: File too large


Max Reitz (2):
  qemu-img: Use strerror() for generic resize error
  qcow2: Avoid making the L1 table too big

 block/qcow2-cluster.c | 2 +-
 qemu-img.c            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.8.3

[Qemu-devel] [PATCH 1/2] qemu-img: Use strerror() for generic resize error

[Max Reitz]

Emitting the plain error number is not very helpful. Use strerror()
instead.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 14e2661..d5ccd9a 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3277,7 +3277,7 @@ static int img_resize(int argc, char **argv)
         error_report("Image is read-only");
         break;
     default:
-        error_report("Error resizing image (%d)", -ret);
+        error_report("Error resizing image: %s", strerror(-ret));
         break;
     }
 out:
-- 
2.8.3

Re: [Qemu-devel] [PATCH 1/2] qemu-img: Use strerror() for generic resize error

[Eric Blake]

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

On 06/15/2016 09:36 AM, Max Reitz wrote:
> Emitting the plain error number is not very helpful. Use strerror()
> instead.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qemu-img.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 14e2661..d5ccd9a 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -3277,7 +3277,7 @@ static int img_resize(int argc, char **argv)
>          error_report("Image is read-only");
>          break;
>      default:
> -        error_report("Error resizing image (%d)", -ret);
> +        error_report("Error resizing image: %s", strerror(-ret));
>          break;

I've argued in the past that we have lots of error_report() callers
using strerror(), and may want to add an error_report_errno() (similar
to error_setg_errno()); this just adds to the list of things that would
benefit.  But such a conversion would be a separate BiteSized task, and
doesn't invalidate your patch.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [PATCH 2/2] qcow2: Avoid making the L1 table too big

[Max Reitz]

We refuse to open images whose L1 table we deem "too big". Consequently,
we should not produce such images ourselves.

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-cluster.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 893ddf6..335b9b0 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -65,7 +65,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
         }
     }
 
-    if (new_l1_size > INT_MAX / sizeof(uint64_t)) {
+    if (new_l1_size > QCOW_MAX_L1_SIZE / sizeof(uint64_t)) {
         return -EFBIG;
     }
 
-- 
2.8.3

Re: [Qemu-devel] [PATCH 2/2] qcow2: Avoid making the L1 table too big

[Eric Blake]

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

On 06/15/2016 09:36 AM, Max Reitz wrote:
> We refuse to open images whose L1 table we deem "too big". Consequently,
> we should not produce such images ourselves.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2-cluster.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 893ddf6..335b9b0 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -65,7 +65,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
>          }
>      }
>  
> -    if (new_l1_size > INT_MAX / sizeof(uint64_t)) {
> +    if (new_l1_size > QCOW_MAX_L1_SIZE / sizeof(uint64_t)) {
>          return -EFBIG;
>      }
>  
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 2/2] qcow2: Avoid making the L1 table too big

[Kevin Wolf]

Am 15.06.2016 um 17:36 hat Max Reitz geschrieben:
> We refuse to open images whose L1 table we deem "too big". Consequently,
> we should not produce such images ourselves.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2-cluster.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 893ddf6..335b9b0 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -65,7 +65,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
>          }
>      }
>  
> -    if (new_l1_size > INT_MAX / sizeof(uint64_t)) {
> +    if (new_l1_size > QCOW_MAX_L1_SIZE / sizeof(uint64_t)) {
>          return -EFBIG;
>      }

Maybe add a QEMU_BUILD_BUG_ON(QCOW_MAX_L1_SIZE > INT_MAX)?

Kevin

Re: [Qemu-devel] [PATCH 2/2] qcow2: Avoid making the L1 table too big

[Max Reitz]

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

On 06.07.2016 10:40, Kevin Wolf wrote:
> Am 15.06.2016 um 17:36 hat Max Reitz geschrieben:
>> We refuse to open images whose L1 table we deem "too big". Consequently,
>> we should not produce such images ourselves.
>>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  block/qcow2-cluster.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
>> index 893ddf6..335b9b0 100644
>> --- a/block/qcow2-cluster.c
>> +++ b/block/qcow2-cluster.c
>> @@ -65,7 +65,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
>>          }
>>      }
>>  
>> -    if (new_l1_size > INT_MAX / sizeof(uint64_t)) {
>> +    if (new_l1_size > QCOW_MAX_L1_SIZE / sizeof(uint64_t)) {
>>          return -EFBIG;
>>      }
> 
> Maybe add a QEMU_BUILD_BUG_ON(QCOW_MAX_L1_SIZE > INT_MAX)?

Good idea, did that. I changed it directly in my block branch, or do you
think a v2 mail necessary?

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH 2/2] qcow2: Avoid making the L1 table too big

[Kevin Wolf]

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

Am 06.07.2016 um 14:47 hat Max Reitz geschrieben:
> On 06.07.2016 10:40, Kevin Wolf wrote:
> > Am 15.06.2016 um 17:36 hat Max Reitz geschrieben:
> >> We refuse to open images whose L1 table we deem "too big". Consequently,
> >> we should not produce such images ourselves.
> >>
> >> Cc: qemu-stable@nongnu.org
> >> Signed-off-by: Max Reitz <mreitz@redhat.com>
> >> ---
> >>  block/qcow2-cluster.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> >> index 893ddf6..335b9b0 100644
> >> --- a/block/qcow2-cluster.c
> >> +++ b/block/qcow2-cluster.c
> >> @@ -65,7 +65,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
> >>          }
> >>      }
> >>  
> >> -    if (new_l1_size > INT_MAX / sizeof(uint64_t)) {
> >> +    if (new_l1_size > QCOW_MAX_L1_SIZE / sizeof(uint64_t)) {
> >>          return -EFBIG;
> >>      }
> > 
> > Maybe add a QEMU_BUILD_BUG_ON(QCOW_MAX_L1_SIZE > INT_MAX)?
> 
> Good idea, did that. I changed it directly in my block branch, or do you
> think a v2 mail necessary?

No, letting you apply it directly is what I intended.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [PATCH 0/2] qcow2: Avoid making the L1 table too big

[Max Reitz]

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

On 15.06.2016 17:36, Max Reitz wrote:
> See https://bugs.launchpad.net/qemu/+bug/1592590 for a bug report.
> 
> Reproducer:
> 
> $ ./qemu-img create -f qcow2 test.qcow2 1M
> Formatting 'test.qcow2', fmt=qcow2 size=1048576 encryption=off
> cluster_size=65536 lazy_refcounts=off refcount_bits=16
> $ ./qemu-img resize test.qcow2 100000T
> Image resized.
> $ ./qemu-img info test.qcow2
> qemu-img: Could not open 'test.qcow2': Active L1 table too large
> 
> After this series:
> 
> $ ./qemu-img resize test.qcow2 100000T
> qemu-img: Error resizing image: File too large
> 
> 
> Max Reitz (2):
>   qemu-img: Use strerror() for generic resize error
>   qcow2: Avoid making the L1 table too big
> 
>  block/qcow2-cluster.c | 2 +-
>  qemu-img.c            | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Applied to my block branch.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]