diff mbox

[v2,5/5] blockdev: use error_set_file_open_failed

Message ID 1331765714-26209-6-git-send-email-alevy@redhat.com
State New
Headers show

Commit Message

Alon Levy March 14, 2012, 10:55 p.m. UTC
This is a little trickier, since those calls chain in many fun ways and
produce sometimes their own return values reusing existing errno values
for similar meanings. In that respect error_set_file_open_failed
specifically ignores EINVAL, ENOTSUP and ENOENT. The first two simply
are not returned by open (2), but the last is but I chose to ignore it
to allow easy reuse in blockdev to avoid confusion when it is used
internally by the create functions.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 blockdev.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Luiz Capitulino March 15, 2012, 2:56 p.m. UTC | #1
On Thu, 15 Mar 2012 00:55:14 +0200
Alon Levy <alevy@redhat.com> wrote:

> This is a little trickier, since those calls chain in many fun ways and
> produce sometimes their own return values reusing existing errno values
> for similar meanings. In that respect error_set_file_open_failed
> specifically ignores EINVAL, ENOTSUP and ENOENT. The first two simply
> are not returned by open (2), but the last is but I chose to ignore it
> to allow easy reuse in blockdev to avoid confusion when it is used
> internally by the create functions.

Please, just drop this from this series as it's completely unrelated to
the screendump command.

> 
> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
>  blockdev.c |   11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 1a500b8..544d067 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -777,7 +777,7 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
>                                    states->old_bs->drv->format_name,
>                                    NULL, -1, flags);
>              if (ret) {
> -                error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
> +                error_set_file_open_failed(errp, new_image_file, -ret);
>                  goto delete_and_fail;
>              }
>          }
> @@ -787,7 +787,7 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
>          ret = bdrv_open(states->new_bs, new_image_file,
>                          flags | BDRV_O_NO_BACKING, drv);
>          if (ret != 0) {
> -            error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
> +            error_set_file_open_failed(errp, new_image_file, -ret);
>              goto delete_and_fail;
>          }
>      }
> @@ -881,8 +881,11 @@ static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
>                                      int bdrv_flags, BlockDriver *drv,
>                                      const char *password, Error **errp)
>  {
> -    if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) {
> -        error_set(errp, QERR_OPEN_FILE_FAILED, filename);
> +    int ret;
> +
> +    ret = bdrv_open(bs, filename, bdrv_flags, drv);
> +    if (ret < 0) {
> +        error_set_file_open_failed(errp, filename, ret);
>          return;
>      }
>
Alon Levy March 15, 2012, 3:23 p.m. UTC | #2
On Thu, Mar 15, 2012 at 11:56:29AM -0300, Luiz Capitulino wrote:
> On Thu, 15 Mar 2012 00:55:14 +0200
> Alon Levy <alevy@redhat.com> wrote:
> 
> > This is a little trickier, since those calls chain in many fun ways and
> > produce sometimes their own return values reusing existing errno values
> > for similar meanings. In that respect error_set_file_open_failed
> > specifically ignores EINVAL, ENOTSUP and ENOENT. The first two simply
> > are not returned by open (2), but the last is but I chose to ignore it
> > to allow easy reuse in blockdev to avoid confusion when it is used
> > internally by the create functions.
> 
> Please, just drop this from this series as it's completely unrelated to
> the screendump command.

After I went to all the trouble jumping through blockdev.c ? OK :)

> 
> > 
> > Signed-off-by: Alon Levy <alevy@redhat.com>
> > ---
> >  blockdev.c |   11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/blockdev.c b/blockdev.c
> > index 1a500b8..544d067 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -777,7 +777,7 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
> >                                    states->old_bs->drv->format_name,
> >                                    NULL, -1, flags);
> >              if (ret) {
> > -                error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
> > +                error_set_file_open_failed(errp, new_image_file, -ret);
> >                  goto delete_and_fail;
> >              }
> >          }
> > @@ -787,7 +787,7 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
> >          ret = bdrv_open(states->new_bs, new_image_file,
> >                          flags | BDRV_O_NO_BACKING, drv);
> >          if (ret != 0) {
> > -            error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
> > +            error_set_file_open_failed(errp, new_image_file, -ret);
> >              goto delete_and_fail;
> >          }
> >      }
> > @@ -881,8 +881,11 @@ static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
> >                                      int bdrv_flags, BlockDriver *drv,
> >                                      const char *password, Error **errp)
> >  {
> > -    if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) {
> > -        error_set(errp, QERR_OPEN_FILE_FAILED, filename);
> > +    int ret;
> > +
> > +    ret = bdrv_open(bs, filename, bdrv_flags, drv);
> > +    if (ret < 0) {
> > +        error_set_file_open_failed(errp, filename, ret);
> >          return;
> >      }
> >  
>
diff mbox

Patch

diff --git a/blockdev.c b/blockdev.c
index 1a500b8..544d067 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -777,7 +777,7 @@  void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
                                   states->old_bs->drv->format_name,
                                   NULL, -1, flags);
             if (ret) {
-                error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
+                error_set_file_open_failed(errp, new_image_file, -ret);
                 goto delete_and_fail;
             }
         }
@@ -787,7 +787,7 @@  void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
         ret = bdrv_open(states->new_bs, new_image_file,
                         flags | BDRV_O_NO_BACKING, drv);
         if (ret != 0) {
-            error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
+            error_set_file_open_failed(errp, new_image_file, -ret);
             goto delete_and_fail;
         }
     }
@@ -881,8 +881,11 @@  static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
                                     int bdrv_flags, BlockDriver *drv,
                                     const char *password, Error **errp)
 {
-    if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) {
-        error_set(errp, QERR_OPEN_FILE_FAILED, filename);
+    int ret;
+
+    ret = bdrv_open(bs, filename, bdrv_flags, drv);
+    if (ret < 0) {
+        error_set_file_open_failed(errp, filename, ret);
         return;
     }