9:56 a.m.
Refactor the internal_feature_available to return the result for just
one group, so it is easier to know on the library side what was the
actual error, and which group refers to; drop internal_available, as no
more needed after this.
On the library side, implement in available and feature_available the
real logic to iterate through the requested group, and error out or
return whether the groups are available. This also introduces caching
for the features, so each needs to be queried just once in each
appliance run.
The result of this is that there should be much less communication with
the daemon to know about available features; the downside is that
queries for more than one group at once, not already cached, will be
cause more queries to the daemon.
---
daemon/available.c | 51 ++++++----------------------------
generator/actions.ml | 13 ++-------
src/MAX_PROC_NR | 2 +-
src/available.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++--
src/guestfs-internal.h | 10 +++++++
src/handle.c | 7 +++++
6 files changed, 101 insertions(+), 56 deletions(-)
diff --git a/daemon/available.c b/daemon/available.c
index 6409b90..d50c737 100644
--- a/daemon/available.c
+++ b/daemon/available.c
@@ -33,53 +33,20 @@
GUESTFSD_EXT_CMD(str_grep, grep);
GUESTFSD_EXT_CMD(str_modprobe, modprobe);
-static int
-available (char *const *groups, int error_on_unavailable)
+int
+do_internal_feature_available (const char *group)
{
- int av;
- size_t i, j;
-
- for (i = 0; groups[i] != NULL; ++i) {
- for (j = 0; optgroups[j].group != NULL; ++j) {
- if (STREQ (groups[i], optgroups[j].group)) {
- av = optgroups[j].available ();
- if (!av) {
- if (error_on_unavailable) {
- reply_with_error ("%s: group not available", optgroups[j].group);
- return -1;
- }
- else
- return 0;
- }
- break; /* out of for (j) loop */
- }
- }
+ size_t i;
- /* Unknown group? */
- if (optgroups[j].group == NULL) {
- reply_with_error ("%s: unknown group", groups[i]);
- return -1;
+ for (i = 0; optgroups[i].group != NULL; ++i) {
+ if (STREQ (group, optgroups[i].group)) {
+ int av = optgroups[i].available ();
+ return av ? 0 : 1;
}
}
- /* All specified groups available. */
- return 1;
-}
-
-int
-do_internal_feature_available (char *const *groups)
-{
- return available (groups, 0);
-}
-
-int
-do_internal_available (char *const *groups)
-{
- if (available (groups, 1) == -1)
- return -1;
-
- /* No error, so all groups available, just returns no error. */
- return 0;
+ /* Unknown group */
+ return 2;
}
char **
diff --git a/generator/actions.ml b/generator/actions.ml
index 6348c88..7ab8ee4 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12779,18 +12779,9 @@ this will fail and set errno as ENOTSUP.
See also L<ntfsresize(8)>, L<resize2fs(8)>, L<btrfs(8)>,
L<xfs_info(8)>." };
{ defaults with
- name = "internal_available"; added = (1, 31, 25);
- style = RErr, [StringList "groups"], [];
- proc_nr = Some 458;
- visibility = VInternal;
- shortdesc = "test availability of some parts of the API";
- longdesc = "\
-This is the internal call which implements C<guestfs_available>." };
-
- { defaults with
name = "internal_feature_available"; added = (1, 31, 25);
- style = RBool "isavailable", [StringList "groups"], [];
- proc_nr = Some 459;
+ style = RInt "result", [String "group"], [];
+ proc_nr = Some 458;
visibility = VInternal;
shortdesc = "test availability of some parts of the API";
longdesc = "\
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 658bd78..c92ddb6 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-459
+458
diff --git a/src/available.c b/src/available.c
index 6738449..4bdd173 100644
--- a/src/available.c
+++ b/src/available.c
@@ -18,17 +18,87 @@
#include <config.h>
+#include <string.h>
+#include <libintl.h>
+
#include "guestfs.h"
+#include "guestfs-internal.h"
#include "guestfs-internal-actions.h"
+static struct cached_feature *
+find_or_cache_feature (guestfs_h *g, const char *group)
+{
+ struct cached_feature *f;
+ size_t i;
+ int res;
+
+ for (i = 0; i < g->nr_features; ++i) {
+ f = &g->features[i];
+
+ if (STRNEQ (f->group, group))
+ continue;
+
+ return f;
+ }
+
+ res = guestfs_internal_feature_available (g, group);
+ if (res < 0)
+ return 0; /* internal_feature_available sent an error. */
+
+ g->features = safe_realloc (g, g->features,
+ (g->nr_features+1) * sizeof (struct cached_feature));
+ f = &g->features[g->nr_features];
+ ++g->nr_features;
+ f->group = safe_strdup (g, group);
+ f->result = res;
+
+ return f;
+}
+
int
guestfs_impl_available (guestfs_h *g, char *const *groups)
{
- return guestfs_internal_available (g, groups);
+ char *const *ptr;
+
+ for (ptr = groups; *ptr != NULL; ++ptr) {
+ const char *group = *ptr;
+ struct cached_feature *f = find_or_cache_feature (g, group);
+
+ if (f == NULL)
+ return -1;
+
+ if (f->result == 2) {
+ error (g, _("%s: unknown group"), group);
+ return -1;
+ } else if (f->result == 1) {
+ error (g, _("%s: group not available"), group);
+ return -1;
+ }
+ }
+
+ return 0;
}
int
guestfs_impl_feature_available (guestfs_h *g, char *const *groups)
{
- return guestfs_internal_feature_available (g, groups);
+ char *const *ptr;
+
+ for (ptr = groups; *ptr != NULL; ++ptr) {
+ const char *group = *ptr;
+ struct cached_feature *f = find_or_cache_feature (g, group);
+
+ if (f == NULL)
+ return -1;
+
+ if (f->result == 2) {
+ error (g, _("%s: unknown group"), group);
+ return -1;
+ } else if (f->result == 1) {
+ return 0;
+ }
+ }
+
+ /* All specified groups available. */
+ return 1;
}
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 49da6fe..bc03ccc 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -360,6 +360,12 @@ struct error_cb_stack {
void * error_cb_data;
};
+/* Cached queried features. */
+struct cached_feature {
+ char *group;
+ int result;
+};
+
/* The libguestfs handle. */
struct guestfs_h
{
@@ -502,6 +508,10 @@ struct guestfs_h
unsigned int nr_requested_credentials;
virConnectCredentialPtr requested_credentials;
#endif
+
+ /* Cached features. */
+ struct cached_feature *features;
+ size_t nr_features;
};
/* Per-filesystem data stored for inspect_os. */
diff --git a/src/handle.c b/src/handle.c
index bd66717..f7b0909 100644
--- a/src/handle.c
+++ b/src/handle.c
@@ -422,6 +422,7 @@ static int
shutdown_backend (guestfs_h *g, int check_for_errors)
{
int ret = 0;
+ size_t i;
if (g->state == CONFIG)
return 0;
@@ -444,6 +445,12 @@ shutdown_backend (guestfs_h *g, int check_for_errors)
guestfs_int_free_drives (g);
+ for (i = 0; i < g->nr_features; ++i)
+ free (g->features[i].group);
+ free (g->features);
+ g->features = NULL;
+ g->nr_features = 0;
+
g->state = CONFIG;
return ret;
--
2.1.0