[v3,6/9] libxl: Share logic for finding path between qemuu and pygrub

Commit Message

George Dunlap March 24, 2016, 5:22 p.m. UTC

From: George Dunlap <[e-mail]>

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.

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.

Now if there is no local access to the disk (i.e., because the disk
has a non-local backend, or relies on a custom hotplug script), libxl
will now print a warning and not provide the emulated disk, rather
than providing bogus parameters to qemu which cause it to error out.
(Such disks will still be available via the PV backend.)

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 <[e-mail]>
---
Changes since v2:
- Fix stray comma

Changes since v1:
- Re-port on top of revisions to previous patches
- Address line length issues
- Remove stray space at the end of an if (xxx )
- Break out devicemodel argument rearrangement into a separate patch
- Spell out implications of warning messages which skip emulated disks

CC: Ian Jackson <[e-mail]>
CC: Wei Liu <[e-mail]>
CC: Stefano Stabellini <[e-mail]>
CC: Anthony Perard <[e-mail]>
CC: Roger Pau Monne <[e-mail]>
---
 tools/libxl/libxl.c          | 17 ++++++++++++++---
 tools/libxl/libxl_dm.c       | 27 +++++++++++++++++++++++----
 tools/libxl/libxl_internal.h |  8 ++++++++
 3 files changed, 45 insertions(+), 7 deletions(-)

Comments

Ian Jackson April 1, 2016, 2:19 p.m. UTC | #1

George Dunlap writes ("[PATCH v3 6/9] libxl: Share logic for finding path between qemuu and pygrub"):
> From: George Dunlap <[e-mail]>
> 
> 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.

Acked-by: Ian Jackson <[e-mail]>

Patch

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2d7a154..602bec2 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3000,8 +3000,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;
 
@@ -3022,6 +3023,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;
 }
@@ -3046,7 +3057,7 @@  void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
 
     LOG(DEBUG, "Trying to find local path");
 
-    dls->diskpath = libxl__device_disk_find_local_path(gc, in_disk);
+    dls->diskpath = libxl__device_disk_find_local_path(gc, in_disk, false);
     if (dls->diskpath) {
         LOG(DEBUG, "Local path found, executing callback.");
         dls->callback(egc, dls, 0);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 1effd71..2eef53a 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1188,23 +1188,42 @@  static int libxl__build_device_model_args_new(libxl__gc *gc,
 
             if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) {
                 if (!disks[i].is_cdrom) {
-                    LOG(WARN, "cannot support"" empty disk format for %s",
+                    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",
+                        "Unable to determine disk image format: %s\n"
+                        "Disk will be available via PV drivers but not as an"
+                        "emulated disk.",
                         disks[i].vdev);
                     continue;
                 }
 
+                /* 
+                 * 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     
-                    target_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\n"
+                        "Disk will be available via PV drivers but not as an"
+                        "emulated disk.",
+                        disks[i].vdev);
+                    continue;
+                }
             }
 
             if (disks[i].is_cdrom) {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 345a764..268a2f4 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2688,6 +2688,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