[Libguestfs] [PATCH v2] lib: Use qemu-img info -U option to avoid locking error.

[Richard W.M. Jones]

On Wed, Sep 26, 2018 at 06:36:47PM +0200, Pino Toscano wrote:
> On Friday, 21 September 2018 11:53:52 CEST Richard W.M. Jones wrote:
> > +/**
> > + * Test if the qemu-img info command supports the C<-U> option to
> > + * disable locking.  The result is memoized in the handle.
> > + *
> > + * Note this option was added in qemu 2.11.  We can remove this test
> > + * when we can assume everyone is using qemu >= 2.11.
> > + */
> > +static int
> > +qemu_img_supports_U_option (guestfs_h *g)
> > +{
> > +  if (g->qemu_img_supports_U_option >= 0)
> > +    return g->qemu_img_supports_U_option;
> > +
> > +  CLEANUP_CMD_CLOSE struct command *cmd = guestfs_int_new_command (g);
> > +  int r;
> > +
> > +  guestfs_int_cmd_add_string_unquoted (cmd,
> > +                                       "qemu-img info --help | "
> > +                                       "grep -sq -- 'info.*-U'");
> 
> This may match something else in future, in case some other command of
> qemu-img gets another option with "info" in it...
> 
> TBH this is something we have already, and precisely in the qemu_data
> struct, which is memoized.  One downside is that using it would mean
> memoizing the qemu help/schema in advance, even if the appliance is not
> started; so code like `guestfish disk-format foo` will be slower.
> OTOH, the upside is that there is no need to "reprobe" qemu-img for
> something that we detect already from the QMP schema.

So I had a look into this.  I guess you mean specifically the result
of ‘guestfs_int_qemu_mandatory_locking’ which queries the QMP schema.
This is a reasonable proxy for presence of the qemu-img -U option
since both features were added within a few months.

However the problem is that testing for this requires us to run qemu
before launch is called.  That has API issues because it's unclear
what code like:

  g = guestfs_create ();
  guestfs_disk_format (g, "disk.img");
  guestfs_set_hv (g, "my-weird-qemu");
  guestfs_launch (g);

should do (granted, it is a peculiar corner case and the caller
probably gets what they deserve).

> One possible idea can be to cache the qemu_data struct in the handle,
> so the direct backend would not get a performance regression (since
> either one of the info commands already preloaded a qemu_data, or
> launch would do that).

I'll see what a patch would look like ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top