xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: George Dunlap <george.dunlap@citrix.com>
To: xen-devel@lists.xen.org
Cc: Wei Liu <wei.liu2@citrix.com>,
	George Dunlap <george.dunlap@eu.citrix.com>,
	Stefano Stabellini <stefano.stabellini@citrix.com>,
	Anthony Perard <anthony.perard@citrix.com>,
	Ian Jackson <ian.jackson@citrix.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: [PATCH v2 6/9] libxl: Share logic for finding path between qemuu and pygrub
Date: Mon, 21 Mar 2016 18:16:58 +0000	[thread overview]
Message-ID: <1458584221-24426-7-git-send-email-george.dunlap@citrix.com> (raw)
In-Reply-To: <1458584221-24426-1-git-send-email-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.

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 <george.dunlap@eu.citrix.com>
---
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 <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       | 27 +++++++++++++++++++++++----
 tools/libxl/libxl_internal.h |  8 ++++++++
 3 files changed, 45 insertions(+), 7 deletions(-)

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 dfcf141..1493b84 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1182,23 +1182,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
-- 
2.1.4


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

  parent reply	other threads:[~2016-03-21 18:16 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-21 18:16 [PATCH v2 0/9] tools: Allow HVM domains emulated access to disks provided by hotplug scripts George Dunlap
2016-03-21 18:16 ` [PATCH v2 1/9] tools/hotplug: Add a "dummy" hotplug script for testing George Dunlap
2016-03-21 18:16 ` [PATCH v2 2/9] libxl: Remove redundant setting of phyical-device George Dunlap
2016-03-21 18:16 ` [PATCH v2 3/9] tools/hotplug: Write physical-device-path in addition to physical-device George Dunlap
2016-03-21 18:16 ` [PATCH v2 4/9] libxl: Move check for local access to a funciton George Dunlap
2016-03-21 18:16 ` [PATCH v2 5/9] libxl: Rearrange qemu upstream disk argument code George Dunlap
2016-03-21 18:16 ` George Dunlap [this message]
2016-03-22 10:38   ` [PATCH v2 6/9] libxl: Share logic for finding path between qemuu and pygrub George Dunlap
2016-03-21 18:16 ` [PATCH v2 7/9] libxl: Allow local access for block devices with hotplug scripts George Dunlap
2016-03-21 18:17 ` [PATCH v2 8/9] docs: Document block-script protocol George Dunlap
2016-03-22 12:52   ` Roger Pau Monné
2016-03-23 11:19     ` George Dunlap
2016-03-23 12:14       ` Roger Pau Monné
2016-03-24 16:29         ` George Dunlap
2016-03-24 16:51           ` Roger Pau Monné
2016-03-21 18:17 ` [PATCH v2 9/9] DO NOT APPLY libxl: Change hotplug script interface to use physical-device-path George Dunlap
2016-03-23 10:10 ` [PATCH v2 0/9] tools: Allow HVM domains emulated access to disks provided by hotplug scripts Roger Pau Monné
2016-03-23 10:36   ` George Dunlap

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1458584221-24426-7-git-send-email-george.dunlap@citrix.com \
    --to=george.dunlap@citrix.com \
    --cc=anthony.perard@citrix.com \
    --cc=george.dunlap@eu.citrix.com \
    --cc=ian.jackson@citrix.com \
    --cc=roger.pau@citrix.com \
    --cc=stefano.stabellini@citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xen.org \
    --subject='Re: [PATCH v2 6/9] libxl: Share logic for finding path between qemuu and pygrub' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).