On Thu, Nov 23, 2017 at 02:05:32PM +0000, Richard W.M. Jones wrote:> On Thu, Nov 23, 2017 at 03:00:45PM +0200, Yaniv Kaul wrote: > > On Thu, Nov 23, 2017 at 10:57 AM, Richard W.M. Jones <rjones@redhat.com> > > wrote: > > > > > On Tue, Nov 21, 2017 at 11:43:54PM +0200, Yaniv Kaul wrote: > > > > Since I upgrading to FC27, I *sometimes* fail to virt-sysprep. > > > > The debug messages: > > > > libguestfs: trace: set_verbose true > > > > libguestfs: trace: set_verbose = 0 > > > > libguestfs: create: flags = 0, handle = 0x7f4600005dd0, program = python2 > > > > libguestfs: trace: set_program "lago" > > > > libguestfs: trace: set_program = 0 > > > > libguestfs: trace: add_drive_ro > > > > "/home/ykaul/ovirt-system-tests/deployment-basic-suite- > > > master/default/images/lago-basic-suite-master-host-0_root.qcow2" > > > > libguestfs: trace: add_drive > > > > "/home/ykaul/ovirt-system-tests/deployment-basic-suite- > > > master/default/images/lago-basic-suite-master-host-0_root.qcow2" > > > > "readonly:true" > > > > libguestfs: creating COW overlay to protect original drive content > > > > libguestfs: trace: disk_format > > > > "/home/ykaul/ovirt-system-tests/deployment-basic-suite- > > > master/default/images/lago-basic-suite-master-host-0_root.qcow2" > > > > libguestfs: command: run: qemu-img > > > > libguestfs: command: run: \ info > > > > libguestfs: command: run: \ --output json > > > > libguestfs: command: run: \ /dev/fd/7 > > > > qemu-img: Could not open '/dev/fd/7': Failed to get shared "write" lock > > > > Is another process using the image? > > Looking at this a bit closer, I think this may be a bug in qemu. > > /dev/fd/7 is supposed to be the file descriptor of the image which we > have opened in libguestfs, see this code: > > https://github.com/libguestfs/libguestfs/blob/06df910491c49360c0292c7153ba5e5cd09a4735/lib/info.c#L174-L191 > > I wonder if qemu gets confused by this and thinks that the image is > open in two places? > > I can't reproduce this here however. Having a nice short reproducer > might help.This bug has now been reported by a Debian user: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=884110 Yaniv, did you file an upstream bug report and/or get a reliable reproducer? I cannot reproduce this at all. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
On 12/12/2017 01:13 PM, Richard W.M. Jones wrote:> On Thu, Nov 23, 2017 at 02:05:32PM +0000, Richard W.M. Jones wrote: >> On Thu, Nov 23, 2017 at 03:00:45PM +0200, Yaniv Kaul wrote: >>> On Thu, Nov 23, 2017 at 10:57 AM, Richard W.M. Jones <rjones@redhat.com> >>> wrote: >>> >>>> On Tue, Nov 21, 2017 at 11:43:54PM +0200, Yaniv Kaul wrote: >>>>> Since I upgrading to FC27, I *sometimes* fail to virt-sysprep. >>>>> The debug messages: >>>>> libguestfs: trace: set_verbose true >>>>> libguestfs: trace: set_verbose = 0 >>>>> libguestfs: create: flags = 0, handle = 0x7f4600005dd0, program = python2 >>>>> libguestfs: trace: set_program "lago" >>>>> libguestfs: trace: set_program = 0 >>>>> libguestfs: trace: add_drive_ro >>>>> "/home/ykaul/ovirt-system-tests/deployment-basic-suite- >>>> master/default/images/lago-basic-suite-master-host-0_root.qcow2" >>>>> libguestfs: trace: add_drive >>>>> "/home/ykaul/ovirt-system-tests/deployment-basic-suite- >>>> master/default/images/lago-basic-suite-master-host-0_root.qcow2" >>>>> "readonly:true" >>>>> libguestfs: creating COW overlay to protect original drive content >>>>> libguestfs: trace: disk_format >>>>> "/home/ykaul/ovirt-system-tests/deployment-basic-suite- >>>> master/default/images/lago-basic-suite-master-host-0_root.qcow2" >>>>> libguestfs: command: run: qemu-img >>>>> libguestfs: command: run: \ info >>>>> libguestfs: command: run: \ --output json >>>>> libguestfs: command: run: \ /dev/fd/7 >>>>> qemu-img: Could not open '/dev/fd/7': Failed to get shared "write" lock >>>>> Is another process using the image? >> >> Looking at this a bit closer, I think this may be a bug in qemu. >> >> /dev/fd/7 is supposed to be the file descriptor of the image which we >> have opened in libguestfs, see this code: >> >> https://github.com/libguestfs/libguestfs/blob/06df910491c49360c0292c7153ba5e5cd09a4735/lib/info.c#L174-L191 >> >> I wonder if qemu gets confused by this and thinks that the image is >> open in two places? >> >> I can't reproduce this here however. Having a nice short reproducer >> might help. > > This bug has now been reported by a Debian user: > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=884110Hi Rich Have you tried the reproducer from the bug report ? Using a Debian Testing system with qemu-img 2.10.1 and libguestfs 1.36.11-1 packages I can reproduce it 100 %.
On Tue, Dec 12, 2017 at 2:13 PM, Richard W.M. Jones <rjones@redhat.com> wrote:> On Thu, Nov 23, 2017 at 02:05:32PM +0000, Richard W.M. Jones wrote: > > On Thu, Nov 23, 2017 at 03:00:45PM +0200, Yaniv Kaul wrote: > > > On Thu, Nov 23, 2017 at 10:57 AM, Richard W.M. Jones < > rjones@redhat.com> > > > wrote: > > > > > > > On Tue, Nov 21, 2017 at 11:43:54PM +0200, Yaniv Kaul wrote: > > > > > Since I upgrading to FC27, I *sometimes* fail to virt-sysprep. > > > > > The debug messages: > > > > > libguestfs: trace: set_verbose true > > > > > libguestfs: trace: set_verbose = 0 > > > > > libguestfs: create: flags = 0, handle = 0x7f4600005dd0, program > python2 > > > > > libguestfs: trace: set_program "lago" > > > > > libguestfs: trace: set_program = 0 > > > > > libguestfs: trace: add_drive_ro > > > > > "/home/ykaul/ovirt-system-tests/deployment-basic-suite- > > > > master/default/images/lago-basic-suite-master-host-0_root.qcow2" > > > > > libguestfs: trace: add_drive > > > > > "/home/ykaul/ovirt-system-tests/deployment-basic-suite- > > > > master/default/images/lago-basic-suite-master-host-0_root.qcow2" > > > > > "readonly:true" > > > > > libguestfs: creating COW overlay to protect original drive content > > > > > libguestfs: trace: disk_format > > > > > "/home/ykaul/ovirt-system-tests/deployment-basic-suite- > > > > master/default/images/lago-basic-suite-master-host-0_root.qcow2" > > > > > libguestfs: command: run: qemu-img > > > > > libguestfs: command: run: \ info > > > > > libguestfs: command: run: \ --output json > > > > > libguestfs: command: run: \ /dev/fd/7 > > > > > qemu-img: Could not open '/dev/fd/7': Failed to get shared "write" > lock > > > > > Is another process using the image? > > > > Looking at this a bit closer, I think this may be a bug in qemu. > > > > /dev/fd/7 is supposed to be the file descriptor of the image which we > > have opened in libguestfs, see this code: > > > > https://github.com/libguestfs/libguestfs/blob/ > 06df910491c49360c0292c7153ba5e5cd09a4735/lib/info.c#L174-L191 > > > > I wonder if qemu gets confused by this and thinks that the image is > > open in two places? > > > > I can't reproduce this here however. Having a nice short reproducer > > might help. > > This bug has now been reported by a Debian user: > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=884110 > > Yaniv, did you file an upstream bug report and/or get a reliable > reproducer? I cannot reproduce this at all. >I have not, since I cannot reliably reproduce it (it happens ~ 1 of 5 runs or so). I wonder if there's additional debug information I can provide, though.> > Rich. > > -- > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~ > rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > libguestfs lets you edit virtual machines. Supports shell scripting, > bindings from many languages. http://libguestfs.org >
On Tue, Dec 12, 2017 at 04:56:15PM +0100, Emmanuel Kasper wrote:> > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=884110 > > Hi Rich > > Have you tried the reproducer from the bug report ? > Using a Debian Testing system with qemu-img 2.10.1 and libguestfs > 1.36.11-1 packages I can reproduce it 100 %.Yes I have now, and yes it does reproduce it reliably. I'm pretty sure this must be a qemu bug, but it may also be possible for us to rewrite the code in lib/info.c so that it doesn't do the strange /dev/fd stuff: https://github.com/libguestfs/libguestfs/blob/2c359583b3c556c2f52fa2f27bccd77338a01736/lib/info.c#L191 We did this for rather obscure reasons, see: https://github.com/libguestfs/libguestfs/commit/d50cb7bbb4cc18f69ea1425e9f5cee9685825f95 and I think it's better to undo this commit. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Well the patch is attached ... but ... it doesn't solve the problem at all: libguestfs: trace: disk_virtual_size "test1.vmdk" libguestfs: command: run: qemu-img libguestfs: command: run: \ info libguestfs: command: run: \ --output json libguestfs: command: run: \ test1.vmdk qemu-img: Could not open 'test1.vmdk': Failed to get shared "write" lock Is another process using the image? libguestfs: trace: disk_virtual_size = -1 (error) qemu-img info: test1.vmdk: qemu-img info exited with error status 1, see debug messages above at /var/tmp/test.pl line 17. I think the patch is probably a good thing (aside from not fixing the bug), so I've attached it for review. - - - Now that I've had time to look at the reported bug, I'm in two minds about whether this is really a bug. What's happening [in the Perl test case, not necessarily in Yaniv's case] is that libguestfs is opening the VMDK file for write, and at the same time ‘qemu-img info’ is opening the VMDK file for read. That's not necessarily a safe operation and qemu is preventing it. The VMDK format is ridiculously complicated so it's not really clear if this use case could be made safe in specific cases. For qcow2 this is also a difficult case (which could possibly be made better by allowing selective locking of parts of the header). TL;DR: Ask Kevin or Fam. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/ --HKEL+t8MFpg/ASTE Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="0001-lib-info-Remove-dev-fd-hacking-and-pass-a-true-filen.patch">From 19a236b9e71eea06425bfe252675299562fe7aca Mon Sep 17 00:00:00 2001From: "Richard W.M. Jones" <rjones@redhat.com> Date: Tue, 12 Dec 2017 21:06:01 +0000 Subject: [PATCH] lib/info: Remove /dev/fd hacking and pass a true filename to qemu-img info. It obscures what's really going on and is no longer necessary for the original purpose. This reverts commit d50cb7bbb4cc18f69ea1425e9f5cee9685825f95. See also: https://www.redhat.com/archives/libguestfs/2017-November/thread.html#00226 https://www.redhat.com/archives/libguestfs/2017-December/thread.html#00044 --- lib/info.c | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/lib/info.c b/lib/info.c index f7378adfd..f146dcbfc 100644 --- a/lib/info.c +++ b/lib/info.c @@ -24,7 +24,6 @@ #include <fcntl.h> #include <unistd.h> #include <sys/types.h> -#include <sys/stat.h> #include <sys/wait.h> #include <assert.h> #include <string.h> @@ -166,41 +165,18 @@ static yajl_val get_json_output (guestfs_h *g, const char *filename) { CLEANUP_CMD_CLOSE struct command *cmd = guestfs_int_new_command (g); - int fd, r; - char fdpath[64]; + int r; yajl_val tree = NULL; - struct stat statbuf; - - fd = open (filename, O_RDONLY /* NB: !O_CLOEXEC */); - if (fd == -1) { - perrorf (g, "disk info: %s", filename); - return NULL; - } - - if (fstat (fd, &statbuf) == -1) { - perrorf (g, "disk info: fstat: %s", filename); - close (fd); - return NULL; - } - if (S_ISDIR (statbuf.st_mode)) { - error (g, "disk info: %s is a directory", filename); - close (fd); - return NULL; - } - - snprintf (fdpath, sizeof fdpath, "/dev/fd/%d", fd); - guestfs_int_cmd_clear_close_files (cmd); guestfs_int_cmd_add_arg (cmd, "qemu-img"); guestfs_int_cmd_add_arg (cmd, "info"); guestfs_int_cmd_add_arg (cmd, "--output"); guestfs_int_cmd_add_arg (cmd, "json"); - guestfs_int_cmd_add_arg (cmd, fdpath); + guestfs_int_cmd_add_arg (cmd, filename); guestfs_int_cmd_set_stdout_callback (cmd, parse_json, &tree, CMD_STDOUT_FLAG_WHOLE_BUFFER); set_child_rlimits (cmd); r = guestfs_int_cmd_run (cmd); - close (fd); if (r == -1) return NULL; if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) { -- 2.15.1 --HKEL+t8MFpg/ASTE--