We''ve had several occuranced of confusion on xl due to shadowing the global domid variable with a local one. The main bulk of this series removes that global, the rest is smaller cleanups to allow Wshadow. I''ve already sent the libxl part of this, but am reincluding here as part of the set.
# HG changeset patch # User Ian Campbell <ian.campbell@citrix.com> # Date 1347620983 -3600 # Node ID 58ab3fb73d13e250f334a38599b749640deac190 # Parent 0547430886c507989dd9091ef5f1e2d089a50351 libxl: Enable -Wshadow. It was convenient to invent $(CFLAGS_LIBXL) to do this. Various renamings to avoid shadowing standard functions: - index(3) - listen(2) - link(2) - abort(3) - abs(3) Reduced the scope of some variables to avoid conflicts. Change to libxc is due to the nested hypercall buf macros in set_xen_guest_handle (used in libxl) using the same local private vars. Build tested only. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> diff -r 0547430886c5 -r 58ab3fb73d13 tools/libxc/xenctrl.h --- a/tools/libxc/xenctrl.h Fri Sep 14 11:00:40 2012 +0100 +++ b/tools/libxc/xenctrl.h Fri Sep 14 12:09:43 2012 +0100 @@ -236,10 +236,10 @@ typedef struct xc_hypercall_buffer xc_hy * Returns the hypercall_buffer associated with a variable. */ #define HYPERCALL_BUFFER(_name) \ - ({ xc_hypercall_buffer_t _val1; \ - typeof(XC__HYPERCALL_BUFFER_NAME(_name)) *_val2 = &XC__HYPERCALL_BUFFER_NAME(_name); \ - (void)(&_val1 == _val2); \ - (_val2)->param_shadow ? (_val2)->param_shadow : (_val2); \ + ({ xc_hypercall_buffer_t _buf1; \ + typeof(XC__HYPERCALL_BUFFER_NAME(_name)) *_buf2 = &XC__HYPERCALL_BUFFER_NAME(_name); \ + (void)(&_buf1 == _buf2); \ + (_buf2)->param_shadow ? (_buf2)->param_shadow : (_buf2); \ }) #define HYPERCALL_BUFFER_INIT_NO_BOUNCE .dir = 0, .sz = 0, .ubuf = (void *)-1 @@ -274,10 +274,10 @@ typedef struct xc_hypercall_buffer xc_hy * directly as a hypercall argument. */ #define HYPERCALL_BUFFER_AS_ARG(_name) \ - ({ xc_hypercall_buffer_t _val1; \ - typeof(XC__HYPERCALL_BUFFER_NAME(_name)) *_val2 = HYPERCALL_BUFFER(_name); \ - (void)(&_val1 == _val2); \ - (unsigned long)(_val2)->hbuf; \ + ({ xc_hypercall_buffer_t _bufarg1; \ + typeof(XC__HYPERCALL_BUFFER_NAME(_name)) *_bufarg2 = HYPERCALL_BUFFER(_name); \ + (void)(&_bufarg1 == _bufarg2); \ + (unsigned long)(_bufarg2)->hbuf; \ }) /* @@ -287,10 +287,10 @@ typedef struct xc_hypercall_buffer xc_hy #undef set_xen_guest_handle #define set_xen_guest_handle(_hnd, _val) \ do { \ - xc_hypercall_buffer_t _val1; \ - typeof(XC__HYPERCALL_BUFFER_NAME(_val)) *_val2 = HYPERCALL_BUFFER(_val); \ - (void) (&_val1 == _val2); \ - set_xen_guest_handle_raw(_hnd, (_val2)->hbuf); \ + xc_hypercall_buffer_t _bufhnd1; \ + typeof(XC__HYPERCALL_BUFFER_NAME(_val)) *_bufhnd2 = HYPERCALL_BUFFER(_val); \ + (void) (&_bufhnd1 == _bufhnd2); \ + set_xen_guest_handle_raw(_hnd, (_bufhnd2)->hbuf); \ } while (0) /* Use with set_xen_guest_handle in place of NULL */ diff -r 0547430886c5 -r 58ab3fb73d13 tools/libxl/Makefile --- a/tools/libxl/Makefile Fri Sep 14 11:00:40 2012 +0100 +++ b/tools/libxl/Makefile Fri Sep 14 12:09:43 2012 +0100 @@ -22,6 +22,12 @@ endif LIBXL_LIBS LIBXL_LIBS = $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(LDLIBS_libblktapctl) $(PTYFUNCS_LIBS) $(LIBUUID_LIBS) +CFLAGS_LIBXL += $(CFLAGS_libxenctrl) +CFLAGS_LIBXL += $(CFLAGS_libxenguest) +CFLAGS_LIBXL += $(CFLAGS_libxenstore) +CFLAGS_LIBXL += $(CFLAGS_libblktapctl) +CFLAGS_LIBXL += -Wshadow + CFLAGS += $(PTHREAD_CFLAGS) LDFLAGS += $(PTHREAD_LDFLAGS) LIBXL_LIBS += $(PTHREAD_LIBS) @@ -71,7 +77,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_c libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y) LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o -$(LIBXL_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore) $(CFLAGS_libblktapctl) -include $(XEN_ROOT)/tools/config.h +$(LIBXL_OBJS): CFLAGS += $(CFLAGS_LIBXL) -include $(XEN_ROOT)/tools/config.h AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h _libxl_list.h _paths.h \ _libxl_save_msgs_callout.h _libxl_save_msgs_helper.h diff -r 0547430886c5 -r 58ab3fb73d13 tools/libxl/flexarray.c --- a/tools/libxl/flexarray.c Fri Sep 14 11:00:40 2012 +0100 +++ b/tools/libxl/flexarray.c Fri Sep 14 12:09:43 2012 +0100 @@ -48,19 +48,19 @@ int flexarray_grow(flexarray_t *array, i return 0; } -int flexarray_set(flexarray_t *array, unsigned int index, void *ptr) +int flexarray_set(flexarray_t *array, unsigned int idx, void *ptr) { - if (index >= array->size) { + if (idx >= array->size) { int newsize; if (!array->autogrow) return 1; - newsize = (array->size * 2 < index) ? index + 1 : array->size * 2; + newsize = (array->size * 2 < idx) ? idx + 1 : array->size * 2; if (flexarray_grow(array, newsize - array->size)) return 2; } - if ( index + 1 > array->count ) - array->count = index + 1; - array->data[index] = ptr; + if ( idx + 1 > array->count ) + array->count = idx + 1; + array->data[idx] = ptr; return 0; } @@ -92,11 +92,11 @@ int flexarray_vappend(flexarray_t *array return ret; } -int flexarray_get(flexarray_t *array, int index, void **ptr) +int flexarray_get(flexarray_t *array, int idx, void **ptr) { - if (index >= array->size) + if (idx >= array->size) return 1; - *ptr = array->data[index]; + *ptr = array->data[idx]; return 0; } diff -r 0547430886c5 -r 58ab3fb73d13 tools/libxl/libxl.c --- a/tools/libxl/libxl.c Fri Sep 14 11:00:40 2012 +0100 +++ b/tools/libxl/libxl.c Fri Sep 14 12:09:43 2012 +0100 @@ -665,7 +665,7 @@ out: libxl_vminfo * libxl_list_vm(libxl_ctx *ctx, int *nb_vm_out) { libxl_vminfo *ptr; - int index, i, ret; + int idx, i, ret; xc_domaininfo_t info[1024]; int size = 1024; @@ -678,15 +678,15 @@ libxl_vminfo * libxl_list_vm(libxl_ctx * LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "geting domain info list"); return NULL; } - for (index = i = 0; i < ret; i++) { + for (idx = i = 0; i < ret; i++) { if (libxl_is_stubdom(ctx, info[i].domain, NULL)) continue; - memcpy(&(ptr[index].uuid), info[i].handle, sizeof(xen_domain_handle_t)); - ptr[index].domid = info[i].domain; - - index++; - } - *nb_vm_out = index; + memcpy(&(ptr[idx].uuid), info[i].handle, sizeof(xen_domain_handle_t)); + ptr[idx].domid = info[i].domain; + + idx++; + } + *nb_vm_out = idx; return ptr; } @@ -3354,7 +3354,7 @@ int libxl_set_memory_target(libxl_ctx *c int32_t target_memkb, int relative, int enforce) { GC_INIT(ctx); - int rc = 1, abort = 0; + int rc = 1, abort_transaction = 0; uint32_t memorykb = 0, videoram = 0; uint32_t current_target_memkb = 0, new_target_memkb = 0; char *memmax, *endptr, *videoram_s = NULL, *target = NULL; @@ -3373,7 +3373,7 @@ retry_transaction: xs_transaction_end(ctx->xsh, t, 1); rc = libxl__fill_dom0_memory_info(gc, ¤t_target_memkb); if (rc < 0) { - abort = 1; + abort_transaction = 1; goto out; } goto retry_transaction; @@ -3381,7 +3381,7 @@ retry_transaction: LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "cannot get target memory info from %s/memory/target\n", dompath); - abort = 1; + abort_transaction = 1; goto out; } else { current_target_memkb = strtoul(target, &endptr, 10); @@ -3389,7 +3389,7 @@ retry_transaction: LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "invalid memory target %s from %s/memory/target\n", target, dompath); - abort = 1; + abort_transaction = 1; goto out; } } @@ -3399,7 +3399,7 @@ retry_transaction: LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "cannot get memory info from %s/memory/static-max\n", dompath); - abort = 1; + abort_transaction = 1; goto out; } memorykb = strtoul(memmax, &endptr, 10); @@ -3407,7 +3407,7 @@ retry_transaction: LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "invalid max memory %s from %s/memory/static-max\n", memmax, dompath); - abort = 1; + abort_transaction = 1; goto out; } @@ -3422,7 +3422,7 @@ retry_transaction: LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "memory_dynamic_max must be less than or equal to" " memory_static_max\n"); - abort = 1; + abort_transaction = 1; goto out; } @@ -3430,7 +3430,7 @@ retry_transaction: LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "new target %d for dom0 is below the minimum threshold\n", new_target_memkb); - abort = 1; + abort_transaction = 1; goto out; } videoram_s = libxl__xs_read(gc, t, libxl__sprintf(gc, @@ -3445,7 +3445,7 @@ retry_transaction: LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xc_domain_setmaxmem domid=%d memkb=%d failed " "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT, rc); - abort = 1; + abort_transaction = 1; goto out; } } @@ -3458,7 +3458,7 @@ retry_transaction: "xc_domain_set_pod_target domid=%d, memkb=%d " "failed rc=%d\n", domid, new_target_memkb / 4, rc); - abort = 1; + abort_transaction = 1; goto out; } @@ -3466,7 +3466,7 @@ retry_transaction: dompath), "%"PRIu32, new_target_memkb); rc = xc_domain_getinfolist(ctx->xch, domid, 1, &info); if (rc != 1 || info.domain != domid) { - abort = 1; + abort_transaction = 1; goto out; } xcinfo2xlinfo(&info, &ptr); @@ -3475,7 +3475,8 @@ retry_transaction: "%"PRIu32, new_target_memkb / 1024); out: - if (!xs_transaction_end(ctx->xsh, t, abort) && !abort) + if (!xs_transaction_end(ctx->xsh, t, abort_transaction) + && !abort_transaction) if (errno == EAGAIN) goto retry_transaction; diff -r 0547430886c5 -r 58ab3fb73d13 tools/libxl/libxl_dm.c --- a/tools/libxl/libxl_dm.c Fri Sep 14 11:00:40 2012 +0100 +++ b/tools/libxl/libxl_dm.c Fri Sep 14 12:09:43 2012 +0100 @@ -379,7 +379,7 @@ static char ** libxl__build_device_model } if (vnc) { int display = 0; - const char *listen = "127.0.0.1"; + const char *addr = "127.0.0.1"; char *vncarg = NULL; flexarray_append(dm_args, "-vnc"); @@ -387,16 +387,16 @@ static char ** libxl__build_device_model if (vnc->display) { display = vnc->display; if (vnc->listen && strchr(vnc->listen, '':'') == NULL) { - listen = vnc->listen; + addr = vnc->listen; } } else if (vnc->listen) { - listen = vnc->listen; + addr = vnc->listen; } - if (strchr(listen, '':'') != NULL) - vncarg = libxl__sprintf(gc, "%s", listen); + if (strchr(addr, '':'') != NULL) + vncarg = libxl__sprintf(gc, "%s", addr); else - vncarg = libxl__sprintf(gc, "%s:%d", listen, display); + vncarg = libxl__sprintf(gc, "%s:%d", addr, display); if (vnc->passwd && vnc->passwd[0]) { vncarg = libxl__sprintf(gc, "%s,password", vncarg); } diff -r 0547430886c5 -r 58ab3fb73d13 tools/libxl/libxl_event.c --- a/tools/libxl/libxl_event.c Fri Sep 14 11:00:40 2012 +0100 +++ b/tools/libxl/libxl_event.c Fri Sep 14 12:09:43 2012 +0100 @@ -167,15 +167,15 @@ static void time_insert_finite(libxl__gc } static int time_register_finite(libxl__gc *gc, libxl__ev_time *ev, - struct timeval abs) + struct timeval absolute) { int rc; - rc = OSEVENT_HOOK(timeout_register, &ev->for_app_reg, abs, ev); + rc = OSEVENT_HOOK(timeout_register, &ev->for_app_reg, absolute, ev); if (rc) return rc; ev->infinite = 0; - ev->abs = abs; + ev->abs = absolute; time_insert_finite(gc, ev); return 0; @@ -202,16 +202,16 @@ static void time_done_debug(libxl__gc *g int libxl__ev_time_register_abs(libxl__gc *gc, libxl__ev_time *ev, libxl__ev_time_callback *func, - struct timeval abs) + struct timeval absolute) { int rc; CTX_LOCK; DBG("ev_time=%p register abs=%lu.%06lu", - ev, (unsigned long)abs.tv_sec, (unsigned long)abs.tv_usec); + ev, (unsigned long)absolute.tv_sec, (unsigned long)absolute.tv_usec); - rc = time_register_finite(gc, ev, abs); + rc = time_register_finite(gc, ev, absolute); if (rc) goto out; ev->func = func; @@ -228,7 +228,7 @@ int libxl__ev_time_register_rel(libxl__g libxl__ev_time_callback *func, int milliseconds /* as for poll(2) */) { - struct timeval abs; + struct timeval absolute; int rc; CTX_LOCK; @@ -238,10 +238,10 @@ int libxl__ev_time_register_rel(libxl__g if (milliseconds < 0) { ev->infinite = 1; } else { - rc = time_rel_to_abs(gc, milliseconds, &abs); + rc = time_rel_to_abs(gc, milliseconds, &absolute); if (rc) goto out; - rc = time_register_finite(gc, ev, abs); + rc = time_register_finite(gc, ev, absolute); if (rc) goto out; } @@ -255,26 +255,26 @@ int libxl__ev_time_register_rel(libxl__g } int libxl__ev_time_modify_abs(libxl__gc *gc, libxl__ev_time *ev, - struct timeval abs) + struct timeval absolute) { int rc; CTX_LOCK; DBG("ev_time=%p modify abs==%lu.%06lu", - ev, (unsigned long)abs.tv_sec, (unsigned long)abs.tv_usec); + ev, (unsigned long)absolute.tv_sec, (unsigned long)absolute.tv_usec); assert(libxl__ev_time_isregistered(ev)); if (ev->infinite) { - rc = time_register_finite(gc, ev, abs); + rc = time_register_finite(gc, ev, absolute); if (rc) goto out; } else { - rc = OSEVENT_HOOK(timeout_modify, &ev->for_app_reg, abs); + rc = OSEVENT_HOOK(timeout_modify, &ev->for_app_reg, absolute); if (rc) goto out; LIBXL_TAILQ_REMOVE(&CTX->etimes, ev, entry); - ev->abs = abs; + ev->abs = absolute; time_insert_finite(gc, ev); } @@ -288,7 +288,7 @@ int libxl__ev_time_modify_abs(libxl__gc int libxl__ev_time_modify_rel(libxl__gc *gc, libxl__ev_time *ev, int milliseconds) { - struct timeval abs; + struct timeval absolute; int rc; CTX_LOCK; @@ -304,10 +304,10 @@ int libxl__ev_time_modify_rel(libxl__gc goto out; } - rc = time_rel_to_abs(gc, milliseconds, &abs); + rc = time_rel_to_abs(gc, milliseconds, &absolute); if (rc) goto out; - rc = libxl__ev_time_modify_abs(gc, ev, abs); + rc = libxl__ev_time_modify_abs(gc, ev, absolute); if (rc) goto out; rc = 0; diff -r 0547430886c5 -r 58ab3fb73d13 tools/libxl/libxl_exec.c --- a/tools/libxl/libxl_exec.c Fri Sep 14 11:00:40 2012 +0100 +++ b/tools/libxl/libxl_exec.c Fri Sep 14 12:09:43 2012 +0100 @@ -35,7 +35,7 @@ static void check_open_fds(const char *w #ifdef __linux__ size_t len; char path[PATH_MAX]; - char link[PATH_MAX+1]; + char linkpath[PATH_MAX+1]; #endif flags = fcntl(i, F_GETFD); if ( flags == -1 ) { @@ -52,11 +52,11 @@ static void check_open_fds(const char *w #ifdef __linux__ snprintf(path, PATH_MAX, "/proc/%d/fd/%d", getpid(), i); - len = readlink(path, link, PATH_MAX); + len = readlink(path, linkpath, PATH_MAX); if (len > 0) { - link[len] = ''\0''; + linkpath[len] = ''\0''; fprintf(stderr, "libxl: execing %s: fd %d is open to %s with flags %#x\n", - what, i, link, flags); + what, i, linkpath, flags); } else #endif fprintf(stderr, "libxl: execing %s: fd %d is open with flags %#x\n", diff -r 0547430886c5 -r 58ab3fb73d13 tools/libxl/libxl_json.c --- a/tools/libxl/libxl_json.c Fri Sep 14 11:00:40 2012 +0100 +++ b/tools/libxl/libxl_json.c Fri Sep 14 12:09:43 2012 +0100 @@ -275,7 +275,7 @@ static int json_object_append_to(libxl__ void libxl__json_object_free(libxl__gc *gc, libxl__json_object *obj) { - int index = 0; + int idx = 0; if (obj == NULL) return; @@ -287,8 +287,8 @@ void libxl__json_object_free(libxl__gc * case JSON_MAP: { libxl__json_map_node *node = NULL; - for (index = 0; index < obj->u.map->count; index++) { - if (flexarray_get(obj->u.map, index, (void**)&node) != 0) + for (idx = 0; idx < obj->u.map->count; idx++) { + if (flexarray_get(obj->u.map, idx, (void**)&node) != 0) break; libxl__json_object_free(gc, node->obj); free(node->map_key); @@ -302,8 +302,8 @@ void libxl__json_object_free(libxl__gc * libxl__json_object *node = NULL; break; - for (index = 0; index < obj->u.array->count; index++) { - if (flexarray_get(obj->u.array, index, (void**)&node) != 0) + for (idx = 0; idx < obj->u.array->count; idx++) { + if (flexarray_get(obj->u.array, idx, (void**)&node) != 0) break; libxl__json_object_free(gc, node); node = NULL; @@ -359,14 +359,14 @@ const libxl__json_object *libxl__json_ma libxl__json_node_type expected_type) { flexarray_t *maps = NULL; - int index = 0; + int idx = 0; if (libxl__json_object_is_map(o)) { libxl__json_map_node *node = NULL; maps = o->u.map; - for (index = 0; index < maps->count; index++) { - if (flexarray_get(maps, index, (void**)&node) != 0) + for (idx = 0; idx < maps->count; idx++) { + if (flexarray_get(maps, idx, (void**)&node) != 0) return NULL; if (strcmp(key, node->map_key) == 0) { if (expected_type == JSON_ANY diff -r 0547430886c5 -r 58ab3fb73d13 tools/libxl/libxl_pci.c --- a/tools/libxl/libxl_pci.c Fri Sep 14 11:00:40 2012 +0100 +++ b/tools/libxl/libxl_pci.c Fri Sep 14 12:09:43 2012 +0100 @@ -167,7 +167,6 @@ static int libxl__device_pci_remove_xens char *be_path, *num_devs_path, *num_devs, *xsdev, *tmp, *tmppath; int num, i, j; xs_transaction_t t; - unsigned int domain = 0, bus = 0, dev = 0, func = 0; be_path = libxl__sprintf(gc, "%s/backend/pci/%d/0", libxl__xs_get_dompath(gc, 0), domid); num_devs_path = libxl__sprintf(gc, "%s/num_devs", be_path); @@ -188,6 +187,7 @@ static int libxl__device_pci_remove_xens } for (i = 0; i < num; i++) { + unsigned int domain = 0, bus = 0, dev = 0, func = 0; xsdev = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/dev-%d", be_path, i)); sscanf(xsdev, PCI_BDF, &domain, &bus, &dev, &func); if (domain == pcidev->domain && bus == pcidev->bus && diff -r 0547430886c5 -r 58ab3fb73d13 tools/libxl/libxl_qmp.c --- a/tools/libxl/libxl_qmp.c Fri Sep 14 11:00:40 2012 +0100 +++ b/tools/libxl/libxl_qmp.c Fri Sep 14 12:09:43 2012 +0100 @@ -171,7 +171,7 @@ static int qmp_register_vnc_callback(lib { GC_INIT(qmp->ctx); const libxl__json_object *obj; - const char *listen, *port; + const char *addr, *port; int rc = -1; if (!libxl__json_object_is_map(o)) { @@ -184,17 +184,17 @@ static int qmp_register_vnc_callback(lib } obj = libxl__json_map_get("host", o, JSON_STRING); - listen = libxl__json_object_get_string(obj); + addr = libxl__json_object_get_string(obj); obj = libxl__json_map_get("service", o, JSON_STRING); port = libxl__json_object_get_string(obj); - if (!listen || !port) { + if (!addr || !port) { LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR, "Failed to retreive VNC connect information."); goto out; } - rc = qmp_write_domain_console_item(gc, qmp->domid, "vnc-listen", listen); + rc = qmp_write_domain_console_item(gc, qmp->domid, "vnc-listen", addr); if (!rc) rc = qmp_write_domain_console_item(gc, qmp->domid, "vnc-port", port);
# HG changeset patch # User Ian Campbell <ian.campbell@citrix.com> # Date 1347621684 -3600 # Node ID 192e4a16a057a456cc78e4ee46242315d474d116 # Parent 58ab3fb73d13e250f334a38599b749640deac190 xl: prepare to enable Wshadow Takes care of everything other than the global domid clashes. Avoid galobal functions - stime(2) - time(2) Signed-off-by: Ian Campbell <ian.campbell@citrix.com> diff -r 58ab3fb73d13 -r 192e4a16a057 tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Fri Sep 14 12:09:43 2012 +0100 +++ b/tools/libxl/xl_cmdimpl.c Fri Sep 14 12:21:24 2012 +0100 @@ -676,7 +676,7 @@ static void parse_config_data(const char b_info->max_vcpus = l; if (!xlu_cfg_get_list (config, "cpus", &cpus, 0, 1)) { - int i, n_cpus = 0; + int n_cpus = 0; if (libxl_cpu_bitmap_alloc(ctx, &b_info->cpumap, 0)) { fprintf(stderr, "Unable to allocate cpumap\n"); @@ -1200,7 +1200,6 @@ skip_vfb: } if (!xlu_cfg_get_list (config, "pci", &pcis, 0, 0)) { - int i; d_config->num_pcidevs = 0; d_config->pcidevs = NULL; for(i = 0; (buf = xlu_cfg_get_listitem (pcis, i)) != NULL; i++) { @@ -1223,7 +1222,6 @@ skip_vfb: switch (xlu_cfg_get_list(config, "cpuid", &cpuids, 0, 1)) { case 0: { - int i; const char *errstr; for (i = 0; (buf = xlu_cfg_get_listitem(cpuids, i)) != NULL; i++) { @@ -1553,7 +1551,7 @@ static int preserve_domain(uint32_t *r_d { time_t now; struct tm tm; - char stime[24]; + char strtime[24]; libxl_uuid new_uuid; @@ -1571,7 +1569,7 @@ static int preserve_domain(uint32_t *r_d return 0; } - if (!strftime(&stime[0], sizeof(stime), "-%Y%m%dT%H%MZ", &tm)) { + if (!strftime(&strtime[0], sizeof(strtime), "-%Y%m%dT%H%MZ", &tm)) { LOG("Failed to format time as a string"); return 0; } @@ -1579,9 +1577,9 @@ static int preserve_domain(uint32_t *r_d libxl_uuid_generate(&new_uuid); LOG("Preserving domain %d %s with suffix%s", - *r_domid, d_config->c_info.name, stime); + *r_domid, d_config->c_info.name, strtime); rc = libxl_domain_preserve(ctx, *r_domid, &d_config->c_info, - stime, new_uuid); + strtime, new_uuid); /* * Although the domain still exists it is no longer the one we are @@ -2697,7 +2695,8 @@ static void destroy_domain(const char *p if (rc) { fprintf(stderr,"destroy failed (rc=%d)\n",rc); exit(-1); } } -static void shutdown_domain(const char *p, int wait, int fallback_trigger) +static void shutdown_domain(const char *p, int wait_for_it, + int fallback_trigger) { int rc; libxl_event *event; @@ -2719,7 +2718,7 @@ static void shutdown_domain(const char * fprintf(stderr,"shutdown failed (rc=%d)\n",rc);exit(-1); } - if (wait) { + if (wait_for_it) { libxl_evgen_domain_death *deathw; rc = libxl_evenable_domain_death(ctx, domid, 0, &deathw); @@ -3685,7 +3684,7 @@ int main_destroy(int argc, char **argv) int main_shutdown(int argc, char **argv) { int opt; - int wait = 0; + int wait_for_it = 0; int fallback_trigger = 0; while ((opt = def_getopt(argc, argv, "wF", "shutdown", 1)) != -1) { @@ -3693,7 +3692,7 @@ int main_shutdown(int argc, char **argv) case 0: case 2: return opt; case ''w'': - wait = 1; + wait_for_it = 1; break; case ''F'': fallback_trigger = 1; @@ -3701,7 +3700,7 @@ int main_shutdown(int argc, char **argv) } } - shutdown_domain(argv[optind], wait, fallback_trigger); + shutdown_domain(argv[optind], wait_for_it, fallback_trigger); return 0; } @@ -4461,7 +4460,7 @@ static void output_topologyinfo(void) return; } -static void info(int numa) +static void print_info(int numa) { output_nodeinfo(); @@ -4504,7 +4503,7 @@ int main_info(int argc, char **argv) } } - info(numa); + print_info(numa); return 0; } @@ -5573,19 +5572,19 @@ int main_blockdetach(int argc, char **ar return rc; } -static char *uptime_to_string(unsigned long time, int short_mode) +static char *uptime_to_string(unsigned long uptime, int short_mode) { int sec, min, hour, day; char *time_string; int ret; - day = (int)(time / 86400); - time -= (day * 86400); - hour = (int)(time / 3600); - time -= (hour * 3600); - min = (int)(time / 60); - time -= (min * 60); - sec = time; + day = (int)(uptime / 86400); + uptime -= (day * 86400); + hour = (int)(uptime / 3600); + uptime -= (hour * 3600); + min = (int)(uptime / 60); + uptime -= (min * 60); + sec = uptime; if (short_mode) if (day > 1) diff -r 58ab3fb73d13 -r 192e4a16a057 tools/libxl/xl_sxp.c --- a/tools/libxl/xl_sxp.c Fri Sep 14 12:09:43 2012 +0100 +++ b/tools/libxl/xl_sxp.c Fri Sep 14 12:21:24 2012 +0100 @@ -78,7 +78,6 @@ void printf_info_sexp(int domid, libxl_d libxl_defbool_to_string(b_info->disable_migrate)); if (c_info->type == LIBXL_DOMAIN_TYPE_PV && b_info->u.pv.bootloader) { - int i; printf("\t(bootloader %s)\n", b_info->u.pv.bootloader); if (b_info->u.pv.bootloader_args) { printf("\t(bootloader_args");
Ian Campbell
2012-Sep-14 12:42 UTC
[PATCH 3 of 3] xl: Remove global domid and enable -Wshadow
# HG changeset patch # User Ian Campbell <ian.campbell@citrix.com> # Date 1347626449 -3600 # Node ID 8fad3481f8c90bb66fff4973b4c5653ebb7ca52e # Parent 192e4a16a057a456cc78e4ee46242315d474d116 xl: Remove global domid and enable -Wshadow Lots of functions loop over a list of domain and others take a domid as a parameter, shadowing the global one and leading to all sorts of confusion. Therefore remove the global domid and explicitly pass it around as necessary. Adds a domid to the parameters for many functions and switches many others from taking a char * domain specifier to taking a domid, pushing the domid lookup to the toplevel. Replaces some open-coded domain_qualifier_to_domid error checking with find_domain. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- I find it hard to believe I won''t have broken anything here... I tested basic lifecycle ops only. diff -r 192e4a16a057 -r 8fad3481f8c9 tools/libxl/Makefile --- a/tools/libxl/Makefile Fri Sep 14 12:21:24 2012 +0100 +++ b/tools/libxl/Makefile Fri Sep 14 13:40:49 2012 +0100 @@ -89,10 +89,13 @@ LIBXLU_OBJS = libxlu_cfg_y.o libxlu_cfg_ CLIENTS = xl testidl libxl-save-helper +CFLAGS_XL += $(CFLAGS_libxenlight) +CFLAGS_XL += -Wshadow + XL_OBJS = xl.o xl_cmdimpl.o xl_cmdtable.o xl_sxp.o $(XL_OBJS) _libxl.api-for-check: \ CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h -$(XL_OBJS): CFLAGS += $(CFLAGS_libxenlight) +$(XL_OBJS): CFLAGS += $(CFLAGS_XL) $(XL_OBJS): CFLAGS += -include $(XEN_ROOT)/tools/config.h # libxl_json.h needs it. SAVE_HELPER_OBJS = libxl_save_helper.o _libxl_save_msgs_helper.o diff -r 192e4a16a057 -r 8fad3481f8c9 tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Fri Sep 14 12:21:24 2012 +0100 +++ b/tools/libxl/xl_cmdimpl.c Fri Sep 14 13:40:49 2012 +0100 @@ -67,9 +67,7 @@ libxl_ctx *ctx; xlchild children[child_max]; -/* when we operate on a domain, it is this one: */ #define INVALID_DOMID ~0 -static uint32_t domid = INVALID_DOMID; static const char *common_domname; static int fd_lock = -1; @@ -197,8 +195,9 @@ static int cpupool_qualifier_to_cpupooli return was_name ? libxl_name_to_cpupoolid(ctx, p, poolid_r) : 0; } -static void find_domain(const char *p) -{ +static uint32_t find_domain(const char *p) +{ + uint32_t domid; int rc, was_name; rc = domain_qualifier_to_domid(p, &domid, &was_name); @@ -207,6 +206,7 @@ static void find_domain(const char *p) exit(2); } common_domname = was_name ? p : libxl_domid_to_name(ctx, domid); + return domid; } static int vncviewer(uint32_t domid, int autopass) @@ -1590,7 +1590,7 @@ static int preserve_domain(uint32_t *r_d return rc == 0 ? 1 : 0; } -static int freemem(libxl_domain_build_info *b_info) +static int freemem(uint32_t domid, libxl_domain_build_info *b_info) { int rc, retries = 3; uint32_t need_memkb, free_memkb; @@ -1670,7 +1670,7 @@ static void autoconnect_console(libxl_ct _exit(1); } -static int domain_wait_event(libxl_event **event_r) +static int domain_wait_event(uint32_t domid, libxl_event **event_r) { int ret; for (;;) { @@ -1704,8 +1704,10 @@ static void evdisable_disk_ejects(libxl_ } } -static int create_domain(struct domain_create *dom_info) -{ +static uint32_t create_domain(struct domain_create *dom_info) +{ + uint32_t domid = INVALID_DOMID; + libxl_domain_config d_config; int debug = dom_info->debug; @@ -1881,7 +1883,7 @@ start: if (rc < 0) goto error_out; - ret = freemem(&d_config.b_info); + ret = freemem(domid, &d_config.b_info); if (ret < 0) { fprintf(stderr, "failed to free memory for the domain\n"); ret = ERROR_FAIL; @@ -2031,7 +2033,7 @@ start: } while (1) { libxl_event *event; - ret = domain_wait_event(&event); + ret = domain_wait_event(domid, &event); if (ret) goto out; switch (event->type) { @@ -2243,13 +2245,11 @@ static int def_getopt(int argc, char * c return -1; } -static int set_memory_max(const char *p, const char *mem) +static int set_memory_max(uint32_t domid, const char *mem) { int64_t memorykb; int rc; - find_domain(p); - memorykb = parse_mem_size_kb(mem); if (memorykb == -1) { fprintf(stderr, "invalid memory size: %s\n", mem); @@ -2263,17 +2263,18 @@ static int set_memory_max(const char *p, int main_memmax(int argc, char **argv) { + uint32_t domid; int opt = 0; - char *p = NULL, *mem; + char *mem; int rc; if ((opt = def_getopt(argc, argv, "", "mem-max", 2)) != -1) return opt; - p = argv[optind]; + domid = find_domain(argv[optind]); mem = argv[optind + 1]; - rc = set_memory_max(p, mem); + rc = set_memory_max(domid, mem); if (rc) { fprintf(stderr, "cannot set domid %d static max memory to : %s\n", domid, mem); return 1; @@ -2282,12 +2283,10 @@ int main_memmax(int argc, char **argv) return 0; } -static void set_memory_target(const char *p, const char *mem) +static void set_memory_target(uint32_t domid, const char *mem) { long long int memorykb; - find_domain(p); - memorykb = parse_mem_size_kb(mem); if (memorykb == -1) { fprintf(stderr, "invalid memory size: %s\n", mem); @@ -2299,26 +2298,26 @@ static void set_memory_target(const char int main_memset(int argc, char **argv) { + uint32_t domid; int opt = 0; - const char *p = NULL, *mem; + const char *mem; if ((opt = def_getopt(argc, argv, "", "mem-set", 2)) != -1) return opt; - p = argv[optind]; + domid = find_domain(argv[optind]); mem = argv[optind + 1]; - set_memory_target(p, mem); + set_memory_target(domid, mem); return 0; } -static void cd_insert(const char *dom, const char *virtdev, char *phys) +static void cd_insert(uint32_t domid, const char *virtdev, char *phys) { libxl_device_disk disk; /* we don''t free disk''s contents */ char *buf = NULL; XLU_Config *config = 0; - find_domain(dom); if (asprintf(&buf, "vdev=%s,access=r,devtype=cdrom,target=%s", virtdev, phys ? phys : "") < 0) { @@ -2338,38 +2337,41 @@ static void cd_insert(const char *dom, c int main_cd_eject(int argc, char **argv) { + uint32_t domid; int opt = 0; - const char *p = NULL, *virtdev; + const char *virtdev; if ((opt = def_getopt(argc, argv, "", "cd-eject", 2)) != -1) return opt; - p = argv[optind]; + domid = find_domain(argv[optind]); virtdev = argv[optind + 1]; - cd_insert(p, virtdev, NULL); + cd_insert(domid, virtdev, NULL); return 0; } int main_cd_insert(int argc, char **argv) { + uint32_t domid; int opt = 0; - const char *p = NULL, *virtdev; + const char *virtdev; char *file = NULL; /* modified by cd_insert tokenising it */ if ((opt = def_getopt(argc, argv, "", "cd-insert", 3)) != -1) return opt; - p = argv[optind]; + domid = find_domain(argv[optind]); virtdev = argv[optind + 1]; file = argv[optind + 2]; - cd_insert(p, virtdev, file); + cd_insert(domid, virtdev, file); return 0; } int main_console(int argc, char **argv) { + uint32_t domid; int opt = 0, num = 0; libxl_console_type type = 0; @@ -2393,7 +2395,7 @@ int main_console(int argc, char **argv) } } - find_domain(argv[optind]); + domid = find_domain(argv[optind]); if (!type) libxl_primary_console_exec(ctx, domid); else @@ -2410,6 +2412,7 @@ int main_vncviewer(int argc, char **argv {"help", 0, 0, ''h''}, {0, 0, 0, 0} }; + uint32_t domid; int opt, autopass = 0; while (1) { @@ -2435,20 +2438,18 @@ int main_vncviewer(int argc, char **argv return 2; } - find_domain(argv[optind]); + domid = find_domain(argv[optind]); if (vncviewer(domid, autopass)) return 1; return 0; } -static void pcilist(const char *dom) +static void pcilist(uint32_t domid) { libxl_device_pci *pcidevs; int num, i; - find_domain(dom); - pcidevs = libxl_device_pci_list(ctx, domid, &num); if (pcidevs == NULL) return; @@ -2464,27 +2465,25 @@ static void pcilist(const char *dom) int main_pcilist(int argc, char **argv) { + uint32_t domid; int opt; - const char *domname = NULL; if ((opt = def_getopt(argc, argv, "", "pci-list", 1)) != -1) return opt; - domname = argv[optind]; - - pcilist(domname); + domid = find_domain(argv[optind]); + + pcilist(domid); return 0; } -static void pcidetach(const char *dom, const char *bdf, int force) +static void pcidetach(uint32_t domid, const char *bdf, int force) { libxl_device_pci pcidev; XLU_Config *config; - find_domain(dom); - libxl_device_pci_init(&pcidev); - + config = xlu_cfg_init(stderr, "command line"); if (!config) { perror("xlu_cfg_inig"); exit(-1); } @@ -2503,9 +2502,10 @@ static void pcidetach(const char *dom, c int main_pcidetach(int argc, char **argv) { + uint32_t domid; int opt; int force = 0; - const char *domname = NULL, *bdf = NULL; + const char *bdf = NULL; while ((opt = def_getopt(argc, argv, "f", "pci-detach", 2)) != -1) { switch (opt) { @@ -2517,19 +2517,17 @@ int main_pcidetach(int argc, char **argv } } - domname = argv[optind]; + domid = find_domain(argv[optind]); bdf = argv[optind + 1]; - pcidetach(domname, bdf, force); + pcidetach(domid, bdf, force); return 0; } -static void pciattach(const char *dom, const char *bdf, const char *vs) +static void pciattach(uint32_t domid, const char *bdf, const char *vs) { libxl_device_pci pcidev; XLU_Config *config; - find_domain(dom); - libxl_device_pci_init(&pcidev); config = xlu_cfg_init(stderr, "command line"); @@ -2547,19 +2545,20 @@ static void pciattach(const char *dom, c int main_pciattach(int argc, char **argv) { + uint32_t domid; int opt; - const char *domname = NULL, *bdf = NULL, *vs = NULL; + const char *bdf = NULL, *vs = NULL; if ((opt = def_getopt(argc, argv, "", "pci-attach", 2)) != -1) return opt; - domname = argv[optind]; + domid = find_domain(argv[optind]); bdf = argv[optind + 1]; if (optind + 1 < argc) vs = argv[optind + 2]; - pciattach(domname, bdf, vs); + pciattach(domid, bdf, vs); return 0; } @@ -2671,22 +2670,20 @@ int main_pciassignable_remove(int argc, return 0; } -static void pause_domain(const char *p) -{ - find_domain(p); +static void pause_domain(uint32_t domid) +{ libxl_domain_pause(ctx, domid); } -static void unpause_domain(const char *p) -{ - find_domain(p); +static void unpause_domain(uint32_t domid) +{ libxl_domain_unpause(ctx, domid); } -static void destroy_domain(const char *p) +static void destroy_domain(uint32_t domid) { int rc; - find_domain(p); + if (domid == 0) { fprintf(stderr, "Cannot destroy privileged domain 0.\n\n"); exit(-1); @@ -2695,13 +2692,12 @@ static void destroy_domain(const char *p if (rc) { fprintf(stderr,"destroy failed (rc=%d)\n",rc); exit(-1); } } -static void shutdown_domain(const char *p, int wait_for_it, +static void shutdown_domain(uint32_t domid, int wait_for_it, int fallback_trigger) { int rc; libxl_event *event; - find_domain(p); rc=libxl_domain_shutdown(ctx, domid); if (rc == ERROR_NOPARAVIRT) { if (fallback_trigger) { @@ -2728,7 +2724,7 @@ static void shutdown_domain(const char * } for (;;) { - rc = domain_wait_event(&event); + rc = domain_wait_event(domid, &event); if (rc) exit(-1); switch (event->type) { @@ -2755,10 +2751,10 @@ static void shutdown_domain(const char * } } -static void reboot_domain(const char *p, int fallback_trigger) +static void reboot_domain(uint32_t domid, int fallback_trigger) { int rc; - find_domain(p); + rc=libxl_domain_reboot(ctx, domid); if (rc == ERROR_NOPARAVIRT) { if (fallback_trigger) { @@ -2815,7 +2811,7 @@ static void list_domains_details(const l if (default_output_format == OUTPUT_FORMAT_JSON) s = printf_info_one_json(hand, info[i].domid, &d_config); else - printf_info_sexp(domid, &d_config); + printf_info_sexp(info[i].domid, &d_config); libxl_domain_config_dispose(&d_config); free(data); free(config_source); @@ -2917,15 +2913,13 @@ static void list_vm(void) libxl_vminfo_list_free(info, nb_vm); } -static void save_domain_core_begin(const char *domain_spec, +static void save_domain_core_begin(uint32_t domid, const char *override_config_file, uint8_t **config_data_r, int *config_len_r) { int rc; - find_domain(domain_spec); - /* configuration file in optional data: */ if (override_config_file) { @@ -2982,14 +2976,15 @@ static void save_domain_core_writeconfig hdr.optional_data_len); } -static int save_domain(const char *p, const char *filename, int checkpoint, +static int save_domain(uint32_t domid, const char *filename, int checkpoint, const char *override_config_file) { int fd; uint8_t *config_data; int config_len; - save_domain_core_begin(p, override_config_file, &config_data, &config_len); + save_domain_core_begin(domid, override_config_file, + &config_data, &config_len); if (!config_len) { fputs(" Savefile will not contain xl domain config\n", stderr); @@ -3159,7 +3154,7 @@ static void migrate_do_preamble(int send } -static void migrate_domain(const char *domain_spec, const char *rune, +static void migrate_domain(uint32_t domid, const char *rune, const char *override_config_file) { pid_t child = -1; @@ -3170,7 +3165,7 @@ static void migrate_domain(const char *d uint8_t *config_data; int config_len; - save_domain_core_begin(domain_spec, override_config_file, + save_domain_core_begin(domid, override_config_file, &config_data, &config_len); if (!config_len) { @@ -3297,10 +3292,10 @@ static void migrate_domain(const char *d exit(-ERROR_BADFAIL); } -static void core_dump_domain(const char *domain_spec, const char *filename) +static void core_dump_domain(uint32_t domid, const char *filename) { int rc; - find_domain(domain_spec); + rc=libxl_domain_core_dump(ctx, domid, filename, NULL); if (rc) { fprintf(stderr,"core dump failed (rc=%d)\n",rc);exit(-1); } } @@ -3308,6 +3303,7 @@ static void core_dump_domain(const char static void migrate_receive(int debug, int daemonize, int monitor, int send_fd, int recv_fd, int remus) { + uint32_t domid; int rc, rc2; char rc_buf; char *migration_domname; @@ -3339,6 +3335,8 @@ static void migrate_receive(int debug, i exit(-rc); } + domid = rc; + if (remus) { /* If we are here, it means that the sender (primary) has crashed. * TODO: Split-Brain Check. @@ -3557,7 +3555,8 @@ int main_migrate_receive(int argc, char int main_save(int argc, char **argv) { - const char *filename, *p; + uint32_t domid; + const char *filename; const char *config_filename = NULL; int checkpoint = 0; int opt; @@ -3577,18 +3576,18 @@ int main_save(int argc, char **argv) return 2; } - p = argv[optind]; + domid = find_domain(argv[optind]); filename = argv[optind + 1]; if ( argc - optind >= 3 ) config_filename = argv[optind + 2]; - save_domain(p, filename, checkpoint, config_filename); + save_domain(domid, filename, checkpoint, config_filename); return 0; } int main_migrate(int argc, char **argv) { - const char *p = NULL; + uint32_t domid; const char *config_filename = NULL; const char *ssh_command = "ssh"; char *rune = NULL; @@ -3618,7 +3617,7 @@ int main_migrate(int argc, char **argv) } } - p = argv[optind]; + domid = find_domain(argv[optind]); host = argv[optind + 1]; if (!ssh_command[0]) { @@ -3631,7 +3630,7 @@ int main_migrate(int argc, char **argv) return 1; } - migrate_domain(p, rune, config_filename); + migrate_domain(domid, rune, config_filename); return 0; } @@ -3642,7 +3641,7 @@ int main_dump_core(int argc, char **argv if ((opt = def_getopt(argc, argv, "", "dump-core", 2)) != -1) return opt; - core_dump_domain(argv[optind], argv[optind + 1]); + core_dump_domain(find_domain(argv[optind]), argv[optind + 1]); return 0; } @@ -3653,7 +3652,7 @@ int main_pause(int argc, char **argv) if ((opt = def_getopt(argc, argv, "", "pause", 1)) != -1) return opt; - pause_domain(argv[optind]); + pause_domain(find_domain(argv[optind])); return 0; } @@ -3665,7 +3664,7 @@ int main_unpause(int argc, char **argv) if ((opt = def_getopt(argc, argv, "", "unpause", 1)) != -1) return opt; - unpause_domain(argv[optind]); + unpause_domain(find_domain(argv[optind])); return 0; } @@ -3677,7 +3676,7 @@ int main_destroy(int argc, char **argv) if ((opt = def_getopt(argc, argv, "", "destroy", 1)) != -1) return opt; - destroy_domain(argv[optind]); + destroy_domain(find_domain(argv[optind])); return 0; } @@ -3700,7 +3699,7 @@ int main_shutdown(int argc, char **argv) } } - shutdown_domain(argv[optind], wait_for_it, fallback_trigger); + shutdown_domain(find_domain(argv[optind]), wait_for_it, fallback_trigger); return 0; } @@ -3719,7 +3718,7 @@ int main_reboot(int argc, char **argv) } } - reboot_domain(argv[optind], fallback_trigger); + reboot_domain(find_domain(argv[optind]), fallback_trigger); return 0; } @@ -3773,7 +3772,7 @@ int main_list(int argc, char **argv) } info_free = info; } else if (optind == argc-1) { - find_domain(argv[optind]); + uint32_t domid = find_domain(argv[optind]); rc = libxl_domain_info(ctx, &info_buf, domid); if (rc == ERROR_INVAL) { fprintf(stderr, "Error: Domain \''%s\'' does not exist.\n", @@ -3922,6 +3921,7 @@ int main_create(int argc, char **argv) int main_config_update(int argc, char **argv) { + uint32_t domid; const char *filename = NULL; char *p; char extra_config[1024]; @@ -3943,7 +3943,7 @@ int main_config_update(int argc, char ** exit(1); } - find_domain(argv[1]); + domid = find_domain(argv[1]); argc--; argv++; if (argv[1] && argv[1][0] != ''-'' && !strchr(argv[1], ''='')) { @@ -4034,12 +4034,10 @@ int main_config_update(int argc, char ** return 0; } -static void button_press(const char *p, const char *b) +static void button_press(uint32_t domid, const char *b) { libxl_trigger trigger; - find_domain(p); - if (!strcmp(b, "power")) { trigger = LIBXL_TRIGGER_POWER; } else if (!strcmp(b, "sleep")) { @@ -4062,7 +4060,7 @@ int main_button_press(int argc, char **a if ((opt = def_getopt(argc, argv, "", "button-press", 2)) != -1) return opt; - button_press(argv[optind], argv[optind + 1]); + button_press(find_domain(argv[optind]), argv[optind + 1]); return 0; } @@ -4188,11 +4186,7 @@ static void vcpulist(int argc, char **ar libxl_dominfo_list_free(dominfo, nb_domain); } else { for (; argc > 0; ++argv, --argc) { - if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) { - fprintf(stderr, "%s is an invalid domain identifier\n", *argv); - goto vcpulist_out; - } - + uint32_t domid = find_domain(*argv); print_domain_vcpuinfo(domid, physinfo.nr_cpus); } } @@ -4211,7 +4205,7 @@ int main_vcpulist(int argc, char **argv) return 0; } -static void vcpupin(const char *d, const char *vcpu, char *cpu) +static void vcpupin(uint32_t domid, const char *vcpu, char *cpu) { libxl_vcpuinfo *vcpuinfo; libxl_bitmap cpumap; @@ -4229,8 +4223,6 @@ static void vcpupin(const char *d, const vcpuid = -1; } - find_domain(d); - if (libxl_cpu_bitmap_alloc(ctx, &cpumap, 0)) { goto vcpupin_out; } @@ -4270,11 +4262,11 @@ int main_vcpupin(int argc, char **argv) if ((opt = def_getopt(argc, argv, "", "vcpu-pin", 3)) != -1) return opt; - vcpupin(argv[optind], argv[optind+1] , argv[optind+2]); + vcpupin(find_domain(argv[optind]), argv[optind+1] , argv[optind+2]); return 0; } -static void vcpuset(const char *d, const char* nr_vcpus) +static void vcpuset(uint32_t domid, const char* nr_vcpus) { char *endptr; unsigned int max_vcpus, i; @@ -4286,8 +4278,6 @@ static void vcpuset(const char *d, const return; } - find_domain(d); - if (libxl_cpu_bitmap_alloc(ctx, &cpumap, 0)) { fprintf(stderr, "libxl_cpu_bitmap_alloc failed\n"); return; @@ -4308,7 +4298,7 @@ int main_vcpuset(int argc, char **argv) if ((opt = def_getopt(argc, argv, "", "vcpu-set", 2)) != -1) return opt; - vcpuset(argv[optind], argv[optind+1]); + vcpuset(find_domain(argv[optind]), argv[optind+1]); return 0; } @@ -4545,7 +4535,7 @@ int main_sharing(int argc, char **argv) } info_free = info; } else if (optind == argc-1) { - find_domain(argv[optind]); + uint32_t domid = find_domain(argv[optind]); rc = libxl_domain_info(ctx, &info_buf, domid); if (rc == ERROR_INVAL) { fprintf(stderr, "Error: Domain \''%s\'' does not exist.\n", @@ -4905,7 +4895,7 @@ int main_sched_credit(int argc, char **a sched_credit_pool_output, cpupool); } else { - find_domain(dom); + uint32_t domid = find_domain(dom); if (!opt_w && !opt_c) { /* output credit scheduler info */ sched_credit_domain_output(-1); @@ -4982,7 +4972,7 @@ int main_sched_credit2(int argc, char ** sched_default_pool_output, cpupool); } else { - find_domain(dom); + uint32_t domid = find_domain(dom); if (!opt_w) { /* output credit2 scheduler info */ sched_credit2_domain_output(-1); @@ -5085,7 +5075,7 @@ int main_sched_sedf(int argc, char **arg sched_default_pool_output, cpupool); } else { - find_domain(dom); + uint32_t domid = find_domain(dom); if (!opt_p && !opt_s && !opt_l && !opt_e && !opt_w) { /* output sedf scheduler info */ @@ -5125,6 +5115,7 @@ int main_sched_sedf(int argc, char **arg int main_domid(int argc, char **argv) { + uint32_t domid; int opt; const char *domname = NULL; @@ -5145,6 +5136,7 @@ int main_domid(int argc, char **argv) int main_domname(int argc, char **argv) { + uint32_t domid; int opt; char *domname = NULL; char *endptr = NULL; @@ -5173,18 +5165,17 @@ int main_domname(int argc, char **argv) int main_rename(int argc, char **argv) { + uint32_t domid; int opt; - const char *dom; - const char *new_name; + const char *dom, *new_name; if ((opt = def_getopt(argc, argv, "", "rename", 2)) != -1) return opt; dom = argv[optind++]; - - find_domain(dom); new_name = argv[optind]; + domid = find_domain(dom); if (libxl_domain_rename(ctx, domid, common_domname, new_name)) { fprintf(stderr, "Can''t rename domain ''%s''.\n", dom); return 1; @@ -5195,9 +5186,9 @@ int main_rename(int argc, char **argv) int main_trigger(int argc, char **argv) { + uint32_t domid; int opt; char *endptr = NULL; - const char *dom = NULL; int vcpuid = 0; const char *trigger_name = NULL; libxl_trigger trigger; @@ -5205,9 +5196,7 @@ int main_trigger(int argc, char **argv) if ((opt = def_getopt(argc, argv, "", "trigger", 2)) != -1) return opt; - dom = argv[optind++]; - - find_domain(dom); + domid = find_domain(argv[optind++]); trigger_name = argv[optind++]; if (libxl_trigger_from_string(trigger_name, &trigger)) { @@ -5230,16 +5219,14 @@ int main_trigger(int argc, char **argv) int main_sysrq(int argc, char **argv) { + uint32_t domid; int opt; const char *sysrq = NULL; - const char *dom = NULL; if ((opt = def_getopt(argc, argv, "", "sysrq", 2)) != -1) return opt; - dom = argv[optind++]; - - find_domain(dom); + domid = find_domain(argv[optind++]); sysrq = argv[optind]; @@ -5313,6 +5300,7 @@ int main_top(int argc, char **argv) int main_networkattach(int argc, char **argv) { + uint32_t domid; int opt; libxl_device_nic nic; XLU_Config *config = 0; @@ -5329,10 +5317,7 @@ int main_networkattach(int argc, char ** return 0; } - if (domain_qualifier_to_domid(argv[optind], &domid, 0) < 0) { - fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]); - return 1; - } + domid = find_domain(argv[optind]); config= xlu_cfg_init(stderr, "command line"); if (!config) { @@ -5418,10 +5403,7 @@ int main_networklist(int argc, char **ar printf("%-3s %-2s %-17s %-6s %-5s %-6s %5s/%-5s %-30s\n", "Idx", "BE", "Mac Addr.", "handle", "state", "evt-ch", "tx-", "rx-ring-ref", "BE-path"); for (argv += optind, argc -= optind; argc > 0; --argc, ++argv) { - if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) { - fprintf(stderr, "%s is an invalid domain identifier\n", *argv); - continue; - } + uint32_t domid = find_domain(*argv); nics = libxl_device_nic_list(ctx, domid, &nb); if (!nics) { continue; @@ -5447,16 +5429,14 @@ int main_networklist(int argc, char **ar int main_networkdetach(int argc, char **argv) { + uint32_t domid; int opt; libxl_device_nic nic; if ((opt = def_getopt(argc, argv, "", "network-detach", 2)) != -1) return opt; - if (domain_qualifier_to_domid(argv[optind], &domid, 0) < 0) { - fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]); - return 1; - } + domid = find_domain(argv[optind]); if (!strchr(argv[optind+1], '':'')) { if (libxl_devid_to_device_nic(ctx, domid, atoi(argv[optind+1]), &nic)) { @@ -5525,6 +5505,7 @@ int main_blocklist(int argc, char **argv printf("%-5s %-3s %-6s %-5s %-6s %-8s %-30s\n", "Vdev", "BE", "handle", "state", "evt-ch", "ring-ref", "BE-path"); for (argv += optind, argc -= optind; argc > 0; --argc, ++argv) { + uint32_t domid; if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) { fprintf(stderr, "%s is an invalid domain identifier\n", *argv); continue; @@ -5550,16 +5531,15 @@ int main_blocklist(int argc, char **argv int main_blockdetach(int argc, char **argv) { + uint32_t domid; int opt, rc = 0; libxl_device_disk disk; if ((opt = def_getopt(argc, argv, "", "block-detach", 2)) != -1) return opt; - if (domain_qualifier_to_domid(argv[optind], &domid, 0) < 0) { - fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]); - return 1; - } + domid = find_domain(argv[optind]); + if (libxl_vdev_to_device_disk(ctx, domid, argv[optind+1], &disk)) { fprintf(stderr, "Error: Device %s not connected.\n", argv[optind+1]); return 1; @@ -5733,7 +5713,7 @@ static void print_uptime(int short_mode, int main_uptime(int argc, char **argv) { - const char *dom = NULL; + const char *dom; int short_mode = 0; uint32_t domains[100]; int nb_doms = 0; @@ -5749,10 +5729,8 @@ int main_uptime(int argc, char **argv) } } - for (;(dom = argv[optind]) != NULL; nb_doms++,optind++) { - find_domain(dom); - domains[nb_doms] = domid; - } + for (;(dom = argv[optind]) != NULL; nb_doms++,optind++) + domains[nb_doms] = find_domain(dom); print_uptime(short_mode, domains, nb_doms); @@ -5761,6 +5739,7 @@ int main_uptime(int argc, char **argv) int main_tmem_list(int argc, char **argv) { + uint32_t domid; const char *dom = NULL; char *buf = NULL; int use_long = 0; @@ -5788,9 +5767,9 @@ int main_tmem_list(int argc, char **argv } if (all) - domid = -1; + domid = INVALID_DOMID; else - find_domain(dom); + domid = find_domain(dom); buf = libxl_tmem_list(ctx, domid, use_long); if (buf == NULL) @@ -5803,6 +5782,7 @@ int main_tmem_list(int argc, char **argv int main_tmem_freeze(int argc, char **argv) { + uint32_t domid; const char *dom = NULL; int all = 0; int opt; @@ -5825,7 +5805,7 @@ int main_tmem_freeze(int argc, char **ar } if (all) - domid = -1; + domid = INVALID_DOMID; else find_domain(dom); @@ -5835,6 +5815,7 @@ int main_tmem_freeze(int argc, char **ar int main_tmem_thaw(int argc, char **argv) { + uint32_t domid; const char *dom = NULL; int all = 0; int opt; @@ -5857,7 +5838,7 @@ int main_tmem_thaw(int argc, char **argv } if (all) - domid = -1; + domid = INVALID_DOMID; else find_domain(dom); @@ -5867,6 +5848,7 @@ int main_tmem_thaw(int argc, char **argv int main_tmem_set(int argc, char **argv) { + uint32_t domid; const char *dom = NULL; uint32_t weight = 0, cap = 0, compress = 0; int opt_w = 0, opt_c = 0, opt_p = 0; @@ -5903,7 +5885,7 @@ int main_tmem_set(int argc, char **argv) } if (all) - domid = -1; + domid = INVALID_DOMID; else find_domain(dom); @@ -5925,6 +5907,7 @@ int main_tmem_set(int argc, char **argv) int main_tmem_shared_auth(int argc, char **argv) { + uint32_t domid; const char *autharg = NULL; char *endptr = NULL; const char *dom = NULL; @@ -5957,7 +5940,7 @@ int main_tmem_shared_auth(int argc, char } if (all) - domid = -1; + domid = INVALID_DOMID; else find_domain(dom); @@ -6712,9 +6695,10 @@ done: int main_remus(int argc, char **argv) { + uint32_t domid; int opt, rc, daemonize = 1; const char *ssh_command = "ssh"; - char *host = NULL, *rune = NULL, *domain = NULL; + char *host = NULL, *rune = NULL; libxl_domain_remus_info r_info; int send_fd = -1, recv_fd = -1; pid_t child = -1; @@ -6750,11 +6734,10 @@ int main_remus(int argc, char **argv) } } - domain = argv[optind]; + domid = find_domain(argv[optind]); host = argv[optind + 1]; if (r_info.blackhole) { - find_domain(domain); send_fd = open("/dev/null", O_RDWR, 0644); if (send_fd < 0) { perror("failed to open /dev/null"); @@ -6771,7 +6754,7 @@ int main_remus(int argc, char **argv) return 1; } - save_domain_core_begin(domain, NULL, &config_data, &config_len); + save_domain_core_begin(domid, NULL, &config_data, &config_len); if (!config_len) { fprintf(stderr, "No config file stored for running domain and "
Ian Campbell
2012-Sep-14 12:47 UTC
Re: [PATCH 3 of 3] xl: Remove global domid and enable -Wshadow
On Fri, 2012-09-14 at 13:42 +0100, Ian Campbell wrote:> I find it hard to believe I won''t have broken anything here... I > tested basic lifecycle ops only.I should have taken this bet :-/ Incremental fixup to the patch below... (could be folded in) 8<-------------------------------------------------- xl: make sure we always use the result of find_domain This is a leftover from when domid was a global. Annotate it with want_unused_result and fix the handful of errors. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> diff -r 8fad3481f8c9 tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Fri Sep 14 13:40:49 2012 +0100 +++ b/tools/libxl/xl_cmdimpl.c Fri Sep 14 13:45:23 2012 +0100 @@ -195,6 +195,7 @@ static int cpupool_qualifier_to_cpupooli return was_name ? libxl_name_to_cpupoolid(ctx, p, poolid_r) : 0; } +static uint32_t find_domain(const char *p) __attribute__((warn_unused_result)); static uint32_t find_domain(const char *p) { uint32_t domid; @@ -5807,7 +5808,7 @@ int main_tmem_freeze(int argc, char **ar if (all) domid = INVALID_DOMID; else - find_domain(dom); + domid = find_domain(dom); libxl_tmem_freeze(ctx, domid); return 0; @@ -5840,7 +5841,7 @@ int main_tmem_thaw(int argc, char **argv if (all) domid = INVALID_DOMID; else - find_domain(dom); + domid = find_domain(dom); libxl_tmem_thaw(ctx, domid); return 0; @@ -5887,7 +5888,7 @@ int main_tmem_set(int argc, char **argv) if (all) domid = INVALID_DOMID; else - find_domain(dom); + domid = find_domain(dom); if (!opt_w && !opt_c && !opt_p) { fprintf(stderr, "No set value specified.\n\n"); @@ -5942,7 +5943,7 @@ int main_tmem_shared_auth(int argc, char if (all) domid = INVALID_DOMID; else - find_domain(dom); + domid = find_domain(dom); if (uuid == NULL || autharg == NULL) { fprintf(stderr, "No uuid or auth specified.\n\n");
Ian Campbell writes ("[PATCH 2 of 3] xl: prepare to enable Wshadow"):> xl: prepare to enable WshadowAcked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian Jackson
2012-Sep-14 14:54 UTC
Re: [PATCH 3 of 3] xl: Remove global domid and enable -Wshadow
Ian Campbell writes ("[PATCH 3 of 3] xl: Remove global domid and enable -Wshadow"):> xl: Remove global domid and enable -WshadowAcked-by: Ian Jackson <ian.jackson@eu.citrix.com>> I find it hard to believe I won''t have broken anything here... I tested > basic lifecycle ops only.You may also have fixed something. I say commit it. Also, sorry for being the ultimate cause of this mess. Ian.
Ian Jackson
2012-Sep-14 14:54 UTC
Re: [PATCH 3 of 3] xl: Remove global domid and enable -Wshadow
Ian Campbell writes ("Re: [Xen-devel] [PATCH 3 of 3] xl: Remove global domid and enable -Wshadow"):> Annotate it with want_unused_result and fix the handful of errors.Oops, Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
On Fri, 2012-09-14 at 13:42 +0100, Ian Campbell wrote:> We''ve had several occuranced of confusion on xl due to shadowing the > global domid variable with a local one. The main bulk of this series > removes that global, the rest is smaller cleanups to allow Wshadow. > > I''ve already sent the libxl part of this, but am reincluding here as > part of the set.I have applied 2/3 + 3/3 + folded in the incremental fix to 3/3 with Ian''s acks. I applied V2 of 1/3 from the separate "libxl: Enable -Wshadow" thread. Thanks. Ian.