[Libguestfs] failure to virt-sysprep (FC27?)

[Richard W.M. Jones]

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/
-------------- next part --------------
>From 19a236b9e71eea06425bfe252675299562fe7aca Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at 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