9:12 a.m.
From: "Richard W.M. Jones" <rjones(a)redhat.com>
Callers are supposed to use the availability API to check for
functions that may not be available in particular builds of
libguestfs. If they don't do this, currently they tend to get obscure
error messages, eg:
libguestfs: error: zerofree: /dev/vda1: zerofree: No such file or directory
This commit changes the error message to explain what callers ought to
be doing instead:
libguestfs: error: zerofree: feature 'zerofree' is not available in this
build of libguestfs. Read 'AVAILABILITY' in the guestfs(3) man page for
how to check for the availability of features.
This patch makes the stubs check for availability. The stub code
changes to:
static void
zerofree_stub (XDR *xdr_in)
{
[...]
/* The caller should have checked before calling this. */
if (! optgroup_zerofree_available ()) {
reply_with_error ("feature '%s' is not available in this\n"
"build of libguestfs. Read 'AVAILABILITY' in the
guestfs(3) man page for\n"
"how to check for the availability of features.",
"zerofree");
goto done;
}
[...]
---
daemon/augeas.c | 158 +++++++++++++++++++++++++++--------------
daemon/daemon.h | 10 ---
daemon/inotify.c | 81 +++++++++++++--------
daemon/mknod.c | 45 +++++++++---
daemon/realpath.c | 26 +++++--
daemon/selinux.c | 40 ++++++++---
daemon/xattr.c | 23 ++++---
generator/generator_daemon.ml | 24 ++++++-
8 files changed, 271 insertions(+), 136 deletions(-)
diff --git a/daemon/augeas.c b/daemon/augeas.c
index f52c091..4a09f57 100644
--- a/daemon/augeas.c
+++ b/daemon/augeas.c
@@ -63,19 +63,11 @@ optgroup_augeas_available (void)
{
return 1;
}
-#else /* !HAVE_AUGEAS */
-int
-optgroup_augeas_available (void)
-{
- return 0;
-}
-#endif
/* We need to rewrite the root path so it is based at /sysroot. */
int
do_aug_init (const char *root, int flags)
{
-#ifdef HAVE_AUGEAS
char *buf;
if (aug) {
@@ -98,24 +90,17 @@ do_aug_init (const char *root, int flags)
}
return 0;
-#else
- NOT_AVAILABLE (-1);
-#endif
}
int
do_aug_close (void)
{
-#ifdef HAVE_AUGEAS
NEED_AUG(-1);
aug_close (aug);
aug = NULL;
return 0;
-#else
- NOT_AVAILABLE (-1);
-#endif
}
int
@@ -133,7 +118,8 @@ do_aug_defvar (const char *name, const char *expr)
}
return r;
#else
- NOT_AVAILABLE (-1);
+ reply_with_error ("function not available");
+ return -1;
#endif
}
@@ -163,14 +149,14 @@ do_aug_defnode (const char *name, const char *expr, const char
*val)
return r;
#else
- NOT_AVAILABLE (NULL);
+ reply_with_error ("function not available");
+ return NULL;
#endif
}
char *
do_aug_get (const char *path)
{
-#ifdef HAVE_AUGEAS
const char *value = NULL;
char *v;
int r;
@@ -204,15 +190,11 @@ do_aug_get (const char *path)
}
return v; /* Caller frees. */
-#else
- NOT_AVAILABLE (NULL);
-#endif
}
int
do_aug_set (const char *path, const char *val)
{
-#ifdef HAVE_AUGEAS
int r;
NEED_AUG (-1);
@@ -224,15 +206,11 @@ do_aug_set (const char *path, const char *val)
}
return 0;
-#else
- NOT_AVAILABLE (-1);
-#endif
}
int
do_aug_clear (const char *path)
{
-#ifdef HAVE_AUGEAS
int r;
NEED_AUG (-1);
@@ -244,15 +222,11 @@ do_aug_clear (const char *path)
}
return 0;
-#else
- NOT_AVAILABLE (-1);
-#endif
}
int
do_aug_insert (const char *path, const char *label, int before)
{
-#ifdef HAVE_AUGEAS
int r;
NEED_AUG (-1);
@@ -264,15 +238,11 @@ do_aug_insert (const char *path, const char *label, int before)
}
return 0;
-#else
- NOT_AVAILABLE (-1);
-#endif
}
int
do_aug_rm (const char *path)
{
-#ifdef HAVE_AUGEAS
int r;
NEED_AUG (-1);
@@ -284,15 +254,11 @@ do_aug_rm (const char *path)
}
return r;
-#else
- NOT_AVAILABLE (-1);
-#endif
}
int
do_aug_mv (const char *src, const char *dest)
{
-#ifdef HAVE_AUGEAS
int r;
NEED_AUG (-1);
@@ -304,15 +270,11 @@ do_aug_mv (const char *src, const char *dest)
}
return 0;
-#else
- NOT_AVAILABLE (-1);
-#endif
}
char **
do_aug_match (const char *path)
{
-#ifdef HAVE_AUGEAS
char **matches = NULL;
void *vp;
int r;
@@ -338,15 +300,11 @@ do_aug_match (const char *path)
matches[r] = NULL;
return matches; /* Caller frees. */
-#else
- NOT_AVAILABLE (NULL);
-#endif
}
int
do_aug_save (void)
{
-#ifdef HAVE_AUGEAS
NEED_AUG (-1);
if (aug_save (aug) == -1) {
@@ -355,9 +313,6 @@ do_aug_save (void)
}
return 0;
-#else
- NOT_AVAILABLE (-1);
-#endif
}
int
@@ -373,7 +328,8 @@ do_aug_load (void)
return 0;
#else
- NOT_AVAILABLE (-1);
+ reply_with_error ("function not available");
+ return -1;
#endif
}
@@ -381,7 +337,6 @@ do_aug_load (void)
char **
do_aug_ls (const char *path)
{
-#ifdef HAVE_AUGEAS
char **matches;
char *buf;
int len;
@@ -420,7 +375,102 @@ do_aug_ls (const char *path)
sort_strings (matches, count_strings ((void *) matches));
return matches; /* Caller frees. */
-#else
- NOT_AVAILABLE (NULL);
-#endif
}
+
+#else /* !HAVE_AUGEAS */
+
+/* Note that the wrapper code (daemon/stubs.c) ensures that the
+ * functions below are never called because optgroup_augeas_available
+ * returns false.
+ */
+int
+optgroup_augeas_available (void)
+{
+ return 0;
+}
+
+int
+do_aug_init (const char *root, int flags)
+{
+ abort ();
+}
+
+int
+do_aug_close (void)
+{
+ abort ();
+}
+
+int
+do_aug_defvar (const char *name, const char *expr)
+{
+ abort ();
+}
+
+guestfs_int_int_bool *
+do_aug_defnode (const char *name, const char *expr, const char *val)
+{
+ abort ();
+}
+
+char *
+do_aug_get (const char *path)
+{
+ abort ();
+}
+
+int
+do_aug_set (const char *path, const char *val)
+{
+ abort ();
+}
+
+int
+do_aug_clear (const char *path)
+{
+ abort ();
+}
+
+int
+do_aug_insert (const char *path, const char *label, int before)
+{
+ abort ();
+}
+
+int
+do_aug_rm (const char *path)
+{
+ abort ();
+}
+
+int
+do_aug_mv (const char *src, const char *dest)
+{
+ abort ();
+}
+
+char **
+do_aug_match (const char *path)
+{
+ abort ();
+}
+
+int
+do_aug_save (void)
+{
+ abort ();
+}
+
+int
+do_aug_load (void)
+{
+ abort ();
+}
+
+char **
+do_aug_ls (const char *path)
+{
+ abort ();
+}
+
+#endif
diff --git a/daemon/daemon.h b/daemon/daemon.h
index babe5bc..b518f92 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -331,16 +331,6 @@ is_zero (const char *buffer, size_t size)
} \
while (0)
-/* Marks functions which are not available.
- * NB. Cannot be used for FileIn functions.
- */
-#define NOT_AVAILABLE(errcode) \
- do { \
- reply_with_error ("%s: function not available", __func__); \
- return (errcode); \
- } \
- while (0)
-
#ifndef __attribute__
# if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 8)
# define __attribute__(x) /* empty */
diff --git a/daemon/inotify.c b/daemon/inotify.c
index add1f14..df6b2e8 100644
--- a/daemon/inotify.c
+++ b/daemon/inotify.c
@@ -56,13 +56,6 @@ optgroup_inotify_available (void)
{
return 1;
}
-#else /* !HAVE_SYS_INOTIFY_H */
-int
-optgroup_inotify_available (void)
-{
- return 0;
-}
-#endif
/* Because inotify_init does NEED_ROOT, NEED_INOTIFY implies NEED_ROOT. */
#define NEED_INOTIFY(errcode) \
@@ -78,7 +71,6 @@ optgroup_inotify_available (void)
int
do_inotify_init (int max_events)
{
-#ifdef HAVE_SYS_INOTIFY_H
FILE *fp;
NEED_ROOT (, return -1);
@@ -129,15 +121,11 @@ do_inotify_init (int max_events)
#endif
return 0;
-#else
- NOT_AVAILABLE (-1);
-#endif
}
int
do_inotify_close (void)
{
-#ifdef HAVE_SYS_INOTIFY_H
NEED_INOTIFY (-1);
if (inotify_fd == -1) {
@@ -154,15 +142,11 @@ do_inotify_close (void)
inotify_posn = 0;
return 0;
-#else
- NOT_AVAILABLE (-1);
-#endif
}
int64_t
do_inotify_add_watch (const char *path, int mask)
{
-#ifdef HAVE_SYS_INOTIFY_H
int64_t r;
char *buf;
@@ -182,15 +166,11 @@ do_inotify_add_watch (const char *path, int mask)
}
return r;
-#else
- NOT_AVAILABLE (-1);
-#endif
}
int
do_inotify_rm_watch (int wd)
{
-#ifdef HAVE_SYS_INOTIFY_H
NEED_INOTIFY (-1);
if (inotify_rm_watch (inotify_fd, wd) == -1) {
@@ -199,15 +179,11 @@ do_inotify_rm_watch (int wd)
}
return 0;
-#else
- NOT_AVAILABLE (-1);
-#endif
}
guestfs_int_inotify_event_list *
do_inotify_read (void)
{
-#ifdef HAVE_SYS_INOTIFY_H
int space;
guestfs_int_inotify_event_list *ret;
@@ -313,15 +289,11 @@ do_inotify_read (void)
xdr_free ((xdrproc_t) xdr_guestfs_int_inotify_event_list, (char *) ret);
free (ret);
return NULL;
-#else
- NOT_AVAILABLE (NULL);
-#endif
}
char **
do_inotify_files (void)
{
-#ifdef HAVE_SYS_INOTIFY_H
char **ret = NULL;
int size = 0, alloc = 0;
unsigned int i;
@@ -404,7 +376,54 @@ do_inotify_files (void)
unlink (tempfile);
return NULL;
-#else
- NOT_AVAILABLE (NULL);
-#endif
}
+
+#else /* !HAVE_SYS_INOTIFY_H */
+
+/* Note that the wrapper code (daemon/stubs.c) ensures that the
+ * functions below are never called because optgroup_inotify_available
+ * returns false.
+ */
+int
+optgroup_inotify_available (void)
+{
+ return 0;
+}
+
+int
+do_inotify_init (int max_events)
+{
+ abort ();
+}
+
+int
+do_inotify_close (void)
+{
+ abort ();
+}
+
+int64_t
+do_inotify_add_watch (const char *path, int mask)
+{
+ abort ();
+}
+
+int
+do_inotify_rm_watch (int wd)
+{
+ abort ();
+}
+
+guestfs_int_inotify_event_list *
+do_inotify_read (void)
+{
+ abort ();
+}
+
+char **
+do_inotify_files (void)
+{
+ abort ();
+}
+
+#endif
diff --git a/daemon/mknod.c b/daemon/mknod.c
index d5b8467..6e678fd 100644
--- a/daemon/mknod.c
+++ b/daemon/mknod.c
@@ -37,18 +37,10 @@ optgroup_mknod_available (void)
{
return 1;
}
-#else
-int
-optgroup_mknod_available (void)
-{
- return 0;
-}
-#endif
int
do_mknod (int mode, int devmajor, int devminor, const char *path)
{
-#ifdef HAVE_MKNOD
int r;
if (mode < 0) {
@@ -66,9 +58,6 @@ do_mknod (int mode, int devmajor, int devminor, const char *path)
}
return 0;
-#else
- NOT_AVAILABLE (-1);
-#endif
}
int
@@ -88,3 +77,37 @@ do_mknod_c (int mode, int devmajor, int devminor, const char *path)
{
return do_mknod (mode | S_IFCHR, devmajor, devminor, path);
}
+
+#else
+
+int
+optgroup_mknod_available (void)
+{
+ return 0;
+}
+
+int
+do_mknod (int mode, int devmajor, int devminor, const char *path)
+{
+ abort ();
+}
+
+int
+do_mkfifo (int mode, const char *path)
+{
+ abort ();
+}
+
+int
+do_mknod_b (int mode, int devmajor, int devminor, const char *path)
+{
+ abort ();
+}
+
+int
+do_mknod_c (int mode, int devmajor, int devminor, const char *path)
+{
+ abort ();
+}
+
+#endif
diff --git a/daemon/realpath.c b/daemon/realpath.c
index 8ec9674..126ef19 100644
--- a/daemon/realpath.c
+++ b/daemon/realpath.c
@@ -36,20 +36,17 @@
#define NAME_MAX FILENAME_MAX
#endif
+#ifdef HAVE_REALPATH
+
int
optgroup_realpath_available (void)
{
-#ifdef HAVE_REALPATH
return 1;
-#else
- return 0;
-#endif
}
char *
do_realpath (const char *path)
{
-#ifdef HAVE_REALPATH
char *ret;
CHROOT_IN;
@@ -61,11 +58,24 @@ do_realpath (const char *path)
}
return ret; /* caller frees */
-#else
- NOT_AVAILABLE (NULL);
-#endif
}
+#else /* !HAVE_REALPATH */
+
+int
+optgroup_realpath_available (void)
+{
+ return 0;
+}
+
+char *
+do_realpath (const char *path)
+{
+ abort ();
+}
+
+#endif /* !HAVE_REALPATH */
+
static int find_path_element (int fd_cwd, char *name, size_t *name_len_ret);
char *
diff --git a/daemon/selinux.c b/daemon/selinux.c
index 2db05ee..40590e1 100644
--- a/daemon/selinux.c
+++ b/daemon/selinux.c
@@ -32,18 +32,12 @@
#include "optgroups.h"
#if defined(HAVE_LIBSELINUX)
+
int
optgroup_selinux_available (void)
{
return 1;
}
-#else /* !HAVE_LIBSELINUX */
-int
-optgroup_selinux_available (void)
-{
- return 0;
-}
-#endif /* !HAVE_LIBSELINUX */
/* setcon is only valid under the following circumstances:
* - single threaded
@@ -52,7 +46,7 @@ optgroup_selinux_available (void)
int
do_setcon (const char *context)
{
-#if defined(HAVE_LIBSELINUX) && defined(HAVE_SETCON)
+#if defined(HAVE_SETCON)
if (setcon ((char *) context) == -1) {
reply_with_perror ("setcon");
return -1;
@@ -60,14 +54,15 @@ do_setcon (const char *context)
return 0;
#else
- NOT_AVAILABLE (-1);
+ reply_with_error ("function not available");
+ return -1;
#endif
}
char *
do_getcon (void)
{
-#if defined(HAVE_LIBSELINUX) && defined(HAVE_GETCON)
+#if defined(HAVE_GETCON)
security_context_t context;
char *r;
@@ -85,6 +80,29 @@ do_getcon (void)
return r; /* caller frees */
#else
- NOT_AVAILABLE (NULL);
+ reply_with_error ("function not available");
+ return NULL;
#endif
}
+
+#else /* !HAVE_LIBSELINUX */
+
+int
+optgroup_selinux_available (void)
+{
+ return 0;
+}
+
+int
+do_setcon (const char *context)
+{
+ abort ();
+}
+
+char *
+do_getcon (void)
+{
+ abort ();
+}
+
+#endif /* !HAVE_LIBSELINUX */
diff --git a/daemon/xattr.c b/daemon/xattr.c
index 2445748..92d0cf1 100644
--- a/daemon/xattr.c
+++ b/daemon/xattr.c
@@ -511,6 +511,11 @@ do_lgetxattr (const char *path, const char *name, size_t *size_r)
}
#else /* no xattr.h */
+
+/* Note that the wrapper code (daemon/stubs.c) ensures that the
+ * functions below are never called because
+ * optgroup_linuxxattrs_available returns false.
+ */
int
optgroup_linuxxattrs_available (void)
{
@@ -520,55 +525,55 @@ optgroup_linuxxattrs_available (void)
guestfs_int_xattr_list *
do_getxattrs (const char *path)
{
- NOT_AVAILABLE (NULL);
+ abort ();
}
guestfs_int_xattr_list *
do_lgetxattrs (const char *path)
{
- NOT_AVAILABLE (NULL);
+ abort ();
}
int
do_setxattr (const char *xattr, const char *val, int vallen, const char *path)
{
- NOT_AVAILABLE (-1);
+ abort ();
}
int
do_lsetxattr (const char *xattr, const char *val, int vallen, const char *path)
{
- NOT_AVAILABLE (-1);
+ abort ();
}
int
do_removexattr (const char *xattr, const char *path)
{
- NOT_AVAILABLE (-1);
+ abort ();
}
int
do_lremovexattr (const char *xattr, const char *path)
{
- NOT_AVAILABLE (-1);
+ abort ();
}
guestfs_int_xattr_list *
do_lxattrlist (const char *path, char *const *names)
{
- NOT_AVAILABLE (NULL);
+ abort ();
}
char *
do_getxattr (const char *path, const char *name, size_t *size_r)
{
- NOT_AVAILABLE (NULL);
+ abort ();
}
char *
do_lgetxattr (const char *path, const char *name, size_t *size_r)
{
- NOT_AVAILABLE (NULL);
+ abort ();
}
#endif /* no xattr.h */
diff --git a/generator/generator_daemon.ml b/generator/generator_daemon.ml
index 88cc8bf..0eb2446 100644
--- a/generator/generator_daemon.ml
+++ b/generator/generator_daemon.ml
@@ -75,12 +75,14 @@ and generate_daemon_actions () =
pr "#include \"c-ctype.h\"\n";
pr "#include \"guestfs_protocol.h\"\n";
pr "#include \"actions.h\"\n";
+ pr "#include \"optgroups.h\"\n";
pr "\n";
List.iter (
- fun (name, (ret, args, optargs), _, _, _, _, _) ->
+ fun (name, (ret, args, optargs), _, flags, _, _, _) ->
(* Generate server-side stubs. *)
- pr "static void %s_stub (XDR *xdr_in)\n" name;
+ pr "static void\n";
+ pr "%s_stub (XDR *xdr_in)\n" name;
pr "{\n";
(match ret with
| RErr | RInt _ -> pr " int r;\n"
@@ -122,6 +124,24 @@ and generate_daemon_actions () =
let is_filein =
List.exists (function FileIn _ -> true | _ -> false) args in
+ (* Reject Optional functions that are not available (RHBZ#679737). *)
+ List.iter (
+ function
+ | Optional group ->
+ pr " /* The caller should have checked before calling this. */\n";
+ pr " if (! optgroup_%s_available ()) {\n" group;
+ if is_filein then
+ pr " cancel_receive ();\n";
+ pr " reply_with_error (\"feature '%%s' is not available in
this\\n\"\n";
+ pr " \"build of libguestfs. Read
'AVAILABILITY' in the guestfs(3) man page for\\n\"\n";
+ pr " \"how to check for the availability of
features.\",\n";
+ pr " \"%s\");\n" group;
+ pr " goto done;\n";
+ pr " }\n";
+ pr "\n"
+ | _ -> ()
+ ) flags;
+
(* Reject unknown optional arguments.
* Note this code is included even for calls with no optional
* args because the caller must not pass optargs_bitmask != 0
--
1.7.6