[PATCH 5/8] libxl: Share logic for finding path between qemuu and pygrub

[George Dunlap <george.dunlap@citrix.com>]

From: George Dunlap <george.dunlap@eu.citrix.com>

qemu can also access disks which will be provided with a qdisk backend
directly; add a flag to libxl__device_disk_find_local_path to indicate
whether to check for qdisk direct access.

Reorganize the qemuu disk argument code to make a clean separation
between finding a file to use (if any), and constructing the
parameters.

Only use qemu_disk_format_string() in circumstances where qemu may be
interpreting the disk (i.e., backend==QDISK).  In all other cases, it
should use RAW.

Share as much as possible between the is_cdrom path and the normal path.

Call libxl__device_disk_find_local_path() for most paths.  If we can't find
a local path, print an error and skip the disk, rather than using a bogus path.

This should fix two bugs:

* In the case of a block script, or a non-dom0 backend, qemuu will now
print a warning and skip the disk, rather than adding bogus
parameters that will cause qemuu to error out.

* You should now be able to use any backend for a cdrom that you can
use for a normal disk.  (Before it was limited to RAW files or things
that qemu could handle directly.)

I left the libxl__blktap_devpath in the qemuu-specific code rather than sharing
it with the pyrgub code because:

1) When the pygrub path runs the guest disks have not yet been set up

2) libxl__blktap_devpath() will give you the existing devpath if it
already exists, but will set one up for you if you don't.  So on the pygrub path,
this would end up setting up a new tap device.

3) There is no TAP-specific teardown code on the pygrub path, and I
don't want to add any (particularly since I'm hoping to remove tapdisk
altogether soon).

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
CC: Anthony Perard <anthony.perard@citrix.com>
CC: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl.c          | 17 ++++++++++--
 tools/libxl/libxl_dm.c       | 66 +++++++++++++++++++++++++++++---------------
 tools/libxl/libxl_internal.h |  8 ++++++
 3 files changed, 66 insertions(+), 25 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index a57568f..d6302a9 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3001,8 +3001,9 @@ static char * libxl__alloc_vdev(libxl__gc *gc, void *get_vdev_user,
 
 /* Callbacks */
 
-static char * libxl__device_disk_find_local_path(libxl__gc *gc, 
-                                                 const libxl_device_disk *disk) {
+char * libxl__device_disk_find_local_path(libxl__gc *gc, 
+                                          const libxl_device_disk *disk,
+                                          bool qdisk_direct) {
     char *path = NULL;
 
     /* No local paths for driver domains */
@@ -3023,6 +3024,16 @@ static char * libxl__device_disk_find_local_path(libxl__gc *gc,
         goto out;
     } 
 
+    /*
+     * If we're being called for a qemu path, we can pass the target
+     * string directly as well
+     */
+    if (qdisk_direct && disk->backend == LIBXL_DISK_BACKEND_QDISK ) {
+        path = libxl__strdup(gc, disk->pdev_path);
+        LOG(DEBUG, "Directly accessing local QDISK target %s", path);
+        goto out;
+    }
+
  out:
     return path;
 }
@@ -3047,7 +3058,7 @@ void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
 
     LOG(DEBUG, "Trying to find local path");
 
-    if ((dls->diskpath = libxl__device_disk_find_local_path(gc, in_disk))) {
+    if ((dls->diskpath = libxl__device_disk_find_local_path(gc, in_disk, false))) {
         LOG(DEBUG, "Local path found, executing callback.");
         dls->callback(egc, dls, 0);
     } else {
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 4aca38e..917ebbf 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1161,9 +1161,9 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
             int disk, part;
             int dev_number =
                 libxl__device_disk_dev_number(disks[i].vdev, &disk, &part);
-            const char *format = qemu_disk_format_string(disks[i].format);
+            const char *format;
             char *drive;
-            const char *pdev_path;
+            const char *target_path;
 
             if (dev_number == -1) {
                 LOG(WARN, "unable to determine"" disk number for %s",
@@ -1171,22 +1171,22 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                 continue;
             }
 
-            if (disks[i].is_cdrom) {
-                if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY)
-                    drive = libxl__sprintf
-                        (gc, "if=ide,index=%d,readonly=%s,media=cdrom,cache=writeback,id=ide-%i",
-                         disk, disks[i].readwrite ? "off" : "on", dev_number);
-                else
-                    drive = libxl__sprintf
-                        (gc, "file=%s,if=ide,index=%d,readonly=%s,media=cdrom,format=%s,cache=writeback,id=ide-%i",
-                         disks[i].pdev_path, disk, disks[i].readwrite ? "off" : "on", format, dev_number);
-            } else {
-                if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) {
+            /* 
+             * If qemu isn't doing the interpreting, the parameter is
+             * always raw
+             */
+            if (disks[i].backend == LIBXL_DISK_BACKEND_QDISK)
+                format = qemu_disk_format_string(disks[i].format);
+            else
+                format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
+
+            if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) {
+                if (!disks[i].is_cdrom) {
                     LOG(WARN, "cannot support"" empty disk format for %s",
                         disks[i].vdev);
                     continue;
                 }
-
+            } else {
                 if (format == NULL) {
                     LOG(WARN,
                         "unable to determine"" disk image format %s",
@@ -1194,14 +1194,36 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                     continue;
                 }
 
-                if (disks[i].backend == LIBXL_DISK_BACKEND_TAP) {
-                    format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
-                    pdev_path = libxl__blktap_devpath(gc, disks[i].pdev_path,
+                /* 
+                 * We can't call libxl__blktap_devpath from
+                 * libxl__device_disk_find_local_path for now because
+                 * the bootloader is called before the disks are set
+                 * up, so this function would set up a blktap node,
+                 * but there's no TAP tear-down on error conditions in
+                 * the bootloader path.
+                 */
+                if (disks[i].backend == LIBXL_DISK_BACKEND_TAP)
+                    target_path = libxl__blktap_devpath(gc, disks[i].pdev_path,
                                                       disks[i].format);
-                } else {
-                    pdev_path = disks[i].pdev_path;
+                else
+                    target_path = libxl__device_disk_find_local_path(gc, 
+                                                               &disks[i], true);
+
+                if (!target_path) {
+                    LOG(WARN, "No way to get local access disk to image: %s", disks[i].vdev);
+                    continue;
                 }
+            }
+
+            if (disks[i].is_cdrom) {
+                drive = libxl__sprintf
+                    (gc, "if=ide,index=%d,readonly=%s,media=cdrom,cache=writeback,id=ide-%i",
+                     disk, disks[i].readwrite ? "off" : "on", dev_number);
 
+                if (target_path)
+                    drive = libxl__sprintf(gc, "%s,file=%s,format=%s",
+                                           drive, target_path, format);
+            } else {
                 /*
                  * Explicit sd disks are passed through as is.
                  *
@@ -1211,7 +1233,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                 if (strncmp(disks[i].vdev, "sd", 2) == 0) {
                     drive = libxl__sprintf
                         (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,readonly=%s,cache=writeback",
-                         pdev_path, disk, format, disks[i].readwrite ? "off" : "on");
+                         target_path, disk, format, disks[i].readwrite ? "off" : "on");
                 } else if (strncmp(disks[i].vdev, "xvd", 3) == 0) {
                     /*
                      * Do not add any emulated disk when PV disk are
@@ -1225,7 +1247,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                     }
                     flexarray_vappend(dm_args, "-drive",
                         GCSPRINTF("file=%s,if=none,id=ahcidisk-%d,format=%s,cache=writeback",
-                        pdev_path, disk, format),
+                        target_path, disk, format),
                         "-device", GCSPRINTF("ide-hd,bus=ahci0.%d,unit=0,drive=ahcidisk-%d",
                         disk, disk), NULL);
                     continue;
@@ -1236,7 +1258,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                     }
                     drive = libxl__sprintf
                         (gc, "file=%s,if=ide,index=%d,media=disk,format=%s,cache=writeback",
-                         pdev_path, disk, format);
+                         target_path, disk, format);
                 } else {
                     continue; /* Do not emulate this disk */
                 }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 005fe53..a511c6d 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2670,6 +2670,14 @@ static inline void libxl__device_disk_local_init(libxl__disk_local_state *dls)
     dls->rc = 0;
 }
 
+/* 
+ * See if we can find a way to access a disk locally
+ */
+_hidden char * libxl__device_disk_find_local_path(libxl__gc *gc, 
+                                                  const libxl_device_disk *disk,
+                                                  bool qdisk_direct);
+
+
 /* Make a disk available in this (the control) domain. Always calls
  * dls->callback when finished.
  * State Idle -> Attaching
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel