On Tuesday, 5 September 2017 16:55:08 CEST Richard W.M. Jones wrote: > Since qemu 2.10, qemu locks all drives even when they are opened > readonly. This disables this locking in this safe case. > > Incomplete fix for > https://bugzilla.redhat.com/show_bug.cgi?id=1417306 > --- I guess that, according to the other message in this thread, the commit message will be amended to use RHBZ# in the subject, and drop the last paragraph? > lib/launch-direct.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/lib/launch-direct.c b/lib/launch-direct.c > index 3b848165c..e5465539d 100644 > --- a/lib/launch-direct.c > +++ b/lib/launch-direct.c > @@ -221,8 +221,11 @@ add_drive_standard_params (guestfs_h *g, struct backend_direct_data *data, > file = guestfs_int_drive_source_qemu_param (g, &drv->src); > append_list_format ("file=%s", file); > > - if (drv->readonly) > + if (drv->readonly) { > append_list ("snapshot=on"); > + if (guestfs_int_version_ge (&data->qemu_version, 2, 10, 0)) > + append_list ("locking=off"); > + } > append_list_format ("cache=%s", > drv->cachemode ? drv->cachemode : "writeback"); LGTM. IMHO locking=off ought to be used also for the appliance disk, otherwise starting two or more libguestfs instances for the same user will fail, no? Even if not, disabling the locking should still be good, as in not making qemu do more work than needed.

On Tue, Sep 05, 2017 at 03:55:08PM +0100, Richard W.M. Jones wrote: > Since qemu 2.10, qemu locks all drives even when they are opened > readonly. This disables this locking in this safe case. > > Incomplete fix for > https://bugzilla.redhat.com/show_bug.cgi?id=1417306 > --- > lib/launch-direct.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/lib/launch-direct.c b/lib/launch-direct.c > index 3b848165c..e5465539d 100644 > --- a/lib/launch-direct.c > +++ b/lib/launch-direct.c > @@ -221,8 +221,11 @@ add_drive_standard_params (guestfs_h *g, struct backend_direct_data *data, > file = guestfs_int_drive_source_qemu_param (g, &drv->src); > append_list_format ("file=%s", file); > > - if (drv->readonly) > + if (drv->readonly) { > append_list ("snapshot=on"); > + if (guestfs_int_version_ge (&data->qemu_version, 2, 10, 0)) > + append_list ("locking=off"); Don't we have a way to enumerate supported options?

Depending on QEMU version is not going to play nice with vendor-shipped QEMU where features are often backported. > + } > append_list_format ("cache=%s", > drv->cachemode ? drv->cachemode : "writeback"); > if (drv->src.format) Roman.