diff mbox

[03/28] libxl: Provide libxl__dm_support_*

Message ID 1450809903-3393-4-git-send-email-ian.jackson@eu.citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ian Jackson Dec. 22, 2015, 6:44 p.m. UTC
This allows code elsewhere in libxl to find out what options a device
model executable supports.  This is done by searching the usage
message for fixed strings.

Because this involves calling fork, callers must use the async
machinery.  To make this easier, and to avoid repeatedly invoking the
dm, we provide a cache within the libxl__ctx.

No call site yet.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
v6: Completely rewritten.
    Cache is now in ctx only, not in xenstore.
    We use the proper libxl__ev_child machinery for fork.
---
 tools/libxl/libxl.c          |    4 +
 tools/libxl/libxl_dm.c       |  231 ++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.h |   76 ++++++++++++++
 3 files changed, 311 insertions(+)

Comments

Ian Campbell Jan. 7, 2016, 5:13 p.m. UTC | #1
On Tue, 2015-12-22 at 18:44 +0000, Ian Jackson wrote:
> This allows code elsewhere in libxl to find out what options a device
> model executable supports.  This is done by searching the usage
> message for fixed strings.

Has anyone (ever, not necessarily a Xen person nor in this context)
approached upstream QEMU about a machine readable output of some sort?

I know libvirt does something similar to this, but they want to support
older versions, whereas we at least have the luxury of not caring about
versions before the point this code lands.

Ian.
Ian Jackson Jan. 8, 2016, 12:13 p.m. UTC | #2
Ian Campbell writes ("Re: [PATCH 03/28] libxl: Provide libxl__dm_support_*"):
> On Tue, 2015-12-22 at 18:44 +0000, Ian Jackson wrote:
> > This allows code elsewhere in libxl to find out what options a device
> > model executable supports.  This is done by searching the usage
> > message for fixed strings.
> 
> Has anyone (ever, not necessarily a Xen person nor in this context)
> approached upstream QEMU about a machine readable output of some sort?

Not as far as I'm aware.  Stefano, could you perhaps put out some
feelers ?

Ian.
Jim Fehlig Jan. 11, 2016, 5 p.m. UTC | #3
On 01/07/2016 10:13 AM, Ian Campbell wrote:
> On Tue, 2015-12-22 at 18:44 +0000, Ian Jackson wrote:
>> This allows code elsewhere in libxl to find out what options a device
>> model executable supports.  This is done by searching the usage
>> message for fixed strings.
> Has anyone (ever, not necessarily a Xen person nor in this context)
> approached upstream QEMU about a machine readable output of some sort?
>
> I know libvirt does something similar to this, but they want to support
> older versions, whereas we at least have the luxury of not caring about
> versions before the point this code lands.

Since qemu 1.2.0, libvirt has been using the various QMP commands to probe for
qemu capabilities, instead of parsing help output.

Regards,
Jim
Ian Campbell Jan. 14, 2016, 10:14 a.m. UTC | #4
On Mon, 2016-01-11 at 10:00 -0700, Jim Fehlig wrote:
> On 01/07/2016 10:13 AM, Ian Campbell wrote:
> > On Tue, 2015-12-22 at 18:44 +0000, Ian Jackson wrote:
> > > This allows code elsewhere in libxl to find out what options a device
> > > model executable supports.  This is done by searching the usage
> > > message for fixed strings.
> > Has anyone (ever, not necessarily a Xen person nor in this context)
> > approached upstream QEMU about a machine readable output of some sort?
> > 
> > I know libvirt does something similar to this, but they want to support
> > older versions, whereas we at least have the luxury of not caring about
> > versions before the point this code lands.
> 
> Since qemu 1.2.0, libvirt has been using the various QMP commands to probe for
> qemu capabilities, instead of parsing help output.

As in it spawns a qemu specifically to ask the questions and then kills it
and starts what it needs _or_ it starts the qemu with minimal command line
cfg and then dynamically pokes in the full config via qmp?

Ian.

> 
> Regards,
> Jim
>
Jim Fehlig Jan. 14, 2016, 6:31 p.m. UTC | #5
Ian Campbell wrote:
> On Mon, 2016-01-11 at 10:00 -0700, Jim Fehlig wrote:
>> On 01/07/2016 10:13 AM, Ian Campbell wrote:
>>> On Tue, 2015-12-22 at 18:44 +0000, Ian Jackson wrote:
>>>> This allows code elsewhere in libxl to find out what options a device
>>>> model executable supports.  This is done by searching the usage
>>>> message for fixed strings.
>>> Has anyone (ever, not necessarily a Xen person nor in this context)
>>> approached upstream QEMU about a machine readable output of some sort?
>>>
>>> I know libvirt does something similar to this, but they want to support
>>> older versions, whereas we at least have the luxury of not caring about
>>> versions before the point this code lands.
>> Since qemu 1.2.0, libvirt has been using the various QMP commands to probe for
>> qemu capabilities, instead of parsing help output.
> 
> As in it spawns a qemu specifically to ask the questions and then kills it
> and starts what it needs _or_ it starts the qemu with minimal command line
> cfg and then dynamically pokes in the full config via qmp?

The latter. When the qemu driver loads, it starts qemu, pokes it for
capabilities via qmp, caches what it finds, then terminates it. The cached
capabilities are used until the associated qemu binary is changed/updated.

If interested in peeking at the code, see virQEMUCapsInitQMP() and the functions
it calls in $libvirt_src/src/qemu/qemu_capabilities.c. E.g.

virQEMUCapsInitQMP()
  -> virQEMUCapsInitQMPMonitor()
    -> virQEMUCapsInitQMPBasic()

Regards,
Jim
Ian Campbell Jan. 15, 2016, 9:56 a.m. UTC | #6
On Thu, 2016-01-14 at 11:31 -0700, Jim Fehlig wrote:
> Ian Campbell wrote:
> > On Mon, 2016-01-11 at 10:00 -0700, Jim Fehlig wrote:
> > > On 01/07/2016 10:13 AM, Ian Campbell wrote:
> > > > On Tue, 2015-12-22 at 18:44 +0000, Ian Jackson wrote:
> > > > > This allows code elsewhere in libxl to find out what options a
> > > > > device
> > > > > model executable supports.  This is done by searching the usage
> > > > > message for fixed strings.
> > > > Has anyone (ever, not necessarily a Xen person nor in this context)
> > > > approached upstream QEMU about a machine readable output of some
> > > > sort?
> > > > 
> > > > I know libvirt does something similar to this, but they want to
> > > > support
> > > > older versions, whereas we at least have the luxury of not caring
> > > > about
> > > > versions before the point this code lands.
> > > Since qemu 1.2.0, libvirt has been using the various QMP commands to
> > > probe for
> > > qemu capabilities, instead of parsing help output.
> > 
> > As in it spawns a qemu specifically to ask the questions and then kills
> > it
> > and starts what it needs _or_ it starts the qemu with minimal command
> > line
> > cfg and then dynamically pokes in the full config via qmp?
> 
> The latter.

From the description I think you meant former?

>  When the qemu driver loads, it starts qemu, pokes it for
> capabilities via qmp, caches what it finds, then terminates it. The cached
> capabilities are used until the associated qemu binary is changed/updated.
> 
> If interested in peeking at the code, see virQEMUCapsInitQMP() and the functions
> it calls in $libvirt_src/src/qemu/qemu_capabilities.c. E.g.
> 
> virQEMUCapsInitQMP()
>   -> virQEMUCapsInitQMPMonitor()
>     -> virQEMUCapsInitQMPBasic()

Thanks.

When I spoke to Ian J yesterday we decided doing this properly (via QMP as
above) would be nice it was out of scope for this series for now. It would
make a nice future bit of cleanup though for sure.

Ian.

> 
> Regards,
> Jim
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
Jim Fehlig Jan. 15, 2016, 2:54 p.m. UTC | #7
On 01/15/2016 02:56 AM, Ian Campbell wrote:
> On Thu, 2016-01-14 at 11:31 -0700, Jim Fehlig wrote:
>> Ian Campbell wrote:
>>> On Mon, 2016-01-11 at 10:00 -0700, Jim Fehlig wrote:
>>>> On 01/07/2016 10:13 AM, Ian Campbell wrote:
>>>>> On Tue, 2015-12-22 at 18:44 +0000, Ian Jackson wrote:
>>>>>> This allows code elsewhere in libxl to find out what options a
>>>>>> device
>>>>>> model executable supports.  This is done by searching the usage
>>>>>> message for fixed strings.
>>>>> Has anyone (ever, not necessarily a Xen person nor in this context)
>>>>> approached upstream QEMU about a machine readable output of some
>>>>> sort?
>>>>>
>>>>> I know libvirt does something similar to this, but they want to
>>>>> support
>>>>> older versions, whereas we at least have the luxury of not caring
>>>>> about
>>>>> versions before the point this code lands.
>>>> Since qemu 1.2.0, libvirt has been using the various QMP commands to
>>>> probe for
>>>> qemu capabilities, instead of parsing help output.
>>> As in it spawns a qemu specifically to ask the questions and then kills
>>> it
>>> and starts what it needs _or_ it starts the qemu with minimal command
>>> line
>>> cfg and then dynamically pokes in the full config via qmp?
>> The latter.
> From the description I think you meant former?

Yes :-). Brain thought one way, fingers typed another...

Regards,
Jim

>
>>  When the qemu driver loads, it starts qemu, pokes it for
>> capabilities via qmp, caches what it finds, then terminates it. The cached
>> capabilities are used until the associated qemu binary is changed/updated.
>>
>> If interested in peeking at the code, see virQEMUCapsInitQMP() and the functions
>> it calls in $libvirt_src/src/qemu/qemu_capabilities.c. E.g.
>>
>> virQEMUCapsInitQMP()
>>   -> virQEMUCapsInitQMPMonitor()
>>     -> virQEMUCapsInitQMPBasic()
> Thanks.
>
> When I spoke to Ian J yesterday we decided doing this properly (via QMP as
> above) would be nice it was out of scope for this series for now. It would
> make a nice future bit of cleanup though for sure.
>
> Ian.
>
>> Regards,
>> Jim
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
diff mbox

Patch

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 9207621..d96189d 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -87,6 +87,8 @@  int libxl_ctx_alloc(libxl_ctx **pctx, int version,
     ctx->sigchld_selfpipe[1] = -1;
     libxl__ev_fd_init(&ctx->sigchld_selfpipe_efd);
 
+    LIBXL_LIST_INIT(&ctx->dm_support_cache);
+
     /* The mutex is special because we can't idempotently destroy it */
 
     if (libxl__init_recursive_mutex(ctx, &ctx->lock) < 0) {
@@ -208,6 +210,8 @@  int libxl_ctx_free(libxl_ctx *ctx)
     libxl__sigchld_notneeded(gc);
     libxl__pipe_close(ctx->sigchld_selfpipe);
 
+    libxl__dm_support_cache_destroy(gc);
+
     CTX_UNLOCK;
     pthread_mutex_destroy(&ctx->lock);
 
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 0aaefd9..675e859 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2169,6 +2169,237 @@  out:
     return ret;
 }
 
+/*----- libxl__dm_support_* -----*/
+
+/* cache */
+
+static libxl__dm_support_cached *
+dm_support_cache_lookup(libxl__gc *gc, const char *dm)
+{
+    libxl__dm_support_cached *search;
+
+    LIBXL_LIST_FOREACH(search, &CTX->dm_support_cache, entry)
+        if (!strcmp(search->dm, dm))
+            return search;
+
+    return 0;
+}
+
+_hidden void libxl__dm_support_cache_destroy(libxl__gc *gc)
+{
+    libxl__dm_support_cached *iter, *next;
+
+    LIBXL_LIST_FOREACH_SAFE(iter, &CTX->dm_support_cache, entry, next) {
+        free(iter->dm);
+        free(iter);
+    }
+}
+
+_hidden bool libxl__dm_supported(libxl__gc *gc, const char *dm,
+                                 libxl__dm_support_check__index opt)
+{
+    libxl__dm_support_cached *cached = dm_support_cache_lookup(gc, dm);
+    assert(cached);
+    return !!(cached->bitmap & (1UL << opt));
+}
+
+/* `check', the long-running operation to check for support */
+
+static void dm_support_check_immed_cb(libxl__egc *egc, libxl__ev_time *ev,
+                                        const struct timeval *requested_abs,
+                                        int rc);
+static void dm_support_check_child_cb(libxl__egc *egc, libxl__ev_child*,
+                                      pid_t pid, int status);
+static void dm_support_check_done(libxl__egc *gc,
+                                  libxl__dm_support_check_state *ch,
+                                  int rc);
+
+_hidden void libxl__dm_support_check_init(libxl__dm_support_check_state *ch)
+{
+    FILLZERO(*ch);
+    libxl__ev_child_init(&ch->child);
+    libxl__ev_time_init(&ch->immediate);
+}
+
+static void dm_support_check_free(libxl__gc *gc,
+                                  libxl__dm_support_check_state *ch)
+{
+    if (ch->helpmsg) fclose(ch->helpmsg);
+    assert(!libxl__ev_child_inuse(&ch->child));
+    libxl__ev_time_deregister(gc, &ch->immediate);
+}
+
+_hidden int libxl__dm_support_check_start(libxl__dm_support_check_state *ch)
+{
+    int r, rc;
+
+    /* convenience aliases */
+    const char *const dm = ch->dm;
+    libxl__ao *const ao = ch->ao;
+    AO_GC;
+
+    libxl__dm_support_check_init(ch);
+
+    r = stat(ch->dm, &ch->stab);
+    if (r<0) {
+        LOGE(ERROR, "device model %s is not accessible", dm);
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    libxl__dm_support_cached *cached = dm_support_cache_lookup(gc, dm);
+    if (cached &&
+        (ch->stab.st_mode  == cached->stab.st_mode  &&
+         ch->stab.st_dev   == cached->stab.st_dev   &&
+         ch->stab.st_ino   == cached->stab.st_ino   &&
+         ch->stab.st_mtime == cached->stab.st_mtime &&
+         ch->stab.st_ctime == cached->stab.st_ctime)) {
+
+        LOG(DEBUG, "returning cached support options for %s: %"PRIx32"",
+            dm, cached->bitmap);
+
+        rc = libxl__ev_time_register_rel(ao, &ch->immediate,
+                                         dm_support_check_immed_cb, 0);
+        if (rc) goto out;
+
+        /* we have queued a callback, our work in this function is done */
+        return 0;
+    }
+
+    ch->revalidate = cached;
+    if (cached) {
+        LOG(DEBUG, "revalidating cached support options for %s", dm);
+    }
+
+    ch->helpmsg = tmpfile();
+    if (!ch->helpmsg) {
+        LOGE(ERROR, "create tempfile for help message");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    pid_t pid = libxl__ev_child_fork(gc, &ch->child,
+                                     dm_support_check_child_cb);
+    if (pid < 0) {
+        LOGE(ERROR, "fork for %s -help", dm);
+        rc = pid;
+        goto out;
+    }
+
+    if (!pid) {
+        const char *const args[] = { dm, "--help", NULL };
+        int outfd = fileno(ch->helpmsg);
+        r = putenv("LC_ALL=C");
+        if (r) libxl__alloc_failed(CTX,__func__,1,0);
+        libxl__exec(gc, -1,outfd,-1, dm, (char**)args, NULL);
+    }
+    return 0;
+
+ out:
+    dm_support_check_free(gc,ch);
+    return rc;
+}
+
+static void dm_support_check_immed_cb(libxl__egc *egc, libxl__ev_time *ev,
+                                      const struct timeval *requested_abs,
+                                      int rc)
+{
+    libxl__dm_support_check_state *ch = CONTAINER_OF(ev, *ch, immediate);
+    dm_support_check_done(egc, ch, rc);
+}
+
+static void dm_support_check_lookfor(uint32_t *result_io,
+                                     const void *data, int datalen,
+                                     const char *str, int strlen,
+                                     int opt)
+{
+    if (memmem(data,datalen, str,strlen))
+        *result_io |= (1UL << opt);
+}
+
+void dm_support_check_child_cb(libxl__egc *egc, libxl__ev_child *child,
+                               pid_t pid, int status)
+{
+    libxl__dm_support_check_state *ch = CONTAINER_OF(child, *ch, child);
+    STATE_AO_GC(ch->ao);
+    int rc;
+
+    /* convenience aliases */
+    const char *const dm = ch->dm;
+
+    if (status) {
+        libxl_report_child_exitstatus(CTX, XTL_ERROR,
+                                      GCSPRINTF("%s -HELP", dm),
+                                      pid, status);
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    rewind(ch->helpmsg);
+
+    void *data;
+    int datalen;
+    rc = libxl_read_file_contents(CTX, "(temp file for dm help output)",
+                                  &data, &datalen);
+    if (rc) goto out;
+    libxl__ptr_add(gc, data);
+
+    if (!datalen) {
+        LOG(ERROR, "dm help output (from %s) was empty!", dm);
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    ((char*)data)[datalen-1] = 0; /* a null-terminated string new */
+
+    uint32_t result = 0;
+
+#define DM_SUPPORT_CHECK_ENTRY(name, string)                    \
+    dm_support_check_lookfor(&result,                           \
+                             data, datalen,                     \
+                             string, sizeof(string)-1,          \
+                             libxl__dm_support_check__##name);
+    DM_SUPPORT_CHECK_LIST
+#undef DM_SUPPORT_CHECK_ENTRY
+
+    libxl__dm_support_cached *cached = ch->revalidate;
+    if (cached) {
+        if (cached->bitmap == result)
+            LOG(DEBUG, "confirmed cached support options for %s: %"PRIx32"",
+                dm, result);
+        else
+            LOG(DEBUG, "updating cached support options for %s:"
+                " %"PRIx32" -> %"PRIx32"", dm, cached->bitmap, result);
+    }        
+    if (!cached) {
+        LOG(DEBUG, "multiple in-parallel checks for %s", dm);
+        cached = dm_support_cache_lookup(gc, dm);
+        /* if already there, take the one which finished last, willy-nilly */
+    }
+    if (!cached) {
+        LOG(DEBUG, "caching support options for %s: %"PRIx32"", dm, result);
+        cached = libxl__zalloc(NOGC, sizeof(*cached));
+        cached->dm = libxl__strdup(NOGC, dm);
+        LIBXL_LIST_INSERT_HEAD(&CTX->dm_support_cache, cached, entry);
+    }
+
+    cached->stab = ch->stab;
+    cached->bitmap = result;
+
+    rc = 0;
+
+ out:
+    dm_support_check_done(egc, ch, rc);
+}
+
+static void dm_support_check_done(libxl__egc *egc,
+                                  libxl__dm_support_check_state *ch,
+                                  int rc)
+{
+    STATE_AO_GC(ch->ao);
+    dm_support_check_free(gc, ch);
+    ch->callback(egc, ch, rc);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index e442996..f963ad6 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -181,6 +181,7 @@  typedef struct libxl__ao libxl__ao;
 typedef struct libxl__aop_occurred libxl__aop_occurred;
 typedef struct libxl__osevent_hook_nexus libxl__osevent_hook_nexus;
 typedef struct libxl__osevent_hook_nexi libxl__osevent_hook_nexi;
+typedef struct libxl__dm_support_cached libxl__dm_support_cached;
 
 _hidden void libxl__alloc_failed(libxl_ctx *, const char *func,
                          size_t nmemb, size_t size) __attribute__((noreturn));
@@ -398,6 +399,13 @@  struct libxl__poller {
     bool fds_changed;
 };
 
+struct libxl__dm_support_cached {
+    LIBXL_LIST_ENTRY(libxl__dm_support_cached) entry;
+    char *dm;
+    struct stat stab;
+    uint32_t bitmap;
+};
+
 struct libxl__gc {
     /* mini-GC */
     int alloc_maxsize; /* -1 means this is the dummy non-gc gc */
@@ -471,6 +479,8 @@  struct libxl__ctx {
     bool sigchld_user_registered;
     LIBXL_LIST_ENTRY(libxl_ctx) sigchld_users_entry;
 
+    LIBXL_LIST_HEAD(, libxl__dm_support_cached) dm_support_cache;
+
     libxl_version_info version_info;
 };
 
@@ -3198,6 +3208,72 @@  _hidden void libxl__bootloader_init(libxl__bootloader_state *bl);
  * If callback is passed rc==0, will have updated st->info appropriately */
 _hidden void libxl__bootloader_run(libxl__egc*, libxl__bootloader_state *st);
 
+/*----- Device model support check -----*/
+
+/*
+ * This can be used to determine whether a particular qemu supports
+ * a particular option.  (Strictly, whether a particular literal
+ * string appears in the help text.)
+ *
+ * Callers should:
+ *   1. Some time it is convenient:
+ *         Set up a libxl__dm_support_check_state
+ *         Invoke libxl__dm_support_check_start
+ *         Await the callback and bomb if it fails
+ *   2. Any time after that, perhaps repeatedly
+ *         Call libxl__dm_supported
+ */
+
+typedef struct libxl__dm_support_check_state libxl__dm_support_check_state;
+
+#define DM_SUPPORT_CHECK_LIST                                 \
+    DM_SUPPORT_CHECK_ENTRY(emulator_id, "emulator_id")        \
+    DM_SUPPORT_CHECK_ENTRY(xsrestrict,  "xsrestrict")
+
+typedef enum {
+#define DM_SUPPORT_CHECK_ENTRY(name, string) \
+    libxl__dm_support_check__##name,
+DM_SUPPORT_CHECK_LIST
+#undef DM_SUPPORT_CHECK_ENTRY
+    libxl__dm_support_check__max
+} libxl__dm_support_check__index;
+
+typedef struct libxl__dm_support_check_state
+  libxl__dm_support_check_state;
+
+/* Cannot be called reentrantly */
+typedef void libxl__dm_support_check_callback(libxl__egc *egc,
+    libxl__dm_support_check_state *checking, int rc);
+
+_hidden void libxl__dm_support_cache_destroy(libxl__gc *gc);
+
+struct libxl__dm_support_check_state {
+    /* caller must fill these in, and they must all remain valid */
+    libxl__ao *ao;
+    const char *dm;
+    libxl__dm_support_check_callback *callback;
+    /* private */
+    FILE *helpmsg;
+    struct stat stab;
+    libxl__ev_child child;
+    libxl__ev_time immediate;
+    libxl__dm_support_cached *revalidate; /* points into dm_support_cache */
+};
+
+_hidden void libxl__dm_support_check_init(
+    libxl__dm_support_check_state *checking);
+
+_hidden int libxl__dm_support_check_start(
+    libxl__dm_support_check_state *checking);
+
+/*
+ * Eg
+ *  libxl__dm_supported(gc, my_dm, libxl__dm_support_check__xsrestrict);
+ * (opt is from DM_SUPPORT_CHECK_LIST, above).
+ */
+_hidden bool libxl__dm_supported(libxl__gc *gc,
+    const char *dm, libxl__dm_support_check__index opt);
+
 /*----- Domain destruction -----*/
 
 /* Domain destruction has been split into two functions: