diff mbox series

[1/1] block/crypto: better error message when creating too large files

Message ID 20200416095019.4406-2-mlevitsk@redhat.com (mailing list archive)
State New, archived
Headers show
Series LUKS: Fix error message when underlying fs don't support large enough files | expand

Commit Message

Maxim Levitsky April 16, 2020, 9:50 a.m. UTC 
Currently if you attampt to create too large file with luks you
get the following error message:

Formatting 'test.luks', fmt=luks size=17592186044416 key-secret=sec0
qemu-img: test.luks: Could not resize file: File too large

While for raw format the error message is
qemu-img: test.img: The image size is too large for file format 'raw'


The reason for this is that qemu-img checks for errno of the failure,
and presents the later error when it is -EFBIG

However crypto generic code 'swallows' the errno and replaces it
with -EIO.

As an attempt to make it better, we can make luks driver,
detect -EFBIG and in this case present a better error message,
which is what this patch does

The new error message is:

qemu-img: error creating test.luks: The requested file size is too large

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534898
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 block/crypto.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

Comments

Kevin Wolf April 20, 2020, 9:02 a.m. UTC | #1 
Am 16.04.2020 um 11:50 hat Maxim Levitsky geschrieben:
> Currently if you attampt to create too large file with luks you
> get the following error message:
> 
> Formatting 'test.luks', fmt=luks size=17592186044416 key-secret=sec0
> qemu-img: test.luks: Could not resize file: File too large
> 
> While for raw format the error message is
> qemu-img: test.img: The image size is too large for file format 'raw'
> 
> 
> The reason for this is that qemu-img checks for errno of the failure,
> and presents the later error when it is -EFBIG
> 
> However crypto generic code 'swallows' the errno and replaces it
> with -EIO.
> 
> As an attempt to make it better, we can make luks driver,
> detect -EFBIG and in this case present a better error message,
> which is what this patch does
> 
> The new error message is:
> 
> qemu-img: error creating test.luks: The requested file size is too large
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534898
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  block/crypto.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/block/crypto.c b/block/crypto.c
> index d577f89659..1b604beb30 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -104,18 +104,35 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
>                                        Error **errp)
>  {
>      struct BlockCryptoCreateData *data = opaque;
> +    Error *local_error = NULL;
> +    int ret;
>  
>      if (data->size > INT64_MAX || headerlen > INT64_MAX - data->size) {
> -        error_setg(errp, "The requested file size is too large");
> -        return -EFBIG;
> +        ret = -EFBIG;
> +        goto error;
>      }
>  
>      /* User provided size should reflect amount of space made
>       * available to the guest, so we must take account of that
>       * which will be used by the crypto header
>       */
> -    return blk_truncate(data->blk, data->size + headerlen, false,
> +    ret = blk_truncate(data->blk, data->size + headerlen, false,
>                          data->prealloc, errp);

I think you intended to use &local_error (which is by the way spelt
local_err in most other places) here instead of passing errp and never
assigning local_err at all.

Kevin

> +
> +    if (ret >= 0) {
> +        return ret;
> +    }
> +
> +error:
> +    if (ret == -EFBIG) {
> +        /* Replace the error message with a better one */
> +        error_free(local_error);
> +        error_setg(errp, "The requested file size is too large");
> +    } else {
> +        error_propagate(errp, local_error);
> +    }
> +
> +    return ret;
>  }
>  
>  
> -- 
> 2.17.2
>
Maxim Levitsky April 20, 2020, 9:16 a.m. UTC | #2 
On Mon, 2020-04-20 at 11:02 +0200, Kevin Wolf wrote:
> Am 16.04.2020 um 11:50 hat Maxim Levitsky geschrieben:
> > Currently if you attampt to create too large file with luks you
> > get the following error message:
> > 
> > Formatting 'test.luks', fmt=luks size=17592186044416 key-secret=sec0
> > qemu-img: test.luks: Could not resize file: File too large
> > 
> > While for raw format the error message is
> > qemu-img: test.img: The image size is too large for file format 'raw'
> > 
> > 
> > The reason for this is that qemu-img checks for errno of the failure,
> > and presents the later error when it is -EFBIG
> > 
> > However crypto generic code 'swallows' the errno and replaces it
> > with -EIO.
> > 
> > As an attempt to make it better, we can make luks driver,
> > detect -EFBIG and in this case present a better error message,
> > which is what this patch does
> > 
> > The new error message is:
> > 
> > qemu-img: error creating test.luks: The requested file size is too large
> > 
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534898
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  block/crypto.c | 23 ++++++++++++++++++++---
> >  1 file changed, 20 insertions(+), 3 deletions(-)
> > 
> > diff --git a/block/crypto.c b/block/crypto.c
> > index d577f89659..1b604beb30 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -104,18 +104,35 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
> >                                        Error **errp)
> >  {
> >      struct BlockCryptoCreateData *data = opaque;
> > +    Error *local_error = NULL;
> > +    int ret;
> >  
> >      if (data->size > INT64_MAX || headerlen > INT64_MAX - data->size) {
> > -        error_setg(errp, "The requested file size is too large");
> > -        return -EFBIG;
> > +        ret = -EFBIG;
> > +        goto error;
> >      }
> >  
> >      /* User provided size should reflect amount of space made
> >       * available to the guest, so we must take account of that
> >       * which will be used by the crypto header
> >       */
> > -    return blk_truncate(data->blk, data->size + headerlen, false,
> > +    ret = blk_truncate(data->blk, data->size + headerlen, false,
> >                          data->prealloc, errp);
> 
> I think you intended to use &local_error (which is by the way spelt
> local_err in most other places) here instead of passing errp and never
> assigning local_err at all.

Absolutely. This is very old patch that I used to keep with the LUKs patchset,
and I remember I rebased it few times. I probably didn't fix a conflict correctly or so.

Best regards,
	Maxim Levitsky

> 
> Kevin
> 
> > +
> > +    if (ret >= 0) {
> > +        return ret;
> > +    }
> > +
> > +error:
> > +    if (ret == -EFBIG) {
> > +        /* Replace the error message with a better one */
> > +        error_free(local_error);
> > +        error_setg(errp, "The requested file size is too large");
> > +    } else {
> > +        error_propagate(errp, local_error);
> > +    }
> > +
> > +    return ret;
> >  }
> >  
> >  
> > -- 
> > 2.17.2
> >
diff mbox series

Patch

diff --git a/block/crypto.c b/block/crypto.c
index d577f89659..1b604beb30 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -104,18 +104,35 @@  static ssize_t block_crypto_init_func(QCryptoBlock *block,
                                       Error **errp)
 {
     struct BlockCryptoCreateData *data = opaque;
+    Error *local_error = NULL;
+    int ret;
 
     if (data->size > INT64_MAX || headerlen > INT64_MAX - data->size) {
-        error_setg(errp, "The requested file size is too large");
-        return -EFBIG;
+        ret = -EFBIG;
+        goto error;
     }
 
     /* User provided size should reflect amount of space made
      * available to the guest, so we must take account of that
      * which will be used by the crypto header
      */
-    return blk_truncate(data->blk, data->size + headerlen, false,
+    ret = blk_truncate(data->blk, data->size + headerlen, false,
                         data->prealloc, errp);
+
+    if (ret >= 0) {
+        return ret;
+    }
+
+error:
+    if (ret == -EFBIG) {
+        /* Replace the error message with a better one */
+        error_free(local_error);
+        error_setg(errp, "The requested file size is too large");
+    } else {
+        error_propagate(errp, local_error);
+    }
+
+    return ret;
 }