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
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
