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