Patchwork [v3,2/5] add qemu_fopen_err

login
register
mail settings
Submitter Alon Levy
Date March 18, 2012, 6:29 p.m.
Message ID <1332095353-19120-3-git-send-email-alevy@redhat.com>
Download mbox | patch
Permalink /patch/147415/
State New
Headers show

Comments

Alon Levy - March 18, 2012, 6:29 p.m.
This adds a helper to conveniently set the correct error based on the
errno after a failed fopen.

The added function is placed in it's own c file to allow libcacard to
not develop dependencies on everything that qerror will bring in, which
includes monitor and half of qemu. I tried to make it as less ugly as I
could, by naming an osdep-no-qerror-obj-y and having that included in
osdep-obj-y, and using only the former for libcacard.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 Makefile.objs      |    8 +++++---
 libcacard/Makefile |    2 +-
 osdep-qerror.c     |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 osdep-qerror.h     |    8 ++++++++
 osdep.c            |    1 -
 5 files changed, 66 insertions(+), 5 deletions(-)
 create mode 100644 osdep-qerror.c
 create mode 100644 osdep-qerror.h
Luiz Capitulino - March 23, 2012, 2:22 p.m.
On Sun, 18 Mar 2012 19:29:10 +0100
Alon Levy <alevy@redhat.com> wrote:

> This adds a helper to conveniently set the correct error based on the
> errno after a failed fopen.
> 
> The added function is placed in it's own c file to allow libcacard to
> not develop dependencies on everything that qerror will bring in, which
> includes monitor and half of qemu. I tried to make it as less ugly as I
> could, by naming an osdep-no-qerror-obj-y and having that included in
> osdep-obj-y, and using only the former for libcacard.

I'm not sure I like this, how will libcacard report errors then?

> 
> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
>  Makefile.objs      |    8 +++++---
>  libcacard/Makefile |    2 +-
>  osdep-qerror.c     |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  osdep-qerror.h     |    8 ++++++++
>  osdep.c            |    1 -
>  5 files changed, 66 insertions(+), 5 deletions(-)
>  create mode 100644 osdep-qerror.c
>  create mode 100644 osdep-qerror.h
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 226b01d..fb5a73a 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -20,9 +20,11 @@ universal-obj-y += $(qom-obj-y)
>  
>  #######################################################################
>  # oslib-obj-y is code depending on the OS (win32 vs posix)
> -oslib-obj-y = osdep.o
> -oslib-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o
> -oslib-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o
> +oslib-no-qerror-obj-y = osdep.o
> +oslib-no-qerror-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o
> +oslib-no-qerror-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o
> +oslib-obj-y = $(oslib-no-qerror-obj-y)
> +oslib-obj-y += osdep-qerror.o
>  
>  #######################################################################
>  # coroutines
> diff --git a/libcacard/Makefile b/libcacard/Makefile
> index c6a896a..83f483f 100644
> --- a/libcacard/Makefile
> +++ b/libcacard/Makefile
> @@ -8,7 +8,7 @@ libcacard_includedir=$(includedir)/cacard
>  $(call set-vpath, $(SRC_PATH):$(libcacard_srcpath))
>  
>  # objects linked against normal qemu binaries, not compiled with libtool
> -QEMU_OBJS=$(addprefix ../,$(oslib-obj-y) qemu-timer-common.o $(trace-obj-y))
> +QEMU_OBJS=$(addprefix ../,$(oslib-no-qerror-obj-y) qemu-timer-common.o $(trace-obj-y))
>  
>  # objects linked into a shared library, built with libtool with -fPIC if required
>  QEMU_OBJS_LIB=$(addsuffix .lo,$(basename $(QEMU_OBJS)))
> diff --git a/osdep-qerror.c b/osdep-qerror.c
> new file mode 100644
> index 0000000..6dac984
> --- /dev/null
> +++ b/osdep-qerror.c
> @@ -0,0 +1,52 @@
> +#include "qerror.h"
> +
> +#include "osdep-qerror.h"
> +
> +/*
> + * Helper to set an Error after a failed fopen.
> + *
> + * Uses errno so it must not be changed by another intermediate call.
> + */
> +void qemu_fopen_err(Error **errp, const char *file_name)
> +{
> +    const char *fmt = NULL;

Where's the fopen() call and the mode argument?

> +
> +    switch (errno) {
> +    case EACCES:
> +        fmt = QERR_EACCES;
> +        break;
> +    case EINTR:
> +        fmt = QERR_EINTR;
> +        break;
> +    case EEXIST:
> +        fmt = QERR_EEXIST;
> +        break;
> +    case EMFILE:
> +        fmt = QERR_OPEN_FILE_EMFILE;
> +        break;
> +    case ENOSPC:
> +        fmt = QERR_ENOSPC;
> +        break;
> +    case EPERM:
> +        fmt = QERR_EPERM;
> +        break;
> +    case EROFS:
> +        fmt = QERR_READ_ONLY;
> +        break;
> +    case ENOTDIR:
> +        fmt = QERR_ENOTDIR;
> +        break;
> +    case EFBIG:
> +        fmt = QERR_EFBIG;
> +        break;
> +    default:
> +        /*
> +         * EINVAL and ENOTSUP will result in the default
> +         *
> +         * ENOENT too, it's used by (for instance) bdrv_create_file for
> +         * a different purpose then open (2) so just give a generic error.
> +         */
> +        fmt = QERR_OPEN_FILE_FAILED;
> +    }
> +    error_set(errp, fmt, file_name);
> +}
> diff --git a/osdep-qerror.h b/osdep-qerror.h
> new file mode 100644
> index 0000000..7320f4a
> --- /dev/null
> +++ b/osdep-qerror.h
> @@ -0,0 +1,8 @@
> +#ifndef OSDEP_QERROR_H
> +#define OSDEP_QERROR_H
> +
> +#include "error.h"
> +
> +void qemu_fopen_err(Error **errp, const char *file_name);
> +
> +#endif
> diff --git a/osdep.c b/osdep.c
> index 3e6bada..efdd21c 100644
> --- a/osdep.c
> +++ b/osdep.c
> @@ -241,4 +241,3 @@ ssize_t qemu_recv_full(int fd, void *buf, size_t count, int flags)
>  
>      return total;
>  }
> -
Alon Levy - March 23, 2012, 7:55 p.m.
On Fri, Mar 23, 2012 at 11:22:48AM -0300, Luiz Capitulino wrote:
> On Sun, 18 Mar 2012 19:29:10 +0100
> Alon Levy <alevy@redhat.com> wrote:
> 
> > This adds a helper to conveniently set the correct error based on the
> > errno after a failed fopen.
> > 
> > The added function is placed in it's own c file to allow libcacard to
> > not develop dependencies on everything that qerror will bring in, which
> > includes monitor and half of qemu. I tried to make it as less ugly as I
> > could, by naming an osdep-no-qerror-obj-y and having that included in
> > osdep-obj-y, and using only the former for libcacard.
> 
> I'm not sure I like this, how will libcacard report errors then?

libcacard is a standalone library, it reports errors via it's existing
interfaces. Changing it to use qerror is possibly a good idea but not
part of this patchset.

> 
> > 
> > Signed-off-by: Alon Levy <alevy@redhat.com>
> > ---
> >  Makefile.objs      |    8 +++++---
> >  libcacard/Makefile |    2 +-
> >  osdep-qerror.c     |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  osdep-qerror.h     |    8 ++++++++
> >  osdep.c            |    1 -
> >  5 files changed, 66 insertions(+), 5 deletions(-)
> >  create mode 100644 osdep-qerror.c
> >  create mode 100644 osdep-qerror.h
> > 
> > diff --git a/Makefile.objs b/Makefile.objs
> > index 226b01d..fb5a73a 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -20,9 +20,11 @@ universal-obj-y += $(qom-obj-y)
> >  
> >  #######################################################################
> >  # oslib-obj-y is code depending on the OS (win32 vs posix)
> > -oslib-obj-y = osdep.o
> > -oslib-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o
> > -oslib-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o
> > +oslib-no-qerror-obj-y = osdep.o
> > +oslib-no-qerror-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o
> > +oslib-no-qerror-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o
> > +oslib-obj-y = $(oslib-no-qerror-obj-y)
> > +oslib-obj-y += osdep-qerror.o
> >  
> >  #######################################################################
> >  # coroutines
> > diff --git a/libcacard/Makefile b/libcacard/Makefile
> > index c6a896a..83f483f 100644
> > --- a/libcacard/Makefile
> > +++ b/libcacard/Makefile
> > @@ -8,7 +8,7 @@ libcacard_includedir=$(includedir)/cacard
> >  $(call set-vpath, $(SRC_PATH):$(libcacard_srcpath))
> >  
> >  # objects linked against normal qemu binaries, not compiled with libtool
> > -QEMU_OBJS=$(addprefix ../,$(oslib-obj-y) qemu-timer-common.o $(trace-obj-y))
> > +QEMU_OBJS=$(addprefix ../,$(oslib-no-qerror-obj-y) qemu-timer-common.o $(trace-obj-y))
> >  
> >  # objects linked into a shared library, built with libtool with -fPIC if required
> >  QEMU_OBJS_LIB=$(addsuffix .lo,$(basename $(QEMU_OBJS)))
> > diff --git a/osdep-qerror.c b/osdep-qerror.c
> > new file mode 100644
> > index 0000000..6dac984
> > --- /dev/null
> > +++ b/osdep-qerror.c
> > @@ -0,0 +1,52 @@
> > +#include "qerror.h"
> > +
> > +#include "osdep-qerror.h"
> > +
> > +/*
> > + * Helper to set an Error after a failed fopen.
> > + *
> > + * Uses errno so it must not be changed by another intermediate call.
> > + */
> > +void qemu_fopen_err(Error **errp, const char *file_name)
> > +{
> > +    const char *fmt = NULL;
> 
> Where's the fopen() call and the mode argument?
> 
> > +
> > +    switch (errno) {
> > +    case EACCES:
> > +        fmt = QERR_EACCES;
> > +        break;
> > +    case EINTR:
> > +        fmt = QERR_EINTR;
> > +        break;
> > +    case EEXIST:
> > +        fmt = QERR_EEXIST;
> > +        break;
> > +    case EMFILE:
> > +        fmt = QERR_OPEN_FILE_EMFILE;
> > +        break;
> > +    case ENOSPC:
> > +        fmt = QERR_ENOSPC;
> > +        break;
> > +    case EPERM:
> > +        fmt = QERR_EPERM;
> > +        break;
> > +    case EROFS:
> > +        fmt = QERR_READ_ONLY;
> > +        break;
> > +    case ENOTDIR:
> > +        fmt = QERR_ENOTDIR;
> > +        break;
> > +    case EFBIG:
> > +        fmt = QERR_EFBIG;
> > +        break;
> > +    default:
> > +        /*
> > +         * EINVAL and ENOTSUP will result in the default
> > +         *
> > +         * ENOENT too, it's used by (for instance) bdrv_create_file for
> > +         * a different purpose then open (2) so just give a generic error.
> > +         */
> > +        fmt = QERR_OPEN_FILE_FAILED;
> > +    }
> > +    error_set(errp, fmt, file_name);
> > +}
> > diff --git a/osdep-qerror.h b/osdep-qerror.h
> > new file mode 100644
> > index 0000000..7320f4a
> > --- /dev/null
> > +++ b/osdep-qerror.h
> > @@ -0,0 +1,8 @@
> > +#ifndef OSDEP_QERROR_H
> > +#define OSDEP_QERROR_H
> > +
> > +#include "error.h"
> > +
> > +void qemu_fopen_err(Error **errp, const char *file_name);
> > +
> > +#endif
> > diff --git a/osdep.c b/osdep.c
> > index 3e6bada..efdd21c 100644
> > --- a/osdep.c
> > +++ b/osdep.c
> > @@ -241,4 +241,3 @@ ssize_t qemu_recv_full(int fd, void *buf, size_t count, int flags)
> >  
> >      return total;
> >  }
> > -
> 
>

Patch

diff --git a/Makefile.objs b/Makefile.objs
index 226b01d..fb5a73a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -20,9 +20,11 @@  universal-obj-y += $(qom-obj-y)
 
 #######################################################################
 # oslib-obj-y is code depending on the OS (win32 vs posix)
-oslib-obj-y = osdep.o
-oslib-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o
-oslib-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o
+oslib-no-qerror-obj-y = osdep.o
+oslib-no-qerror-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o
+oslib-no-qerror-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o
+oslib-obj-y = $(oslib-no-qerror-obj-y)
+oslib-obj-y += osdep-qerror.o
 
 #######################################################################
 # coroutines
diff --git a/libcacard/Makefile b/libcacard/Makefile
index c6a896a..83f483f 100644
--- a/libcacard/Makefile
+++ b/libcacard/Makefile
@@ -8,7 +8,7 @@  libcacard_includedir=$(includedir)/cacard
 $(call set-vpath, $(SRC_PATH):$(libcacard_srcpath))
 
 # objects linked against normal qemu binaries, not compiled with libtool
-QEMU_OBJS=$(addprefix ../,$(oslib-obj-y) qemu-timer-common.o $(trace-obj-y))
+QEMU_OBJS=$(addprefix ../,$(oslib-no-qerror-obj-y) qemu-timer-common.o $(trace-obj-y))
 
 # objects linked into a shared library, built with libtool with -fPIC if required
 QEMU_OBJS_LIB=$(addsuffix .lo,$(basename $(QEMU_OBJS)))
diff --git a/osdep-qerror.c b/osdep-qerror.c
new file mode 100644
index 0000000..6dac984
--- /dev/null
+++ b/osdep-qerror.c
@@ -0,0 +1,52 @@ 
+#include "qerror.h"
+
+#include "osdep-qerror.h"
+
+/*
+ * Helper to set an Error after a failed fopen.
+ *
+ * Uses errno so it must not be changed by another intermediate call.
+ */
+void qemu_fopen_err(Error **errp, const char *file_name)
+{
+    const char *fmt = NULL;
+
+    switch (errno) {
+    case EACCES:
+        fmt = QERR_EACCES;
+        break;
+    case EINTR:
+        fmt = QERR_EINTR;
+        break;
+    case EEXIST:
+        fmt = QERR_EEXIST;
+        break;
+    case EMFILE:
+        fmt = QERR_OPEN_FILE_EMFILE;
+        break;
+    case ENOSPC:
+        fmt = QERR_ENOSPC;
+        break;
+    case EPERM:
+        fmt = QERR_EPERM;
+        break;
+    case EROFS:
+        fmt = QERR_READ_ONLY;
+        break;
+    case ENOTDIR:
+        fmt = QERR_ENOTDIR;
+        break;
+    case EFBIG:
+        fmt = QERR_EFBIG;
+        break;
+    default:
+        /*
+         * EINVAL and ENOTSUP will result in the default
+         *
+         * ENOENT too, it's used by (for instance) bdrv_create_file for
+         * a different purpose then open (2) so just give a generic error.
+         */
+        fmt = QERR_OPEN_FILE_FAILED;
+    }
+    error_set(errp, fmt, file_name);
+}
diff --git a/osdep-qerror.h b/osdep-qerror.h
new file mode 100644
index 0000000..7320f4a
--- /dev/null
+++ b/osdep-qerror.h
@@ -0,0 +1,8 @@ 
+#ifndef OSDEP_QERROR_H
+#define OSDEP_QERROR_H
+
+#include "error.h"
+
+void qemu_fopen_err(Error **errp, const char *file_name);
+
+#endif
diff --git a/osdep.c b/osdep.c
index 3e6bada..efdd21c 100644
--- a/osdep.c
+++ b/osdep.c
@@ -241,4 +241,3 @@  ssize_t qemu_recv_full(int fd, void *buf, size_t count, int flags)
 
     return total;
 }
-