thr3ads.net - Xen devel - [Xen-devel] [PATCH 0/7] libxl: QMP client improvement + pci passthrougth insert through QMP [Oct 2011]

If this information is useful, please help other people find it:
Share via:

Anthony PERARD

2011-Oct-07 12:10 UTC

[Xen-devel] [PATCH 0/7] libxl: QMP client improvement + pci passthrougth insert through QMP

This patch series improve the QMP client in lib XenLight to be able to insert a
PCI passthrough device with the upstream QEMU. This require to apply the patch
(RFC) series post earlier this week for QEMU.

The first patch create a key in xenstore with the version of the running device
model, here: /local/domain/$domid/dm-version

The patch series require one patch from Ian:
"libxl: IDL: autogenerate functions to produce JSON from libxl data
structures"

Anthony PERARD (7):
  libxl, Introduce dm-version xenstore key.
  libxl_qmp, Introduce an opaque argument to the callbacks.
  libxl_qmp, Introduce list of arguments to qmp_send
  libxl_qmp, Always insert a command id in the callback_list.
  libxl_qmp, Return the callback return code in qmp_next.
  libxl_qmp, Introduce libxl__qmp_pci_add.
  libxl, Use QMP to insert a passthrough device when using upstream
    QEMU

 tools/libxl/libxl_create.c   |    5 ++
 tools/libxl/libxl_internal.c |   19 +++++
 tools/libxl/libxl_internal.h |    7 ++
 tools/libxl/libxl_pci.c      |   58 ++++++++++------
 tools/libxl/libxl_qmp.c      |  164 ++++++++++++++++++++++++++++++++++-------
 5 files changed, 204 insertions(+), 49 deletions(-)

-- 
Anthony PERARD


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

Anthony PERARD

2011-Oct-07 12:10 UTC

head link

[Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

The key is /local/domain/$domid/dm-version.
This come with libxl__device_model_version_running helper function.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_create.c   |    5 +++++
 tools/libxl/libxl_internal.c |   19 +++++++++++++++++++
 tools/libxl/libxl_internal.h |    5 +++++
 3 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index e1e3258..3f33d90 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -175,6 +175,11 @@ int libxl__domain_build(libxl__gc *gc,
 
     gettimeofday(&start_time, NULL);
 
+    localents = libxl__calloc(gc, 3, sizeof (char *));
+    i = 0;
+    localents[i++] = "dm-version";
+    localents[i++] = libxl__strdup(gc,
libxl_device_model_version_to_string(dm_info->device_model_version));
+
     switch (info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
         ret = libxl__build_hvm(gc, domid, info, dm_info, state);
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index 3b8a41f..e535c0c 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -318,3 +318,22 @@ int libxl__fd_set_cloexec(int fd)
     }
     return fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
 }
+
+libxl_device_model_version libxl__device_model_version_running(libxl__gc *gc,
+                                                               uint32_t domid)
+{
+    char *path = NULL;
+    char *dm_version = NULL;
+    libxl_device_model_version value;
+
+    path = libxl__sprintf(gc, "/local/domain/%d/dm-version", domid);
+    dm_version = libxl__xs_read(gc, XBT_NULL, path);
+    if (!dm_version) {
+        return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+    }
+
+    if (libxl_device_model_version_from_string(dm_version, &value) < 0)
{
+        return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+    }
+    return value;
+}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 935c899..4dd0f91 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -554,4 +554,9 @@ _hidden void libxl__json_object_free(libxl__gc *gc,
libxl__json_object *obj);
 
 _hidden libxl__json_object *libxl__json_parse(libxl__gc *gc, const char *s);
 
+  /* Based on /local/domain/$domid/dm-version xenstore key
+   * default is qemu xen traditional */
+_hidden libxl_device_model_version
+libxl__device_model_version_running(libxl__gc *gc, uint32_t domid);
+
 #endif
-- 
Anthony PERARD


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

Anthony PERARD

2011-Oct-07 12:10 UTC

head link

[Xen-devel] [PATCH 2/7] libxl_qmp, Introduce an opaque argument to the callbacks.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_qmp.c |   29 +++++++++++++++++++----------
 1 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 34db29f..5ab9acc 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -43,11 +43,13 @@
 #define QMP_RECEIVE_BUFFER_SIZE 4096
 
 typedef int (*qmp_callback_t)(libxl__qmp_handler *qmp,
-                              const libxl__json_object *tree);
+                              const libxl__json_object *tree,
+                              void *opaque);
 
 typedef struct callback_id_pair {
     int id;
     qmp_callback_t callback;
+    void *opaque;
     SIMPLEQ_ENTRY(callback_id_pair) next;
 } callback_id_pair;
 
@@ -70,7 +72,8 @@ struct libxl__qmp_handler {
 };
 
 static int qmp_send(libxl__qmp_handler *qmp,
-                    const char *cmd, qmp_callback_t callback);
+                    const char *cmd,
+                    qmp_callback_t callback, void *opaque);
 
 static const int QMP_SOCKET_CONNECT_TIMEOUT = 5;
 
@@ -100,7 +103,8 @@ static int store_serial_port_info(libxl__qmp_handler *qmp,
 }
 
 static int register_serials_chardev_callback(libxl__qmp_handler *qmp,
-                                             const libxl__json_object *o)
+                                             const libxl__json_object *o,
+                                             void *unused)
 {
     const libxl__json_object *obj = NULL;
     const libxl__json_object *label = NULL;
@@ -144,7 +148,7 @@ static int
register_serials_chardev_callback(libxl__qmp_handler *qmp,
 }
 
 static int qmp_capabilities_callback(libxl__qmp_handler *qmp,
-                                     const libxl__json_object *o)
+                                     const libxl__json_object *o, void *unused)
 {
     qmp->connected = true;
 
@@ -157,7 +161,7 @@ static int qmp_capabilities_callback(libxl__qmp_handler
*qmp,
 
 static int enable_qmp_capabilities(libxl__qmp_handler *qmp)
 {
-    return qmp_send(qmp, "qmp_capabilities",
qmp_capabilities_callback);
+    return qmp_send(qmp, "qmp_capabilities",
qmp_capabilities_callback, NULL);
 }
 
 /*
@@ -208,7 +212,7 @@ static void qmp_handle_error_response(libxl__qmp_handler
*qmp,
     resp = libxl__json_map_get("desc", resp, JSON_STRING);
 
     if (pp) {
-        pp->callback(qmp, NULL);
+        pp->callback(qmp, NULL, pp->opaque);
         if (pp->id == qmp->wait_for_id) {
             /* tell that the id have been processed */
             qmp->wait_for_id = 0;
@@ -241,7 +245,8 @@ static int qmp_handle_response(libxl__qmp_handler *qmp,
 
         if (pp) {
             pp->callback(qmp,
-                         libxl__json_map_get("return", resp,
JSON_ANY));
+                         libxl__json_map_get("return", resp,
JSON_ANY),
+                         pp->opaque);
             if (pp->id == qmp->wait_for_id) {
                 /* tell that the id have been processed */
                 qmp->wait_for_id = 0;
@@ -423,7 +428,8 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
 }
 
 static int qmp_send(libxl__qmp_handler *qmp,
-                    const char *cmd, qmp_callback_t callback)
+                    const char *cmd,
+                    qmp_callback_t callback, void *opaque)
 {
     yajl_gen_config conf = { 0, NULL };
     const unsigned char *buf;
@@ -461,6 +467,7 @@ static int qmp_send(libxl__qmp_handler *qmp,
         }
         elm->id = qmp->last_id_used;
         elm->callback = callback;
+        elm->opaque = opaque;
         SIMPLEQ_INSERT_TAIL(&qmp->callback_list, elm, next);
     }
 
@@ -483,13 +490,14 @@ error:
 }
 
 static int qmp_synchronous_send(libxl__qmp_handler *qmp, const char *cmd,
-                                qmp_callback_t callback, int ask_timeout)
+                                qmp_callback_t callback, void *opaque,
+                                int ask_timeout)
 {
     int id = 0;
     int ret = 0;
     libxl__gc gc = LIBXL_INIT_GC(qmp->ctx);
 
-    id = qmp_send(qmp, cmd, callback);
+    id = qmp_send(qmp, cmd, callback, opaque);
     if (id <= 0) {
         return -1;
     }
@@ -579,6 +587,7 @@ int libxl__qmp_query_serial(libxl__qmp_handler *qmp)
 {
     return qmp_synchronous_send(qmp, "query-chardev",
                                 register_serials_chardev_callback,
+                                NULL,
                                 qmp->timeout);
 }
 
-- 
Anthony PERARD


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

Anthony PERARD

2011-Oct-07 12:10 UTC

head link

[Xen-devel] [PATCH 3/7] libxl_qmp, Introduce list of arguments to qmp_send

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_qmp.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 5ab9acc..002fb13 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -72,7 +72,7 @@ struct libxl__qmp_handler {
 };
 
 static int qmp_send(libxl__qmp_handler *qmp,
-                    const char *cmd,
+                    const char *cmd, libxl_key_value_list *args,
                     qmp_callback_t callback, void *opaque);
 
 static const int QMP_SOCKET_CONNECT_TIMEOUT = 5;
@@ -161,7 +161,8 @@ static int qmp_capabilities_callback(libxl__qmp_handler
*qmp,
 
 static int enable_qmp_capabilities(libxl__qmp_handler *qmp)
 {
-    return qmp_send(qmp, "qmp_capabilities",
qmp_capabilities_callback, NULL);
+    return qmp_send(qmp, "qmp_capabilities", NULL,
+                    qmp_capabilities_callback, NULL);
 }
 
 /*
@@ -428,7 +429,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
 }
 
 static int qmp_send(libxl__qmp_handler *qmp,
-                    const char *cmd,
+                    const char *cmd, libxl_key_value_list *args,
                     qmp_callback_t callback, void *opaque)
 {
     yajl_gen_config conf = { 0, NULL };
@@ -447,6 +448,10 @@ static int qmp_send(libxl__qmp_handler *qmp,
     libxl__yajl_gen_asciiz(hand, cmd);
     libxl__yajl_gen_asciiz(hand, "id");
     yajl_gen_integer(hand, ++qmp->last_id_used);
+    if (args) {
+        libxl__yajl_gen_asciiz(hand, "arguments");
+        libxl_key_value_list_gen_json(hand, args);
+    }
     yajl_gen_map_close(hand);
 
     s = yajl_gen_get_buf(hand, &buf, &len);
@@ -490,6 +495,7 @@ error:
 }
 
 static int qmp_synchronous_send(libxl__qmp_handler *qmp, const char *cmd,
+                                libxl_key_value_list *args,
                                 qmp_callback_t callback, void *opaque,
                                 int ask_timeout)
 {
@@ -497,7 +503,7 @@ static int qmp_synchronous_send(libxl__qmp_handler *qmp,
const char *cmd,
     int ret = 0;
     libxl__gc gc = LIBXL_INIT_GC(qmp->ctx);
 
-    id = qmp_send(qmp, cmd, callback, opaque);
+    id = qmp_send(qmp, cmd, args, callback, opaque);
     if (id <= 0) {
         return -1;
     }
@@ -585,7 +591,7 @@ void libxl__qmp_cleanup(libxl__gc *gc, uint32_t domid)
 
 int libxl__qmp_query_serial(libxl__qmp_handler *qmp)
 {
-    return qmp_synchronous_send(qmp, "query-chardev",
+    return qmp_synchronous_send(qmp, "query-chardev", NULL,
                                 register_serials_chardev_callback,
                                 NULL,
                                 qmp->timeout);
-- 
Anthony PERARD


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

Anthony PERARD

2011-Oct-07 12:10 UTC

head link

[Xen-devel] [PATCH 4/7] libxl_qmp, Always insert a command id in the callback_list.

Because the function qmp_synchronous_send rely on the presence of the id
in the callback_list.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_qmp.c |   34 ++++++++++++++++++----------------
 1 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 002fb13..1594a4f 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -213,7 +213,9 @@ static void qmp_handle_error_response(libxl__qmp_handler
*qmp,
     resp = libxl__json_map_get("desc", resp, JSON_STRING);
 
     if (pp) {
-        pp->callback(qmp, NULL, pp->opaque);
+        if (pp->callback) {
+            pp->callback(qmp, NULL, pp->opaque);
+        }
         if (pp->id == qmp->wait_for_id) {
             /* tell that the id have been processed */
             qmp->wait_for_id = 0;
@@ -245,9 +247,11 @@ static int qmp_handle_response(libxl__qmp_handler *qmp,
         callback_id_pair *pp = qmp_get_callback_from_id(qmp, resp);
 
         if (pp) {
-            pp->callback(qmp,
-                         libxl__json_map_get("return", resp,
JSON_ANY),
-                         pp->opaque);
+            if (pp->callback) {
+                pp->callback(qmp,
+                             libxl__json_map_get("return", resp,
JSON_ANY),
+                             pp->opaque);
+            }
             if (pp->id == qmp->wait_for_id) {
                 /* tell that the id have been processed */
                 qmp->wait_for_id = 0;
@@ -437,6 +441,7 @@ static int qmp_send(libxl__qmp_handler *qmp,
     unsigned int len = 0;
     yajl_gen_status s;
     yajl_gen hand;
+    callback_id_pair *elm = NULL;
 
     hand = yajl_gen_alloc(&conf, NULL);
     if (!hand) {
@@ -462,19 +467,16 @@ static int qmp_send(libxl__qmp_handler *qmp,
         return -1;
     }
 
-    if (callback) {
-        callback_id_pair *elm = malloc(sizeof (callback_id_pair));
-        if (elm == NULL) {
-            LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR,
-                             "Failed to allocate a QMP callback");
-            yajl_gen_free(hand);
-            return -1;
-        }
-        elm->id = qmp->last_id_used;
-        elm->callback = callback;
-        elm->opaque = opaque;
-        SIMPLEQ_INSERT_TAIL(&qmp->callback_list, elm, next);
+    elm = malloc(sizeof (callback_id_pair));
+    if (elm == NULL) {
+        LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR,
+                         "Failed to allocate a QMP callback");
+        goto error;
     }
+    elm->id = qmp->last_id_used;
+    elm->callback = callback;
+    elm->opaque = opaque;
+    SIMPLEQ_INSERT_TAIL(&qmp->callback_list, elm, next);
 
     LIBXL__LOG(qmp->ctx, LIBXL__LOG_DEBUG, "next qmp command:
''%s''", buf);
 
-- 
Anthony PERARD


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

Anthony PERARD

2011-Oct-07 12:10 UTC

head link

[Xen-devel] [PATCH 5/7] libxl_qmp, Return the callback return code in qmp_next.

So, if there is an error in the answer given by QEMU, the function
qmp_synchronous_send while know.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_qmp.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 1594a4f..cd3e4e4 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -233,6 +233,7 @@ static int qmp_handle_response(libxl__qmp_handler *qmp,
                                const libxl__json_object *resp)
 {
     libxl__qmp_message_type type = LIBXL__QMP_MESSAGE_TYPE_INVALID;
+    int rc = 0;
 
     type = qmp_response_type(qmp, resp);
     LIBXL__LOG(qmp->ctx, LIBXL__LOG_DEBUG,
@@ -241,14 +242,14 @@ static int qmp_handle_response(libxl__qmp_handler *qmp,
     switch (type) {
     case LIBXL__QMP_MESSAGE_TYPE_QMP:
         /* On the greeting message from the server, enable QMP capabilities */
-        enable_qmp_capabilities(qmp);
+        rc = enable_qmp_capabilities(qmp);
         break;
     case LIBXL__QMP_MESSAGE_TYPE_RETURN: {
         callback_id_pair *pp = qmp_get_callback_from_id(qmp, resp);
 
         if (pp) {
             if (pp->callback) {
-                pp->callback(qmp,
+                rc = pp->callback(qmp,
                              libxl__json_map_get("return", resp,
JSON_ANY),
                              pp->opaque);
             }
@@ -263,13 +264,13 @@ static int qmp_handle_response(libxl__qmp_handler *qmp,
     }
     case LIBXL__QMP_MESSAGE_TYPE_ERROR:
         qmp_handle_error_response(qmp, resp);
-        break;
+        return -1;
     case LIBXL__QMP_MESSAGE_TYPE_EVENT:
         break;
     case LIBXL__QMP_MESSAGE_TYPE_INVALID:
         return -1;
     }
-    return 0;
+    return rc;
 }
 
 /*
@@ -358,6 +359,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
 
     char *incomplete = NULL;
     size_t incomplete_size = 0;
+    int rc = 0;
 
     do {
         fd_set rfds;
@@ -416,7 +418,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
                 s = end + 2;
 
                 if (o) {
-                    qmp_handle_response(qmp, o);
+                    rc = qmp_handle_response(qmp, o);
                     libxl__json_object_free(gc, o);
                 } else {
                     LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR,
@@ -429,7 +431,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
         } while (s < s_end);
    } while (s < s_end);
 
-    return 1;
+    return rc;
 }
 
 static int qmp_send(libxl__qmp_handler *qmp,
-- 
Anthony PERARD


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

Anthony PERARD

2011-Oct-07 12:10 UTC

head link

[Xen-devel] [PATCH 6/7] libxl_qmp, Introduce libxl__qmp_pci_add.

This function insert a PCI passthrough device in qemu.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_internal.h |    2 +
 tools/libxl/libxl_qmp.c      |   89 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 4dd0f91..78e1be2 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -119,6 +119,7 @@ typedef struct {
 } libxl__device;
 
 #define XC_PCI_BDF             "0x%x, 0x%x, 0x%x, 0x%x"
+#define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func)
& 0x07))
 #define AUTO_PHP_SLOT          0x100
 #define SYSFS_PCI_DEV          "/sys/bus/pci/devices"
 #define SYSFS_PCIBACK_DRIVER   "/sys/bus/pci/drivers/pciback"
@@ -444,6 +445,7 @@ _hidden libxl__qmp_handler *libxl__qmp_initialize(libxl_ctx
*ctx,
                                                   uint32_t domid);
 /* ask to QEMU the serial port information and store it in xenstore. */
 _hidden int libxl__qmp_query_serial(libxl__qmp_handler *qmp);
+_hidden int libxl__qmp_pci_add(libxl__gc *gc, int d, libxl_device_pci *pcidev);
 /* close and free the QMP handler */
 _hidden void libxl__qmp_close(libxl__qmp_handler *qmp);
 /* remove the socket file, if the file has already been removed,
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index cd3e4e4..c39a1ac 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -41,6 +41,7 @@
  */
 
 #define QMP_RECEIVE_BUFFER_SIZE 4096
+#define PCI_PT_QDEV_ID "pci-pt-%02x_%02x.%01x"
 
 typedef int (*qmp_callback_t)(libxl__qmp_handler *qmp,
                               const libxl__json_object *tree,
@@ -601,6 +602,94 @@ int libxl__qmp_query_serial(libxl__qmp_handler *qmp)
                                 qmp->timeout);
 }
 
+static int pci_add_callback(libxl__qmp_handler *qmp,
+                            const libxl__json_object *response, void *opaque)
+{
+    libxl_device_pci *pcidev = opaque;
+    const libxl__json_object *bus = NULL;
+    libxl__gc gc = LIBXL_INIT_GC(qmp->ctx);
+    int i, j, rc = -1;
+    char *asked_id = libxl__sprintf(&gc, PCI_PT_QDEV_ID,
+                                    pcidev->bus, pcidev->dev,
pcidev->func);
+
+    for (i = 0; (bus = libxl__json_array_get(response, i)); i++) {
+        const libxl__json_object *devices = NULL;
+        const libxl__json_object *device = NULL;
+        const libxl__json_object *o = NULL;
+        const char *id = NULL;
+
+        devices = libxl__json_map_get("devices", bus, JSON_ARRAY);
+
+        for (j = 0; (device = libxl__json_array_get(devices, j)); j++) {
+             o = libxl__json_map_get("qdev_id", device, JSON_STRING);
+             id = libxl__json_object_get_string(o);
+
+             if (id && strcmp(asked_id, id) == 0) {
+                 int dev_slot, dev_func;
+
+                 o = libxl__json_map_get("slot", device,
JSON_INTEGER);
+                 if (!o)
+                     goto out;
+                 dev_slot = libxl__json_object_get_integer(o);
+                 o = libxl__json_map_get("function", device,
JSON_INTEGER);
+                 if (!o)
+                     goto out;
+                 dev_func = libxl__json_object_get_integer(o);
+
+                 pcidev->vdevfn = PCI_DEVFN(dev_slot, dev_func);
+
+                 rc = 0;
+                 goto out;
+             }
+        }
+    }
+
+
+out:
+    libxl__free_all(&gc);
+    return rc;
+}
+
+int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
+{
+    libxl__qmp_handler *qmp = NULL;
+    flexarray_t *parameters = NULL;
+    libxl_key_value_list args = NULL;
+    char *hostaddr = NULL;
+    int rc = 0;
+
+    qmp = libxl__qmp_initialize(libxl__gc_owner(gc), domid);
+    if (!qmp)
+        return -1;
+
+    hostaddr = libxl__sprintf(gc, "%04x:%02x:%02x.%01x",
pcidev->domain,
+                              pcidev->bus, pcidev->dev, pcidev->func);
+    if (!hostaddr)
+        return -1;
+
+    parameters = flexarray_make(6, 1);
+    flexarray_append_pair(parameters, "driver",
"xen-pci-passthrough");
+    flexarray_append_pair(parameters, "id",
+                          libxl__sprintf(gc, PCI_PT_QDEV_ID,
+                                         pcidev->bus, pcidev->dev,
+                                         pcidev->func));
+    flexarray_append_pair(parameters, "hostaddr", hostaddr);
+    args = libxl__xs_kvs_of_flexarray(gc, parameters, 6);
+    if (!args)
+        return -1;
+
+    rc = qmp_synchronous_send(qmp, "device_add", &args,
+                              NULL, NULL, qmp->timeout);
+    if (rc == 0) {
+        rc = qmp_synchronous_send(qmp, "query-pci", NULL,
+                                  pci_add_callback, pcidev, qmp->timeout);
+    }
+
+    flexarray_free(parameters);
+    libxl__qmp_close(qmp);
+    return rc;
+}
+
 int libxl__qmp_initializations(libxl_ctx *ctx, uint32_t domid)
 {
     libxl__qmp_handler *qmp = NULL;
-- 
Anthony PERARD


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

Anthony PERARD

2011-Oct-07 12:10 UTC

head link

[Xen-devel] [PATCH 7/7] libxl, Use QMP to insert a passthrough device when using upstream QEMU

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_pci.c |   58 ++++++++++++++++++++++++++++++-----------------
 1 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 1523cf0..b6cb4a3 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -613,27 +613,43 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid,
libxl_device_pci *pcidev, i
                                          NULL, NULL, NULL) < 0) {
             return ERROR_FAIL;
         }
-        path = libxl__sprintf(gc,
"/local/domain/0/device-model/%d/state", domid);
-        state = libxl__xs_read(gc, XBT_NULL, path);
-        path = libxl__sprintf(gc,
"/local/domain/0/device-model/%d/parameter", domid);
-        if (pcidev->vdevfn)
-            libxl__xs_write(gc, XBT_NULL, path, PCI_BDF_VDEVFN,
pcidev->domain,
-                           pcidev->bus, pcidev->dev, pcidev->func,
pcidev->vdevfn);
-        else
-            libxl__xs_write(gc, XBT_NULL, path, PCI_BDF, pcidev->domain,
-                           pcidev->bus, pcidev->dev, pcidev->func);
-        path = libxl__sprintf(gc,
"/local/domain/0/device-model/%d/command", domid);
-        xs_write(ctx->xsh, XBT_NULL, path, "pci-ins",
strlen("pci-ins"));
-        rc = libxl__wait_for_device_model(gc, domid, NULL, NULL,
-                                          pci_ins_check, state);
-        path = libxl__sprintf(gc,
"/local/domain/0/device-model/%d/parameter", domid);
-        vdevfn = libxl__xs_read(gc, XBT_NULL, path);
-        path = libxl__sprintf(gc,
"/local/domain/0/device-model/%d/state", domid);
-        if ( rc < 0 )
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "qemu refused to add device:
%s", vdevfn);
-        else if ( sscanf(vdevfn, "0x%x", &pcidev->vdevfn) != 1
)
-            rc = -1;
-        xs_write(ctx->xsh, XBT_NULL, path, state, strlen(state));
+        if (libxl__device_model_version_running(gc, domid)
+            == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
+            rc = libxl__qmp_pci_add(gc, domid, pcidev);
+        } else {
+            path = libxl__sprintf(gc,
"/local/domain/0/device-model/%d/state",
+                                  domid);
+            state = libxl__xs_read(gc, XBT_NULL, path);
+            path = libxl__sprintf(gc,
+                                 
"/local/domain/0/device-model/%d/parameter",
+                                  domid);
+            if (pcidev->vdevfn) {
+                libxl__xs_write(gc, XBT_NULL, path, PCI_BDF_VDEVFN,
+                                pcidev->domain, pcidev->bus,
pcidev->dev,
+                                pcidev->func, pcidev->vdevfn);
+            } else {
+                libxl__xs_write(gc, XBT_NULL, path, PCI_BDF, pcidev->domain,
+                                pcidev->bus, pcidev->dev,
pcidev->func);
+            }
+            path = libxl__sprintf(gc,
+                                 
"/local/domain/0/device-model/%d/command",
+                                  domid);
+            xs_write(ctx->xsh, XBT_NULL, path, "pci-ins",
strlen("pci-ins"));
+            rc = libxl__wait_for_device_model(gc, domid, NULL, NULL,
+                                              pci_ins_check, state);
+            path = libxl__sprintf(gc,
+                                 
"/local/domain/0/device-model/%d/parameter",
+                                  domid);
+            vdevfn = libxl__xs_read(gc, XBT_NULL, path);
+            path = libxl__sprintf(gc,
"/local/domain/0/device-model/%d/state",
+                                  domid);
+            if ( rc < 0 )
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                           "qemu refused to add device: %s", vdevfn);
+            else if ( sscanf(vdevfn, "0x%x", &pcidev->vdevfn)
!= 1 )
+                rc = -1;
+            xs_write(ctx->xsh, XBT_NULL, path, state, strlen(state));
+        }
         if ( rc )
             return ERROR_FAIL;
         break;
-- 
Anthony PERARD


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

Ian Campbell

2011-Oct-07 12:28 UTC

head link

Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

On Fri, 2011-10-07 at 13:10 +0100, Anthony PERARD wrote:> The key is /local/domain/$domid/dm-version.
I''ve been wondering if we should introduce /libxl/$domid/ as a place
for
keeping tooltack internal droppings like this. The danger with putting
stuff in /local/domain is that domains come to rely on them.
> This come with libxl__device_model_version_running helper function.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  tools/libxl/libxl_create.c   |    5 +++++
>  tools/libxl/libxl_internal.c |   19 +++++++++++++++++++
>  tools/libxl/libxl_internal.h |    5 +++++
>  3 files changed, 29 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index e1e3258..3f33d90 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -175,6 +175,11 @@ int libxl__domain_build(libxl__gc *gc,
>  
>      gettimeofday(&start_time, NULL);
>  
> +    localents = libxl__calloc(gc, 3, sizeof (char *));
> +    i = 0;
> +    localents[i++] = "dm-version";
> +    localents[i++] = libxl__strdup(gc,
libxl_device_model_version_to_string(dm_info->device_model_version));
> +
You don''t seem to use this anywhere?
>      switch (info->type) {
>      case LIBXL_DOMAIN_TYPE_HVM:
>          ret = libxl__build_hvm(gc, domid, info, dm_info, state);
> diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> index 3b8a41f..e535c0c 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
> @@ -318,3 +318,22 @@ int libxl__fd_set_cloexec(int fd)
>      }
>      return fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
>  }
> +
> +libxl_device_model_version libxl__device_model_version_running(libxl__gc
*gc,
> +                                                               uint32_t
domid)
> +{
> +    char *path = NULL;
> +    char *dm_version = NULL;
> +    libxl_device_model_version value;
> +
> +    path = libxl__sprintf(gc, "/local/domain/%d/dm-version",
domid);
> +    dm_version = libxl__xs_read(gc, XBT_NULL, path);
> +    if (!dm_version) {
This would be a bug, since it would imply an inconsistent version of
libxl was used to create the domain? (not sure what our policy around
this actually is / should be).
> +        return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> +    }
> +
> +    if (libxl_device_model_version_from_string(dm_version, &value)
< 0) {
> +        return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
> +    }
> +    return value;
> +}
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 935c899..4dd0f91 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -554,4 +554,9 @@ _hidden void libxl__json_object_free(libxl__gc *gc,
libxl__json_object *obj);
>  
>  _hidden libxl__json_object *libxl__json_parse(libxl__gc *gc, const char
*s);
>  
> +  /* Based on /local/domain/$domid/dm-version xenstore key
> +   * default is qemu xen traditional */
> +_hidden libxl_device_model_version
> +libxl__device_model_version_running(libxl__gc *gc, uint32_t domid);
> +
>  #endif


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

Ian Campbell

2011-Oct-07 12:28 UTC

head link

Re: [Xen-devel] [PATCH 2/7] libxl_qmp, Introduce an opaque argument to the callbacks.

On Fri, 2011-10-07 at 13:10 +0100, Anthony PERARD wrote:> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  tools/libxl/libxl_qmp.c |   29 +++++++++++++++++++----------
>  1 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index 34db29f..5ab9acc 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -43,11 +43,13 @@
>  #define QMP_RECEIVE_BUFFER_SIZE 4096
>  
>  typedef int (*qmp_callback_t)(libxl__qmp_handler *qmp,
> -                              const libxl__json_object *tree);
> +                              const libxl__json_object *tree,
> +                              void *opaque);
>  
>  typedef struct callback_id_pair {
>      int id;
>      qmp_callback_t callback;
> +    void *opaque;
>      SIMPLEQ_ENTRY(callback_id_pair) next;
>  } callback_id_pair;
>  
> @@ -70,7 +72,8 @@ struct libxl__qmp_handler {
>  };
>  
>  static int qmp_send(libxl__qmp_handler *qmp,
> -                    const char *cmd, qmp_callback_t callback);
> +                    const char *cmd,
> +                    qmp_callback_t callback, void *opaque);
>  
>  static const int QMP_SOCKET_CONNECT_TIMEOUT = 5;
>  
> @@ -100,7 +103,8 @@ static int store_serial_port_info(libxl__qmp_handler
*qmp,
>  }
>  
>  static int register_serials_chardev_callback(libxl__qmp_handler *qmp,
> -                                             const libxl__json_object *o)
> +                                             const libxl__json_object *o,
> +                                             void *unused)
>  {
>      const libxl__json_object *obj = NULL;
>      const libxl__json_object *label = NULL;
> @@ -144,7 +148,7 @@ static int
register_serials_chardev_callback(libxl__qmp_handler *qmp,
>  }
>  
>  static int qmp_capabilities_callback(libxl__qmp_handler *qmp,
> -                                     const libxl__json_object *o)
> +                                     const libxl__json_object *o, void
*unused)
>  {
>      qmp->connected = true;
>  
> @@ -157,7 +161,7 @@ static int qmp_capabilities_callback(libxl__qmp_handler
*qmp,
>  
>  static int enable_qmp_capabilities(libxl__qmp_handler *qmp)
>  {
> -    return qmp_send(qmp, "qmp_capabilities",
qmp_capabilities_callback);
> +    return qmp_send(qmp, "qmp_capabilities",
qmp_capabilities_callback, NULL);
>  }
>  
>  /*
> @@ -208,7 +212,7 @@ static void
qmp_handle_error_response(libxl__qmp_handler *qmp,
>      resp = libxl__json_map_get("desc", resp, JSON_STRING);
>  
>      if (pp) {
> -        pp->callback(qmp, NULL);
> +        pp->callback(qmp, NULL, pp->opaque);
>          if (pp->id == qmp->wait_for_id) {
>              /* tell that the id have been processed */
>              qmp->wait_for_id = 0;
> @@ -241,7 +245,8 @@ static int qmp_handle_response(libxl__qmp_handler *qmp,
>  
>          if (pp) {
>              pp->callback(qmp,
> -                         libxl__json_map_get("return", resp,
JSON_ANY));
> +                         libxl__json_map_get("return", resp,
JSON_ANY),
> +                         pp->opaque);
>              if (pp->id == qmp->wait_for_id) {
>                  /* tell that the id have been processed */
>                  qmp->wait_for_id = 0;
> @@ -423,7 +428,8 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler
*qmp)
>  }
>  
>  static int qmp_send(libxl__qmp_handler *qmp,
> -                    const char *cmd, qmp_callback_t callback)
> +                    const char *cmd,
> +                    qmp_callback_t callback, void *opaque)
>  {
>      yajl_gen_config conf = { 0, NULL };
>      const unsigned char *buf;
> @@ -461,6 +467,7 @@ static int qmp_send(libxl__qmp_handler *qmp,
>          }
>          elm->id = qmp->last_id_used;
>          elm->callback = callback;
> +        elm->opaque = opaque;
>          SIMPLEQ_INSERT_TAIL(&qmp->callback_list, elm, next);
>      }
>  
> @@ -483,13 +490,14 @@ error:
>  }
>  
>  static int qmp_synchronous_send(libxl__qmp_handler *qmp, const char *cmd,
> -                                qmp_callback_t callback, int ask_timeout)
> +                                qmp_callback_t callback, void *opaque,
> +                                int ask_timeout)
>  {
>      int id = 0;
>      int ret = 0;
>      libxl__gc gc = LIBXL_INIT_GC(qmp->ctx);
>  
> -    id = qmp_send(qmp, cmd, callback);
> +    id = qmp_send(qmp, cmd, callback, opaque);
>      if (id <= 0) {
>          return -1;
>      }
> @@ -579,6 +587,7 @@ int libxl__qmp_query_serial(libxl__qmp_handler *qmp)
>  {
>      return qmp_synchronous_send(qmp, "query-chardev",
>                                  register_serials_chardev_callback,
> +                                NULL,
>                                  qmp->timeout);
>  }
>  


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

Ian Campbell

2011-Oct-07 12:29 UTC

head link

Re: [Xen-devel] [PATCH 3/7] libxl_qmp, Introduce list of arguments to qmp_send

On Fri, 2011-10-07 at 13:10 +0100, Anthony PERARD wrote:> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  tools/libxl/libxl_qmp.c |   16 +++++++++++-----
>  1 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index 5ab9acc..002fb13 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -72,7 +72,7 @@ struct libxl__qmp_handler {
>  };
>  
>  static int qmp_send(libxl__qmp_handler *qmp,
> -                    const char *cmd,
> +                    const char *cmd, libxl_key_value_list *args,
>                      qmp_callback_t callback, void *opaque);
>  
>  static const int QMP_SOCKET_CONNECT_TIMEOUT = 5;
> @@ -161,7 +161,8 @@ static int qmp_capabilities_callback(libxl__qmp_handler
*qmp,
>  
>  static int enable_qmp_capabilities(libxl__qmp_handler *qmp)
>  {
> -    return qmp_send(qmp, "qmp_capabilities",
qmp_capabilities_callback, NULL);
> +    return qmp_send(qmp, "qmp_capabilities", NULL,
> +                    qmp_capabilities_callback, NULL);
>  }
>  
>  /*
> @@ -428,7 +429,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler
*qmp)
>  }
>  
>  static int qmp_send(libxl__qmp_handler *qmp,
> -                    const char *cmd,
> +                    const char *cmd, libxl_key_value_list *args,
>                      qmp_callback_t callback, void *opaque)
>  {
>      yajl_gen_config conf = { 0, NULL };
> @@ -447,6 +448,10 @@ static int qmp_send(libxl__qmp_handler *qmp,
>      libxl__yajl_gen_asciiz(hand, cmd);
>      libxl__yajl_gen_asciiz(hand, "id");
>      yajl_gen_integer(hand, ++qmp->last_id_used);
> +    if (args) {
> +        libxl__yajl_gen_asciiz(hand, "arguments");
> +        libxl_key_value_list_gen_json(hand, args);
> +    }
>      yajl_gen_map_close(hand);
>  
>      s = yajl_gen_get_buf(hand, &buf, &len);
> @@ -490,6 +495,7 @@ error:
>  }
>  
>  static int qmp_synchronous_send(libxl__qmp_handler *qmp, const char *cmd,
> +                                libxl_key_value_list *args,
>                                  qmp_callback_t callback, void *opaque,
>                                  int ask_timeout)
>  {
> @@ -497,7 +503,7 @@ static int qmp_synchronous_send(libxl__qmp_handler
*qmp, const char *cmd,
>      int ret = 0;
>      libxl__gc gc = LIBXL_INIT_GC(qmp->ctx);
>  
> -    id = qmp_send(qmp, cmd, callback, opaque);
> +    id = qmp_send(qmp, cmd, args, callback, opaque);
>      if (id <= 0) {
>          return -1;
>      }
> @@ -585,7 +591,7 @@ void libxl__qmp_cleanup(libxl__gc *gc, uint32_t domid)
>  
>  int libxl__qmp_query_serial(libxl__qmp_handler *qmp)
>  {
> -    return qmp_synchronous_send(qmp, "query-chardev",
> +    return qmp_synchronous_send(qmp, "query-chardev", NULL,
>                                  register_serials_chardev_callback,
>                                  NULL,
>                                  qmp->timeout);


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

Ian Campbell

2011-Oct-07 12:32 UTC

head link

Re: [Xen-devel] [PATCH 4/7] libxl_qmp, Always insert a command id in the callback_list.

On Fri, 2011-10-07 at 13:10 +0100, Anthony PERARD wrote:> Because the function qmp_synchronous_send rely on the presence of the id
> in the callback_list.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  tools/libxl/libxl_qmp.c |   34 ++++++++++++++++++----------------
>  1 files changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index 002fb13..1594a4f 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -213,7 +213,9 @@ static void
qmp_handle_error_response(libxl__qmp_handler *qmp,
>      resp = libxl__json_map_get("desc", resp, JSON_STRING);
>  
>      if (pp) {
> -        pp->callback(qmp, NULL, pp->opaque);
> +        if (pp->callback) {
> +            pp->callback(qmp, NULL, pp->opaque);
> +        }
>          if (pp->id == qmp->wait_for_id) {
>              /* tell that the id have been processed */
>              qmp->wait_for_id = 0;
> @@ -245,9 +247,11 @@ static int qmp_handle_response(libxl__qmp_handler
*qmp,
>          callback_id_pair *pp = qmp_get_callback_from_id(qmp, resp);
>  
>          if (pp) {
> -            pp->callback(qmp,
> -                         libxl__json_map_get("return", resp,
JSON_ANY),
> -                         pp->opaque);
> +            if (pp->callback) {
> +                pp->callback(qmp,
> +                             libxl__json_map_get("return", resp,
JSON_ANY),
> +                             pp->opaque);
> +            }
>              if (pp->id == qmp->wait_for_id) {
>                  /* tell that the id have been processed */
>                  qmp->wait_for_id = 0;
> @@ -437,6 +441,7 @@ static int qmp_send(libxl__qmp_handler *qmp,
>      unsigned int len = 0;
>      yajl_gen_status s;
>      yajl_gen hand;
> +    callback_id_pair *elm = NULL;
>  
>      hand = yajl_gen_alloc(&conf, NULL);
>      if (!hand) {
> @@ -462,19 +467,16 @@ static int qmp_send(libxl__qmp_handler *qmp,
>          return -1;
>      }
>  
> -    if (callback) {
> -        callback_id_pair *elm = malloc(sizeof (callback_id_pair));
> -        if (elm == NULL) {
> -            LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR,
> -                             "Failed to allocate a QMP
callback");
> -            yajl_gen_free(hand);
> -            return -1;
> -        }
> -        elm->id = qmp->last_id_used;
> -        elm->callback = callback;
> -        elm->opaque = opaque;
> -        SIMPLEQ_INSERT_TAIL(&qmp->callback_list, elm, next);
> +    elm = malloc(sizeof (callback_id_pair));
> +    if (elm == NULL) {
> +        LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR,
> +                         "Failed to allocate a QMP callback");
> +        goto error;
>      }
> +    elm->id = qmp->last_id_used;
> +    elm->callback = callback;
> +    elm->opaque = opaque;
> +    SIMPLEQ_INSERT_TAIL(&qmp->callback_list, elm, next);
>  
>      LIBXL__LOG(qmp->ctx, LIBXL__LOG_DEBUG, "next qmp command:
''%s''", buf);
>  


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

Ian Campbell

2011-Oct-07 12:36 UTC

head link

Re: [Xen-devel] [PATCH 5/7] libxl_qmp, Return the callback return code in qmp_next.

On Fri, 2011-10-07 at 13:10 +0100, Anthony PERARD wrote:> So, if there is an error in the answer given by QEMU, the function
> qmp_synchronous_send while know.                       "will"
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  tools/libxl/libxl_qmp.c |   14 ++++++++------
>  1 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index 1594a4f..cd3e4e4 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -233,6 +233,7 @@ static int qmp_handle_response(libxl__qmp_handler *qmp,
>                                 const libxl__json_object *resp)
>  {
>      libxl__qmp_message_type type = LIBXL__QMP_MESSAGE_TYPE_INVALID;
> +    int rc = 0;
>  
>      type = qmp_response_type(qmp, resp);
>      LIBXL__LOG(qmp->ctx, LIBXL__LOG_DEBUG,
> @@ -241,14 +242,14 @@ static int qmp_handle_response(libxl__qmp_handler
*qmp,
>      switch (type) {
>      case LIBXL__QMP_MESSAGE_TYPE_QMP:
>          /* On the greeting message from the server, enable QMP
capabilities */
> -        enable_qmp_capabilities(qmp);
> +        rc = enable_qmp_capabilities(qmp);
>          break;
>      case LIBXL__QMP_MESSAGE_TYPE_RETURN: {
>          callback_id_pair *pp = qmp_get_callback_from_id(qmp, resp);
>  
>          if (pp) {
>              if (pp->callback) {
> -                pp->callback(qmp,
> +                rc = pp->callback(qmp,
>                               libxl__json_map_get("return", resp,
JSON_ANY),
>                               pp->opaque);
>              }
> @@ -263,13 +264,13 @@ static int qmp_handle_response(libxl__qmp_handler
*qmp,
>      }
>      case LIBXL__QMP_MESSAGE_TYPE_ERROR:
>          qmp_handle_error_response(qmp, resp);
> -        break;
> +        return -1;
A mixture or "return -1" and a rc variable returned at the outermost
level is a bit odd. You should either always set rc and fall through or
always return at the end of each case.
>      case LIBXL__QMP_MESSAGE_TYPE_EVENT:
>          break;
>      case LIBXL__QMP_MESSAGE_TYPE_INVALID:
>          return -1;
>      }
> -    return 0;
> +    return rc;
>  }
>  
>  /*
> @@ -358,6 +359,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler
*qmp)
>  
>      char *incomplete = NULL;
>      size_t incomplete_size = 0;
> +    int rc = 0;
>  
>      do {
>          fd_set rfds;
> @@ -416,7 +418,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler
*qmp)
>                  s = end + 2;
>  
>                  if (o) {
> -                    qmp_handle_response(qmp, o);
> +                    rc = qmp_handle_response(qmp, o);
If rc now indicates error do we need to bail straight away or need to
keep going around this loop? (Or is it certain we will immediately fall
out of the loop after this?)
>                      libxl__json_object_free(gc, o);
>                  } else {
>                      LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR,
> @@ -429,7 +431,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler
*qmp)
>          } while (s < s_end);
>     } while (s < s_end);
>  
> -    return 1;
> +    return rc;
>  }
>  
>  static int qmp_send(libxl__qmp_handler *qmp,


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

Ian Campbell

2011-Oct-07 12:43 UTC

head link

Re: [Xen-devel] [PATCH 6/7] libxl_qmp, Introduce libxl__qmp_pci_add.

On Fri, 2011-10-07 at 13:10 +0100, Anthony PERARD wrote:> This function insert a PCI passthrough device in qemu.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  tools/libxl/libxl_internal.h |    2 +
>  tools/libxl/libxl_qmp.c      |   89
++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 91 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 4dd0f91..78e1be2 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -119,6 +119,7 @@ typedef struct {
>  } libxl__device;
>  
>  #define XC_PCI_BDF             "0x%x, 0x%x, 0x%x, 0x%x"
> +#define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) |
((func) & 0x07))
No need to include the domain or dev in this? The name is pretty weird
if you don''t include the dev portion.  Looks like this is actually a
vdevfn? xl has an open coded instance of this pattern for that -- worth
adding a public macro?

FWIW pcidev_encode_bdf() exists which encodes the whole lot from a
libxl_device_pci, if that what you end up needing.
>  #define AUTO_PHP_SLOT          0x100
>  #define SYSFS_PCI_DEV          "/sys/bus/pci/devices"
>  #define SYSFS_PCIBACK_DRIVER   "/sys/bus/pci/drivers/pciback"
> @@ -444,6 +445,7 @@ _hidden libxl__qmp_handler
*libxl__qmp_initialize(libxl_ctx *ctx,
>                                                    uint32_t domid);
>  /* ask to QEMU the serial port information and store it in xenstore. */
>  _hidden int libxl__qmp_query_serial(libxl__qmp_handler *qmp);
> +_hidden int libxl__qmp_pci_add(libxl__gc *gc, int d, libxl_device_pci
*pcidev);
>  /* close and free the QMP handler */
>  _hidden void libxl__qmp_close(libxl__qmp_handler *qmp);
>  /* remove the socket file, if the file has already been removed,
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index cd3e4e4..c39a1ac 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -41,6 +41,7 @@
>   */
>  
>  #define QMP_RECEIVE_BUFFER_SIZE 4096
> +#define PCI_PT_QDEV_ID "pci-pt-%02x_%02x.%01x"
>  
>  typedef int (*qmp_callback_t)(libxl__qmp_handler *qmp,
>                                const libxl__json_object *tree,
> @@ -601,6 +602,94 @@ int libxl__qmp_query_serial(libxl__qmp_handler *qmp)
>                                  qmp->timeout);
>  }
>  
> +static int pci_add_callback(libxl__qmp_handler *qmp,
> +                            const libxl__json_object *response, void
*opaque)
> +{
> +    libxl_device_pci *pcidev = opaque;
> +    const libxl__json_object *bus = NULL;
> +    libxl__gc gc = LIBXL_INIT_GC(qmp->ctx);
> +    int i, j, rc = -1;
> +    char *asked_id = libxl__sprintf(&gc, PCI_PT_QDEV_ID,
> +                                    pcidev->bus, pcidev->dev,
pcidev->func);
> +
> +    for (i = 0; (bus = libxl__json_array_get(response, i)); i++) {
> +        const libxl__json_object *devices = NULL;
> +        const libxl__json_object *device = NULL;
> +        const libxl__json_object *o = NULL;
> +        const char *id = NULL;
> +
> +        devices = libxl__json_map_get("devices", bus,
JSON_ARRAY);
> +
> +        for (j = 0; (device = libxl__json_array_get(devices, j)); j++) {
> +             o = libxl__json_map_get("qdev_id", device,
JSON_STRING);
> +             id = libxl__json_object_get_string(o);
> +
> +             if (id && strcmp(asked_id, id) == 0) {
> +                 int dev_slot, dev_func;
> +
> +                 o = libxl__json_map_get("slot", device,
JSON_INTEGER);
> +                 if (!o)
> +                     goto out;
> +                 dev_slot = libxl__json_object_get_integer(o);
> +                 o = libxl__json_map_get("function", device,
JSON_INTEGER);
> +                 if (!o)
> +                     goto out;
> +                 dev_func = libxl__json_object_get_integer(o);
> +
> +                 pcidev->vdevfn = PCI_DEVFN(dev_slot, dev_func);
> +
> +                 rc = 0;
> +                 goto out;
> +             }
> +        }
> +    }
> +
> +
> +out:
> +    libxl__free_all(&gc);
> +    return rc;
> +}
> +
> +int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
> +{
> +    libxl__qmp_handler *qmp = NULL;
> +    flexarray_t *parameters = NULL;
> +    libxl_key_value_list args = NULL;
> +    char *hostaddr = NULL;
> +    int rc = 0;
> +
> +    qmp = libxl__qmp_initialize(libxl__gc_owner(gc), domid);
> +    if (!qmp)
> +        return -1;
> +
> +    hostaddr = libxl__sprintf(gc, "%04x:%02x:%02x.%01x",
pcidev->domain,
> +                              pcidev->bus, pcidev->dev,
pcidev->func);
> +    if (!hostaddr)
> +        return -1;
> +
> +    parameters = flexarray_make(6, 1);
> +    flexarray_append_pair(parameters, "driver",
"xen-pci-passthrough");
> +    flexarray_append_pair(parameters, "id",
> +                          libxl__sprintf(gc, PCI_PT_QDEV_ID,
> +                                         pcidev->bus, pcidev->dev,
> +                                         pcidev->func));
> +    flexarray_append_pair(parameters, "hostaddr", hostaddr);
> +    args = libxl__xs_kvs_of_flexarray(gc, parameters, 6);
> +    if (!args)
> +        return -1;
> +
> +    rc = qmp_synchronous_send(qmp, "device_add", &args,
> +                              NULL, NULL, qmp->timeout);
> +    if (rc == 0) {
> +        rc = qmp_synchronous_send(qmp, "query-pci", NULL,
> +                                  pci_add_callback, pcidev,
qmp->timeout);
> +    }
> +
> +    flexarray_free(parameters);
> +    libxl__qmp_close(qmp);
> +    return rc;
> +}
> +
>  int libxl__qmp_initializations(libxl_ctx *ctx, uint32_t domid)
>  {
>      libxl__qmp_handler *qmp = NULL;


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

Ian Campbell

2011-Oct-07 12:45 UTC

head link

Re: [Xen-devel] [PATCH 7/7] libxl, Use QMP to insert a passthrough device when using upstream QEMU

On Fri, 2011-10-07 at 13:10 +0100, Anthony PERARD wrote:> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Sticking within the 80-column limit is getting pretty hard, perhaps
pulling out the interactions with old-qemu into a new function would
help?
> ---
>  tools/libxl/libxl_pci.c |   58
++++++++++++++++++++++++++++++-----------------
>  1 files changed, 37 insertions(+), 21 deletions(-)
> 
> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index 1523cf0..b6cb4a3 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -613,27 +613,43 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid,
libxl_device_pci *pcidev, i
>                                           NULL, NULL, NULL) < 0) {
>              return ERROR_FAIL;
>          }
> -        path = libxl__sprintf(gc,
"/local/domain/0/device-model/%d/state", domid);
> -        state = libxl__xs_read(gc, XBT_NULL, path);
> -        path = libxl__sprintf(gc,
"/local/domain/0/device-model/%d/parameter", domid);
> -        if (pcidev->vdevfn)
> -            libxl__xs_write(gc, XBT_NULL, path, PCI_BDF_VDEVFN,
pcidev->domain,
> -                           pcidev->bus, pcidev->dev,
pcidev->func, pcidev->vdevfn);
> -        else
> -            libxl__xs_write(gc, XBT_NULL, path, PCI_BDF,
pcidev->domain,
> -                           pcidev->bus, pcidev->dev,
pcidev->func);
> -        path = libxl__sprintf(gc,
"/local/domain/0/device-model/%d/command", domid);
> -        xs_write(ctx->xsh, XBT_NULL, path, "pci-ins",
strlen("pci-ins"));
> -        rc = libxl__wait_for_device_model(gc, domid, NULL, NULL,
> -                                          pci_ins_check, state);
> -        path = libxl__sprintf(gc,
"/local/domain/0/device-model/%d/parameter", domid);
> -        vdevfn = libxl__xs_read(gc, XBT_NULL, path);
> -        path = libxl__sprintf(gc,
"/local/domain/0/device-model/%d/state", domid);
> -        if ( rc < 0 )
> -            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "qemu refused to add
device: %s", vdevfn);
> -        else if ( sscanf(vdevfn, "0x%x", &pcidev->vdevfn)
!= 1 )
> -            rc = -1;
> -        xs_write(ctx->xsh, XBT_NULL, path, state, strlen(state));
> +        if (libxl__device_model_version_running(gc, domid)
> +            == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> +            rc = libxl__qmp_pci_add(gc, domid, pcidev);
> +        } else {
> +            path = libxl__sprintf(gc,
"/local/domain/0/device-model/%d/state",
> +                                  domid);
> +            state = libxl__xs_read(gc, XBT_NULL, path);
> +            path = libxl__sprintf(gc,
> +                                 
"/local/domain/0/device-model/%d/parameter",
> +                                  domid);
> +            if (pcidev->vdevfn) {
> +                libxl__xs_write(gc, XBT_NULL, path, PCI_BDF_VDEVFN,
> +                                pcidev->domain, pcidev->bus,
pcidev->dev,
> +                                pcidev->func, pcidev->vdevfn);
> +            } else {
> +                libxl__xs_write(gc, XBT_NULL, path, PCI_BDF,
pcidev->domain,
> +                                pcidev->bus, pcidev->dev,
pcidev->func);
> +            }
> +            path = libxl__sprintf(gc,
> +                                 
"/local/domain/0/device-model/%d/command",
> +                                  domid);
> +            xs_write(ctx->xsh, XBT_NULL, path, "pci-ins",
strlen("pci-ins"));
> +            rc = libxl__wait_for_device_model(gc, domid, NULL, NULL,
> +                                              pci_ins_check, state);
> +            path = libxl__sprintf(gc,
> +                                 
"/local/domain/0/device-model/%d/parameter",
> +                                  domid);
> +            vdevfn = libxl__xs_read(gc, XBT_NULL, path);
> +            path = libxl__sprintf(gc,
"/local/domain/0/device-model/%d/state",
> +                                  domid);
> +            if ( rc < 0 )
> +                LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                           "qemu refused to add device: %s",
vdevfn);
> +            else if ( sscanf(vdevfn, "0x%x",
&pcidev->vdevfn) != 1 )
> +                rc = -1;
> +            xs_write(ctx->xsh, XBT_NULL, path, state, strlen(state));
> +        }
>          if ( rc )
>              return ERROR_FAIL;
>          break;


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

Anthony PERARD

2011-Oct-07 13:29 UTC

head link

Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

On Fri, Oct 7, 2011 at 13:28, Ian Campbell <Ian.Campbell@citrix.com>
wrote:> On Fri, 2011-10-07 at 13:10 +0100, Anthony PERARD wrote:
>> The key is /local/domain/$domid/dm-version.
>
> I''ve been wondering if we should introduce /libxl/$domid/ as a
place for
> keeping tooltack internal droppings like this. The danger with putting
> stuff in /local/domain is that domains come to rely on them.
I was not sure about where to put the value. So
/libxl/$domid/dm-version is good to me.
>> This come with libxl__device_model_version_running helper function.
>>
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>> ---
>>  tools/libxl/libxl_create.c   |    5 +++++
>>  tools/libxl/libxl_internal.c |   19 +++++++++++++++++++
>>  tools/libxl/libxl_internal.h |    5 +++++
>>  3 files changed, 29 insertions(+), 0 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>> index e1e3258..3f33d90 100644
>> --- a/tools/libxl/libxl_create.c
>> +++ b/tools/libxl/libxl_create.c
>> @@ -175,6 +175,11 @@ int libxl__domain_build(libxl__gc *gc,
>>
>>      gettimeofday(&start_time, NULL);
>>
>> +    localents = libxl__calloc(gc, 3, sizeof (char *));
>> +    i = 0;
>> +    localents[i++] = "dm-version";
>> +    localents[i++] = libxl__strdup(gc,
libxl_device_model_version_to_string(dm_info->device_model_version));
>> +
>
> You don''t seem to use this anywhere?
This is magic ! :-)

This pointer was not used in this functions, but still given to
libxl__build_post() (and written in xenstore to /local/domain).
My other option from this function was to write the value in /vm/
which does not seams to be a good place.
>>      switch (info->type) {
>>      case LIBXL_DOMAIN_TYPE_HVM:
>>          ret = libxl__build_hvm(gc, domid, info, dm_info, state);
>> diff --git a/tools/libxl/libxl_internal.c
b/tools/libxl/libxl_internal.c
>> index 3b8a41f..e535c0c 100644
>> --- a/tools/libxl/libxl_internal.c
>> +++ b/tools/libxl/libxl_internal.c
>> @@ -318,3 +318,22 @@ int libxl__fd_set_cloexec(int fd)
>>      }
>>      return fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
>>  }
>> +
>> +libxl_device_model_version
libxl__device_model_version_running(libxl__gc *gc,
>> +                                                              
uint32_t domid)
>> +{
>> +    char *path = NULL;
>> +    char *dm_version = NULL;
>> +    libxl_device_model_version value;
>> +
>> +    path = libxl__sprintf(gc, "/local/domain/%d/dm-version",
domid);
>> +    dm_version = libxl__xs_read(gc, XBT_NULL, path);
>> +    if (!dm_version) {
>
> This would be a bug, since it would imply an inconsistent version of
> libxl was used to create the domain? (not sure what our policy around
> this actually is / should be).
I just want to give libxl the ability to run after an update. But it''s
maybe better to print and return an error, so that will give a chance
to the user to update is xenstore :). (or use the tool that created
the domain).
>> +        return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
>> +    }
>> +
>> +    if (libxl_device_model_version_from_string(dm_version, &value)
< 0) {
>> +        return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
>> +    }
>> +    return value;
>> +}
>> diff --git a/tools/libxl/libxl_internal.h
b/tools/libxl/libxl_internal.h
>> index 935c899..4dd0f91 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
>> @@ -554,4 +554,9 @@ _hidden void libxl__json_object_free(libxl__gc *gc,
libxl__json_object *obj);
>>
>>  _hidden libxl__json_object *libxl__json_parse(libxl__gc *gc, const
char *s);
>>
>> +  /* Based on /local/domain/$domid/dm-version xenstore key
>> +   * default is qemu xen traditional */
>> +_hidden libxl_device_model_version
>> +libxl__device_model_version_running(libxl__gc *gc, uint32_t domid);
>> +
>>  #endif

-- 
Anthony PERARD

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

Anthony PERARD

2011-Oct-07 15:23 UTC

head link

Re: [Xen-devel] [PATCH 5/7] libxl_qmp, Return the callback return code in qmp_next.

On Fri, Oct 7, 2011 at 13:36, Ian Campbell <Ian.Campbell@citrix.com>
wrote:> On Fri, 2011-10-07 at 13:10 +0100, Anthony PERARD wrote:
>> So, if there is an error in the answer given by QEMU, the function
>> qmp_synchronous_send while know.
>                       "will"
>
>>
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>> ---
>>  tools/libxl/libxl_qmp.c |   14 ++++++++------
>>  1 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
>> index 1594a4f..cd3e4e4 100644
>> --- a/tools/libxl/libxl_qmp.c
>> +++ b/tools/libxl/libxl_qmp.c
>> @@ -233,6 +233,7 @@ static int qmp_handle_response(libxl__qmp_handler
*qmp,
>>                                 const libxl__json_object *resp)
>>  {
>>      libxl__qmp_message_type type = LIBXL__QMP_MESSAGE_TYPE_INVALID;
>> +    int rc = 0;
>>
>>      type = qmp_response_type(qmp, resp);
>>      LIBXL__LOG(qmp->ctx, LIBXL__LOG_DEBUG,
>> @@ -241,14 +242,14 @@ static int qmp_handle_response(libxl__qmp_handler
*qmp,
>>      switch (type) {
>>      case LIBXL__QMP_MESSAGE_TYPE_QMP:
>>          /* On the greeting message from the server, enable QMP
capabilities */
>> -        enable_qmp_capabilities(qmp);
>> +        rc = enable_qmp_capabilities(qmp);
>>          break;
>>      case LIBXL__QMP_MESSAGE_TYPE_RETURN: {
>>          callback_id_pair *pp = qmp_get_callback_from_id(qmp, resp);
>>
>>          if (pp) {
>>              if (pp->callback) {
>> -                pp->callback(qmp,
>> +                rc = pp->callback(qmp,
>>                               libxl__json_map_get("return",
resp, JSON_ANY),
>>                               pp->opaque);
>>              }
>> @@ -263,13 +264,13 @@ static int qmp_handle_response(libxl__qmp_handler
*qmp,
>>      }
>>      case LIBXL__QMP_MESSAGE_TYPE_ERROR:
>>          qmp_handle_error_response(qmp, resp);
>> -        break;
>> +        return -1;
>
> A mixture or "return -1" and a rc variable returned at the
outermost
> level is a bit odd. You should either always set rc and fall through or
> always return at the end of each case.
OK, I will change that.
>>      case LIBXL__QMP_MESSAGE_TYPE_EVENT:
>>          break;
>>      case LIBXL__QMP_MESSAGE_TYPE_INVALID:
>>          return -1;
>>      }
>> -    return 0;
>> +    return rc;
>>  }
>>
>>  /*
>> @@ -358,6 +359,7 @@ static int qmp_next(libxl__gc *gc,
libxl__qmp_handler *qmp)
>>
>>      char *incomplete = NULL;
>>      size_t incomplete_size = 0;
>> +    int rc = 0;
>>
>>      do {
>>          fd_set rfds;
>> @@ -416,7 +418,7 @@ static int qmp_next(libxl__gc *gc,
libxl__qmp_handler *qmp)
>>                  s = end + 2;
>>
>>                  if (o) {
>> -                    qmp_handle_response(qmp, o);
>> +                    rc = qmp_handle_response(qmp, o);
>
> If rc now indicates error do we need to bail straight away or need to
> keep going around this loop? (Or is it certain we will immediately fall
> out of the loop after this?)
We can not be sure that we will return, because it could be another
message in the butffer. So I should return if there is a protocol
error. But I think that I should keep seperate the return code of a
callback, so only the interested function (qmp_synchronous_send) will
read it (and return to the caller).
>>                      libxl__json_object_free(gc, o);
>>                  } else {
>>                      LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR,
>> @@ -429,7 +431,7 @@ static int qmp_next(libxl__gc *gc,
libxl__qmp_handler *qmp)
>>          } while (s < s_end);
>>     } while (s < s_end);
>>
>> -    return 1;
>> +    return rc;
>>  }
>>
>>  static int qmp_send(libxl__qmp_handler *qmp,
-- 
Anthony PERARD

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

Anthony PERARD

2011-Oct-07 16:01 UTC

head link

Re: [Xen-devel] [PATCH 6/7] libxl_qmp, Introduce libxl__qmp_pci_add.

On Fri, Oct 7, 2011 at 13:43, Ian Campbell <Ian.Campbell@citrix.com>
wrote:> On Fri, 2011-10-07 at 13:10 +0100, Anthony PERARD wrote:
>> This function insert a PCI passthrough device in qemu.
>>
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>> ---
>>  tools/libxl/libxl_internal.h |    2 +
>>  tools/libxl/libxl_qmp.c      |   89
++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 91 insertions(+), 0 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_internal.h
b/tools/libxl/libxl_internal.h
>> index 4dd0f91..78e1be2 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
>> @@ -119,6 +119,7 @@ typedef struct {
>>  } libxl__device;
>>
>>  #define XC_PCI_BDF             "0x%x, 0x%x, 0x%x, 0x%x"
>> +#define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) |
((func) & 0x07))
>
> No need to include the domain or dev in this? The name is pretty weird
> if you don''t include the dev portion.  Looks like this is actually
a
> vdevfn? xl has an open coded instance of this pattern for that -- worth
> adding a public macro?
I have actual copy the macro from qemu, and I think "slot === dev". I
think the macro come from linux/pci.h.
But yes, I use it to store the vdevfn.
> FWIW pcidev_encode_bdf() exists which encodes the whole lot from a
> libxl_device_pci, if that what you end up needing.
If the vdefvn should contain the virtual bus and domain number, so
yes, that will be the function I need.

-- 
Anthony PERARD

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

Anthony PERARD

2011-Oct-07 16:03 UTC

head link

Re: [Xen-devel] [PATCH 7/7] libxl, Use QMP to insert a passthrough device when using upstream QEMU

On Fri, Oct 7, 2011 at 13:45, Ian Campbell <Ian.Campbell@citrix.com>
wrote:> On Fri, 2011-10-07 at 13:10 +0100, Anthony PERARD wrote:
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>
> Sticking within the 80-column limit is getting pretty hard, perhaps
> pulling out the interactions with old-qemu into a new function would
> help?
I think it does, so I will probably create this new function.

-- 
Anthony PERARD

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

Ian Jackson

2011-Oct-07 17:35 UTC

head link

Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/7] libxl, Introduce
dm-version xenstore key."):> On Fri, 2011-10-07 at 13:10 +0100, Anthony PERARD wrote:
> > The key is /local/domain/$domid/dm-version.
> 
> I''ve been wondering if we should introduce /libxl/$domid/ as a
place for
> keeping tooltack internal droppings like this. The danger with putting
> stuff in /local/domain is that domains come to rely on them.
Also, can''t the domain write to /local/domain/$domid ?  Which we
don''t
want, in this case.
> > +    localents[i++] = libxl__strdup(gc,
libxl_device_model_version_to_string(dm_info->device_model_version));
> > +
> 
> You don''t seem to use this anywhere?
Also, like several other lines here, it needs to be wrapped
(preferably, to 75 columns or so).
> This would be a bug, since it would imply an inconsistent version of
> libxl was used to create the domain? (not sure what our policy around
> this actually is / should be).
No, because the same toolstack can create domains with either version
of qemu.

Ian.

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

Ian Jackson

2011-Oct-07 17:38 UTC

head link

Re: [Xen-devel] [PATCH 7/7] libxl, Use QMP to insert a passthrough device when using upstream QEMU

Anthony PERARD writes ("[Xen-devel] [PATCH 7/7] libxl, Use QMP to insert a
passt> +            else if ( sscanf(vdevfn, "0x%x",
&pcidev->vdevfn) != 1 )> +                rc = -1;
If this ever happens, it would be very mysterious.  Perhaps add a
logging call ?

Ian.

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

Ian Campbell

2011-Oct-07 17:53 UTC

head link

Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

On Fri, 2011-10-07 at 18:35 +0100, Ian Jackson wrote:> Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/7] libxl, Introduce
dm-version xenstore key."):
> > This would be a bug, since it would imply an inconsistent version of
> > libxl was used to create the domain? (not sure what our policy around
> > this actually is / should be).
> 
> No, because the same toolstack can create domains with either version
> of qemu.
Such a toolstack would always write the node to one value or the other.

The case I''m referring to is when the domain was created by one version
of libxl (which doesn''t write the node) but is now being acted upon by
a
new one (which does write & check it), perhaps due to an intervening
upgrade.

There is no way to know which version of qemu was used in this scenario
but I question whether it is something we should support in any case (at
least outside of stable branches).

Ian.
> 
> Ian.

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

Ian Campbell

2011-Oct-07 18:24 UTC

head link

Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

On Fri, 2011-10-07 at 18:35 +0100, Ian Jackson wrote:> Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/7] libxl, Introduce
dm-version xenstore key."):
> > On Fri, 2011-10-07 at 13:10 +0100, Anthony PERARD wrote:
> > > The key is /local/domain/$domid/dm-version.
> > 
> > I''ve been wondering if we should introduce /libxl/$domid/ as
a place for
> > keeping tooltack internal droppings like this. The danger with putting
> > stuff in /local/domain is that domains come to rely on them.
> 
> Also, can''t the domain write to /local/domain/$domid ?  Which we
don''t
> want, in this case.
Do you think /libxl/$domid is a good idea then or do you have somewhere
else in mind?

Ian.



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

Ian Campbell

2011-Oct-07 18:29 UTC

head link

Re: [Xen-devel] [PATCH 6/7] libxl_qmp, Introduce libxl__qmp_pci_add.

On Fri, 2011-10-07 at 17:01 +0100, Anthony PERARD wrote:> On Fri, Oct 7, 2011 at 13:43, Ian Campbell <Ian.Campbell@citrix.com>
wrote:
> > On Fri, 2011-10-07 at 13:10 +0100, Anthony PERARD wrote:
> >> This function insert a PCI passthrough device in qemu.
> >>
> >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> >> ---
> >>  tools/libxl/libxl_internal.h |    2 +
> >>  tools/libxl/libxl_qmp.c      |   89
++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 91 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/tools/libxl/libxl_internal.h
b/tools/libxl/libxl_internal.h
> >> index 4dd0f91..78e1be2 100644
> >> --- a/tools/libxl/libxl_internal.h
> >> +++ b/tools/libxl/libxl_internal.h
> >> @@ -119,6 +119,7 @@ typedef struct {
> >>  } libxl__device;
> >>
> >>  #define XC_PCI_BDF             "0x%x, 0x%x, 0x%x, 0x%x"
> >> +#define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3)
| ((func) & 0x07))
> >
> > No need to include the domain or dev in this? The name is pretty weird
> > if you don''t include the dev portion.  Looks like this is
actually a
> > vdevfn? xl has an open coded instance of this pattern for that --
worth
> > adding a public macro?
> 
> I have actual copy the macro from qemu, and I think "slot ===
dev".
It appears to yes.
>  I
> think the macro come from linux/pci.h.
> But yes, I use it to store the vdevfn.
> 
> > FWIW pcidev_encode_bdf() exists which encodes the whole lot from a
> > libxl_device_pci, if that what you end up needing.
> 
> If the vdefvn should contain the virtual bus and domain number, so
> yes, that will be the function I need.
I don''t think it does in the old qemu stuff, but I don''t know
about
upstream, I suspect not.> 


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

Ian Campbell

2011-Oct-07 18:40 UTC

head link

Re: [Xen-devel] [PATCH 5/7] libxl_qmp, Return the callback return code in qmp_next.

On Fri, 2011-10-07 at 16:23 +0100, Anthony PERARD wrote:> >> @@ -416,7 +418,7 @@ static int qmp_next(libxl__gc *gc,
> libxl__qmp_handler *qmp)
> >>                  s = end + 2;
> >>
> >>                  if (o) {
> >> -                    qmp_handle_response(qmp, o);
> >> +                    rc = qmp_handle_response(qmp, o);
> >
> > If rc now indicates error do we need to bail straight away or need
> to
> > keep going around this loop? (Or is it certain we will immediately
> fall
> > out of the loop after this?)
> 
> We can not be sure that we will return, because it could be another
> message in the butffer. So I should return if there is a protocol
> error. But I think that I should keep seperate the return code of a
> callback, so only the interested function (qmp_synchronous_send) will
> read it (and return to the caller).
I think this means the caller cannot tell which request or how many
errors have occurred in total.

I think that the callbacks should take a success/error code. If that
particular function is interested in propagating the error then they can
save it in their opaque data pointer to be retrieved by the caller of
the sync_send.

Another alternative could be to add an rc member to callback_id_pair
(making it more of a qmp request handle than an callback/id pair) and
push the allocation/free of that datastructure back up into the caller
of qmp_send (perhaps many callers of sync_send would just use an on
stack variable). Callers could then check the result of a specific
request by looking at that new member.

Ian.

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

Ian Jackson

2011-Oct-10 10:19 UTC

head link

Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/7] libxl, Introduce
dm-version xenstore key."):> Do you think /libxl/$domid is a good idea then or do you have somewhere
> else in mind?
/libxl/$domid is fine I think.  We need a clear rule that nothing
other than libxl will ever be expected to look there, so it''s entirely
a private path and does not ever form part of any API.

Ian.

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

Ian Campbell

2011-Oct-10 10:28 UTC

head link

Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

On Mon, 2011-10-10 at 11:19 +0100, Ian Jackson wrote:> Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/7] libxl, Introduce
dm-version xenstore key."):
> > Do you think /libxl/$domid is a good idea then or do you have
somewhere
> > else in mind?
> 
> /libxl/$domid is fine I think.  We need a clear rule that nothing
> other than libxl will ever be expected to look there, so it''s
entirely
> a private path and does not ever form part of any API.
Agreed, this includes toolstacks using libxl accessing directly also.

If we come up with a valid reason for toolstacks to store stuff in
xenstore then libxl can provide a user-data style interface to mediate
this use.

The permissions should be such that only the toolstack domain can access
it.

Ian.


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

Xen devel - Oct 2011 - [PATCH 0/7] libxl: QMP client improvement + pci passthrougth insert through QMP

[Xen-devel] [PATCH 0/7] libxl: QMP client improvement + pci passthrougth insert through QMP

[Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

[Xen-devel] [PATCH 2/7] libxl_qmp, Introduce an opaque argument to the callbacks.

[Xen-devel] [PATCH 3/7] libxl_qmp, Introduce list of arguments to qmp_send

[Xen-devel] [PATCH 4/7] libxl_qmp, Always insert a command id in the callback_list.

[Xen-devel] [PATCH 5/7] libxl_qmp, Return the callback return code in qmp_next.

[Xen-devel] [PATCH 6/7] libxl_qmp, Introduce libxl__qmp_pci_add.

[Xen-devel] [PATCH 7/7] libxl, Use QMP to insert a passthrough device when using upstream QEMU

Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

Re: [Xen-devel] [PATCH 2/7] libxl_qmp, Introduce an opaque argument to the callbacks.

Re: [Xen-devel] [PATCH 3/7] libxl_qmp, Introduce list of arguments to qmp_send

Re: [Xen-devel] [PATCH 4/7] libxl_qmp, Always insert a command id in the callback_list.

Re: [Xen-devel] [PATCH 5/7] libxl_qmp, Return the callback return code in qmp_next.

Re: [Xen-devel] [PATCH 6/7] libxl_qmp, Introduce libxl__qmp_pci_add.

Re: [Xen-devel] [PATCH 7/7] libxl, Use QMP to insert a passthrough device when using upstream QEMU

Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

Re: [Xen-devel] [PATCH 5/7] libxl_qmp, Return the callback return code in qmp_next.

Re: [Xen-devel] [PATCH 6/7] libxl_qmp, Introduce libxl__qmp_pci_add.

Re: [Xen-devel] [PATCH 7/7] libxl, Use QMP to insert a passthrough device when using upstream QEMU

Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

Re: [Xen-devel] [PATCH 7/7] libxl, Use QMP to insert a passthrough device when using upstream QEMU

Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

Re: [Xen-devel] [PATCH 6/7] libxl_qmp, Introduce libxl__qmp_pci_add.

Re: [Xen-devel] [PATCH 5/7] libxl_qmp, Return the callback return code in qmp_next.

Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.