On Thursday 12 May 2016 22:26:25 Richard W.M. Jones wrote:
qemu feature detection takes about 95ms on my laptop. The overhead is almost all due to the time taken by the glibc link loader opening the 170+ libraries that qemu is linked to (×2 because we need to run qemu twice).
Fixing that is seriously hard work.
Therefore memoize the results of guestfs_int_test_qemu.
This is keyed on the size and mtime of the qemu binary, so if the user changes the qemu binary (eg. setting LIBGUESTFS_HV) we discard the memoized result and rerun the qemu commands. There is also a generation number so we can bump the generation in future versions of libguestfs to invalidate all previously cached data.
The memo is stored in the supermin cache directory (eg. /var/tmp/.guestfs-*) in the files:
qemu.stat Result of 'stat(2)' of the qemu binary qemu.help qemu -help output qemu.devices qemu -devices ? output
Note these files are only stored when using the 'direct' backend. For the libvirt backend, libvirt itself memoizes this data in its own place. ---
The idea looks ok, just one note below.
src/qemu.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 146 insertions(+), 8 deletions(-)
diff --git a/src/qemu.c b/src/qemu.c index feb8bd6..259a6fd 100644 --- a/src/qemu.c +++ b/src/qemu.c @@ -58,23 +58,163 @@ struct qemu_data { guestfs_int_qemu_supports_virtio_scsi */ };
+static int test_qemu (guestfs_h *g, struct qemu_data *data); static void parse_qemu_version (guestfs_h *g, struct qemu_data *data); static void read_all (guestfs_h *g, void *retv, const char *buf, size_t len);
+/* This is saved in the qemu.stat file, so if we decide to change the + * test_qemu memoization format/data in future, we should increment + * this to discard any memoized data cached by previous versions of + * libguestfs. + */ +#define MEMO_GENERATION 1 + /** * Test qemu binary (or wrapper) runs, and do C<qemu -help> so we know * the version of qemu what options this qemu supports, and * C<qemu -device ?> so we know what devices are available. + * + * This caches the results in the cachedir so that as long as the qemu + * binary does not change, calling this is effectively free. */ struct qemu_data * guestfs_int_test_qemu (guestfs_h *g) { + struct qemu_data *data; + struct stat statbuf; + CLEANUP_FREE char *cachedir = NULL, *qemu_stat_filename = NULL, + *qemu_help_filename = NULL, *qemu_devices_filename = NULL; + FILE *fp; + int generation; + uint64_t prev_size, prev_mtime; + + if (stat (g->hv, &statbuf) == -1) { + perrorf (g, "stat: %s", g->hv); + return NULL; + } + + cachedir = guestfs_int_lazy_make_supermin_appliance_dir (g); + if (cachedir == NULL) + return NULL; + + qemu_stat_filename = safe_asprintf (g, "%s/qemu.stat", cachedir); + qemu_help_filename = safe_asprintf (g, "%s/qemu.help", cachedir); + qemu_devices_filename = safe_asprintf (g, "%s/qemu.devices", cachedir);
Instead of writing the generation number to the stat file, what about doing a simplier approach, i.e. add it to the filenames: - $cachedir/qemu.$generation.stat - $cachedir/qemu.$generation.help - etc Less data to read, fopen will just open the data for the right generation. -- Pino Toscano