[PATCH 0/11 OPW] All outstanding and corrected patches

[PATCH 0/11 OPW] All outstanding and corrected patches

[Kelley Nielsen]

These patches update libxl_qmp.c and libxl_utils.c to use the new
convenience macros defined in libxl_internal.h. These have all been
submitted before, and some are acked; but since some changes and
corrections have been made, they're collected here together to keep 
them organized.

[PATCH OPW 01/11] libxl: change most remaining LIBXL_LOG to LOG in
[PATCH OPW 02/11] libxl: Use new macro LOGE() in libxl_qmp.c
[PATCH OPW 03/11] libxl: use GCSPRINTF in place of libxl_sprintf() in
[PATCH OPW 04/11] libxl: use GCSPRINTF instead of libxl__sprintf
[PATCH OPW 05/11] libxl: use LOGE instead of LIBXL__LOG_ERRNO in
[PATCH OPW 06/11] libxl: use LOG and LOGE instead of LIBXL__LOG* in
[PATCH OPW 07/11] libxl: use LOG instead of LIBXL__LOG in
[PATCH OPW 09/11] opw: libxl: use macro CTX in libxl_qmp.c
[PATCH OPW 11/11] opw: libxl: use CTX macro in libxl_utils.c

[PATCH OPW 01/11] libxl: change most remaining LIBXL_LOG to LOG in libxl_qmp.c

[Kelley Nielsen]

Coding style has recently been changed for libxl. The convenience
macro LOG() has been introduced, and invocations of the old macro
LIBXL__LOG() are to be replaced with it. Change occurences of the
old macro to the new one in the functions qmp_handle_response()
and qmp_handle_error_response(). The new macros need access to a
local libxl__gc *gc, so add it as a parameter to both these functions,
and pass the instance in qmp_next() down the call chain to
qmp_handle_response() and in turn to qmp_handle_error_response().

Suggested-by: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
---
 tools/libxl/libxl_qmp.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 1f76d53..04cab58 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -138,7 +138,7 @@ static int register_serials_chardev_callback(libxl__qmp_handler *qmp,
             port_number = strtol(s, &endptr, 10);
             if (*s == 0 || *endptr != 0) {
                 LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR,
-                           "Invalid serial port number: %s", s);
+			   "Invalid serial port number: %s", s);
                 return -1;
             }
             ret = store_serial_port_info(qmp, chardev, port_number);
@@ -260,7 +260,7 @@ static callback_id_pair *qmp_get_callback_from_id(libxl__qmp_handler *qmp,
     return NULL;
 }
 
-static void qmp_handle_error_response(libxl__qmp_handler *qmp,
+static void qmp_handle_error_response(libxl__gc *gc, libxl__qmp_handler *qmp,
                                       const libxl__json_object *resp)
 {
     callback_id_pair *pp = qmp_get_callback_from_id(qmp, resp);
@@ -283,19 +283,17 @@ static void qmp_handle_error_response(libxl__qmp_handler *qmp,
         free(pp);
     }
 
-    LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR,
-               "received an error message from QMP server: %s",
-               libxl__json_object_get_string(resp));
+    LOG(ERROR, "received an error message from QMP server: %s",
+        libxl__json_object_get_string(resp));
 }
 
-static int qmp_handle_response(libxl__qmp_handler *qmp,
+static int qmp_handle_response(libxl__gc *gc, libxl__qmp_handler *qmp,
                                const libxl__json_object *resp)
 {
     libxl__qmp_message_type type = LIBXL__QMP_MESSAGE_TYPE_INVALID;
 
     type = qmp_response_type(qmp, resp);
-    LIBXL__LOG(qmp->ctx, LIBXL__LOG_DEBUG,
-               "message type: %s", libxl__qmp_message_type_to_string(type));
+    LOG(DEBUG, "message type: %s", libxl__qmp_message_type_to_string(type));
 
     switch (type) {
     case LIBXL__QMP_MESSAGE_TYPE_QMP:
@@ -317,14 +315,14 @@ static int qmp_handle_response(libxl__qmp_handler *qmp,
                 /* tell that the id have been processed */
                 qmp->wait_for_id = 0;
             }
-            LIBXL_STAILQ_REMOVE(
-                &qmp->callback_list, pp, callback_id_pair, next);
+            LIBXL_STAILQ_REMOVE(&qmp->callback_list, pp, callback_id_pair,
+			        next);
             free(pp);
         }
-        return 0;
+	return 0;
     }
     case LIBXL__QMP_MESSAGE_TYPE_ERROR:
-        qmp_handle_error_response(qmp, resp);
+        qmp_handle_error_response(gc, qmp, resp);
         return -1;
     case LIBXL__QMP_MESSAGE_TYPE_EVENT:
         return 0;
@@ -481,7 +479,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
                 o = libxl__json_parse(gc, s);
 
                 if (o) {
-                    rc = qmp_handle_response(qmp, o);
+                    rc = qmp_handle_response(gc, qmp, o);
                 } else {
                     LOG(ERROR, "Parse error of : %s\n", s);
                     return -1;
-- 
1.8.1.2

[PATCH OPW 02/11] libxl: Use new macro LOGE() in libxl_qmp.c

[Kelley Nielsen]

Code cleanup -- no functional changes

Coding style has recently been changed for libxl. The convenience
macro LOGE() has been introduced, and invocations of the old macro
LIBXL__LOG_ERROR() are to be replaced with it. Change all occurences
of the old macro (in functions that have a local libxl_gc *gc) except
the one in register_serials_chardev_callback() to the new one. (This
function lacks a local libxl__gc *gc, which LOGE() requires.)

Suggested-by: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_qmp.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 04cab58..9228165 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -342,8 +342,7 @@ static libxl__qmp_handler *qmp_init_handler(libxl__gc *gc, uint32_t domid)
 
     qmp = calloc(1, sizeof (libxl__qmp_handler));
     if (qmp == NULL) {
-        LIBXL__LOG_ERRNO(libxl__gc_owner(gc), LIBXL__LOG_ERROR,
-                         "Failed to allocate qmp_handler");
+        LOGE(ERROR, "Failed to allocate qmp_handler");
         return NULL;
     }
     qmp->ctx = libxl__gc_owner(gc);
@@ -437,7 +436,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
         } else if (ret < 0) {
             if (errno == EINTR)
                 continue;
-            LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR, "Select error");
+            LOGE(ERROR, "Select error");
             return -1;
         }
 
@@ -446,7 +445,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
             LOG(ERROR, "Unexpected end of socket");
             return -1;
         } else if (rd < 0) {
-            LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR, "Socket read error");
+            LOGE(ERROR, "Socket read error");
             return rd;
         }
 
@@ -533,8 +532,7 @@ static char *qmp_send_prepare(libxl__gc *gc, libxl__qmp_handler *qmp,
 
     elm = malloc(sizeof (callback_id_pair));
     if (elm == NULL) {
-        LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR,
-                         "Failed to allocate a QMP callback");
+        LOGE(ERROR, "Failed to allocate a QMP callback");
         goto out;
     }
     elm->id = qmp->last_id_used;
@@ -692,7 +690,7 @@ libxl__qmp_handler *libxl__qmp_initialize(libxl__gc *gc, uint32_t domid)
     qmp_socket = libxl__sprintf(gc, "%s/qmp-libxl-%d",
                                 libxl__run_dir_path(), domid);
     if ((ret = qmp_open(qmp, qmp_socket, QMP_SOCKET_CONNECT_TIMEOUT)) < 0) {
-        LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR, "Connection error");
+        LOGE(ERROR, "Connection error");
         qmp_free_handler(qmp);
         return NULL;
     }
@@ -724,16 +722,13 @@ void libxl__qmp_close(libxl__qmp_handler *qmp)
 
 void libxl__qmp_cleanup(libxl__gc *gc, uint32_t domid)
 {
-    libxl_ctx *ctx = libxl__gc_owner(gc);
     char *qmp_socket;
 
     qmp_socket = libxl__sprintf(gc, "%s/qmp-libxl-%d",
                                 libxl__run_dir_path(), domid);
     if (unlink(qmp_socket) == -1) {
         if (errno != ENOENT) {
-            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
-                             "Failed to remove QMP socket file %s",
-                             qmp_socket);
+            LOGE(ERROR, "Failed to remove QMP socket file %s", qmp_socket);
         }
     }
 }
-- 
1.8.1.2

[PATCH OPW 03/11] libxl: use GCSPRINTF in place of libxl_sprintf() in libxl_qmp.c

[Kelley Nielsen]

Code cleanup -- no functional changes

The convenience macro GCSPRINTF has been written to be used in place of
libxl_sprintf. Change all calls to libxl_sprintf() in libxl_qmp.c to
invocations of the new macro.

Suggested-by: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_qmp.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 9228165..b4985fe 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -101,7 +101,7 @@ static int store_serial_port_info(libxl__qmp_handler *qmp,
     }
 
     path = libxl__xs_get_dompath(gc, qmp->domid);
-    path = libxl__sprintf(gc, "%s/serial/%d/tty", path, port);
+    path = GCSPRINTF("%s/serial/%d/tty", path, port);
 
     ret = libxl__xs_write(gc, XBT_NULL, path, "%s", chardev + 4);
 
@@ -160,7 +160,7 @@ static int qmp_write_domain_console_item(libxl__gc *gc, int domid,
     char *path;
 
     path = libxl__xs_get_dompath(gc, domid);
-    path = libxl__sprintf(gc, "%s/console/%s", path, item);
+    path = GCSPRINTF("%s/console/%s", path, item);
 
     return libxl__xs_write(gc, XBT_NULL, path, "%s", value);
 }
@@ -672,8 +672,7 @@ static void qmp_parameters_add_integer(libxl__gc *gc,
 }
 
 #define QMP_PARAMETERS_SPRINTF(args, name, format, ...) \
-    qmp_parameters_add_string(gc, args, name, \
-                              libxl__sprintf(gc, format, __VA_ARGS__))
+    qmp_parameters_add_string(gc, args, name, GCSPRINTF(format, __VA_ARGS__))
 
 /*
  * API
@@ -687,8 +686,7 @@ libxl__qmp_handler *libxl__qmp_initialize(libxl__gc *gc, uint32_t domid)
 
     qmp = qmp_init_handler(gc, domid);
 
-    qmp_socket = libxl__sprintf(gc, "%s/qmp-libxl-%d",
-                                libxl__run_dir_path(), domid);
+    qmp_socket = GCSPRINTF("%s/qmp-libxl-%d", libxl__run_dir_path(), domid);
     if ((ret = qmp_open(qmp, qmp_socket, QMP_SOCKET_CONNECT_TIMEOUT)) < 0) {
         LOGE(ERROR, "Connection error");
         qmp_free_handler(qmp);
@@ -724,8 +722,7 @@ void libxl__qmp_cleanup(libxl__gc *gc, uint32_t domid)
 {
     char *qmp_socket;
 
-    qmp_socket = libxl__sprintf(gc, "%s/qmp-libxl-%d",
-                                libxl__run_dir_path(), domid);
+    qmp_socket = GCSPRINTF("%s/qmp-libxl-%d", libxl__run_dir_path(), domid);
     if (unlink(qmp_socket) == -1) {
         if (errno != ENOENT) {
             LOGE(ERROR, "Failed to remove QMP socket file %s", qmp_socket);
@@ -754,8 +751,8 @@ static int pci_add_callback(libxl__qmp_handler *qmp,
     const libxl__json_object *bus = NULL;
     GC_INIT(qmp->ctx);
     int i, j, rc = -1;
-    char *asked_id = libxl__sprintf(gc, PCI_PT_QDEV_ID,
-                                    pcidev->bus, pcidev->dev, pcidev->func);
+    char *asked_id = GCSPRINTF(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;
@@ -823,8 +820,8 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
     if (!qmp)
         return -1;
 
-    hostaddr = libxl__sprintf(gc, "%04x:%02x:%02x.%01x", pcidev->domain,
-                              pcidev->bus, pcidev->dev, pcidev->func);
+    hostaddr = GCSPRINTF("%04x:%02x:%02x.%01x", pcidev->domain,
+                         pcidev->bus, pcidev->dev, pcidev->func);
     if (!hostaddr)
         return -1;
 
@@ -860,8 +857,7 @@ int libxl__qmp_pci_del(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
 {
     char *id = NULL;
 
-    id = libxl__sprintf(gc, PCI_PT_QDEV_ID,
-                        pcidev->bus, pcidev->dev, pcidev->func);
+    id = GCSPRINTF(PCI_PT_QDEV_ID, pcidev->bus, pcidev->dev, pcidev->func);
 
     return qmp_device_del(gc, domid, id);
 }
-- 
1.8.1.2

[PATCH OPW 04/11] libxl: use GCSPRINTF instead of libxl__sprintf

[Kelley Nielsen]

Code cleanup - no functional changes

The convenience macro GCSPRINTF has been written to be used in place
of libxl__sprintf(). Replace all calls to libxl__sprintf() in
libxl_utils.c with invocations of the new macro.

Suggested-by: Ian Campbell <Ian.Campbell@citrix.com>
Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_utils.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 682f874..53ebf73 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -173,8 +173,8 @@ int libxl_get_stubdom_id(libxl_ctx *ctx, int guest_domid)
     int ret;
 
     stubdom_id_s = libxl__xs_read(gc, XBT_NULL,
-                                 libxl__sprintf(gc, "%s/image/device-model-domid",
-                                               libxl__xs_get_dompath(gc, guest_domid)));
+                                  GCSPRINTF("%s/image/device-model-domid",
+                                  libxl__xs_get_dompath(gc, guest_domid)));
     if (stubdom_id_s)
         ret = atoi(stubdom_id_s);
     else
@@ -190,7 +190,8 @@ int libxl_is_stubdom(libxl_ctx *ctx, uint32_t domid, uint32_t *target_domid)
     uint32_t value;
     int ret = 0;
 
-    target = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/target", libxl__xs_get_dompath(gc, domid)));
+    target = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/target",
+			    libxl__xs_get_dompath(gc, domid)));
     if (!target)
         goto out;
     value = strtol(target, &endptr, 10);
@@ -227,20 +228,20 @@ int libxl_create_logfile(libxl_ctx *ctx, const char *name, char **full_name)
     char *logfile, *logfile_new;
     int i, rc;
 
-    logfile = libxl__sprintf(gc, "/var/log/xen/%s.log", name);
+    logfile = GCSPRINTF("/var/log/xen/%s.log", name);
     if (stat(logfile, &stat_buf) == 0) {
         /* file exists, rotate */
-        logfile = libxl__sprintf(gc, "/var/log/xen/%s.log.10", name);
+        logfile = GCSPRINTF("/var/log/xen/%s.log.10", name);
         unlink(logfile);
         for (i = 9; i > 0; i--) {
-            logfile = libxl__sprintf(gc, "/var/log/xen/%s.log.%d", name, i);
-            logfile_new = libxl__sprintf(gc, "/var/log/xen/%s.log.%d", name, i + 1);
+            logfile = GCSPRINTF("/var/log/xen/%s.log.%d", name, i);
+            logfile_new = GCSPRINTF("/var/log/xen/%s.log.%d", name, i + 1);
             rc = logrename(gc, logfile, logfile_new);
             if (rc)
                 goto out;
         }
-        logfile = libxl__sprintf(gc, "/var/log/xen/%s.log", name);
-        logfile_new = libxl__sprintf(gc, "/var/log/xen/%s.log.1", name);
+        logfile = GCSPRINTF("/var/log/xen/%s.log", name);
+        logfile_new = GCSPRINTF("/var/log/xen/%s.log.1", name);
 
         rc = logrename(gc, logfile, logfile_new);
         if (rc)
-- 
1.8.1.2

[PATCH OPW 05/11] libxl: use LOGE instead of LIBXL__LOG_ERRNO in libxl_utils.c

[Kelley Nielsen]

Code cleanup - no functional changes

The convenience macro LOGE has been written to take the place of
LIBXL__LOG_ERRNO. LOGE depends on the existence of a local libgl__gc
*gc. Replace two invocations of LIBXL__LOG_ERRNO, which are in
functions that already have a libxl__gc *gc present, to invocations
of the new macro.

Suggested-by: Ian Campbell <Ian.Campbell@citrix.com>
Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
---
 tools/libxl/libxl_utils.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 53ebf73..ca3406b 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -207,15 +207,14 @@ out:
 
 static int logrename(libxl__gc *gc, const char *old, const char *new)
 {
-    libxl_ctx *ctx = libxl__gc_owner(gc);
     int r;
 
     r = rename(old, new);
     if (r) {
         if (errno == ENOENT) return 0; /* ok */
 
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to rotate logfile - could not"
-                     " rename %s to %s", old, new);
+        LOGE(ERROR, "failed to rotate logfile - "
+	     "could not rename %s to %s", old, new);
         return ERROR_FAIL;
     }
     return 0;
@@ -248,9 +247,9 @@ int libxl_create_logfile(libxl_ctx *ctx, const char *name, char **full_name)
             goto out;
     } else {
         if (errno != ENOENT)
-            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_WARNING, "problem checking existence of"
-                         " logfile %s, which might have needed to be rotated",
-                         name);
+            LOGE(WARN, "problem checking existence of logfile %s, "
+	         "which might have needed to be rotated",
+                 name);
     }
     *full_name = strdup(logfile);
     rc = 0;
-- 
1.8.1.2

[PATCH OPW 06/11] libxl: use LOG and LOGE instead of LIBXL__LOG* in libxl_utils.c

[Kelley Nielsen]

Code cleanup - no functional changes

The convenience macros LOG and LOGE have been written to take the
place of the old macros in the LIBXL__LOG* family. Replace the
invocations of the old macros in the function libxl_read_file_contents()
with invocations of the corresponding new ones. Create a local
libxl__gc gc* for the new macros to use by invoking GC_INIT(ctx) at the
top of the function, and clean it up by invoking GC_FREE at the two
exit points.

Suggested-by: Ian Campbell <Ian.Campbell@citrix.com>
Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_utils.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index ca3406b..cc6c342 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -290,6 +290,7 @@ out:
 
 int libxl_read_file_contents(libxl_ctx *ctx, const char *filename,
                              void **data_r, int *datalen_r) {
+    GC_INIT(ctx);
     FILE *f = 0;
     uint8_t *data = 0;
     int datalen = 0;
@@ -300,23 +301,23 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char *filename,
     f = fopen(filename, "r");
     if (!f) {
         if (errno == ENOENT) return ENOENT;
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to open %s", filename);
+        LOGE(ERROR, "failed to open %s", filename);
         goto xe;
     }
 
     if (fstat(fileno(f), &stab)) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to fstat %s", filename);
+        LOGE(ERROR, "failed to fstat %s", filename);
         goto xe;
     }
 
     if (!S_ISREG(stab.st_mode)) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "%s is not a plain file", filename);
+        LOGE(ERROR, "%s is not a plain file", filename);
         errno = ENOTTY;
         goto xe;
     }
 
     if (stab.st_size > INT_MAX) {
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "file %s is far too large", filename);
+        LOG(ERROR, "file %s is far too large", filename);
         errno = EFBIG;
         goto xe;
     }
@@ -330,10 +331,10 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char *filename,
         rs = fread(data, 1, datalen, f);
         if (rs != datalen) {
             if (ferror(f))
-                LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to read %s", filename);
+                LOGE(ERROR, "failed to read %s", filename);
             else if (feof(f))
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "%s changed size while we"
-                       " were reading it", filename);
+                LOG(ERROR, "%s changed size while we were reading it",
+		    filename);
             else
                 abort();
             goto xe;
@@ -342,16 +343,18 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char *filename,
 
     if (fclose(f)) {
         f = 0;
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to close %s", filename);
+        LOGE(ERROR, "failed to close %s", filename);
         goto xe;
     }
 
     if (data_r) *data_r = data;
     if (datalen_r) *datalen_r = datalen;
 
+    GC_FREE;
     return 0;
 
  xe:
+    GC_FREE;
     e = errno;
     assert(e != ENOENT);
     if (f) fclose(f);
-- 
1.8.1.2

[PATCH OPW 07/11] libxl: use LOG instead of LIBXL__LOG in libxl_utils.c

[Kelley Nielsen]

To conform to the new coding style, replace the invocation of
LIBXL__LOG in the function libxl_pipe() in the file libxl_utils.c
with an invocation of LOG. Create a local libxl__gc gc* for LOG
to use by invoking GC_INIT(ctx) at the top of the function, and
clean it up by invoking GC_FREE at the exit. Create a variable,
ret, to consolidate exits in one place and avoid invoking GC_FREE
twice.

Suggested-by: Ian Campbell <Ian.Campbell@citrix.com>
Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl_utils.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index cc6c342..cac54e6 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -478,11 +478,14 @@ int libxl__remove_directory(libxl__gc *gc, const char *dirpath)
 
 int libxl_pipe(libxl_ctx *ctx, int pipes[2])
 {
+    GC_INIT(ctx);
+    int ret = 0;
     if (pipe(pipes) < 0) {
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Failed to create a pipe");
-        return -1;
+        LOG(ERROR, "Failed to create a pipe");
+        ret = -1;
     }
-    return 0;
+    GC_FREE;
+    return ret;
 }
 
 int libxl_uuid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
-- 
1.8.1.2

[PATCH OPW 08/11] opw: libxl: add convenience macros to qmp_send() in libxl_qmp.c

[Kelley Nielsen]

Update qmp_send() in libxl_qmp.c to use the new convenience macros
declared in libxl_internal.h. Uses GC_INIT at the top of the function,
and GC_FREE at the exit. Since GC_INIT returns a libxl__gc by reference
and not by value, remove the address operator from the left of the
variable gc where it is passed as a parameter.

Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
Suggested-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_qmp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index b4985fe..9a66c6d 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -557,9 +557,9 @@ static int qmp_send(libxl__qmp_handler *qmp,
 {
     char *buf = NULL;
     int rc = -1;
-    libxl__gc gc; LIBXL_INIT_GC(gc,qmp->ctx);
+    GC_INIT(qmp->ctx);
 
-    buf = qmp_send_prepare(&gc, qmp, cmd, args, callback, opaque, context);
+    buf = qmp_send_prepare(gc, qmp, cmd, args, callback, opaque, context);
 
     if (buf == NULL) {
         goto out;
@@ -574,7 +574,7 @@ static int qmp_send(libxl__qmp_handler *qmp,
 
     rc = qmp->last_id_used;
 out:
-    libxl__free_all(&gc);
+    GC_FREE;
     return rc;
 }
 
-- 
1.8.1.2

[PATCH OPW 09/11] opw: libxl: use macro CTX in libxl_qmp.c

[Kelley Nielsen]

The new coding style uses the convenience macro CTX as declared in
libxl_internal.h. Substitute an invocation of this macro for its body
at the one place it occurs in libxl_qmp.c.

Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
Suggested-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_qmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 9a66c6d..f1f6352 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -345,7 +345,7 @@ static libxl__qmp_handler *qmp_init_handler(libxl__gc *gc, uint32_t domid)
         LOGE(ERROR, "Failed to allocate qmp_handler");
         return NULL;
     }
-    qmp->ctx = libxl__gc_owner(gc);
+    qmp->ctx = CTX;
     qmp->domid = domid;
     qmp->timeout = 5;
 
-- 
1.8.1.2

[PATCH OPW 10/11] opw: libxl: use macro GCNEW in libxl_qmp.c

[Kelley Nielsen]

The new coding style uses the convenience macro GCNEW as declared in
libxl_internal.h. Substitute an invocation of this macro for its
body at the one place it occurs in libxl_qmp.c.

Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
Suggested-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_qmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index f1f6352..20d925f 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -628,7 +628,7 @@ static void qmp_parameters_common_add(libxl__gc *gc,
         *param = libxl__json_object_alloc(gc, JSON_MAP);
     }
 
-    arg = libxl__zalloc(gc, sizeof(*arg));
+    GCNEW(arg);
 
     arg->map_key = libxl__strdup(gc, name);
     arg->obj = obj;
-- 
1.8.1.2

[PATCH OPW 11/11] opw: libxl: use CTX macro in libxl_utils.c

[Kelley Nielsen]

The new coding style uses the convenience macro CTX as declared in
libxl_internal.h. Substitute an invocation of this macro for its
body at the two places it occurs in libxl_utils.c.

Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
Suggested-by: Ian Campbell <Ian.Campbell@citrix.com>
---
 tools/libxl/libxl_utils.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index cac54e6..f078caa 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -57,7 +57,7 @@ char *libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid)
 
 char *libxl__domid_to_name(libxl__gc *gc, uint32_t domid)
 {
-    char *s = libxl_domid_to_name(libxl__gc_owner(gc), domid);
+    char *s = libxl_domid_to_name(CTX, domid);
     libxl__ptr_add(gc, s);
     return s;
 }
@@ -135,7 +135,7 @@ int libxl_cpupoolid_is_valid(libxl_ctx *ctx, uint32_t poolid)
 
 char *libxl__cpupoolid_to_name(libxl__gc *gc, uint32_t poolid)
 {
-    char *s = libxl_cpupoolid_to_name(libxl__gc_owner(gc), poolid);
+    char *s = libxl_cpupoolid_to_name(CTX, poolid);
     libxl__ptr_add(gc, s);
     return s;
 }
-- 
1.8.1.2

Re: [PATCH 0/11 OPW] All outstanding and corrected patches

[Matt Wilson]

On Mon, Nov 11, 2013 at 03:23:49PM -0800, Kelley Nielsen wrote:
> These patches update libxl_qmp.c and libxl_utils.c to use the new
> convenience macros defined in libxl_internal.h. These have all been
> submitted before, and some are acked; but since some changes and
> corrections have been made, they're collected here together to keep 
> them organized.

Hi Kelley,

Two minor points of feedback on process, rather than these actual
changes:

1) Patch 10 is missing below.
2) There's not a summary of what's changed since the last time you
   posted the series. It'd be nice to have a summary of which patches
   have been reviewed / Ack'ed in the cover letter. For a more complex
   example, see other cover letters like:
     http://lists.xen.org/archives/html/xen-devel/2013-11/msg01374.html

--msw

> [PATCH OPW 01/11] libxl: change most remaining LIBXL_LOG to LOG in
> [PATCH OPW 02/11] libxl: Use new macro LOGE() in libxl_qmp.c
> [PATCH OPW 03/11] libxl: use GCSPRINTF in place of libxl_sprintf() in
> [PATCH OPW 04/11] libxl: use GCSPRINTF instead of libxl__sprintf
> [PATCH OPW 05/11] libxl: use LOGE instead of LIBXL__LOG_ERRNO in
> [PATCH OPW 06/11] libxl: use LOG and LOGE instead of LIBXL__LOG* in
> [PATCH OPW 07/11] libxl: use LOG instead of LIBXL__LOG in
> [PATCH OPW 09/11] opw: libxl: use macro CTX in libxl_qmp.c
> [PATCH OPW 11/11] opw: libxl: use CTX macro in libxl_utils.c

Re: [PATCH OPW 01/11] libxl: change most remaining LIBXL_LOG to LOG in libxl_qmp.c

[Anthony PERARD]

On Mon, Nov 11, 2013 at 03:23:50PM -0800, Kelley Nielsen wrote:
> Coding style has recently been changed for libxl. The convenience
> macro LOG() has been introduced, and invocations of the old macro
> LIBXL__LOG() are to be replaced with it. Change occurences of the
> old macro to the new one in the functions qmp_handle_response()
> and qmp_handle_error_response(). The new macros need access to a
> local libxl__gc *gc, so add it as a parameter to both these functions,
> and pass the instance in qmp_next() down the call chain to
> qmp_handle_response() and in turn to qmp_handle_error_response().
> 
> Suggested-by: Anthony PERARD <anthony.perard@citrix.com>
> Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
> ---
>  tools/libxl/libxl_qmp.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index 1f76d53..04cab58 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -138,7 +138,7 @@ static int register_serials_chardev_callback(libxl__qmp_handler *qmp,
>              port_number = strtol(s, &endptr, 10);
>              if (*s == 0 || *endptr != 0) {
>                  LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR,
> -                           "Invalid serial port number: %s", s);
> +			   "Invalid serial port number: %s", s);

You are introducing <TAB>s here, there should be no tabs in C files.

>                  return -1;
>              }
>              ret = store_serial_port_info(qmp, chardev, port_number);
> @@ -260,7 +260,7 @@ static callback_id_pair *qmp_get_callback_from_id(libxl__qmp_handler *qmp,
>      return NULL;
>  }
>  
> -static void qmp_handle_error_response(libxl__qmp_handler *qmp,
> +static void qmp_handle_error_response(libxl__gc *gc, libxl__qmp_handler *qmp,
>                                        const libxl__json_object *resp)
>  {
>      callback_id_pair *pp = qmp_get_callback_from_id(qmp, resp);
> @@ -283,19 +283,17 @@ static void qmp_handle_error_response(libxl__qmp_handler *qmp,
>          free(pp);
>      }
>  
> -    LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR,
> -               "received an error message from QMP server: %s",
> -               libxl__json_object_get_string(resp));
> +    LOG(ERROR, "received an error message from QMP server: %s",
> +        libxl__json_object_get_string(resp));
>  }
>  
> -static int qmp_handle_response(libxl__qmp_handler *qmp,
> +static int qmp_handle_response(libxl__gc *gc, libxl__qmp_handler *qmp,
>                                 const libxl__json_object *resp)
>  {
>      libxl__qmp_message_type type = LIBXL__QMP_MESSAGE_TYPE_INVALID;
>  
>      type = qmp_response_type(qmp, resp);
> -    LIBXL__LOG(qmp->ctx, LIBXL__LOG_DEBUG,
> -               "message type: %s", libxl__qmp_message_type_to_string(type));
> +    LOG(DEBUG, "message type: %s", libxl__qmp_message_type_to_string(type));
>  
>      switch (type) {
>      case LIBXL__QMP_MESSAGE_TYPE_QMP:
> @@ -317,14 +315,14 @@ static int qmp_handle_response(libxl__qmp_handler *qmp,
>                  /* tell that the id have been processed */
>                  qmp->wait_for_id = 0;
>              }
> -            LIBXL_STAILQ_REMOVE(
> -                &qmp->callback_list, pp, callback_id_pair, next);
> +            LIBXL_STAILQ_REMOVE(&qmp->callback_list, pp, callback_id_pair,
> +			        next);

Same here (tabs).

>              free(pp);
>          }
> -        return 0;
> +	return 0;

And here.

>      }
>      case LIBXL__QMP_MESSAGE_TYPE_ERROR:
> -        qmp_handle_error_response(qmp, resp);
> +        qmp_handle_error_response(gc, qmp, resp);
>          return -1;
>      case LIBXL__QMP_MESSAGE_TYPE_EVENT:
>          return 0;
> @@ -481,7 +479,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
>                  o = libxl__json_parse(gc, s);
>  
>                  if (o) {
> -                    rc = qmp_handle_response(qmp, o);
> +                    rc = qmp_handle_response(gc, qmp, o);
>                  } else {
>                      LOG(ERROR, "Parse error of : %s\n", s);
>                      return -1;
> -- 
> 1.8.1.2
> 

Once the tabs are replace by space:
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

-- 
Anthony PERARD

Re: [PATCH OPW 05/11] libxl: use LOGE instead of LIBXL__LOG_ERRNO in libxl_utils.c

[Anthony PERARD]

On Mon, Nov 11, 2013 at 03:23:54PM -0800, Kelley Nielsen wrote:
> Code cleanup - no functional changes
> 
> The convenience macro LOGE has been written to take the place of
> LIBXL__LOG_ERRNO. LOGE depends on the existence of a local libgl__gc
> *gc. Replace two invocations of LIBXL__LOG_ERRNO, which are in
> functions that already have a libxl__gc *gc present, to invocations
> of the new macro.
> 
> Suggested-by: Ian Campbell <Ian.Campbell@citrix.com>
> Signed-off-by: Kelley Nielsen <kelleynnn@gmail.com>
> ---
>  tools/libxl/libxl_utils.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> index 53ebf73..ca3406b 100644
> --- a/tools/libxl/libxl_utils.c
> +++ b/tools/libxl/libxl_utils.c
> @@ -207,15 +207,14 @@ out:
>  
>  static int logrename(libxl__gc *gc, const char *old, const char *new)
>  {
> -    libxl_ctx *ctx = libxl__gc_owner(gc);
>      int r;
>  
>      r = rename(old, new);
>      if (r) {
>          if (errno == ENOENT) return 0; /* ok */
>  
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to rotate logfile - could not"
> -                     " rename %s to %s", old, new);
> +        LOGE(ERROR, "failed to rotate logfile - "
> +	     "could not rename %s to %s", old, new);

You are introducing TABs here.

>          return ERROR_FAIL;
>      }
>      return 0;
> @@ -248,9 +247,9 @@ int libxl_create_logfile(libxl_ctx *ctx, const char *name, char **full_name)
>              goto out;
>      } else {
>          if (errno != ENOENT)
> -            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_WARNING, "problem checking existence of"
> -                         " logfile %s, which might have needed to be rotated",
> -                         name);
> +            LOGE(WARN, "problem checking existence of logfile %s, "
> +	         "which might have needed to be rotated",

Same here.

> +                 name);
>      }
>      *full_name = strdup(logfile);
>      rc = 0;

Once the TABs are replaced:
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

-- 
Anthony PERARD

Re: [PATCH 0/11 OPW] All outstanding and corrected patches

[Ian Campbell]

On Mon, 2013-11-11 at 15:23 -0800, Kelley Nielsen wrote:
> These patches update libxl_qmp.c and libxl_utils.c to use the new
> convenience macros defined in libxl_internal.h. These have all been
> submitted before, and some are acked; but since some changes and
> corrections have been made, they're collected here together to keep 
> them organized.

George gave me a release Ack on IRC so I've gone ahead and picked these
up, including v2 of #1 and #5. For future reference please either post
these as replies to the relevant patch (git send-email --reply-to) or
resend the whole series, otherwise it's very confusing to figure out
what needs to be applied.

A few patches were already applied, I suspect because you were basing
your series on master and not staging (the latter is propagated to the
former when the automated tests pass). There were one or two minor
rejects for the same reason which I fixed up.

Please check the staging branch in case I missed one, picked up the
wrong thing or messed something up etc.

> [PATCH OPW 01/11] libxl: change most remaining LIBXL_LOG to LOG in
> [PATCH OPW 02/11] libxl: Use new macro LOGE() in libxl_qmp.c
> [PATCH OPW 03/11] libxl: use GCSPRINTF in place of libxl_sprintf() in
> [PATCH OPW 04/11] libxl: use GCSPRINTF instead of libxl__sprintf
> [PATCH OPW 05/11] libxl: use LOGE instead of LIBXL__LOG_ERRNO in
> [PATCH OPW 06/11] libxl: use LOG and LOGE instead of LIBXL__LOG* in
> [PATCH OPW 07/11] libxl: use LOG instead of LIBXL__LOG in
> [PATCH OPW 09/11] opw: libxl: use macro CTX in libxl_qmp.c
> [PATCH OPW 11/11] opw: libxl: use CTX macro in libxl_utils.c