xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC for-4.6 1/4] libxl: Move check for local access to a funciton
@ 2015-08-10 19:11 George Dunlap
  2015-08-10 19:11 ` [PATCH RFC for-4.6 2/4] libxl: Share logic for finding path between qemuu and pygrub George Dunlap
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: George Dunlap @ 2015-08-10 19:11 UTC (permalink / raw)
  To: xen-devel
  Cc: George Dunlap, Ian Jackson, Wei Liu, Ian Campbell, Roger Pau Monne

Move pygrub checks for local access ability into a separate function.

Also reorganize libxl__device_disk_local_initiate_attach so that we
don't initialize dls->disk unless we actually end up doing a local
attach.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl.c | 66 +++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 46 insertions(+), 20 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 083f099..e402c80 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3046,13 +3046,38 @@ 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 *path = NULL;
+
+    /* No local paths for driver domains */
+    if (disk->backend_domname != NULL) {
+        LOG(DEBUG, "Non-local backend, can't access locally.\n");
+        goto out;
+    }
+
+    /*
+     * If this is in raw format, and we're not using a script or a
+     * driver domain, we can access the target path directly.
+     */
+    if (disk->format == LIBXL_DISK_FORMAT_RAW
+        && disk->script == NULL
+        && disk->pdev_path) {
+        path = libxl__strdup(gc, disk->pdev_path);
+        LOG(DEBUG, "Directly accessing local RAW disk %s", path);
+        goto out;
+    } 
+
+ out:
+    return path;
+}
+
 static void local_device_attach_cb(libxl__egc *egc, libxl__ao_device *aodev);
 
 void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
                                      libxl__disk_local_state *dls)
 {
     STATE_AO_GC(dls->ao);
-    char *dev = NULL;
     int rc;
     const libxl_device_disk *in_disk = dls->in_disk;
     libxl_device_disk *disk = &dls->disk;
@@ -3060,34 +3085,35 @@ void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
 
     assert(in_disk->pdev_path);
 
-    memcpy(disk, in_disk, sizeof(libxl_device_disk));
-    disk->pdev_path = libxl__strdup(gc, in_disk->pdev_path);
-    if (in_disk->script != NULL)
-        disk->script = libxl__strdup(gc, in_disk->script);
     disk->vdev = NULL;
 
-    rc = libxl__device_disk_setdefault(gc, disk);
-    if (rc) goto out;
+    if(dls->diskpath)
+        LOG(DEBUG, "Strange, dls->diskpath already set: %s", dls->diskpath);
 
-    /* If this is in a driver domain, or it's not a raw format, or it involves
-     * running a script, we have to do a local attach. */
-    if (disk->backend_domname != NULL
-        || disk->format != LIBXL_DISK_FORMAT_RAW
-        || disk->script != NULL) {
+    LOG(DEBUG, "Trying to find local path");
+
+    if ((dls->diskpath = libxl__device_disk_find_local_path(gc, in_disk))) {
+        LOG(DEBUG, "Local path found, executing callback.");
+        dls->callback(egc, dls, 0);
+    } else {
+        LOG(DEBUG, "Local path not found, initiating attach.");
+
+        memcpy(disk, in_disk, sizeof(libxl_device_disk));
+        disk->pdev_path = libxl__strdup(gc, in_disk->pdev_path);
+        if (in_disk->script != NULL)
+            disk->script = libxl__strdup(gc, in_disk->script);
+        disk->vdev = NULL;
+        
+        rc = libxl__device_disk_setdefault(gc, disk);
+        if (rc) goto out;
+
+        /* If we can't find a local path, attach it */
         libxl__prepare_ao_device(ao, &dls->aodev);
         dls->aodev.callback = local_device_attach_cb;
         device_disk_add(egc, LIBXL_TOOLSTACK_DOMID, disk, &dls->aodev,
                         libxl__alloc_vdev, (void *) blkdev_start);
-        return;
     }
 
-    LOG(DEBUG, "locally attaching RAW disk %s", disk->pdev_path);
-    dev = disk->pdev_path;
-
-    if (dev != NULL)
-        dls->diskpath = libxl__strdup(gc, dev);
-
-    dls->callback(egc, dls, 0);
     return;
 
  out:
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH RFC for-4.6 2/4] libxl: Share logic for finding path between qemuu and pygrub
  2015-08-10 19:11 [PATCH RFC for-4.6 1/4] libxl: Move check for local access to a funciton George Dunlap
@ 2015-08-10 19:11 ` George Dunlap
  2015-08-10 19:11 ` [PATCH RFC for-4.6 3/4] tools/hotplug: Add a "dummy" hotplug script for testing George Dunlap
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: George Dunlap @ 2015-08-10 19:11 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Ian Campbell, George Dunlap, Stefano Stabellini,
	Anthony Perard, Ian Jackson, Roger Pau Monne

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 Campbell <ian.campbell@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       | 67 +++++++++++++++++++++++++++++---------------
 tools/libxl/libxl_internal.h |  8 ++++++
 3 files changed, 67 insertions(+), 25 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index e402c80..c67caee 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3046,8 +3046,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 */
@@ -3068,6 +3069,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;
 }
@@ -3092,7 +3103,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 02c0162..a54e758 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1097,9 +1097,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) {
                 LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "unable to determine"
@@ -1107,36 +1107,59 @@ 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,media=cdrom,cache=writeback,id=ide-%i",
-                         disk, dev_number);
-                else
-                    drive = libxl__sprintf
-                        (gc, "file=%s,if=ide,index=%d,media=cdrom,format=%s,cache=writeback,id=ide-%i",
-                         disks[i].pdev_path, disk, 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) {
                     LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "cannot support"
                                " empty disk format for %s", disks[i].vdev);
                     continue;
                 }
-
+            } else {
                 if (format == NULL) {
                     LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "unable to determine"
                                " disk image format %s", disks[i].vdev);
                     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) {
+                    LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "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,media=cdrom,cache=writeback,id=ide-%i",
+                     disk, 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.
                  *
@@ -1146,18 +1169,18 @@ 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,cache=writeback",
-                         pdev_path, disk, format);
+                         target_path, disk, format);
                 else if (disk < 6 && b_info->u.hvm.hdtype == LIBXL_HDTYPE_AHCI) {
                     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;
                 } else if (disk < 4)
                     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 6ea6c83..f58b8c9 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2638,6 +2638,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
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH RFC for-4.6 3/4] tools/hotplug: Add a "dummy" hotplug script for testing
  2015-08-10 19:11 [PATCH RFC for-4.6 1/4] libxl: Move check for local access to a funciton George Dunlap
  2015-08-10 19:11 ` [PATCH RFC for-4.6 2/4] libxl: Share logic for finding path between qemuu and pygrub George Dunlap
@ 2015-08-10 19:11 ` George Dunlap
  2015-08-10 19:11 ` [PATCH RFC for-4.6 4/4] libxl: Allow local access for block devices with hotplug scripts George Dunlap
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: George Dunlap @ 2015-08-10 19:11 UTC (permalink / raw)
  To: xen-devel
  Cc: George Dunlap, Ian Jackson, Wei Liu, Ian Campbell, Roger Pau Monne

Testing the hotplug external script path at the moment involves
actually setting up one of the alternate datapaths (blktap, iscsi,
&c).  Simplify testing by making a script which does a simple loopback,
but still has a target that can't be used directly.

To use:

script=block-dummy,vdev=xvda,target=dummy:<file>

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/hotplug/Linux/Makefile    |   1 +
 tools/hotplug/Linux/block-dummy | 107 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+)
 create mode 100644 tools/hotplug/Linux/block-dummy

diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile
index 6e10118..dcfd58a 100644
--- a/tools/hotplug/Linux/Makefile
+++ b/tools/hotplug/Linux/Makefile
@@ -27,6 +27,7 @@ XEN_SCRIPTS += vscsi
 XEN_SCRIPTS += block-iscsi
 XEN_SCRIPTS += block-tap
 XEN_SCRIPTS += block-drbd-probe
+XEN_SCRIPTS += block-dummy
 XEN_SCRIPTS += $(XEN_SCRIPTS-y)
 
 SUBDIRS-$(CONFIG_SYSTEMD) += systemd
diff --git a/tools/hotplug/Linux/block-dummy b/tools/hotplug/Linux/block-dummy
new file mode 100644
index 0000000..57d40b5
--- /dev/null
+++ b/tools/hotplug/Linux/block-dummy
@@ -0,0 +1,107 @@
+#!/bin/bash -e
+#
+# dummy Xen block device hotplug script
+#
+# Author George Dunlap <george.dunlap@eu.citrix.com>
+#
+# Based on block-iscsi by Roger Pau Monné <roger.pau@citrix.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License as published
+# by the Free Software Foundation; version 2.1 only. with the special
+# exception on linking described in file LICENSE.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Lesser General Public License for more details.
+#
+# Usage:
+#
+# Target should be specified using the following syntax:
+#
+# script=block-dummy,vdev=xvda,target=dummy:<file>
+
+dir=$(dirname "$0")
+. "$dir/block-common.sh"
+
+check_tools()
+{
+    if ! command -v losetup > /dev/null 2>&1; then
+        fatal "Unable to find losetup"
+    fi
+}
+
+# Sets the following global variables based on the params field passed in as
+# a parameter: type file
+parse_target()
+{
+    params=($(echo "$1" | tr ":" "\n"))
+
+    type=${params[0]}
+    file=${params[1]}
+    if [ -z "$type" ] || [ -z "$file" ]; then
+        fatal "Cannot parse required parameters"
+    fi
+
+    if [ "$type" != "dummy" ] ; then
+	fatal "Invalid type: $type"
+    fi
+}
+
+# Attaches the device and writes xenstore backend entries to connect
+# the device
+add()
+{
+    test -f "$file" || fatal "$file does not exist."
+
+    loopdev=$(losetup -f 2>/dev/null || find_free_loopback_dev)
+    if [ "$loopdev" = '' ]
+    then
+        fatal 'Failed to find an unused loop device'
+    fi
+    
+    if LANG=C losetup -h 2>&1 | grep read-only >/dev/null
+    then
+        roflag="-$mode"; roflag="${roflag#-w}"; roflag="${roflag#-!}"
+    else
+        roflag=''
+    fi
+
+    do_or_die losetup $roflag "$loopdev" "$file"
+    # FIXME Is this OK?
+    xenstore_write "$XENBUS_PATH/node" "$loopdev"
+    write_dev "$loopdev"
+}
+
+# Disconnects the device
+remove()
+{
+    node=$(xenstore_read "$XENBUS_PATH/node")
+    losetup -d "$node"
+}
+
+command=$1
+target=$(xenstore-read $XENBUS_PATH/params || true)
+if [ -z "$target" ]; then
+    fatal "No information about the target"
+fi
+
+parse_target "$target"
+
+mode=$(xenstore_read "$XENBUS_PATH/mode")
+mode=$(canonicalise_mode "$mode")
+
+check_tools || exit 1
+
+case $command in
+add)
+    add
+    ;;
+remove)
+    remove
+    ;;
+*)
+    exit 1
+    ;;
+esac
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH RFC for-4.6 4/4] libxl: Allow local access for block devices with hotplug scripts
  2015-08-10 19:11 [PATCH RFC for-4.6 1/4] libxl: Move check for local access to a funciton George Dunlap
  2015-08-10 19:11 ` [PATCH RFC for-4.6 2/4] libxl: Share logic for finding path between qemuu and pygrub George Dunlap
  2015-08-10 19:11 ` [PATCH RFC for-4.6 3/4] tools/hotplug: Add a "dummy" hotplug script for testing George Dunlap
@ 2015-08-10 19:11 ` George Dunlap
  2015-08-10 19:14 ` [PATCH RFC for-4.6 1/4] libxl: Move check for local access to a funciton George Dunlap
  2015-11-06 14:19 ` Wei Liu
  4 siblings, 0 replies; 7+ messages in thread
From: George Dunlap @ 2015-08-10 19:11 UTC (permalink / raw)
  To: xen-devel
  Cc: George Dunlap, Ian Jackson, Wei Liu, Ian Campbell, Roger Pau Monne

pygrub and qemuu need to be able to access a VM's disks locally in
order to be able to pull out the kernel and provide emulated disk
access, respectively.  This can be done either by accessing the local
disk directly, or by plugging the target disk into dom0 to allow
access.

Unfortunately, while the plugging machinery works for pygrub, it does
not yet work for qemuu; meaning that disk hotplug scripts cannot be
used with HVM domains.

Fortunately, disks using hotplug scripts created in dom0 do create a
block device as part of set-up, which can be accessed locally.

First, modify the write_dev function in block-common to add an extra
parameter: "physical-device-path", the path to the physical device
which is passed to blkback.  All hotplug scripts that use write_dev
will automatically gain the new functionality.

Second, modify libxl__device_disk_setdefault() to be able to fish this
path out of xenstore and pass it back.

We need the target domid to find the appropriate xenstore node, so add that to
libxl__disk_local_state.

This allows qemuu to boot with block devices created with hotplug scripts.

Unfortunately it does not allow pygrub to access a device locally
without having to plug it in, because when pygrub runs the guests'
disks have not yet been set up.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/hotplug/Linux/block-common.sh |  5 ++++-
 tools/libxl/libxl.c                 | 42 ++++++++++++++++++++++++++++++++++---
 tools/libxl/libxl_bootloader.c      |  1 +
 tools/libxl/libxl_dm.c              |  2 +-
 tools/libxl/libxl_internal.h        |  4 +++-
 5 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/tools/hotplug/Linux/block-common.sh b/tools/hotplug/Linux/block-common.sh
index ee95009..1aecd7e 100644
--- a/tools/hotplug/Linux/block-common.sh
+++ b/tools/hotplug/Linux/block-common.sh
@@ -58,8 +58,9 @@ device_major_minor()
 #
 write_dev() {
   local mm
+  local dev="$1"
   
-  mm=$(device_major_minor "$1")
+  mm=$(device_major_minor "$dev")
  
   if [ -z $mm ]
   then
@@ -68,6 +69,8 @@ write_dev() {
  
   xenstore_write "$XENBUS_PATH/physical-device" "$mm"
 
+  xenstore_write "$XENBUS_PATH/physical-device-path" "$dev"
+
   success
 }
 
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index c67caee..680539e 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2325,7 +2325,7 @@ int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
 }
 
 int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
-                                   libxl_device_disk *disk,
+                                   const libxl_device_disk *disk,
                                    libxl__device *device)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -2485,6 +2485,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
                     if (!libxl__device_physdisk_major_minor(dev, &major, &minor))
                         flexarray_append_pair(back, "physical-device",
                                               libxl__sprintf(gc, "%x:%x", major, minor));
+                    flexarray_append_pair(back, "physical-device-path", dev);
                 }
 
                 assert(device->backend_kind == LIBXL__DEVICE_KIND_VBD);
@@ -3047,6 +3048,7 @@ static char * libxl__alloc_vdev(libxl__gc *gc, void *get_vdev_user,
 /* Callbacks */
 
 char * libxl__device_disk_find_local_path(libxl__gc *gc, 
+                                          libxl_domid domid,
                                           const libxl_device_disk *disk,
                                           bool qdisk_direct) {
     char *path = NULL;
@@ -3077,6 +3079,40 @@ char * libxl__device_disk_find_local_path(libxl__gc *gc,
         path = libxl__strdup(gc, disk->pdev_path);
         LOG(DEBUG, "Directly accessing local QDISK target %s", path);
         goto out;
+    } 
+
+    /* 
+     * If the format isn't raw and / or we're using a script, then see
+     * if the script has written a path to the "cooked" node
+     */
+    if(disk->script) {
+        libxl__device device;
+        char * be_path, *pdpath;
+        int rc;
+
+        LOG(DEBUG, "Run from a script; checking for physical-device-path (vdev %s)",
+            disk->vdev);
+
+        rc = libxl__device_from_disk(gc, domid, disk, &device);
+        if (rc < 0)
+            goto out;
+
+        /* Needs: dompath, device type, domid */
+        be_path = libxl__device_backend_path(gc, &device);
+
+        /* FIXME: Do we need to wait for things to be set up here? */
+        
+        pdpath = libxl__sprintf(gc, "%s/physical-device-path", be_path);
+
+        LOG(DEBUG, "Attempting to read node %s", pdpath);
+        path = libxl__xs_read(gc, XBT_NULL, pdpath);
+
+        if (path)
+            LOG(DEBUG, "Accessing cooked block device %s", path);
+        else
+            LOG(DEBUG, "No physical-device-path, can't access locally.");
+
+        goto out;
     }
 
  out:
@@ -3103,7 +3139,8 @@ 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, false))) {
+    if ((dls->diskpath = libxl__device_disk_find_local_path(gc, dls->domid,
+                                                            in_disk, false))) {
         LOG(DEBUG, "Local path found, executing callback.");
         dls->callback(egc, dls, 0);
     } else {
@@ -3118,7 +3155,6 @@ void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
         rc = libxl__device_disk_setdefault(gc, disk);
         if (rc) goto out;
 
-        /* If we can't find a local path, attach it */
         libxl__prepare_ao_device(ao, &dls->aodev);
         dls->aodev.callback = local_device_attach_cb;
         device_disk_add(egc, LIBXL_TOOLSTACK_DOMID, disk, &dls->aodev,
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index 95dde98..bd0c81f 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -382,6 +382,7 @@ void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl)
     /* This sets the state of the dls struct from Undefined to Idle */
     libxl__device_disk_local_init(&bl->dls);
     bl->dls.ao = ao;
+    bl->dls.domid = bl->domid;
     bl->dls.in_disk = bl->disk;
     bl->dls.blkdev_start = info->blkdev_start;
     bl->dls.callback = bootloader_disk_attached_cb;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index a54e758..df1acd0 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1142,7 +1142,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                                                       disks[i].format);
                 else
                     target_path = libxl__device_disk_find_local_path(gc, 
-                                                               &disks[i], true);
+                                                  guest_domid, &disks[i], true);
 
                 if (!target_path) {
                     LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "No way to get local"
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index f58b8c9..294c1a5 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1677,7 +1677,7 @@ _hidden char *libxl__blktap_devpath(libxl__gc *gc,
 _hidden int libxl__device_destroy_tapdisk(libxl__gc *gc, const char *params);
 
 _hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
-                                   libxl_device_disk *disk,
+                                   const libxl_device_disk *disk,
                                    libxl__device *device);
 
 /* Calls poll() again - useful to check whether a signaled condition
@@ -2618,6 +2618,7 @@ typedef void libxl__disk_local_state_callback(libxl__egc*,
 struct libxl__disk_local_state {
     /* filled by the user */
     libxl__ao *ao;
+    libxl_domid domid;
     const libxl_device_disk *in_disk;
     libxl_device_disk disk;
     const char *blkdev_start;
@@ -2642,6 +2643,7 @@ static inline void libxl__device_disk_local_init(libxl__disk_local_state *dls)
  * See if we can find a way to access a disk locally
  */
 _hidden char * libxl__device_disk_find_local_path(libxl__gc *gc, 
+                                                  libxl_domid domid,
                                                   const libxl_device_disk *disk,
                                                   bool qdisk_direct);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC for-4.6 1/4] libxl: Move check for local access to a funciton
  2015-08-10 19:11 [PATCH RFC for-4.6 1/4] libxl: Move check for local access to a funciton George Dunlap
                   ` (2 preceding siblings ...)
  2015-08-10 19:11 ` [PATCH RFC for-4.6 4/4] libxl: Allow local access for block devices with hotplug scripts George Dunlap
@ 2015-08-10 19:14 ` George Dunlap
  2015-11-06 14:19 ` Wei Liu
  4 siblings, 0 replies; 7+ messages in thread
From: George Dunlap @ 2015-08-10 19:14 UTC (permalink / raw)
  To: xen-devel
  Cc: George Dunlap, Ian Jackson, Wei Liu, Ian Campbell, Roger Pau Monne

On Mon, Aug 10, 2015 at 8:11 PM, George Dunlap
<george.dunlap@eu.citrix.com> wrote:
> Move pygrub checks for local access ability into a separate function.
>
> Also reorganize libxl__device_disk_local_initiate_attach so that we
> don't initialize dls->disk unless we actually end up doing a local
> attach.
>
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

This series works for me with the "block-dummy" script, both for HVM
and for PV (with pygrub).

It probably still needs a bit of work to get in, but I won't be able
to work on it anymore.  I hope someone can pick it up and get it in.
Otherwise hotplug scripts for qemu will have to wait until 4.7.

Thanks,
 -George

> ---
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Roger Pau Monne <roger.pau@citrix.com>
> ---
>  tools/libxl/libxl.c | 66 +++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 46 insertions(+), 20 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 083f099..e402c80 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -3046,13 +3046,38 @@ 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 *path = NULL;
> +
> +    /* No local paths for driver domains */
> +    if (disk->backend_domname != NULL) {
> +        LOG(DEBUG, "Non-local backend, can't access locally.\n");
> +        goto out;
> +    }
> +
> +    /*
> +     * If this is in raw format, and we're not using a script or a
> +     * driver domain, we can access the target path directly.
> +     */
> +    if (disk->format == LIBXL_DISK_FORMAT_RAW
> +        && disk->script == NULL
> +        && disk->pdev_path) {
> +        path = libxl__strdup(gc, disk->pdev_path);
> +        LOG(DEBUG, "Directly accessing local RAW disk %s", path);
> +        goto out;
> +    }
> +
> + out:
> +    return path;
> +}
> +
>  static void local_device_attach_cb(libxl__egc *egc, libxl__ao_device *aodev);
>
>  void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
>                                       libxl__disk_local_state *dls)
>  {
>      STATE_AO_GC(dls->ao);
> -    char *dev = NULL;
>      int rc;
>      const libxl_device_disk *in_disk = dls->in_disk;
>      libxl_device_disk *disk = &dls->disk;
> @@ -3060,34 +3085,35 @@ void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
>
>      assert(in_disk->pdev_path);
>
> -    memcpy(disk, in_disk, sizeof(libxl_device_disk));
> -    disk->pdev_path = libxl__strdup(gc, in_disk->pdev_path);
> -    if (in_disk->script != NULL)
> -        disk->script = libxl__strdup(gc, in_disk->script);
>      disk->vdev = NULL;
>
> -    rc = libxl__device_disk_setdefault(gc, disk);
> -    if (rc) goto out;
> +    if(dls->diskpath)
> +        LOG(DEBUG, "Strange, dls->diskpath already set: %s", dls->diskpath);
>
> -    /* If this is in a driver domain, or it's not a raw format, or it involves
> -     * running a script, we have to do a local attach. */
> -    if (disk->backend_domname != NULL
> -        || disk->format != LIBXL_DISK_FORMAT_RAW
> -        || disk->script != NULL) {
> +    LOG(DEBUG, "Trying to find local path");
> +
> +    if ((dls->diskpath = libxl__device_disk_find_local_path(gc, in_disk))) {
> +        LOG(DEBUG, "Local path found, executing callback.");
> +        dls->callback(egc, dls, 0);
> +    } else {
> +        LOG(DEBUG, "Local path not found, initiating attach.");
> +
> +        memcpy(disk, in_disk, sizeof(libxl_device_disk));
> +        disk->pdev_path = libxl__strdup(gc, in_disk->pdev_path);
> +        if (in_disk->script != NULL)
> +            disk->script = libxl__strdup(gc, in_disk->script);
> +        disk->vdev = NULL;
> +
> +        rc = libxl__device_disk_setdefault(gc, disk);
> +        if (rc) goto out;
> +
> +        /* If we can't find a local path, attach it */
>          libxl__prepare_ao_device(ao, &dls->aodev);
>          dls->aodev.callback = local_device_attach_cb;
>          device_disk_add(egc, LIBXL_TOOLSTACK_DOMID, disk, &dls->aodev,
>                          libxl__alloc_vdev, (void *) blkdev_start);
> -        return;
>      }
>
> -    LOG(DEBUG, "locally attaching RAW disk %s", disk->pdev_path);
> -    dev = disk->pdev_path;
> -
> -    if (dev != NULL)
> -        dls->diskpath = libxl__strdup(gc, dev);
> -
> -    dls->callback(egc, dls, 0);
>      return;
>
>   out:
> --
> 1.9.1
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC for-4.6 1/4] libxl: Move check for local access to a funciton
  2015-08-10 19:11 [PATCH RFC for-4.6 1/4] libxl: Move check for local access to a funciton George Dunlap
                   ` (3 preceding siblings ...)
  2015-08-10 19:14 ` [PATCH RFC for-4.6 1/4] libxl: Move check for local access to a funciton George Dunlap
@ 2015-11-06 14:19 ` Wei Liu
  2015-11-06 14:30   ` Wei Liu
  4 siblings, 1 reply; 7+ messages in thread
From: Wei Liu @ 2015-11-06 14:19 UTC (permalink / raw)
  To: George Dunlap
  Cc: Ian Jackson, Roger Pau Monne, Wei Liu, Ian Campbell, xen-devel

On Mon, Aug 10, 2015 at 08:11:22PM +0100, George Dunlap wrote:
> Move pygrub checks for local access ability into a separate function.
> 
> Also reorganize libxl__device_disk_local_initiate_attach so that we
> don't initialize dls->disk unless we actually end up doing a local
> attach.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Roger Pau Monne <roger.pau@citrix.com>
> ---
>  tools/libxl/libxl.c | 66 +++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 46 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 083f099..e402c80 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -3046,13 +3046,38 @@ 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 *path = NULL;
> +
> +    /* No local paths for driver domains */
> +    if (disk->backend_domname != NULL) {
> +        LOG(DEBUG, "Non-local backend, can't access locally.\n");
> +        goto out;
> +    }
> +
> +    /*
> +     * If this is in raw format, and we're not using a script or a
> +     * driver domain, we can access the target path directly.
> +     */
> +    if (disk->format == LIBXL_DISK_FORMAT_RAW
> +        && disk->script == NULL
> +        && disk->pdev_path) {
> +        path = libxl__strdup(gc, disk->pdev_path);
> +        LOG(DEBUG, "Directly accessing local RAW disk %s", path);
> +        goto out;
> +    } 
> +
> + out:
> +    return path;
> +}
> +
>  static void local_device_attach_cb(libxl__egc *egc, libxl__ao_device *aodev);
>  
>  void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
>                                       libxl__disk_local_state *dls)
>  {
>      STATE_AO_GC(dls->ao);
> -    char *dev = NULL;
>      int rc;
>      const libxl_device_disk *in_disk = dls->in_disk;
>      libxl_device_disk *disk = &dls->disk;
> @@ -3060,34 +3085,35 @@ void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
>  
>      assert(in_disk->pdev_path);
>  
> -    memcpy(disk, in_disk, sizeof(libxl_device_disk));
> -    disk->pdev_path = libxl__strdup(gc, in_disk->pdev_path);
> -    if (in_disk->script != NULL)
> -        disk->script = libxl__strdup(gc, in_disk->script);
>      disk->vdev = NULL;
>  
> -    rc = libxl__device_disk_setdefault(gc, disk);
> -    if (rc) goto out;
> +    if(dls->diskpath)

Space after `if'.

> +        LOG(DEBUG, "Strange, dls->diskpath already set: %s", dls->diskpath);
>  

And if this is not expected, maybe just assert(!dls->diskpath) ?

> -    /* If this is in a driver domain, or it's not a raw format, or it involves
> -     * running a script, we have to do a local attach. */
> -    if (disk->backend_domname != NULL
> -        || disk->format != LIBXL_DISK_FORMAT_RAW
> -        || disk->script != NULL) {
> +    LOG(DEBUG, "Trying to find local path");
> +
> +    if ((dls->diskpath = libxl__device_disk_find_local_path(gc, in_disk))) {
> +        LOG(DEBUG, "Local path found, executing callback.");
> +        dls->callback(egc, dls, 0);
> +    } else {
> +        LOG(DEBUG, "Local path not found, initiating attach.");
> +
> +        memcpy(disk, in_disk, sizeof(libxl_device_disk));
> +        disk->pdev_path = libxl__strdup(gc, in_disk->pdev_path);
> +        if (in_disk->script != NULL)
> +            disk->script = libxl__strdup(gc, in_disk->script);
> +        disk->vdev = NULL;
> +        
> +        rc = libxl__device_disk_setdefault(gc, disk);
> +        if (rc) goto out;
> +
> +        /* If we can't find a local path, attach it */
>          libxl__prepare_ao_device(ao, &dls->aodev);
>          dls->aodev.callback = local_device_attach_cb;
>          device_disk_add(egc, LIBXL_TOOLSTACK_DOMID, disk, &dls->aodev,
>                          libxl__alloc_vdev, (void *) blkdev_start);
> -        return;
>      }
>  
> -    LOG(DEBUG, "locally attaching RAW disk %s", disk->pdev_path);
> -    dev = disk->pdev_path;
> -
> -    if (dev != NULL)
> -        dls->diskpath = libxl__strdup(gc, dev);
> -
> -    dls->callback(egc, dls, 0);
>      return;
>  
>   out:
> -- 
> 1.9.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC for-4.6 1/4] libxl: Move check for local access to a funciton
  2015-11-06 14:19 ` Wei Liu
@ 2015-11-06 14:30   ` Wei Liu
  0 siblings, 0 replies; 7+ messages in thread
From: Wei Liu @ 2015-11-06 14:30 UTC (permalink / raw)
  To: George Dunlap
  Cc: Ian Jackson, Roger Pau Monne, Wei Liu, Ian Campbell, xen-devel

This is sent by mistake.  Please ignore this email until I finish the
whole series.

Wei.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-11-06 14:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-10 19:11 [PATCH RFC for-4.6 1/4] libxl: Move check for local access to a funciton George Dunlap
2015-08-10 19:11 ` [PATCH RFC for-4.6 2/4] libxl: Share logic for finding path between qemuu and pygrub George Dunlap
2015-08-10 19:11 ` [PATCH RFC for-4.6 3/4] tools/hotplug: Add a "dummy" hotplug script for testing George Dunlap
2015-08-10 19:11 ` [PATCH RFC for-4.6 4/4] libxl: Allow local access for block devices with hotplug scripts George Dunlap
2015-08-10 19:14 ` [PATCH RFC for-4.6 1/4] libxl: Move check for local access to a funciton George Dunlap
2015-11-06 14:19 ` Wei Liu
2015-11-06 14:30   ` Wei Liu

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).