All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] Use new QEMU xenstore protocol
@ 2015-04-09 18:49 Wei Liu
  2015-04-09 18:49 ` [PATCH v4 1/3] qemu-trad: xenstore: use relative path for device-model node Wei Liu
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Wei Liu @ 2015-04-09 18:49 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell

This patch sereis contains 1 patch to QEMU traditional tree and 2 patches to
Xen tree.

Note that QEMU trad patch and libxl patch need to be committed at the same
time. And please fold in the change to  update Config.mk to libxl patch to
avoid regression.

The third patch has not been acked, but the reversion doesn't affect the libxl
and QEMU patch so we can commit it any time we see fit.

Wei.

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

* [PATCH v4 1/3] qemu-trad: xenstore: use relative path for device-model node
  2015-04-09 18:49 [PATCH v4 0/3] Use new QEMU xenstore protocol Wei Liu
@ 2015-04-09 18:49 ` Wei Liu
  2015-04-09 18:49 ` [PATCH v4 2/3] libxl: use new QEMU xenstore protocol Wei Liu
  2015-04-09 18:49 ` [PATCH v4 3/3] Revert "x86/hvm: wait for at least one ioreq server to be enabled" Wei Liu
  2 siblings, 0 replies; 6+ messages in thread
From: Wei Liu @ 2015-04-09 18:49 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell

For QEMU traditional running in Dom0, there is no functional change
because it will still write to the same /local/domain/0 path.

For QEMU traditional stubdom, this is an incompatible startup
protocol change.  There is a corresponding libxl changeset "libxl:
use new QEMU xenstore protocol", which has not yet been committed
(54c2e621 was reverted in 84066dd4).

QEMU traditional stubdom was broken by a0731cca "ioreq-server: on-demand
creation of ioreq server" in 4.5. Currently there is a workaround in
-unstable dd748d12 "x86/hvm: wait for at least one ioreq server to be
enabled" (which should be backported to 4.5). QEMU traditional stubdom
works with that workaround in -unstable but it's not ideal situation.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
Note:
In order to not cause a regression in -unstable, this change and libxl
side change need to be committed at the same time. And libxl side patch
needs to contain a updated tag that points to this commit.
---
 xenstore.c | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/xenstore.c b/xenstore.c
index b0d6f77..8af2715 100644
--- a/xenstore.c
+++ b/xenstore.c
@@ -711,15 +711,13 @@ void xenstore_parse_domain_config(int hvm_domid)
 
 
     /* Set a watch for log-dirty commands from the migration tools */
-    if (pasprintf(&buf, "/local/domain/0/device-model/%u/logdirty/cmd",
-                  domid) != -1) {
+    if (pasprintf(&buf, "device-model/%u/logdirty/cmd", domid) != -1) {
         xs_watch(xsh, buf, "logdirty");
         fprintf(logfile, "Watching %s\n", buf);
     }
 
     /* Set a watch for suspend requests from the migration tools */
-    if (pasprintf(&buf, 
-                  "/local/domain/0/device-model/%u/command", domid) != -1) {
+    if (pasprintf(&buf, "device-model/%u/command", domid) != -1) {
         xs_watch(xsh, buf, "dm-command");
         fprintf(logfile, "Watching %s\n", buf);
     }
@@ -777,7 +775,7 @@ int xenstore_parse_disable_pf_config ()
     int disable_pf = 0;
     unsigned int len;
 
-    if (pasprintf(&buf, "/local/domain/0/device-model/%u/disable_pf",domid) == -1)
+    if (pasprintf(&buf, "device-model/%u/disable_pf",domid) == -1)
         goto out;
 
     params = xs_read(xsh, XBT_NULL, buf, &len);
@@ -807,15 +805,11 @@ static void xenstore_process_logdirty_event(void)
     unsigned int len;
 
     /* Remember the paths for the command and response entries */
-    if (pasprintf(&ret_path,
-                "/local/domain/0/device-model/%u/logdirty/ret",
-                domid) == -1) {
+    if (pasprintf(&ret_path, "device-model/%u/logdirty/ret", domid) == -1) {
         fprintf(logfile, "Log-dirty: out of memory\n");
         exit(1);
     }
-    if (pasprintf(&cmd_path,
-                "/local/domain/0/device-model/%u/logdirty/cmd",
-                domid) == -1) {
+    if (pasprintf(&cmd_path, "device-model/%u/logdirty/cmd", domid) == -1) {
         fprintf(logfile, "Log-dirty: out of memory\n");
         exit(1);
     }
@@ -854,8 +848,7 @@ static void xenstore_process_dm_command_event(void)
     char *path = NULL, *command = NULL, *par = NULL;
     unsigned int len;
 
-    if (pasprintf(&path, 
-                  "/local/domain/0/device-model/%u/command", domid) == -1) {
+    if (pasprintf(&path, "device-model/%u/command", domid) == -1) {
         fprintf(logfile, "out of memory reading dm command\n");
         goto out;
     }
@@ -874,8 +867,7 @@ static void xenstore_process_dm_command_event(void)
         xen_pause_requested = 0;
     } else if (!strncmp(command, "usb-add", len)) {
         fprintf(logfile, "dm-command: usb-add a usb device\n");
-        if (pasprintf(&path,
-                "/local/domain/0/device-model/%u/parameter", domid) == -1) {
+        if (pasprintf(&path, "device-model/%u/parameter", domid) == -1) {
             fprintf(logfile, "out of memory reading dm command parameter\n");
             goto out;
         }
@@ -888,8 +880,7 @@ static void xenstore_process_dm_command_event(void)
         fprintf(logfile, "dm-command: finish usb-add a usb device:%s\n",par);
     } else if (!strncmp(command, "usb-del", len)) {
         fprintf(logfile, "dm-command: usb-del a usb device\n");
-        if (pasprintf(&path,
-                "/local/domain/0/device-model/%u/parameter", domid) == -1) {
+        if (pasprintf(&path, "device-model/%u/parameter", domid) == -1) {
             fprintf(logfile, "out of memory reading dm command parameter\n");
             goto out;
         }
@@ -904,8 +895,7 @@ static void xenstore_process_dm_command_event(void)
     } else if (!strncmp(command, "pci-rem", len)) {
         fprintf(logfile, "dm-command: hot remove pass-through pci dev \n");
 
-        if (pasprintf(&path, 
-                      "/local/domain/0/device-model/%u/parameter", domid) == -1) {
+        if (pasprintf(&path, "device-model/%u/parameter", domid) == -1) {
             fprintf(logfile, "out of memory reading dm command parameter\n");
             goto out;
         }
@@ -918,8 +908,7 @@ static void xenstore_process_dm_command_event(void)
     } else if (!strncmp(command, "pci-ins", len)) {
         fprintf(logfile, "dm-command: hot insert pass-through pci dev \n");
 
-        if (pasprintf(&path, 
-                      "/local/domain/0/device-model/%u/parameter", domid) == -1) {
+        if (pasprintf(&path, "device-model/%u/parameter", domid) == -1) {
             fprintf(logfile, "out of memory reading dm command parameter\n");
             goto out;
         }
@@ -943,8 +932,7 @@ void xenstore_record_dm(const char *subpath, const char *state)
 {
     char *path = NULL;
 
-    if (pasprintf(&path, 
-                  "/local/domain/0/device-model/%u/%s", domid, subpath) == -1) {
+    if (pasprintf(&path, "device-model/%u/%s", domid, subpath) == -1) {
         fprintf(logfile, "out of memory recording dm \n");
         goto out;
     }
@@ -1521,7 +1509,7 @@ char *xenstore_device_model_read(int domid, const char *key, unsigned int *len)
 {
     char *path = NULL, *value = NULL;
 
-    if (pasprintf(&path, "/local/domain/0/device-model/%d/%s", domid, key) == -1)
+    if (pasprintf(&path, "device-model/%d/%s", domid, key) == -1)
         return NULL;
 
     value = xs_read(xsh, XBT_NULL, path, len);
-- 
1.9.1

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

* [PATCH v4 2/3] libxl: use new QEMU xenstore protocol
  2015-04-09 18:49 [PATCH v4 0/3] Use new QEMU xenstore protocol Wei Liu
  2015-04-09 18:49 ` [PATCH v4 1/3] qemu-trad: xenstore: use relative path for device-model node Wei Liu
@ 2015-04-09 18:49 ` Wei Liu
  2015-04-09 18:56   ` Ian Jackson
  2015-04-09 18:49 ` [PATCH v4 3/3] Revert "x86/hvm: wait for at least one ioreq server to be enabled" Wei Liu
  2 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2015-04-09 18:49 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell

Originally both QEMU traditional and QEMU upstream used hardcoded
/local/domain/0 paths. This patch changes the protocol to use
/local/domain/$dm_domid path.

For QEMU traditional and upstream without stubdom, $dm_domid is 0 so
the path is in fact still /local/domain/0.

For QEMU traditional stubdom, this is incompatible protocol change.
However QEMU traditional is shipped with Xen so we are allowed to do
such change. This change needs to work with corresponding QEMU
traditional changeset.

There is no compatibility issue with QEMU upstream stubdom, because QEMU
upstream stubdom doesn't exist yet.

Watch /local/domain/$dm_domid/device-model/$domid/state, wait until
state turns "running" then unpause guest.

LIBXL_STUBDOM_START_TIMEOUT is the timeout used wait for stubdom to be
ready. My test on a very old machine (Core 2 6400) showed that it might
need more than 20s before the stubdom is ready to serve DomU.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
Note:
To avoid the change of protocol causing regression, this patch should be
committed with QEMU trad patch at the same time, and please fold the
change to Config.mk in.
---
 tools/libxl/libxl.c          |  4 +++-
 tools/libxl/libxl_device.c   |  4 +++-
 tools/libxl/libxl_dm.c       | 46 +++++++++++++++++++++++++++++++++---
 tools/libxl/libxl_dom.c      | 56 +++++++++++++++++++++++++++-----------------
 tools/libxl/libxl_internal.h |  2 ++
 tools/libxl/libxl_pci.c      | 22 +++++++++--------
 6 files changed, 98 insertions(+), 36 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2a735b3..511eef1 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1033,7 +1033,9 @@ int libxl_domain_unpause(libxl_ctx *ctx, uint32_t domid)
     }
 
     if (type == LIBXL_DOMAIN_TYPE_HVM) {
-        path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid);
+        uint32_t dm_domid = libxl_get_stubdom_id(ctx, domid);
+
+        path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
         state = libxl__xs_read(gc, XBT_NULL, path);
         if (state != NULL && !strcmp(state, "paused")) {
             libxl__qemu_traditional_cmd(gc, domid, "continue");
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 0f50d04..0c06dc4 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1188,7 +1188,9 @@ int libxl__wait_for_device_model_deprecated(libxl__gc *gc,
                                  void *check_callback_userdata)
 {
     char *path;
-    path = GCSPRINTF("/local/domain/0/device-model/%d/state", domid);
+    uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
+
+    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
     return libxl__xenstore_child_wait_deprecated(gc, domid,
                                      LIBXL_DEVICE_MODEL_START_TIMEOUT,
                                      "Device Model", path, state, spawning,
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index f20565a..30c1578 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1010,6 +1010,8 @@ static void stubdom_pvqemu_cb(libxl__egc *egc,
 static void spawn_stubdom_pvqemu_destroy_cb(libxl__egc *egc,
                                             libxl__destroy_domid_state *dis,
                                             int rc);
+static void stubdom_xswait_cb(libxl__egc *egc, libxl__xswait_state *xswait,
+                              int rc, const char *p);
 
 char *libxl__stub_dm_name(libxl__gc *gc, const char *guest_name)
 {
@@ -1140,9 +1142,10 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
 retry_transaction:
     t = xs_transaction_start(ctx->xsh);
     xs_mkdir(ctx->xsh, t,
-        libxl__sprintf(gc, "/local/domain/0/device-model/%d", guest_domid));
+             libxl__device_model_xs_path(gc, dm_domid, guest_domid, ""));
     xs_set_permissions(ctx->xsh, t,
-        libxl__sprintf(gc, "/local/domain/0/device-model/%d", guest_domid),
+                       libxl__device_model_xs_path(gc, dm_domid,
+                                                   guest_domid, ""),
                        perm, ARRAY_SIZE(perm));
     if (!xs_transaction_end(ctx->xsh, t, 0))
         if (errno == EAGAIN)
@@ -1288,6 +1291,8 @@ static void stubdom_pvqemu_cb(libxl__egc *egc,
     STATE_AO_GC(sdss->dm.spawn.ao);
     uint32_t dm_domid = sdss->pvqemu.guest_domid;
 
+    libxl__xswait_init(&sdss->xswait);
+
     if (rc) {
         LOGE(ERROR, "error connecting nics devices");
         goto out;
@@ -1296,10 +1301,45 @@ static void stubdom_pvqemu_cb(libxl__egc *egc,
     rc = libxl_domain_unpause(CTX, dm_domid);
     if (rc) goto out;
 
+    sdss->xswait.ao = ao;
+    sdss->xswait.what = GCSPRINTF("Stubdom %u for %u startup",
+                                  dm_domid, sdss->dm.guest_domid);
+    sdss->xswait.path =
+        libxl__device_model_xs_path(gc, dm_domid, sdss->dm.guest_domid,
+                                    "/state");
+    sdss->xswait.timeout_ms = LIBXL_STUBDOM_START_TIMEOUT * 1000;
+    sdss->xswait.callback = stubdom_xswait_cb;
+    rc = libxl__xswait_start(gc, &sdss->xswait);
+    if (rc) goto out;
+
+    return;
+
+ out:
+    stubdom_xswait_cb(egc, &sdss->xswait, rc, NULL);
+}
+
+static void stubdom_xswait_cb(libxl__egc *egc, libxl__xswait_state *xswait,
+                              int rc, const char *p)
+{
+    EGC_GC;
+    libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(xswait, *sdss, xswait);
+    uint32_t dm_domid = sdss->pvqemu.guest_domid;
+
+    if (rc) {
+        if (rc == ERROR_TIMEDOUT)
+            LOG(ERROR, "%s: startup timed out", xswait->what);
+        goto out;
+    }
+
+    if (!p) return;
+
+    if (strcmp(p, "running"))
+        return;
  out:
+    libxl__xswait_stop(gc, xswait);
     if (rc) {
         if (dm_domid) {
-            sdss->dis.ao = ao;
+            sdss->dis.ao = sdss->dm.spawn.ao;
             sdss->dis.domid = dm_domid;
             sdss->dis.callback = spawn_stubdom_pvqemu_destroy_cb;
             libxl__destroy_domid(egc, &sdss->dis);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index a88db69..9711fb6 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1010,7 +1010,8 @@ int libxl__qemu_traditional_cmd(libxl__gc *gc, uint32_t domid,
                                 const char *cmd)
 {
     char *path = NULL;
-    path = GCSPRINTF("/local/domain/0/device-model/%d/command", domid);
+    uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
+    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/command");
     return libxl__xs_write(gc, XBT_NULL, path, "%s", cmd);
 }
 
@@ -1024,11 +1025,13 @@ struct libxl__physmap_info {
 
 #define TOOLSTACK_SAVE_VERSION 1
 
-static inline char *restore_helper(libxl__gc *gc, uint32_t domid,
-        uint64_t phys_offset, char *node)
+static inline char *restore_helper(libxl__gc *gc, uint32_t dm_domid,
+                                   uint32_t domid,
+                                   uint64_t phys_offset, char *node)
 {
-    return GCSPRINTF("/local/domain/0/device-model/%d/physmap/%"PRIx64"/%s",
-            domid, phys_offset, node);
+    return libxl__device_model_xs_path(gc, dm_domid, domid,
+                                       "/physmap/%"PRIx64"/%s",
+                                       phys_offset, node);
 }
 
 int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
@@ -1042,6 +1045,7 @@ int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
     uint32_t count = 0, version = 0;
     struct libxl__physmap_info* pi;
     char *xs_path;
+    uint32_t dm_domid;
 
     LOG(DEBUG,"domain=%"PRIu32" toolstack data size=%"PRIu32, domid, size);
 
@@ -1067,20 +1071,23 @@ int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
         return -1;
     }
 
+    dm_domid = libxl_get_stubdom_id(CTX, domid);
     for (i = 0; i < count; i++) {
         pi = (struct libxl__physmap_info*) ptr;
         ptr += sizeof(struct libxl__physmap_info) + pi->namelen;
 
-        xs_path = restore_helper(gc, domid, pi->phys_offset, "start_addr");
+        xs_path = restore_helper(gc, dm_domid, domid,
+                                 pi->phys_offset, "start_addr");
         ret = libxl__xs_write(gc, 0, xs_path, "%"PRIx64, pi->start_addr);
         if (ret)
             return -1;
-        xs_path = restore_helper(gc, domid, pi->phys_offset, "size");
+        xs_path = restore_helper(gc, dm_domid, domid, pi->phys_offset, "size");
         ret = libxl__xs_write(gc, 0, xs_path, "%"PRIx64, pi->size);
         if (ret)
             return -1;
         if (pi->namelen > 0) {
-            xs_path = restore_helper(gc, domid, pi->phys_offset, "name");
+            xs_path = restore_helper(gc, dm_domid, domid,
+                                     pi->phys_offset, "name");
             ret = libxl__xs_write(gc, 0, xs_path, "%s", pi->name);
             if (ret)
                 return -1;
@@ -1133,10 +1140,11 @@ static void domain_suspend_switch_qemu_xen_traditional_logdirty
     const char *got;
 
     if (!lds->cmd_path) {
-        lds->cmd_path = GCSPRINTF(
-                   "/local/domain/0/device-model/%u/logdirty/cmd", domid);
-        lds->ret_path = GCSPRINTF(
-                   "/local/domain/0/device-model/%u/logdirty/ret", domid);
+        uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
+        lds->cmd_path = libxl__device_model_xs_path(gc, dm_domid, domid,
+                                                    "/logdirty/cmd");
+        lds->ret_path = libxl__device_model_xs_path(gc, dm_domid, domid,
+                                                    "/logdirty/ret");
     }
     lds->cmd = enable ? "enable" : "disable";
 
@@ -1655,11 +1663,13 @@ static void domain_suspend_common_done(libxl__egc *egc,
     dss->callback_common_done(egc, dss, ok);
 }
 
-static inline char *physmap_path(libxl__gc *gc, uint32_t domid,
-        char *phys_offset, char *node)
+static inline char *physmap_path(libxl__gc *gc, uint32_t dm_domid,
+                                 uint32_t domid,
+                                 char *phys_offset, char *node)
 {
-    return GCSPRINTF("/local/domain/0/device-model/%d/physmap/%s/%s",
-            domid, phys_offset, node);
+    return libxl__device_model_xs_path(gc, dm_domid, domid,
+                                       "/physmap/%s/%s",
+                                       phys_offset, node);
 }
 
 int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
@@ -1674,9 +1684,13 @@ int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
     uint8_t *ptr = NULL;
     char **entries = NULL;
     struct libxl__physmap_info *pi;
+    uint32_t dm_domid;
+
+    dm_domid = libxl_get_stubdom_id(CTX, domid);
 
-    entries = libxl__xs_directory(gc, 0, GCSPRINTF(
-                "/local/domain/0/device-model/%d/physmap", domid), &num);
+    entries = libxl__xs_directory(gc, 0,
+                libxl__device_model_xs_path(gc, dm_domid, domid, "/physmap"),
+                &num);
     count = num;
 
     *len = sizeof(version) + sizeof(count);
@@ -1699,21 +1713,21 @@ int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
             return -1;
         }
 
-        xs_path = physmap_path(gc, domid, phys_offset, "start_addr");
+        xs_path = physmap_path(gc, dm_domid, domid, phys_offset, "start_addr");
         start_addr = libxl__xs_read(gc, 0, xs_path);
         if (start_addr == NULL) {
             LOG(ERROR, "%s is NULL", xs_path);
             return -1;
         }
 
-        xs_path = physmap_path(gc, domid, phys_offset, "size");
+        xs_path = physmap_path(gc, dm_domid, domid, phys_offset, "size");
         size = libxl__xs_read(gc, 0, xs_path);
         if (size == NULL) {
             LOG(ERROR, "%s is NULL", xs_path);
             return -1;
         }
 
-        xs_path = physmap_path(gc, domid, phys_offset, "name");
+        xs_path = physmap_path(gc, dm_domid, domid, phys_offset, "name");
         name = libxl__xs_read(gc, 0, xs_path);
         if (name == NULL)
             namelen = 0;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 3aba221..9c22309 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -86,6 +86,7 @@
 #define LIBXL_DESTROY_TIMEOUT 10
 #define LIBXL_HOTPLUG_TIMEOUT 10
 #define LIBXL_DEVICE_MODEL_START_TIMEOUT 10
+#define LIBXL_STUBDOM_START_TIMEOUT 30
 #define LIBXL_QEMU_BODGE_TIMEOUT 2
 #define LIBXL_XENCONSOLE_LIMIT 1048576
 #define LIBXL_XENCONSOLE_PROTOCOL "vt100"
@@ -3075,6 +3076,7 @@ typedef struct {
     libxl__dm_spawn_state pvqemu;
     libxl__destroy_domid_state dis;
     libxl__multidev multidev;
+    libxl__xswait_state xswait;
 } libxl__stub_dm_spawn_state;
 
 _hidden void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state*);
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index f3ae132..394f61c 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -850,11 +850,12 @@ static int qemu_pci_add_xenstore(libxl__gc *gc, uint32_t domid,
     int rc = 0;
     char *path;
     char *state, *vdevfn;
+    uint32_t dm_domid;
 
-    path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid);
+    dm_domid = libxl_get_stubdom_id(CTX, domid);
+    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
     state = libxl__xs_read(gc, XBT_NULL, path);
-    path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/parameter",
-                          domid);
+    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/parameter");
     if (pcidev->vdevfn) {
         libxl__xs_write(gc, XBT_NULL, path, PCI_BDF_VDEVFN","PCI_OPTIONS,
                         pcidev->domain, pcidev->bus, pcidev->dev,
@@ -869,11 +870,9 @@ static int qemu_pci_add_xenstore(libxl__gc *gc, uint32_t domid,
     libxl__qemu_traditional_cmd(gc, domid, "pci-ins");
     rc = libxl__wait_for_device_model_deprecated(gc, domid, NULL, NULL,
                                       pci_ins_check, state);
-    path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/parameter",
-                          domid);
+    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/parameter");
     vdevfn = libxl__xs_read(gc, XBT_NULL, path);
-    path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state",
-                          domid);
+    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
     if ( rc < 0 )
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
                    "qemu refused to add device: %s", vdevfn);
@@ -1175,10 +1174,13 @@ static int qemu_pci_remove_xenstore(libxl__gc *gc, uint32_t domid,
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char *state;
     char *path;
+    uint32_t dm_domid;
 
-    path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid);
+    dm_domid = libxl_get_stubdom_id(CTX, domid);
+
+    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
     state = libxl__xs_read(gc, XBT_NULL, path);
-    path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/parameter", domid);
+    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/parameter");
     libxl__xs_write(gc, XBT_NULL, path, PCI_BDF, pcidev->domain,
                     pcidev->bus, pcidev->dev, pcidev->func);
 
@@ -1196,7 +1198,7 @@ static int qemu_pci_remove_xenstore(libxl__gc *gc, uint32_t domid,
             return ERROR_FAIL;
         }
     }
-    path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid);
+    path = libxl__device_model_xs_path(gc, dm_domid, domid, "/state");
     xs_write(ctx->xsh, XBT_NULL, path, state, strlen(state));
 
     return 0;
-- 
1.9.1

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

* [PATCH v4 3/3] Revert "x86/hvm: wait for at least one ioreq server to be enabled"
  2015-04-09 18:49 [PATCH v4 0/3] Use new QEMU xenstore protocol Wei Liu
  2015-04-09 18:49 ` [PATCH v4 1/3] qemu-trad: xenstore: use relative path for device-model node Wei Liu
  2015-04-09 18:49 ` [PATCH v4 2/3] libxl: use new QEMU xenstore protocol Wei Liu
@ 2015-04-09 18:49 ` Wei Liu
  2015-04-13 14:56   ` Paul Durrant
  2 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2015-04-09 18:49 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Paul Durrant, ian.jackson, ian.campbell, Jan Beulich

This reverts commit dd748d128d86996592afafea02e578cc7d4e6d42.

We don't need this workaround anymore since we have fixed the toolstack
interlock problem that affects stubdom.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Paul Durrant <Paul.Durrant@citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/x86/hvm/hvm.c           | 21 ---------------------
 xen/include/asm-x86/hvm/domain.h |  1 -
 2 files changed, 22 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index bfde380..8b62296 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -893,13 +893,6 @@ static void hvm_ioreq_server_enable(struct hvm_ioreq_server *s,
 
   done:
     spin_unlock(&s->lock);
-
-    /* This check is protected by the domain ioreq server lock. */
-    if ( d->arch.hvm_domain.ioreq_server.waiting )
-    {
-        d->arch.hvm_domain.ioreq_server.waiting = 0;
-        domain_unpause(d);
-    }
 }
 
 static void hvm_ioreq_server_disable(struct hvm_ioreq_server *s,
@@ -1451,20 +1444,6 @@ int hvm_domain_initialise(struct domain *d)
 
     spin_lock_init(&d->arch.hvm_domain.ioreq_server.lock);
     INIT_LIST_HEAD(&d->arch.hvm_domain.ioreq_server.list);
-    
-    /*
-     * In the case where a stub domain is providing emulation for
-     * the guest, there is no interlock in the toolstack to prevent
-     * the guest from running before the stub domain is ready.
-     * Hence the domain must remain paused until at least one ioreq
-     * server is created and enabled.
-     */
-    if ( !is_pvh_domain(d) )
-    {
-        domain_pause(d);
-        d->arch.hvm_domain.ioreq_server.waiting = 1;
-    }
-
     spin_lock_init(&d->arch.hvm_domain.irq_lock);
     spin_lock_init(&d->arch.hvm_domain.uc_lock);
 
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index 0702bf5..2757c7f 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -83,7 +83,6 @@ struct hvm_domain {
     struct {
         spinlock_t       lock;
         ioservid_t       id;
-        bool_t           waiting;
         struct list_head list;
     } ioreq_server;
     struct hvm_ioreq_server *default_ioreq_server;
-- 
1.9.1

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

* Re: [PATCH v4 2/3] libxl: use new QEMU xenstore protocol
  2015-04-09 18:49 ` [PATCH v4 2/3] libxl: use new QEMU xenstore protocol Wei Liu
@ 2015-04-09 18:56   ` Ian Jackson
  0 siblings, 0 replies; 6+ messages in thread
From: Ian Jackson @ 2015-04-09 18:56 UTC (permalink / raw)
  To: Wei Liu; +Cc: Paul Durrant, ian.campbell, Jan Beulich, xen-devel

Wei Liu writes ("[PATCH v4 2/3] libxl: use new QEMU xenstore protocol"):
> Originally both QEMU traditional and QEMU upstream used hardcoded
> /local/domain/0 paths. This patch changes the protocol to use
> /local/domain/$dm_domid path.

Thanks.  I have committed 1/2 and 2/2 (the latter with the appropriate
Config.mk change, and a minor edit to the commit message).

I will leave 3/3 until it has an appropriate ack.

Thanks,
Ian.

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

* Re: [PATCH v4 3/3] Revert "x86/hvm: wait for at least one ioreq server to be enabled"
  2015-04-09 18:49 ` [PATCH v4 3/3] Revert "x86/hvm: wait for at least one ioreq server to be enabled" Wei Liu
@ 2015-04-13 14:56   ` Paul Durrant
  0 siblings, 0 replies; 6+ messages in thread
From: Paul Durrant @ 2015-04-13 14:56 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Wei Liu, Ian Campbell, Jan Beulich

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 09 April 2015 19:49
> To: xen-devel@lists.xen.org
> Cc: Ian Campbell; Ian Jackson; Wei Liu; Paul Durrant; Jan Beulich
> Subject: [PATCH v4 3/3] Revert "x86/hvm: wait for at least one ioreq server
> to be enabled"
> 
> This reverts commit dd748d128d86996592afafea02e578cc7d4e6d42.
> 
> We don't need this workaround anymore since we have fixed the toolstack
> interlock problem that affects stubdom.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Paul Durrant <Paul.Durrant@citrix.com>

Assuming the toolstack bits are ok, I'm happy with this.

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> Cc: Jan Beulich <JBeulich@suse.com>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/x86/hvm/hvm.c           | 21 ---------------------
>  xen/include/asm-x86/hvm/domain.h |  1 -
>  2 files changed, 22 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index bfde380..8b62296 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -893,13 +893,6 @@ static void hvm_ioreq_server_enable(struct
> hvm_ioreq_server *s,
> 
>    done:
>      spin_unlock(&s->lock);
> -
> -    /* This check is protected by the domain ioreq server lock. */
> -    if ( d->arch.hvm_domain.ioreq_server.waiting )
> -    {
> -        d->arch.hvm_domain.ioreq_server.waiting = 0;
> -        domain_unpause(d);
> -    }
>  }
> 
>  static void hvm_ioreq_server_disable(struct hvm_ioreq_server *s,
> @@ -1451,20 +1444,6 @@ int hvm_domain_initialise(struct domain *d)
> 
>      spin_lock_init(&d->arch.hvm_domain.ioreq_server.lock);
>      INIT_LIST_HEAD(&d->arch.hvm_domain.ioreq_server.list);
> -
> -    /*
> -     * In the case where a stub domain is providing emulation for
> -     * the guest, there is no interlock in the toolstack to prevent
> -     * the guest from running before the stub domain is ready.
> -     * Hence the domain must remain paused until at least one ioreq
> -     * server is created and enabled.
> -     */
> -    if ( !is_pvh_domain(d) )
> -    {
> -        domain_pause(d);
> -        d->arch.hvm_domain.ioreq_server.waiting = 1;
> -    }
> -
>      spin_lock_init(&d->arch.hvm_domain.irq_lock);
>      spin_lock_init(&d->arch.hvm_domain.uc_lock);
> 
> diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-
> x86/hvm/domain.h
> index 0702bf5..2757c7f 100644
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -83,7 +83,6 @@ struct hvm_domain {
>      struct {
>          spinlock_t       lock;
>          ioservid_t       id;
> -        bool_t           waiting;
>          struct list_head list;
>      } ioreq_server;
>      struct hvm_ioreq_server *default_ioreq_server;
> --
> 1.9.1

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

end of thread, other threads:[~2015-04-13 14:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-09 18:49 [PATCH v4 0/3] Use new QEMU xenstore protocol Wei Liu
2015-04-09 18:49 ` [PATCH v4 1/3] qemu-trad: xenstore: use relative path for device-model node Wei Liu
2015-04-09 18:49 ` [PATCH v4 2/3] libxl: use new QEMU xenstore protocol Wei Liu
2015-04-09 18:56   ` Ian Jackson
2015-04-09 18:49 ` [PATCH v4 3/3] Revert "x86/hvm: wait for at least one ioreq server to be enabled" Wei Liu
2015-04-13 14:56   ` Paul Durrant

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.