Libguestfs - Aug 2020 - Re: [PATCH] appliance: extract UUID from QCOW2 disk image

On Wed, Aug 12, 2020 at 04:12:28PM +0300, Vladimir Sementsov-Ogievskiy
wrote:
> 12.08.2020 15:52, Vladimir Sementsov-Ogievskiy wrote:
> >12.08.2020 15:39, Andrey Shinkevich wrote:
> >>For the appliance of the QCOW2 format, get the UUID of the disk by
> >>reading the first 256k bytes with 'qemu-img dd' command.
Then pass the
> >>read block to the 'file' command. In case of failure, run
the 'file'
> >>command again directly.
> >>
> >>Suggested-by: Denis V. Lunev <den@openvz.org>
> >>Signed-off-by: Andrey Shinkevich
<andrey.shinkevich@virtuozzo.com>
> >>---
> >>  lib/appliance-kcmdline.c | 70
+++++++++++++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 69 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/lib/appliance-kcmdline.c b/lib/appliance-kcmdline.c
> >>index 211cc46..3bb8bcd 100644
> >>--- a/lib/appliance-kcmdline.c
> >>+++ b/lib/appliance-kcmdline.c
> >>@@ -71,7 +71,7 @@ read_uuid (guestfs_h *g, void *retv, const char
*line, size_t len)
> >>   * The L<file(1)> command does the hard work.
> >>   */
> >>  static char *
> >>-get_root_uuid (guestfs_h *g, const char *appliance)
> >>+do_get_root_uuid (guestfs_h *g, const char *appliance)
> >>  {
> >>    CLEANUP_CMD_CLOSE struct command *cmd = guestfs_int_new_command
(g);
> >>    char *ret = NULL;
> >>@@ -96,6 +96,74 @@ get_root_uuid (guestfs_h *g, const char
*appliance)
> >>  }
> >>  /**
> >>+ * Read the first 256k bytes of the in_file with
L<qemu-img(1)> command
> >>+ * and write them into the out_file. That may be useful to get
UUID of
> >>+ * the QCOW2 disk image with further L<file(1)> command.
> >>+ * The function returns zero if successful, otherwise -1.
> >>+ */
> >>+static int
> >>+run_qemu_img_dd (guestfs_h *g, const char *in_file, char
*out_file)
> >>+{
> >>+  CLEANUP_CMD_CLOSE struct command *cmd = guestfs_int_new_command
(g);
> >>+  int r;
> >>+
> >>+  guestfs_int_cmd_add_arg (cmd, "qemu-img");
> >>+  guestfs_int_cmd_add_arg (cmd, "dd");
> >>+  guestfs_int_cmd_add_arg_format (cmd, "if=%s",
in_file);
> >>+  guestfs_int_cmd_add_arg_format (cmd, "of=%s",
out_file);
> >>+  guestfs_int_cmd_add_arg (cmd, "bs=256k");
> >>+  guestfs_int_cmd_add_arg (cmd, "count=1");
> >>+
> >>+  r = guestfs_int_cmd_run (cmd);
> >>+  if (r == -1) {
> >>+    error (g, "Failed to run qemu-img");
> >>+    return -1;
> >>+  }
> >>+  if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) {
> >>+    guestfs_int_external_command_failed (g, r, "qemu-img
dd", NULL);
> >>+    return -1;
> >>+  }
> >>+
> >>+  return 0;
> >>+}
> >>+
> >>+/**
> >>+ * Get the UUID from the appliance disk image.
> >>+ */
> >>+static char *
> >>+get_root_uuid (guestfs_h *g, const char *appliance)
> >>+{
> >>+  char *UUID = NULL;
> >
> >Generally, don't use uppercase for local variables, I'd rename
it s/UUID/uuid/
> >
> >>+  int ret;
> >>+  char tmp_file[] = "/tmp/libguestfsXXXXXX";
> >>+
> >>+  if (!mktemp (tmp_file)) {
> >>+    error (g, "get_root_uuid: mktemp failed");
> >>+    return NULL;
> >
> >Hmm, if failed to create temp file, we still can try call
do_get_root_uuid on original appliance image.
> >
> 
> Another thought: with this patch, in case of raw appliance, we will do
copying for nothing. Probably better try file command on original image first,
and if failed, try with qemu-img dd.
Agreed.  Also because this path is critical for performance
especially when we launch the handle many times.

Rich.
> >>+  }
> >>+
> >>+  ret = run_qemu_img_dd (g, appliance, tmp_file);
> >>+  if (ret == 0) {
> >>+    UUID = do_get_root_uuid (g, tmp_file);
> >>+    if (UUID) {
> >>+      goto out;
> >>+    }
> >>+  }
> >>+
> >>+  /* We get here in case of failure to extract a UUID from the
temporary file.
> >>+   * Let us try to get the UUID from the appliance directly.
> >>+   */
> >>+  UUID = do_get_root_uuid (g, appliance);
> >>+  if (!UUID) {
> >>+    error (g, "Failed to get the appliance UUID");
> >>+  }
> >>+
> >>+out:
> >>+  unlink (tmp_file);
> >>+  return UUID;
> >>+}
> >>+
> >>+/**
> >>   * Construct the Linux command line passed to the appliance. 
This is
> >>   * used by the C<direct> and C<libvirt> backends, and
is simply
> >>   * located in this file because it's a convenient place for
this
> >>
> >
> >
> 
> 
> -- 
> Best regards,
> Vladimir
-- 
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