Skip to contentSkip to conversation

tools: 3 misc fixes/cleanups

PATCH In review Ian Campbell 3 participants 14 messages Jul 7, 2015 – Jul 8, 2015

lore
Share & tools
lore

Ian Campbell

author
#0 lore raw

cover tools: 3 misc fixes/cleanups

Here are five patches from my "split up libxenctrl" working branch (which clearly isn't going to make 4.6) which are independent fixes which could go into 4.6.

Ian.

author
#1 lore raw

PATCH 1/5 tools: libxc: fix "alocated" typo in comment

Signed-off-by Ian Campbell <[email protected]>
---
 tools/libxc/include/xenctrl.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/libxc/include/xenctrl.h +1 -1
index d1d2ab3..74f2e12 100644
@@ -376,7 +376,7 @@ void xc__hypercall_buffer_free_pages(xc_interface *xch, xc_hypercall_buffer_t *b
  * buffer and call xc_hypercall_buffer_array_get().
  *
  * Destroy the array with xc_hypercall_buffer_array_destroy() to free
- * the array and all its alocated hypercall buffers.
+ * the array and all its allocated hypercall buffers.
  */
 struct xc_hypercall_buffer_array;
 typedef struct xc_hypercall_buffer_array xc_hypercall_buffer_array_t;
Signature · 1 line
2.1.4
author
#3 lore raw

PATCH 2/5 libxc: Correct log message in xc_map_foreign_bulk

In reply to Ian Campbell

Things are confusing enough as it is without using another function's name here.

Signed-off-by Ian Campbell <[email protected]>
---
 tools/libxc/xc_freebsd_osdep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/libxc/xc_freebsd_osdep.c +2 -2
index e6613ef..011112b 100644
@@ -160,7 +160,7 @@ static void *freebsd_privcmd_map_foreign_bulk(xc_interface *xch,
     addr = mmap(NULL, num << XC_PAGE_SHIFT, prot, MAP_SHARED, fd, 0);
     if ( addr == MAP_FAILED )
     {
-        PERROR("xc_map_foreign_batch: mmap failed");
+        PERROR("xc_map_foreign_bulk: mmap failed");
         return NULL;
     }
 

@@ -174,7 +174,7 @@ static void *freebsd_privcmd_map_foreign_bulk(xc_interface *xch,
     if ( rc < 0 )
     {
         int saved_errno = errno;
-        PERROR("xc_map_foreign_batch: ioctl failed");
+        PERROR("xc_map_foreign_bulk: ioctl failed");
         (void)munmap(addr, num << XC_PAGE_SHIFT);
         errno = saved_errno;
         return NULL;
Signature · 1 line
2.1.4
Acked-by on v1 2/5
#4 lore raw

Re: [PATCH 2/5] libxc: Correct log message in xc_map_foreign_bulk

Ian Campbell writes ("[PATCH 2/5] libxc: Correct log message in xc_map_foreign_bulk"):

Quoted 2 lines

Things are confusing enough as it is without using another function's name here.

Acked-by Ian Jackson <[email protected]>
author
#5 lore raw

PATCH 3/5 tools: libxl: Log on more error paths on domain create failure

In reply to Ian Campbell

The setdefault functions do not generally log why they didn't like things.

Signed-off-by Ian Campbell <[email protected]>
---
 tools/libxl/libxl_create.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)
tools/libxl/libxl_create.c +16 -4
index f366a09..9d91e89 100644
@@ -899,7 +899,10 @@ static void initiate_domain_create(libxl__egc *egc,
     }
 
     ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info);
-    if (ret) goto error_out;
+    if (ret) {
+        LOG(ERROR, "Unable to set domain create info defaults");
+        goto error_out;
+    }
 
     ret = libxl__domain_make(gc, d_config, &domid, &state->config);
     if (ret) {

@@ -913,7 +916,10 @@ static void initiate_domain_create(libxl__egc *egc,
     dcs->dmss.dm.guest_domid = 0; /* means we haven't spawned */
 
     ret = libxl__domain_build_info_setdefault(gc, &d_config->b_info);
-    if (ret) goto error_out;
+    if (ret) {
+        LOG(ERROR, "Unable to set domain build info defaults");
+        goto error_out;
+    }
 
     if (!sched_params_valid(gc, domid, &d_config->b_info.sched_params)) {
         LOG(ERROR, "Invalid scheduling parameters\n");

@@ -923,7 +929,10 @@ static void initiate_domain_create(libxl__egc *egc,
 
     for (i = 0; i < d_config->num_disks; i++) {
         ret = libxl__device_disk_setdefault(gc, &d_config->disks[i]);
-        if (ret) goto error_out;
+        if (ret) {
+            LOG(ERROR, "Unable to set disk defaults for disk %d", i);
+            goto error_out;
+        }
     }
 
     dcs->bl.ao = ao;

@@ -943,7 +952,10 @@ static void initiate_domain_create(libxl__egc *egc,
          * but qemu needs the nic information to be complete.
          */
         ret = libxl__device_nic_setdefault(gc, &d_config->nics[i], domid);
-        if (ret) goto error_out;
+        if (ret) {
+            LOG(ERROR, "Unable to set nic defaults for nic %d", i);
+            goto error_out;
+        }
 
         if (d_config->nics[i].devid > last_devid)
             last_devid = d_config->nics[i].devid;
Signature · 1 line
2.1.4
Acked-by on v1 3/5
#6 lore raw

Re: [PATCH 3/5] tools: libxl: Log on more error paths on domain create failure

Ian Campbell writes ("[PATCH 3/5] tools: libxl: Log on more error paths on domain create failure"):

Quoted 2 lines

The setdefault functions do not generally log why they didn't like things.

Acked-by Ian Jackson <[email protected]>
author
#7 lore raw

PATCH 4/5 tools: Link in-tree libvchan and libblktapctl users against lib....so

In reply to Ian Campbell

As with other in-tree users avoid -L + -l.

This avoids any confusion with versions of these libraries already installed on the system and the possibility of linking against them by mistake.

Signed-off-by Ian Campbell <[email protected]>
---
 tools/Rules.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/Rules.mk +2 -2
index 3c29d07..2c422bd 100644
@@ -51,7 +51,7 @@ LDLIBS_libxenstat  = $(SHLIB_libxenctrl) $(SHLIB_libxenstore) $(XEN_LIBXENSTAT)/
 SHLIB_libxenstat  = -Wl,-rpath-link=$(XEN_LIBXENSTAT)
 
 CFLAGS_libxenvchan = -I$(XEN_LIBVCHAN)
-LDLIBS_libxenvchan = $(SHLIB_libxenctrl) $(SHLIB_libxenstore) -L$(XEN_LIBVCHAN) -lxenvchan
+LDLIBS_libxenvchan = $(SHLIB_libxenctrl) $(SHLIB_libxenstore) $(XEN_LIBVCHAN)/libxenvchan$(libextension)
 SHLIB_libxenvchan  = -Wl,-rpath-link=$(XEN_LIBVCHAN)
 
 ifeq ($(debug),y)

@@ -65,7 +65,7 @@ LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
 
 ifeq ($(LIBXL_BLKTAP),y)
 CFLAGS_libblktapctl = -I$(XEN_BLKTAP2)/control -I$(XEN_BLKTAP2)/include $(CFLAGS_xeninclude)
-LDLIBS_libblktapctl = -L$(XEN_BLKTAP2)/control -lblktapctl
+LDLIBS_libblktapctl = $(XEN_BLKTAP2)/control/libblktapctl$(libextension)
 SHLIB_libblktapctl  = -Wl,-rpath-link=$(XEN_BLKTAP2)/control
 else
 CFLAGS_libblktapctl =
Signature · 1 line
2.1.4
Acked-by on v1 4/5
#8 lore raw

Re: [PATCH 4/5] tools: Link in-tree libvchan and libblktapctl users against lib....so

Ian Campbell writes ("[PATCH 4/5] tools: Link in-tree libvchan and libblktapctl users against lib....so"):

Quoted 5 lines

As with other in-tree users avoid -L + -l.

This avoids any confusion with versions of these libraries already installed on the system and the possibility of linking against them by mistake.

Acked-by Ian Jackson <[email protected]>
author
#9 lore raw

PATCH 5/5 tools: Do not add top-level tools dir to include path

In reply to Ian Campbell

Instead switch to an explicit -include $(XEN_ROOT)/tools/config.h to pickup config.h.

Most places already do this, fixup the rest.

Signed-off-by Ian Campbell <[email protected]>
---
 tools/libfsimage/ext2fs-lib/Makefile     | 4 ++--
 tools/libfsimage/ext2fs-lib/ext2fs-lib.c | 3 ---
 tools/misc/Makefile                      | 4 +++-
 tools/misc/gtraceview.c                  | 3 ---
 tools/ocaml/xenstored/Makefile           | 3 ++-
 tools/ocaml/xenstored/systemd_stubs.c    | 1 -
 tools/xenstat/xentop/Makefile            | 4 ++--
 tools/xenstat/xentop/xentop.c            | 3 ---
 tools/xenstore/Makefile                  | 3 ++-
 tools/xenstore/xenstored_core.c          | 1 -
 10 files changed, 11 insertions(+), 18 deletions(-)
tools/libfsimage/ext2fs-lib/Makefile +2 -2
index 671fbff..0e00fde 100644
@@ -6,8 +6,8 @@ FS = ext2fs-lib
 
 FS_LIBDEPS = $(EXTFS_LIBS)
 
-# Include configure output (config.h) to headers search path
-CFLAGS += -I$(XEN_ROOT)/tools
+# Include configure output (config.h)
+CFLAGS += -include $(XEN_ROOT)/tools/config.h
 
 .PHONY: all
 all: fs-all
tools/libfsimage/ext2fs-lib/ext2fs-lib.c +0 -3
index ed47146..84b6d1e 100644
@@ -21,9 +21,6 @@
  * Use is subject to license terms.
  */
 
-/* Include output from configure */
-#include <config.h>
-
 #include <fsimage_plugin.h>
 #include INCLUDE_EXTFS_H
 #include <errno.h>
tools/misc/Makefile +3 -1
index 82c361f..c4490f3 100644
@@ -1,7 +1,9 @@
 XEN_ROOT=$(CURDIR)/../..
 include $(XEN_ROOT)/tools/Rules.mk
 
-CFLAGS += -Werror -I$(XEN_ROOT)/tools
+CFLAGS += -Werror
+# Include configure output (config.h)
+CFLAGS += -include $(XEN_ROOT)/tools/config.h
 CFLAGS += $(CFLAGS_libxenctrl)
 CFLAGS += $(CFLAGS_xeninclude)
 CFLAGS += $(CFLAGS_libxenstore)
tools/misc/gtraceview.c +0 -3
index 501f86a..d9f1185 100644
@@ -16,9 +16,6 @@
  * Place - Suite 330, Boston, MA 02111-1307 USA.
  */
 
-/* Include output from configure */
-#include <config.h>
-
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
tools/ocaml/xenstored/Makefile +2 -1
index bcf09d4..d861f11 100644
@@ -2,7 +2,8 @@ XEN_ROOT = $(CURDIR)/../../..
 OCAML_TOPLEVEL = $(CURDIR)/..
 include $(OCAML_TOPLEVEL)/common.make
 
-CFLAGS += -I$(XEN_ROOT)/tools/
+# Include configure output (config.h)
+CFLAGS += -include $(XEN_ROOT)/tools/config.h
 CFLAGS-$(CONFIG_SYSTEMD)  += $(SYSTEMD_CFLAGS)
 LDFLAGS-$(CONFIG_SYSTEMD) += $(SYSTEMD_LIBS)
tools/ocaml/xenstored/systemd_stubs.c +0 -1
index 623592c..d924ff1 100644
@@ -22,7 +22,6 @@
 #include <caml/custom.h>
 #include <caml/signals.h>
 #include <caml/fail.h>
-#include <config.h>
 
 #if defined(HAVE_SYSTEMD)
tools/xenstat/xentop/Makefile +2 -2
index 97950b9..e67c9ab 100644
@@ -22,8 +22,8 @@ CFLAGS += -DGCC_PRINTF -Werror $(CFLAGS_libxenstat)
 LDLIBS += $(LDLIBS_libxenstat) $(CURSES_LIBS) $(TINFO_LIBS) $(SOCKET_LIBS) -lm -lyajl
 CFLAGS += -DHOST_$(XEN_OS)
 
-# Include configure output (config.h) to headers search path
-CFLAGS += -I$(XEN_ROOT)/tools
+# Include configure output (config.h)
+CFLAGS += -include $(XEN_ROOT)/tools/config.h
 LDFLAGS += $(APPEND_LDFLAGS)
 
 .PHONY: all
tools/xenstat/xentop/xentop.c +0 -3
index 23b57f1..8173886 100644
@@ -19,9 +19,6 @@
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-/* Include output from configure */
-#include <config.h>
-
 /* get curses header from configure */
 #include INCLUDE_CURSES_H
tools/xenstore/Makefile +2 -1
index 30f3b46..1b4a494 100644
@@ -6,7 +6,8 @@ MINOR = 3
 
 CFLAGS += -Werror
 CFLAGS += -I.
-CFLAGS += -I$(XEN_ROOT)/tools/
+# Include configure output (config.h)
+CFLAGS += -include $(XEN_ROOT)/tools/config.h
 CFLAGS += -I./include
 CFLAGS += $(CFLAGS_libxenctrl)
 CFLAGS += -DXEN_LIB_STORED="\"$(XEN_LIB_STORED)\""
tools/xenstore/xenstored_core.c +0 -1
index 3fd9a20..b18000d 100644
@@ -40,7 +40,6 @@
 #include <signal.h>
 #include <assert.h>
 #include <setjmp.h>
-#include <config.h>
 
 #include "utils.h"
 #include "list.h"
Signature · 1 line
2.1.4
Acked-by on v1 5/5
#10 lore raw

Re: [PATCH 5/5] tools: Do not add top-level tools dir to include path

Ian Campbell writes ("[PATCH 5/5] tools: Do not add top-level tools dir to include path"):

Quoted 4 lines

Instead switch to an explicit -include $(XEN_ROOT)/tools/config.h to pickup config.h.

Most places already do this, fixup the rest.

Acked-by Ian Jackson <[email protected]>
Acked-by on the v1 series
#11 lore raw
In reply to Ian Campbell
Quoted 4 lines · Ian Campbell

Here are five patches from my "split up libxenctrl" working branch (which clearly isn't going to make 4.6) which are independent fixes which could go into 4.6.

Your subject line said 3 but the text said 5. :-)

Whole series:

Acked-by Wei Liu <[email protected]>
Quoted 1 line

Ian.

author
#12 lore raw
Quoted 7 lines · Wei Liu
Quoted 4 lines · Ian Campbell

Here are five patches from my "split up libxenctrl" working branch (which clearly isn't going to make 4.6) which are independent fixes which could go into 4.6.

Your subject line said 3 but the text said 5. :-)

Yeah :-) I found 2 more halfway through preparing and clearly didn't edit aggressively enough!

Quoted 4 lines

Whole series:

Acked-by Wei Liu <[email protected]>

Thanks.

author
#13 lore raw
In reply to Wei Liu
Quoted 11 lines · Wei Liu
Quoted 4 lines · Ian Campbell

Here are five patches from my "split up libxenctrl" working branch (which clearly isn't going to make 4.6) which are independent fixes which could go into 4.6.

Your subject line said 3 but the text said 5. :-)

Whole series:

Acked-by Wei Liu <[email protected]>

All _five_ applied, thanks.