Ian Campbell
2010-Jul-30 09:02 UTC
[Xen-devel] [PATCH 0 of 3] xl: free allocations made at top level
xl: free all allocations made at the top level
The following series makes "xl list" free of memory leaks (as reported
by valgrind) subject to two exceptions outlined below.
Although we may not care about many memory leaks in xl since the
commands are short lived I think it is important that we be able to
use xl to validate the libxl does not leak and/or that users of libxl
can be written which do not leak.
I''ve not extensively tested all possible xl commands so it is possible
that this series may expose a latent double free etc. create,
destroy, console and list seem OK.
The series applies after Ian Jackson''s series consisting of
[PATCH 1/2] libxl: const-correctness for libxl_uuid2string
[PATCH 2/2] xl: support "xl list <domain>"
which was posted to the list yesterday.
I used a version of valgrind which I have patched to understand the
PRIVCMD_HYPERCALL ioctl on /proc/xen/privcmd. It currently understands
exactly one sysctl (XEN_SYSCTL_getdomaininfolist). I will post the
patch shortly and imagine I will be expanding it as I test other xl
commands etc.
I ran the test with the
--leak-check=full --show-reachable=yes --track-origins=yes
options to valgrind and there are two leaks remaining:
==7040== 8 bytes in 1 blocks are still reachable in loss record 1 of 2
==7040== at 0x4022249: calloc (vg_replace_malloc.c:467)
==7040== by 0x40616A3: hcall_buf_prep (xc_private.c:242)
==7040== by 0x4062306: xc_sysctl (xc_private.h:177)
==7040== by 0x4059F61: xc_domain_getinfolist (xc_domain.c:254)
==7040== by 0x403FDBF: libxl_list_domain (libxl.c:496)
==7040== by 0x8054B2E: main_list (xl_cmdimpl.c:3005)
==7040== by 0x804B2FB: main (xl.c:76)
==7040==
==7040== 4,096 bytes in 1 blocks are still reachable in loss record 2 of 2
==7040== at 0x40220F6: memalign (vg_replace_malloc.c:581)
==7040== by 0x4022153: posix_memalign (vg_replace_malloc.c:709)
==7040== by 0x4061110: xc_memalign (xc_private.c:804)
==7040== by 0x40616D8: hcall_buf_prep (xc_private.c:250)
==7040== by 0x4062306: xc_sysctl (xc_private.h:177)
==7040== by 0x4059F61: xc_domain_getinfolist (xc_domain.c:254)
==7040== by 0x403FDBF: libxl_list_domain (libxl.c:496)
==7040== by 0x8054B2E: main_list (xl_cmdimpl.c:3005)
==7040== by 0x804B2FB: main (xl.c:76)
These are the hypercall argument buffers which libxc allocates, locks
into memory and stores in TLS using pthread_setspecific.
The memory should be freed on thread exit (libxc supplies a destructor
to pthread_key_create) but since xl isn''t actually using threads it
never occurs.
I''m not sure that is really necessary to avoid these leaks (other than
to provide an empty base line for future work). They are pretty benign
since they are allocated exactly once per thread and I''m reasonably
happy that in a user of libxc/libxl which used threads they would
actually end up getting freed at the appropriate point.
If there were a way to tell valgrind not to worry about these
allocations that would be nice, as would a palatable workaround which
could be used in xl but I can''t find anything suitable. For example
calling pthread_exit() at the end of main() causes leaks from the C
runtime so that is out. Creating a thread to do the body of the work
(with main just doing pthread_join and returning the result) doesn''t
pass the palatable test IMHO
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Campbell
2010-Jul-30 09:02 UTC
[Xen-devel] [PATCH 1 of 3] xl: return(N) from individual commands to top level instead of exit(N)
# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1280477590 -3600
# Node ID 89a8086b0a054c10552bc32a28026773c9608772
# Parent 54981fca074262cb89fb2d1682dc6170552207b8
xl: return(N) from individual commands to top level instead of exit(N)
This allows the top level command dispatcher to cleanup some of its
own allocations.
This is a fairly mechanical conversion of exit(FOO) into return FOO
for the top-level command functions (i.e. main_*). There are still
code paths which will exit() further down the call chains which will
require actual thought.
At first glance not all the return codes are which one would normally
expect for process exit codes (e.g. some are negative) but I didn''t
attempt to address that here.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r 54981fca0742 -r 89a8086b0a05 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Fri Jul 30 08:47:39 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c Fri Jul 30 09:13:10 2010 +0100
@@ -1701,7 +1701,7 @@ int main_memmax(int argc, char **argv)
}
if (optind >= argc - 1) {
help("mem-max");
- exit(2);
+ return 2;
}
p = argv[optind];
@@ -1710,10 +1710,10 @@ int main_memmax(int argc, char **argv)
rc = set_memory_max(p, mem);
if (rc) {
fprintf(stderr, "cannot set domid %d static max memory to :
%s\n", domid, mem);
- exit(1);
- }
-
- exit(0);
+ return 1;
+ }
+
+ return 0;
}
void set_memory_target(char *p, char *mem)
@@ -1740,7 +1740,7 @@ int main_memset(int argc, char **argv)
switch (opt) {
case ''h'':
help("mem-set");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -1748,14 +1748,14 @@ int main_memset(int argc, char **argv)
}
if (optind >= argc - 1) {
help("mem-set");
- exit(2);
+ return 2;
}
p = argv[optind];
mem = argv[optind + 1];
set_memory_target(p, mem);
- exit(0);
+ return 0;
}
void cd_insert(char *dom, char *virtdev, char *phys)
@@ -1806,7 +1806,7 @@ int main_cd_eject(int argc, char **argv)
switch (opt) {
case ''h'':
help("cd-eject");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -1814,14 +1814,14 @@ int main_cd_eject(int argc, char **argv)
}
if (optind >= argc - 1) {
help("cd-eject");
- exit(2);
+ return 2;
}
p = argv[optind];
virtdev = argv[optind + 1];
cd_insert(p, virtdev, NULL);
- exit(0);
+ return 0;
}
int main_cd_insert(int argc, char **argv)
@@ -1833,7 +1833,7 @@ int main_cd_insert(int argc, char **argv
switch (opt) {
case ''h'':
help("cd-insert");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -1841,7 +1841,7 @@ int main_cd_insert(int argc, char **argv
}
if (optind >= argc - 2) {
help("cd-insert");
- exit(2);
+ return 2;
}
p = argv[optind];
@@ -1849,7 +1849,7 @@ int main_cd_insert(int argc, char **argv
file = argv[optind + 2];
cd_insert(p, virtdev, file);
- exit(0);
+ return 0;
}
int main_console(int argc, char **argv)
@@ -1860,7 +1860,7 @@ int main_console(int argc, char **argv)
switch (opt) {
case ''h'':
help("console");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -1868,7 +1868,7 @@ int main_console(int argc, char **argv)
}
if (optind >= argc) {
help("console");
- exit(2);
+ return 2;
}
find_domain(argv[optind]);
@@ -1906,7 +1906,7 @@ int main_vncviewer(int argc, char **argv
break;
case ''h'':
help("vncviewer");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -1915,12 +1915,12 @@ int main_vncviewer(int argc, char **argv
if (argc - optind != 1) {
help("vncviewer");
- exit(2);
+ return 2;
}
if (vncviewer(argv[optind], autopass))
- exit(1);
- exit(0);
+ return 1;
+ return 0;
}
void pcilist_assignable(void)
@@ -1945,7 +1945,7 @@ int main_pcilist_assignable(int argc, ch
switch (opt) {
case ''h'':
help("pci-list-assignable-devices");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -1953,7 +1953,7 @@ int main_pcilist_assignable(int argc, ch
}
pcilist_assignable();
- exit(0);
+ return 0;
}
void pcilist(char *dom)
@@ -1981,7 +1981,7 @@ int main_pcilist(int argc, char **argv)
switch (opt) {
case ''h'':
help("pci-list");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -1989,13 +1989,13 @@ int main_pcilist(int argc, char **argv)
}
if (optind >= argc) {
help("pci-list");
- exit(2);
+ return 2;
}
domname = argv[optind];
pcilist(domname);
- exit(0);
+ return 0;
}
void pcidetach(char *dom, char *bdf)
@@ -2023,7 +2023,7 @@ int main_pcidetach(int argc, char **argv
switch (opt) {
case ''h'':
help("pci-detach");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -2031,14 +2031,14 @@ int main_pcidetach(int argc, char **argv
}
if (optind >= argc - 1) {
help("pci-detach");
- exit(2);
+ return 2;
}
domname = argv[optind];
bdf = argv[optind + 1];
pcidetach(domname, bdf);
- exit(0);
+ return 0;
}
void pciattach(char *dom, char *bdf, char *vs)
{
@@ -2065,7 +2065,7 @@ int main_pciattach(int argc, char **argv
switch (opt) {
case ''h'':
help("pci-attach");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -2073,7 +2073,7 @@ int main_pciattach(int argc, char **argv
}
if (optind >= argc - 1) {
help("pci-attach");
- exit(2);
+ return 2;
}
domname = argv[optind];
@@ -2083,7 +2083,7 @@ int main_pciattach(int argc, char **argv
vs = argv[optind + 2];
pciattach(domname, bdf, vs);
- exit(0);
+ return 0;
}
void pause_domain(char *p)
@@ -2673,7 +2673,7 @@ int main_restore(int argc, char **argv)
break;
case ''h'':
help("restore");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -2687,7 +2687,7 @@ int main_restore(int argc, char **argv)
checkpoint_file = argv[optind + 1];
} else {
help("restore");
- exit(2);
+ return 2;
}
memset(&dom_info, 0, sizeof(dom_info));
@@ -2700,9 +2700,9 @@ int main_restore(int argc, char **argv)
rc = create_domain(&dom_info);
if (rc < 0)
- exit(-rc);
-
- exit(0);
+ return -rc;
+
+ return 0;
}
int main_migrate_receive(int argc, char **argv)
@@ -2714,7 +2714,7 @@ int main_migrate_receive(int argc, char
switch (opt) {
case ''h'':
help("migrate-receive");
- exit(2);
+ return 2;
break;
case ''e'':
daemonize = 0;
@@ -2730,10 +2730,10 @@ int main_migrate_receive(int argc, char
if (argc-optind != 0) {
help("migrate-receive");
- exit(2);
+ return 2;
}
migrate_receive(debug, daemonize);
- exit(0);
+ return 0;
}
int main_save(int argc, char **argv)
@@ -2750,7 +2750,7 @@ int main_save(int argc, char **argv)
break;
case ''h'':
help("save");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -2759,14 +2759,14 @@ int main_save(int argc, char **argv)
if (argc-optind < 1 || argc-optind > 3) {
help("save");
- exit(2);
+ return 2;
}
p = argv[optind];
filename = argv[optind + 1];
config_filename = argv[optind + 2];
save_domain(p, filename, checkpoint, config_filename);
- exit(0);
+ return 0;
}
int main_migrate(int argc, char **argv)
@@ -2782,7 +2782,7 @@ int main_migrate(int argc, char **argv)
switch (opt) {
case ''h'':
help("migrate");
- exit(0);
+ return 0;
case ''C'':
config_filename = optarg;
break;
@@ -2803,7 +2803,7 @@ int main_migrate(int argc, char **argv)
if (argc-optind < 2 || argc-optind > 2) {
help("migrate");
- exit(2);
+ return 2;
}
p = argv[optind];
@@ -2816,21 +2816,21 @@ int main_migrate(int argc, char **argv)
ssh_command, host,
daemonize ? "" : " -e",
debug ? " -d" : "") < 0)
- exit(1);
+ return 1;
}
migrate_domain(p, rune, config_filename);
- exit(0);
+ return 0;
}
int main_dump_core(int argc, char **argv)
{
if ( argc-optind < 2 ) {
help("dump-core");
- exit(2);
+ return 2;
}
core_dump_domain(argv[optind], argv[optind + 1]);
- exit(0);
+ return 0;
}
int main_pause(int argc, char **argv)
@@ -2843,7 +2843,7 @@ int main_pause(int argc, char **argv)
switch (opt) {
case ''h'':
help("pause");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -2851,13 +2851,13 @@ int main_pause(int argc, char **argv)
}
if (optind >= argc) {
help("pause");
- exit(2);
+ return 2;
}
p = argv[optind];
pause_domain(p);
- exit(0);
+ return 0;
}
int main_unpause(int argc, char **argv)
@@ -2870,7 +2870,7 @@ int main_unpause(int argc, char **argv)
switch (opt) {
case ''h'':
help("unpause");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -2878,13 +2878,13 @@ int main_unpause(int argc, char **argv)
}
if (optind >= argc) {
help("unpause");
- exit(2);
+ return 2;
}
p = argv[optind];
unpause_domain(p);
- exit(0);
+ return 0;
}
int main_destroy(int argc, char **argv)
@@ -2896,7 +2896,7 @@ int main_destroy(int argc, char **argv)
switch (opt) {
case ''h'':
help("destroy");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -2904,13 +2904,13 @@ int main_destroy(int argc, char **argv)
}
if (optind >= argc) {
help("destroy");
- exit(2);
+ return 2;
}
p = argv[optind];
destroy_domain(p);
- exit(0);
+ return 0;
}
int main_shutdown(int argc, char **argv)
@@ -2922,7 +2922,7 @@ int main_shutdown(int argc, char **argv)
switch (opt) {
case ''h'':
help("shutdown");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -2930,13 +2930,13 @@ int main_shutdown(int argc, char **argv)
}
if (optind >= argc) {
help("shutdown");
- exit(2);
+ return 2;
}
p = argv[optind];
shutdown_domain(p);
- exit(0);
+ return 0;
}
int main_reboot(int argc, char **argv)
@@ -2948,7 +2948,7 @@ int main_reboot(int argc, char **argv)
switch (opt) {
case ''h'':
help("reboot");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -2956,13 +2956,13 @@ int main_reboot(int argc, char **argv)
}
if (optind >= argc) {
help("reboot");
- exit(2);
+ return 2;
}
p = argv[optind];
reboot_domain(p);
- exit(0);
+ return 0;
}
int main_list(int argc, char **argv)
{
@@ -2991,7 +2991,7 @@ int main_list(int argc, char **argv)
break;
case ''h'':
help("list");
- exit(0);
+ return 0;
case ''v'':
verbose = 1;
break;
@@ -3005,7 +3005,7 @@ int main_list(int argc, char **argv)
info = libxl_list_domain(&ctx, &nb_domain);
if (!info) {
fprintf(stderr, "libxl_domain_infolist failed.\n");
- exit(1);
+ return 1;
}
info_free = info;
} else if (optind == argc-1) {
@@ -3013,13 +3013,13 @@ int main_list(int argc, char **argv)
rc = libxl_domain_info(&ctx, &info_buf, domid);
if (rc) {
fprintf(stderr, "libxl_domain_info failed (code %d).\n",
rc);
- exit(-rc);
+ return -rc;
}
info = &info_buf;
nb_domain = 1;
} else {
help("list");
- exit(2);
+ return 2;
}
if (details)
@@ -3029,7 +3029,7 @@ int main_list(int argc, char **argv)
free(info_free);
- exit(0);
+ return 0;
}
int main_list_vm(int argc, char **argv)
@@ -3040,7 +3040,7 @@ int main_list_vm(int argc, char **argv)
switch (opt) {
case ''h'':
help("list-vm");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -3048,7 +3048,7 @@ int main_list_vm(int argc, char **argv)
}
list_vm();
- exit(0);
+ return 0;
}
int main_create(int argc, char **argv)
@@ -3091,7 +3091,7 @@ int main_create(int argc, char **argv)
break;
case ''h'':
help("create");
- exit(0);
+ return 0;
case ''n'':
dryrun = 1;
break;
@@ -3116,7 +3116,7 @@ int main_create(int argc, char **argv)
filename = argv[optind];
} else {
help("create");
- exit(2);
+ return 2;
}
optind++;
}
@@ -3134,9 +3134,9 @@ int main_create(int argc, char **argv)
rc = create_domain(&dom_info);
if (rc < 0)
- exit(-rc);
-
- exit(0);
+ return -rc;
+
+ return 0;
}
void button_press(char *p, char *b)
@@ -3167,7 +3167,7 @@ int main_button_press(int argc, char **a
switch (opt) {
case ''h'':
help("button-press");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -3175,14 +3175,14 @@ int main_button_press(int argc, char **a
}
if (optind >= argc - 1) {
help("button-press");
- exit(2);
+ return 2;
}
p = argv[optind];
b = argv[optind + 1];
button_press(p, b);
- exit(0);
+ return 0;
}
static void print_vcpuinfo(uint32_t tdomid,
@@ -3304,7 +3304,7 @@ int main_vcpulist(int argc, char **argv)
switch (opt) {
case ''h'':
help("vcpu-list");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -3312,7 +3312,7 @@ int main_vcpulist(int argc, char **argv)
}
vcpulist(argc - 2, argv + 2);
- exit(0);
+ return 0;
}
void vcpupin(char *d, const char *vcpu, char *cpu)
@@ -3402,13 +3402,13 @@ int main_vcpupin(int argc, char **argv)
if (argc != 5) {
help("vcpu-pin");
- exit(0);
+ return 0;
}
while ((opt = getopt(argc, argv, "h")) != -1) {
switch (opt) {
case ''h'':
help("vcpu-pin");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -3416,7 +3416,7 @@ int main_vcpupin(int argc, char **argv)
}
vcpupin(argv[2], argv[3] , argv[4]);
- exit(0);
+ return 0;
}
void vcpuset(char *d, char* nr_vcpus)
@@ -3443,13 +3443,13 @@ int main_vcpuset(int argc, char **argv)
if (argc != 4) {
help("vcpu-set");
- exit(0);
+ return 0;
}
while ((opt = getopt(argc, argv, "h")) != -1) {
switch (opt) {
case ''h'':
help("vcpu-set");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -3457,7 +3457,7 @@ int main_vcpuset(int argc, char **argv)
}
vcpuset(argv[2], argv[3]);
- exit(0);
+ return 0;
}
static void output_xeninfo(void)
@@ -3564,7 +3564,7 @@ int main_info(int argc, char **argv)
switch (opt) {
case ''h'':
help("info");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -3572,7 +3572,7 @@ int main_info(int argc, char **argv)
}
info();
- exit(0);
+ return 0;
}
static int sched_credit_domain_get(
@@ -3633,7 +3633,7 @@ int main_sched_credit(int argc, char **a
break;
case ''h'':
help("sched-credit");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -3642,21 +3642,21 @@ int main_sched_credit(int argc, char **a
if (!dom && (opt_w || opt_c)) {
fprintf(stderr, "Must specify a domain.\n");
- exit(1);
+ return 1;
}
if (!dom) { /* list all domain''s credit scheduler info */
info = libxl_list_domain(&ctx, &nb_domain);
if (!info) {
fprintf(stderr, "libxl_domain_infolist failed.\n");
- exit(1);
+ return 1;
}
printf("%-33s %4s %6s %4s\n", "Name",
"ID", "Weight", "Cap");
for (i = 0; i < nb_domain; i++) {
rc = sched_credit_domain_get(info[i].domid, &scinfo);
if (rc)
- exit(-rc);
+ return -rc;
sched_credit_domain_output(info[i].domid, &scinfo);
}
} else {
@@ -3664,7 +3664,7 @@ int main_sched_credit(int argc, char **a
rc = sched_credit_domain_get(domid, &scinfo);
if (rc)
- exit(-rc);
+ return -rc;
if (!opt_w && !opt_c) { /* output credit scheduler info */
printf("%-33s %4s %6s %4s\n", "Name",
"ID", "Weight", "Cap");
@@ -3676,11 +3676,11 @@ int main_sched_credit(int argc, char **a
scinfo.cap = cap;
rc = sched_credit_domain_set(domid, &scinfo);
if (rc)
- exit(-rc);
- }
- }
-
- exit(0);
+ return -rc;
+ }
+ }
+
+ return 0;
}
int main_domid(int argc, char **argv)
@@ -3692,7 +3692,7 @@ int main_domid(int argc, char **argv)
switch (opt) {
case ''h'':
help("domid");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -3703,17 +3703,17 @@ int main_domid(int argc, char **argv)
if (!domname) {
fprintf(stderr, "Must specify a domain name.\n\n");
help("domid");
- exit(1);
+ return 1;
}
if (libxl_name_to_domid(&ctx, domname, &domid)) {
fprintf(stderr, "Can''t get domid of domain name
''%s'', maybe this domain does not exist.\n", domname);
- exit(1);
+ return 1;
}
printf("%d\n", domid);
- exit(0);
+ return 0;
}
int main_domname(int argc, char **argv)
@@ -3726,7 +3726,7 @@ int main_domname(int argc, char **argv)
switch (opt) {
case ''h'':
help("domname");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -3736,24 +3736,24 @@ int main_domname(int argc, char **argv)
if (!argv[optind]) {
fprintf(stderr, "Must specify a domain id.\n\n");
help("domname");
- exit(1);
+ return 1;
}
domid = strtol(argv[optind], &endptr, 10);
if (domid == 0 && !strcmp(endptr, argv[optind])) {
/*no digits at all*/
fprintf(stderr, "Invalid domain id.\n\n");
- exit(1);
+ return 1;
}
domname = libxl_domid_to_name(&ctx, domid);
if (!domname) {
fprintf(stderr, "Can''t get domain name of domain id
''%d'', maybe this domain does not exist.\n", domid);
- exit(1);
+ return 1;
}
printf("%s\n", domname);
- exit(0);
+ return 0;
}
int main_rename(int argc, char **argv)
@@ -3766,7 +3766,7 @@ int main_rename(int argc, char **argv)
switch (opt) {
case ''h'':
help("rename");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -3777,7 +3777,7 @@ int main_rename(int argc, char **argv)
if (!dom || !argv[optind]) {
fprintf(stderr, "''xl rename'' requires 2
arguments.\n\n");
help("rename");
- exit(1);
+ return 1;
}
find_domain(dom);
@@ -3785,10 +3785,10 @@ int main_rename(int argc, char **argv)
if (libxl_domain_rename(&ctx, domid, common_domname, new_name, 0)) {
fprintf(stderr, "Can''t rename domain
''%s''.\n", dom);
- exit(1);
- }
-
- exit(0);
+ return 1;
+ }
+
+ return 0;
}
int main_trigger(int argc, char **argv)
@@ -3803,7 +3803,7 @@ int main_trigger(int argc, char **argv)
switch (opt) {
case ''h'':
help("trigger");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -3814,7 +3814,7 @@ int main_trigger(int argc, char **argv)
if (!dom || !argv[optind]) {
fprintf(stderr, "''xl trigger'' requires between 2
and 3 arguments.\n\n");
help("trigger");
- exit(1);
+ return 1;
}
find_domain(dom);
@@ -3830,7 +3830,7 @@ int main_trigger(int argc, char **argv)
libxl_send_trigger(&ctx, domid, trigger_name, vcpuid);
- exit(0);
+ return 0;
}
@@ -3844,7 +3844,7 @@ int main_sysrq(int argc, char **argv)
switch (opt) {
case ''h'':
help("sysrq");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -3855,7 +3855,7 @@ int main_sysrq(int argc, char **argv)
if (!dom || !argv[optind]) {
fprintf(stderr, "''xl sysrq'' requires 2
arguments.\n\n");
help("sysrq");
- exit(1);
+ return 1;
}
find_domain(dom);
@@ -3865,12 +3865,12 @@ int main_sysrq(int argc, char **argv)
if (sysrq[1] != ''\0'') {
fprintf(stderr, "Invalid sysrq.\n\n");
help("sysrq");
- exit(1);
+ return 1;
}
libxl_send_sysrq(&ctx, domid, sysrq[0]);
- exit(0);
+ return 0;
}
int main_debug_keys(int argc, char **argv)
@@ -3882,7 +3882,7 @@ int main_debug_keys(int argc, char **arg
switch (opt) {
case ''h'':
help("debug-keys");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -3890,17 +3890,17 @@ int main_debug_keys(int argc, char **arg
}
if (optind >= argc) {
help("debug-keys");
- exit(2);
+ return 2;
}
keys = argv[optind];
if (libxl_send_debug_keys(&ctx, keys)) {
fprintf(stderr, "cannot send debug keys: %s\n", keys);
- exit(1);
- }
-
- exit(0);
+ return 1;
+ }
+
+ return 0;
}
int main_dmesg(int argc, char **argv)
@@ -3917,7 +3917,7 @@ int main_dmesg(int argc, char **argv)
break;
case ''h'':
help("dmesg");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option not supported\n");
break;
@@ -3933,7 +3933,7 @@ int main_dmesg(int argc, char **argv)
finish:
libxl_xen_console_read_finish(&ctx, cr);
- exit(ret);
+ return ret;
}
int main_top(int argc, char **argv)
@@ -3944,14 +3944,14 @@ int main_top(int argc, char **argv)
switch (opt) {
case ''h'':
help("top");
- exit(0);
- default:
- fprintf(stderr, "option `%c'' not supported.\n",
opt);
- break;
- }
- }
-
- exit(system("xentop"));
+ return 0;
+ default:
+ fprintf(stderr, "option `%c'' not supported.\n",
opt);
+ break;
+ }
+ }
+
+ return system("xentop");
}
int main_networkattach(int argc, char **argv)
@@ -3964,22 +3964,22 @@ int main_networkattach(int argc, char **
if ((argc < 3) || (argc > 11)) {
help("network-attach");
- exit(0);
+ return 0;
}
while ((opt = getopt(argc, argv, "h")) != -1) {
switch (opt) {
case ''h'':
help("network-attach");
- exit(0);
- default:
- fprintf(stderr, "option `%c'' not supported.\n",
opt);
- break;
- }
- }
-
- if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
- fprintf(stderr, "%s is an invalid domain identifier\n",
argv[2]);
- exit(1);
+ return 0;
+ default:
+ fprintf(stderr, "option `%c'' not supported.\n",
opt);
+ break;
+ }
+ }
+
+ if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
+ fprintf(stderr, "%s is an invalid domain identifier\n",
argv[2]);
+ return 1;
}
init_nic_info(&nic, -1);
for (argv += 3, argc -= 3; argc > 0; ++argv, --argc) {
@@ -3990,7 +3990,7 @@ int main_networkattach(int argc, char **
nic.nictype = NICTYPE_IOEMU;
} else {
fprintf(stderr, "Invalid parameter
`type''.\n");
- exit(1);
+ return 1;
}
} else if (!strncmp("mac=", *argv, 4)) {
tok = strtok((*argv) + 4, ":");
@@ -3998,7 +3998,7 @@ int main_networkattach(int argc, char **
val = strtoul(tok, &endptr, 16);
if ((tok == endptr) || (val > 255)) {
fprintf(stderr, "Invalid parameter
`mac''.\n");
- exit(1);
+ return 1;
}
nic.mac[i] = val;
}
@@ -4007,7 +4007,7 @@ int main_networkattach(int argc, char **
} else if (!strncmp("ip=", *argv, 3)) {
if (!inet_aton((*argv) + 3, &(nic.ip))) {
fprintf(stderr, "Invalid parameter
`ip''.\n");
- exit(1);
+ return 1;
}
} else if (!strncmp("script=", *argv, 6)) {
nic.script = (*argv) + 6;
@@ -4015,7 +4015,7 @@ int main_networkattach(int argc, char **
val = strtoul((*argv) + 8, &endptr, 10);
if (((*argv) + 8) == endptr) {
fprintf(stderr, "Invalid parameter
`backend''.\n");
- exit(1);
+ return 1;
}
nic.backend_domid = val;
} else if (!strncmp("vifname=", *argv, 8)) {
@@ -4026,15 +4026,15 @@ int main_networkattach(int argc, char **
} else if (!strncmp("accel=", *argv, 6)) {
} else {
fprintf(stderr, "unrecognized argument `%s''\n",
*argv);
- exit(1);
+ return 1;
}
}
nic.domid = domid;
if (libxl_device_nic_add(&ctx, domid, &nic)) {
fprintf(stderr, "libxl_device_nic_add failed.\n");
- exit(1);
- }
- exit(0);
+ return 1;
+ }
+ return 0;
}
int main_networklist(int argc, char **argv)
@@ -4045,13 +4045,13 @@ int main_networklist(int argc, char **ar
if (argc < 3) {
help("network-list");
- exit(1);
+ return 1;
}
while ((opt = getopt(argc, argv, "h")) != -1) {
switch (opt) {
case ''h'':
help("network-list");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not
supported.\n", opt);
break;
@@ -4082,7 +4082,7 @@ int main_networklist(int argc, char **ar
nics->rref_tx, nics->rref_rx, nics->backend);
}
}
- exit(0);
+ return 0;
}
int main_networkdetach(int argc, char **argv)
@@ -4092,40 +4092,40 @@ int main_networkdetach(int argc, char **
if (argc != 4) {
help("network-detach");
- exit(0);
+ return 0;
}
while ((opt = getopt(argc, argv, "h")) != -1) {
switch (opt) {
case ''h'':
help("network-detach");
- exit(0);
- default:
- fprintf(stderr, "option `%c'' not supported.\n",
opt);
- break;
- }
- }
-
- if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
- fprintf(stderr, "%s is an invalid domain identifier\n",
argv[2]);
- exit(1);
+ return 0;
+ default:
+ fprintf(stderr, "option `%c'' not supported.\n",
opt);
+ break;
+ }
+ }
+
+ if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
+ fprintf(stderr, "%s is an invalid domain identifier\n",
argv[2]);
+ return 1;
}
if (!strchr(argv[3], '':'')) {
if (libxl_devid_to_device_nic(&ctx, domid, argv[3], &nic)) {
fprintf(stderr, "Unknown device %s.\n", argv[3]);
- exit(1);
+ return 1;
}
} else {
if (libxl_mac_to_device_nic(&ctx, domid, argv[3], &nic)) {
fprintf(stderr, "Unknown device %s.\n", argv[3]);
- exit(1);
+ return 1;
}
}
if (libxl_device_nic_del(&ctx, &nic, 1)) {
fprintf(stderr, "libxl_device_nic_del failed.\n");
- exit(1);
- }
- exit(0);
+ return 1;
+ }
+ return 0;
}
int main_blockattach(int argc, char **argv)
@@ -4137,13 +4137,13 @@ int main_blockattach(int argc, char **ar
if ((argc < 5) || (argc > 7)) {
help("block-attach");
- exit(0);
+ return 0;
}
while ((opt = getopt(argc, argv, "h")) != -1) {
switch (opt) {
case ''h'':
help("block-attach");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -4167,16 +4167,16 @@ int main_blockattach(int argc, char **ar
disk.phystype = PHYSTYPE_QCOW2;
} else {
fprintf(stderr, "Error: `%s'' is not a valid disk
image.\n", tok);
- exit(1);
+ return 1;
}
} else {
fprintf(stderr, "Error: `%s'' is not a valid block
device.\n", tok);
- exit(1);
+ return 1;
}
disk.physpath = strtok(NULL, "\0");
if (!disk.physpath) {
fprintf(stderr, "Error: missing path to disk image.\n");
- exit(1);
+ return 1;
}
disk.virtpath = argv[4];
disk.unpluggable = 1;
@@ -4184,12 +4184,12 @@ int main_blockattach(int argc, char **ar
if (domain_qualifier_to_domid(argv[2], &fe_domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n",
argv[2]);
- exit(1);
+ return 1;
}
if (argc == 7) {
if (domain_qualifier_to_domid(argv[6], &be_domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n",
argv[6]);
- exit(1);
+ return 1;
}
}
disk.domid = fe_domid;
@@ -4197,7 +4197,7 @@ int main_blockattach(int argc, char **ar
if (libxl_device_disk_add(&ctx, fe_domid, &disk)) {
fprintf(stderr, "libxl_device_disk_add failed.\n");
}
- exit(0);
+ return 0;
}
int main_blocklist(int argc, char **argv)
@@ -4209,13 +4209,13 @@ int main_blocklist(int argc, char **argv
if (argc < 3) {
help("block-list");
- exit(0);
+ return 0;
}
while ((opt = getopt(argc, argv, "h")) != -1) {
switch (opt) {
case ''h'':
help("block-list");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -4242,7 +4242,7 @@ int main_blocklist(int argc, char **argv
}
}
}
- exit(0);
+ return 0;
}
int main_blockdetach(int argc, char **argv)
@@ -4252,31 +4252,31 @@ int main_blockdetach(int argc, char **ar
if (argc != 4) {
help("block-detach");
- exit(0);
+ return 0;
}
while ((opt = getopt(argc, argv, "h")) != -1) {
switch (opt) {
case ''h'':
help("block-detach");
- exit(0);
- default:
- fprintf(stderr, "option `%c'' not supported.\n",
opt);
- break;
- }
- }
-
- if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
- fprintf(stderr, "%s is an invalid domain identifier\n",
argv[2]);
- exit(1);
+ return 0;
+ default:
+ fprintf(stderr, "option `%c'' not supported.\n",
opt);
+ break;
+ }
+ }
+
+ if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
+ fprintf(stderr, "%s is an invalid domain identifier\n",
argv[2]);
+ return 1;
}
if (libxl_devid_to_device_disk(&ctx, domid, argv[3], &disk)) {
fprintf(stderr, "Error: Device %s not connected.\n",
argv[3]);
- exit(1);
+ return 1;
}
if (libxl_device_disk_del(&ctx, &disk, 1)) {
fprintf(stderr, "libxl_device_del failed.\n");
}
- exit(0);
+ return 0;
}
int main_network2attach(int argc, char **argv)
@@ -4290,13 +4290,13 @@ int main_network2attach(int argc, char *
if ((argc < 3) || (argc > 12)) {
help("network2-attach");
- exit(0);
+ return 0;
}
while ((opt = getopt(argc, argv, "h")) != -1) {
switch (opt) {
case ''h'':
help("network2-attach");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -4305,7 +4305,7 @@ int main_network2attach(int argc, char *
if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n",
argv[1]);
- exit(1);
+ return 1;
}
init_net2_info(&net2, -1);
for (argv += 3, argc -= 3; argc > 0; --argc, ++argv) {
@@ -4315,7 +4315,7 @@ int main_network2attach(int argc, char *
val = strtoul(tok, &endptr, 16);
if ((tok == endptr) || (val > 255)) {
fprintf(stderr, "Invalid parameter
`front_mac''.\n");
- exit(1);
+ return 1;
}
net2.front_mac[i] = val;
}
@@ -4325,7 +4325,7 @@ int main_network2attach(int argc, char *
val = strtoul(tok, &endptr, 16);
if ((tok == endptr) || (val > 255)) {
fprintf(stderr, "Invalid parameter
back_mac=%s.\n", *argv + 9);
- exit(1);
+ return 1;
}
net2.back_mac[i] = val;
}
@@ -4345,26 +4345,26 @@ int main_network2attach(int argc, char *
val = strtoul(*argv + 5, &endptr, 10);
if (endptr == (*argv + 5)) {
fprintf(stderr, "Invalid parameter pdev=%s.\n", *argv
+ 5);
- exit(1);
+ return 1;
}
net2.pdev = val;
} else if (!strncmp("max_bypasses=", *argv, 13)) {
val = strtoul(*argv + 13, &endptr, 10);
if (endptr == (*argv + 13)) {
fprintf(stderr, "Invalid parameter
max_bypasses=%s.\n", *argv + 13);
- exit(1);
+ return 1;
}
net2.max_bypasses = val;
} else {
fprintf(stderr, "unrecognized argument `%s''\n",
*argv);
- exit(1);
+ return 1;
}
}
if (back_dom) {
if (domain_qualifier_to_domid(back_dom, &back_domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n",
back_dom);
- exit(1);
+ return 1;
}
}
net2.domid = domid;
@@ -4372,7 +4372,7 @@ int main_network2attach(int argc, char *
if (libxl_device_net2_add(&ctx, domid, &net2)) {
fprintf(stderr, "libxl_device_net2_add failed.\n");
}
- exit(0);
+ return 0;
}
int main_network2list(int argc, char **argv)
@@ -4383,13 +4383,13 @@ int main_network2list(int argc, char **a
if (argc < 3) {
help("network2-list");
- exit(0);
+ return 0;
}
while ((opt = getopt(argc, argv, "h")) != -1) {
switch (opt) {
case ''h'':
help("network2-list");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -4417,7 +4417,7 @@ int main_network2list(int argc, char **a
}
}
}
- exit(0);
+ return 0;
}
int main_network2detach(int argc, char **argv)
@@ -4427,32 +4427,32 @@ int main_network2detach(int argc, char *
if (argc != 4) {
help("network2-detach");
- exit(0);
+ return 0;
}
while ((opt = getopt(argc, argv, "h")) != -1) {
switch (opt) {
case ''h'':
help("network2-detach");
- exit(0);
- default:
- fprintf(stderr, "option `%c'' not supported.\n",
opt);
- break;
- }
- }
-
- if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
- fprintf(stderr, "%s is an invalid domain identifier\n",
argv[2]);
- exit(1);
+ return 0;
+ default:
+ fprintf(stderr, "option `%c'' not supported.\n",
opt);
+ break;
+ }
+ }
+
+ if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
+ fprintf(stderr, "%s is an invalid domain identifier\n",
argv[2]);
+ return 1;
}
if (libxl_devid_to_device_net2(&ctx, domid, argv[3], &net2)) {
fprintf(stderr, "Error: Device %s not connected.\n", argv[3]);
- exit(1);
+ return 1;
}
if (libxl_device_net2_del(&ctx, &net2, 1)) {
fprintf(stderr, "libxl_device_net2_del failed.\n");
- exit(1);
- }
- exit(0);
+ return 1;
+ }
+ return 0;
}
static char *uptime_to_string(unsigned long time, int short_mode)
@@ -4624,7 +4624,7 @@ int main_uptime(int argc, char **argv)
break;
case ''h'':
help("uptime");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -4638,7 +4638,7 @@ int main_uptime(int argc, char **argv)
print_uptime(short_mode, domains, nb_doms);
- exit(0);
+ return 0;
}
int main_tmem_list(int argc, char **argv)
@@ -4659,7 +4659,7 @@ int main_tmem_list(int argc, char **argv
break;
case ''h'':
help("tmem-list");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -4670,7 +4670,7 @@ int main_tmem_list(int argc, char **argv
if (!dom && all == 0) {
fprintf(stderr, "You must specify -a or a domain id.\n\n");
help("tmem-list");
- exit(1);
+ return 1;
}
if (all)
@@ -4680,11 +4680,11 @@ int main_tmem_list(int argc, char **argv
buf = libxl_tmem_list(&ctx, domid, use_long);
if (buf == NULL)
- exit(-1);
+ return -1;
printf("%s\n", buf);
free(buf);
- exit(0);
+ return 0;
}
int main_tmem_freeze(int argc, char **argv)
@@ -4700,7 +4700,7 @@ int main_tmem_freeze(int argc, char **ar
break;
case ''h'':
help("tmem-freeze");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -4711,7 +4711,7 @@ int main_tmem_freeze(int argc, char **ar
if (!dom && all == 0) {
fprintf(stderr, "You must specify -a or a domain id.\n\n");
help("tmem-freeze");
- exit(1);
+ return 1;
}
if (all)
@@ -4720,7 +4720,7 @@ int main_tmem_freeze(int argc, char **ar
find_domain(dom);
libxl_tmem_freeze(&ctx, domid);
- exit(0);
+ return 0;
}
int main_tmem_destroy(int argc, char **argv)
@@ -4736,7 +4736,7 @@ int main_tmem_destroy(int argc, char **a
break;
case ''h'':
help("tmem-destroy");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -4747,7 +4747,7 @@ int main_tmem_destroy(int argc, char **a
if (!dom && all == 0) {
fprintf(stderr, "You must specify -a or a domain id.\n\n");
help("tmem-destroy");
- exit(1);
+ return 1;
}
if (all)
@@ -4756,7 +4756,7 @@ int main_tmem_destroy(int argc, char **a
find_domain(dom);
libxl_tmem_destroy(&ctx, domid);
- exit(0);
+ return 0;
}
int main_tmem_thaw(int argc, char **argv)
@@ -4772,7 +4772,7 @@ int main_tmem_thaw(int argc, char **argv
break;
case ''h'':
help("tmem-thaw");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -4783,7 +4783,7 @@ int main_tmem_thaw(int argc, char **argv
if (!dom && all == 0) {
fprintf(stderr, "You must specify -a or a domain id.\n\n");
help("tmem-thaw");
- exit(1);
+ return 1;
}
if (all)
@@ -4792,7 +4792,7 @@ int main_tmem_thaw(int argc, char **argv
find_domain(dom);
libxl_tmem_thaw(&ctx, domid);
- exit(0);
+ return 0;
}
int main_tmem_set(int argc, char **argv)
@@ -4822,7 +4822,7 @@ int main_tmem_set(int argc, char **argv)
break;
case ''h'':
help("tmem-set");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -4833,7 +4833,7 @@ int main_tmem_set(int argc, char **argv)
if (!dom && all == 0) {
fprintf(stderr, "You must specify -a or a domain id.\n\n");
help("tmem-set");
- exit(1);
+ return 1;
}
if (all)
@@ -4844,7 +4844,7 @@ int main_tmem_set(int argc, char **argv)
if (!opt_w && !opt_c && !opt_p) {
fprintf(stderr, "No set value specified.\n\n");
help("tmem-set");
- exit(1);
+ return 1;
}
if (opt_w)
@@ -4854,7 +4854,7 @@ int main_tmem_set(int argc, char **argv)
if (opt_p)
libxl_tmem_set(&ctx, domid, "compress", compress);
- exit(0);
+ return 0;
}
int main_tmem_shared_auth(int argc, char **argv)
@@ -4880,7 +4880,7 @@ int main_tmem_shared_auth(int argc, char
break;
case ''h'':
help("tmem-shared-auth");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -4891,7 +4891,7 @@ int main_tmem_shared_auth(int argc, char
if (!dom && all == 0) {
fprintf(stderr, "You must specify -a or a domain id.\n\n");
help("tmem-shared-auth");
- exit(1);
+ return 1;
}
if (all)
@@ -4902,18 +4902,18 @@ int main_tmem_shared_auth(int argc, char
if (uuid == NULL || autharg == NULL) {
fprintf(stderr, "No uuid or auth specified.\n\n");
help("tmem-shared-auth");
- exit(1);
+ return 1;
}
auth = strtol(autharg, &endptr, 10);
if (*endptr != ''\0'') {
fprintf(stderr, "Invalid auth, valid auth are
<0|1>.\n\n");
- exit(1);
+ return 1;
}
libxl_tmem_shared_auth(&ctx, domid, uuid, auth);
- exit(0);
+ return 0;
}
int main_tmem_freeable(int argc, char **argv)
@@ -4925,7 +4925,7 @@ int main_tmem_freeable(int argc, char **
switch (opt) {
case ''h'':
help("tmem-freeable");
- exit(0);
+ return 0;
default:
fprintf(stderr, "option `%c'' not supported.\n",
opt);
break;
@@ -4934,8 +4934,8 @@ int main_tmem_freeable(int argc, char **
mb = libxl_tmem_freeable(&ctx);
if (mb == -1)
- exit(-1);
+ return -1;
printf("%d\n", mb);
- exit(0);
-}
+ return 0;
+}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Campbell
2010-Jul-30 09:02 UTC
[Xen-devel] [PATCH 2 of 3] xl: free the libxl context before exit
# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1280477591 -3600
# Node ID 8b2d8f18090171e161c398019bacedbb5f43455b
# Parent 89a8086b0a054c10552bc32a28026773c9608772
xl: free the libxl context before exit
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r 89a8086b0a05 -r 8b2d8f180901 tools/libxl/xl.c
--- a/tools/libxl/xl.c Fri Jul 30 09:13:10 2010 +0100
+++ b/tools/libxl/xl.c Fri Jul 30 09:13:11 2010 +0100
@@ -40,6 +40,7 @@ int main(int argc, char **argv)
int opt = 0;
char *cmd = 0;
struct cmd_spec *cspec;
+ int ret;
while ((opt = getopt(argc, argv, "+v")) >= 0) {
switch (opt) {
@@ -72,12 +73,16 @@ int main(int argc, char **argv)
cspec = cmdtable_lookup(cmd);
if (cspec)
- return cspec->cmd_impl(argc, argv);
+ ret = cspec->cmd_impl(argc, argv);
else if (!strcmp(cmd, "help")) {
help(argv[optind]);
- exit(0);
+ ret = 0;
} else {
fprintf(stderr, "command not implemented\n");
- exit(1);
+ ret = 1;
}
+
+ libxl_ctx_free(&ctx);
+
+ return ret;
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Campbell
2010-Jul-30 09:02 UTC
[Xen-devel] [PATCH 3 of 3] xl: destroy the logger before exit
# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1280477591 -3600
# Node ID f5f5949d98f0104ad1422ddacded20875f23d38d
# Parent 8b2d8f18090171e161c398019bacedbb5f43455b
xl: destroy the logger before exit
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r 8b2d8f180901 -r f5f5949d98f0 tools/libxl/xl.c
--- a/tools/libxl/xl.c Fri Jul 30 09:13:11 2010 +0100
+++ b/tools/libxl/xl.c Fri Jul 30 09:13:11 2010 +0100
@@ -83,6 +83,7 @@ int main(int argc, char **argv)
}
libxl_ctx_free(&ctx);
+ xtl_logger_destroy((xentoollog_logger*)logger);
return ret;
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Campbell
2010-Jul-30 09:28 UTC
[Xen-devel] Re: [PATCH 0 of 3] xl: free allocations made at top level
On Fri, 2010-07-30 at 10:02 +0100, Ian Campbell wrote:> > If there were a way to tell valgrind not to worry about these > allocations that would be nice, as would a palatable workaround which > could be used in xl but I can''t find anything suitable. For example > calling pthread_exit() at the end of main() causes leaks from the C > runtime so that is out. Creating a thread to do the body of the work > (with main just doing pthread_join and returning the result) doesn''t > pass the palatable test IMHOThis seems to work. I''m not entirely sure about it though -- it''s not clear what happens if the thread which calls xc_interface_close is not the final thread to exit and whether we would still leak the buffer from the thread which does the final exit() call. It''s enough to make xl valgrind clean though, so perhaps that is enough. # HG changeset patch # User Ian Campbell <ian.campbell@citrix.com> # Date 1280481944 -3600 # Node ID b8bf3e732b9a4b3a1614dff87af48560d3839e3b # Parent f5f5949d98f0104ad1422ddacded20875f23d38d libxc: free thread specific hypercall buffer on xc_interface_close The per-thread hypercall buffer is usually cleaned up on pthread_exit by the destructor passed to pthread_key_create. However if the calling application is not threaded then the destructor is never called. This frees the data for the current thread only but that is OK since any other threads will be cleaned up by the destructor. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> diff -r f5f5949d98f0 -r b8bf3e732b9a tools/libxc/xc_private.c --- a/tools/libxc/xc_private.c Fri Jul 30 09:13:11 2010 +0100 +++ b/tools/libxc/xc_private.c Fri Jul 30 10:25:44 2010 +0100 @@ -57,6 +57,8 @@ xc_interface *xc_interface_open(xentooll return 0; } +static void xc_clean_hcall_buf(void); + int xc_interface_close(xc_interface *xch) { int rc = 0; @@ -68,6 +70,9 @@ int xc_interface_close(xc_interface *xch rc = xc_interface_close_core(xch, xch->fd); if (rc) PERROR("Could not close hypervisor interface"); } + + xc_clean_hcall_buf(); + free(xch); return rc; } @@ -180,6 +185,8 @@ int hcall_buf_prep(void **addr, size_t l int hcall_buf_prep(void **addr, size_t len) { return 0; } void hcall_buf_release(void **addr, size_t len) { } +static void xc_clean_hcall_buf(void) { } + #else /* !__sun__ */ int lock_pages(void *addr, size_t len) @@ -223,6 +230,14 @@ static void _xc_clean_hcall_buf(void *m) } pthread_setspecific(hcall_buf_pkey, NULL); +} + +static void xc_clean_hcall_buf(void) +{ + void *hcall_buf = pthread_getspecific(hcall_buf_pkey); + + if (hcall_buf) + _xc_clean_hcall_buf(hcall_buf); } static void _xc_init_hcall_buf(void) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Jul-30 14:02 UTC
[Xen-devel] Re: [PATCH 0 of 3] xl: free allocations made at top level
On Fri, 2010-07-30 at 10:02 +0100, Ian Campbell wrote:> > I used a version of valgrind which I have patched to understand the > PRIVCMD_HYPERCALL ioctl on /proc/xen/privcmd. It currently understands > exactly one sysctl (XEN_SYSCTL_getdomaininfolist). I will post the > patch shortly and imagine I will be expanding it as I test other xl > commands etc.Here is the patch I used. It''s obviously very basic and terribly incomplete, only understands a single sysctl, only for the version of Xen it is built against, is my first attempt at hacking valgrind, etc etc. I also bailed on the normal valgrind policy of duplicating the interface headers so usage is: ./configure --with-xen=/path/to/headers The path needs to be to an installed set of xen headers, such that "#include <xen/xen.h>" is valid e.g. dist/install/usr/include in your built Xen tree or /usr/include or something equivalent (e.g. libxen-dev installed under Debian etc.) Ian. diff --git a/configure.in b/configure.in index 62e1837..e71ecd6 100644 --- a/configure.in +++ b/configure.in @@ -1558,6 +1558,11 @@ elif test x$VGCONF_PLATFORM_SEC_CAPS = xPPC32_AIX5 ; then mflag_secondary=-q32 fi +AC_ARG_WITH(xen, + [ --with-xen= Specify location of Xen headers], + XEN_CFLAGS=-I$withval +) +AC_SUBST(XEN_CFLAGS) AC_ARG_WITH(mpicc, [ --with-mpicc= Specify name of MPI2-ised C compiler], diff --git a/coregrind/Makefile.am b/coregrind/Makefile.am index d9d1bca..d7216f9 100644 --- a/coregrind/Makefile.am +++ b/coregrind/Makefile.am @@ -211,6 +211,7 @@ noinst_HEADERS = \ m_syswrap/priv_syswrap-aix5.h \ m_syswrap/priv_syswrap-darwin.h \ m_syswrap/priv_syswrap-main.h \ + m_syswrap/priv_syswrap-xen.h \ m_ume/priv_ume.h #---------------------------------------------------------------------------- @@ -338,6 +339,7 @@ COREGRIND_SOURCES_COMMON = \ m_syswrap/syswrap-ppc64-aix5.c \ m_syswrap/syswrap-x86-darwin.c \ m_syswrap/syswrap-amd64-darwin.c \ + m_syswrap/syswrap-xen.c \ m_ume/elf.c \ m_ume/macho.c \ m_ume/main.c \ @@ -350,7 +352,7 @@ nodist_libcoregrind_@VGCONF_ARCH_PRI@_@VGCONF_OS@_a_SOURCES = \ libcoregrind_@VGCONF_ARCH_PRI@_@VGCONF_OS@_a_CPPFLAGS = \ $(AM_CPPFLAGS_@VGCONF_PLATFORM_PRI_CAPS@) libcoregrind_@VGCONF_ARCH_PRI@_@VGCONF_OS@_a_CFLAGS = \ - $(AM_CFLAGS_@VGCONF_PLATFORM_PRI_CAPS@) + $(AM_CFLAGS_@VGCONF_PLATFORM_PRI_CAPS@) @XEN_CFLAGS@ libcoregrind_@VGCONF_ARCH_PRI@_@VGCONF_OS@_a_CCASFLAGS = \ $(AM_CCASFLAGS_@VGCONF_PLATFORM_PRI_CAPS@) if VGCONF_HAVE_PLATFORM_SEC diff --git a/coregrind/m_syswrap/priv_syswrap-xen.h b/coregrind/m_syswrap/priv_syswrap-xen.h new file mode 100644 index 0000000..c65edca --- /dev/null +++ b/coregrind/m_syswrap/priv_syswrap-xen.h @@ -0,0 +1,10 @@ +#ifndef __PRIV_SYSWRAP_XEN_H +#define __PRIV_SYSWRAP_XEN_H + +DECL_TEMPLATE(xen, ioctl_privcmd_hypercall); + +#endif // __PRIV_SYSWRAP_XEN_H + +/*--------------------------------------------------------------------*/ +/*--- end ---*/ +/*--------------------------------------------------------------------*/ diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c index 247402d..42dc4d9 100644 --- a/coregrind/m_syswrap/syswrap-linux.c +++ b/coregrind/m_syswrap/syswrap-linux.c @@ -57,7 +57,7 @@ #include "priv_types_n_macros.h" #include "priv_syswrap-generic.h" #include "priv_syswrap-linux.h" - +#include "priv_syswrap-xen.h" // Run a thread from beginning to end and return the thread''s // scheduler-return-code. @@ -4821,6 +4821,11 @@ PRE(sys_ioctl) } break; + + case VKI_XEN_IOCTL_PRIVCMD_HYPERCALL: + WRAPPER_PRE_NAME(xen, ioctl_privcmd_hypercall)(tid, layout, arrghs, status, flags); + break; + default: /* EVIOC* are variable length and return size written on success */ switch (ARG2 & ~(_VKI_IOC_SIZEMASK << _VKI_IOC_SIZESHIFT)) { @@ -5633,6 +5638,10 @@ POST(sys_ioctl) } break; + case VKI_XEN_IOCTL_PRIVCMD_HYPERCALL: + WRAPPER_POST_NAME(xen, ioctl_privcmd_hypercall)(tid, arrghs, status); + break; + default: /* EVIOC* are variable length and return size written on success */ switch (ARG2 & ~(_VKI_IOC_SIZEMASK << _VKI_IOC_SIZESHIFT)) { diff --git a/coregrind/m_syswrap/syswrap-xen.c b/coregrind/m_syswrap/syswrap-xen.c new file mode 100644 index 0000000..53e6078 --- /dev/null +++ b/coregrind/m_syswrap/syswrap-xen.c @@ -0,0 +1,117 @@ +#include "pub_core_basics.h" +#include "pub_core_vki.h" +#include "pub_core_vkiscnums.h" +#include "pub_core_threadstate.h" +#include "pub_core_aspacemgr.h" +#include "pub_core_debuginfo.h" // VG_(di_notify_*) +#include "pub_core_transtab.h" // VG_(discard_translations) +#include "pub_core_xarray.h" +#include "pub_core_clientstate.h" +#include "pub_core_debuglog.h" +#include "pub_core_libcbase.h" +#include "pub_core_libcassert.h" +#include "pub_core_libcfile.h" +#include "pub_core_libcprint.h" +#include "pub_core_libcproc.h" +#include "pub_core_libcsignal.h" +#include "pub_core_mallocfree.h" +#include "pub_core_tooliface.h" +#include "pub_core_options.h" +#include "pub_core_scheduler.h" +#include "pub_core_signals.h" +#include "pub_core_syscall.h" +#include "pub_core_syswrap.h" + +#include "priv_types_n_macros.h" +#include "priv_syswrap-generic.h" +#include "priv_syswrap-xen.h" + +#include <stdint.h> + +#define __XEN_TOOLS__ + +#include <xen/xen.h> +#include <xen/sysctl.h> + + +#define PRE(name) DEFN_PRE_TEMPLATE(xen, name) +#define POST(name) DEFN_POST_TEMPLATE(xen, name) + +PRE(ioctl_privcmd_hypercall) +{ + struct vki_xen_privcmd_hypercall *args = (struct vki_xen_privcmd_hypercall *)(ARG3); + + if (!args) + return; + + + switch (args->op) { + case __HYPERVISOR_sysctl: { + struct xen_sysctl *sysctl = (struct xen_sysctl *)(unsigned int)args->arg[0]; + + PRINT("__HYPERVISOR_sysctl ( %d )", sysctl->cmd); + + /* Single argument hypercall */ + PRE_MEM_READ("hypercall", ARG3, 8 + ( 8 * 1 ) ); + + /* Common part of xen_sysctl */ + PRE_MEM_READ("__HYPERVISOR_sysctl", args->arg[0], ( 4 + 4 )); + + if (!sysctl || sysctl->interface_version != XEN_SYSCTL_INTERFACE_VERSION) + /* BUG ? */ + return; + + //PRE_REG_READ1(long, "__HYPERVISOR_sysctl",); + switch (sysctl->cmd) { + case XEN_SYSCTL_getdomaininfolist: + PRE_MEM_READ("__HYPERVISOR_sysctl", &sysctl->u.getdomaininfolist.first_domain, sizeof(domid_t)); + PRE_MEM_READ("__HYPERVISOR_sysctl", &sysctl->u.getdomaininfolist.max_domains, sizeof(uint32_t)); + PRE_MEM_READ("__HYPERVISOR_sysctl", &sysctl->u.getdomaininfolist.buffer, sizeof(&sysctl->u.getdomaininfolist.buffer)); + break; + + default: + VG_(printf)("pre sysctl version %x unknown cmd %d\n", + sysctl->interface_version, sysctl->cmd); + break; + } + } + break; + + default: + VG_(printf)("pre unknown hypercall %lld ( 0x%#llx, 0x%#llx, 0x%#llx, 0x%#llx, 0x%#llx )\n", + args->op, args->arg[0], args->arg[1], args->arg[2], args->arg[3], args->arg[4]); + } +} + +POST(ioctl_privcmd_hypercall) +{ + struct vki_xen_privcmd_hypercall *args = (struct vki_xen_privcmd_hypercall *)(ARG3); + + if (!args) + return; + + switch (args->op) { + case __HYPERVISOR_sysctl: { + struct xen_sysctl *sysctl = (struct xen_sysctl *)(unsigned int)args->arg[0]; + + if (!sysctl || sysctl->interface_version != XEN_SYSCTL_INTERFACE_VERSION) + return; + + switch (sysctl->cmd) { + case XEN_SYSCTL_getdomaininfolist: + POST_MEM_WRITE(&sysctl->u.getdomaininfolist.num_domains, sizeof(sysctl->u.getdomaininfolist.num_domains)); + POST_MEM_WRITE(sysctl->u.getdomaininfolist.buffer.p, sizeof(xen_domctl_getdomaininfo_t) * sysctl->u.getdomaininfolist.num_domains); + break; + default: + VG_(printf)("post sysctl version %x cmd %d\n", + sysctl->interface_version, sysctl->cmd); + break; + } + break; + } + default: + VG_(printf)("post unknown hypercall %lld ( 0x%#llx, 0x%#llx, 0x%#llx, 0x%#llx, 0x%#llx )\n", + args->op, args->arg[0], args->arg[1], args->arg[2], args->arg[3], args->arg[4]); + break; + } +} diff --git a/include/Makefile.am b/include/Makefile.am index 33d0857..22bffa7 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -54,7 +54,8 @@ nobase_pkginclude_HEADERS = \ vki/vki-scnums-ppc64-linux.h \ vki/vki-scnums-x86-linux.h \ vki/vki-scnums-arm-linux.h \ - vki/vki-scnums-darwin.h + vki/vki-scnums-darwin.h + vki/vki-xen.h noinst_HEADERS = \ vki/vki-ppc32-aix5.h \ diff --git a/include/pub_tool_vki.h b/include/pub_tool_vki.h index 73a4174..c4c117f 100644 --- a/include/pub_tool_vki.h +++ b/include/pub_tool_vki.h @@ -47,6 +47,7 @@ #if defined(VGO_linux) # include "vki/vki-linux.h" +# include "vki/vki-xen.h" #elif defined(VGP_ppc32_aix5) # include "vki/vki-ppc32-aix5.h" #elif defined(VGP_ppc64_aix5) diff --git a/include/vki/vki-linux.h b/include/vki/vki-linux.h index beff378..86126ea 100644 --- a/include/vki/vki-linux.h +++ b/include/vki/vki-linux.h @@ -2709,6 +2709,17 @@ struct vki_getcpu_cache { #define VKI_EV_MAX 0x1f #define VKI_EV_CNT (VKI_EV_MAX+1) +//---------------------------------------------------------------------- +// Xen privcmd IOCTL +//---------------------------------------------------------------------- + +struct vki_xen_privcmd_hypercall { + __vki_u64 op; + __vki_u64 arg[5]; +}; + +#define VKI_XEN_IOCTL_PRIVCMD_HYPERCALL _VKI_IOC(_VKI_IOC_NONE, ''P'', 0, sizeof(struct vki_xen_privcmd_hypercall)) + #endif // __VKI_LINUX_H /*--------------------------------------------------------------------*/ diff --git a/include/vki/vki-xen.h b/include/vki/vki-xen.h new file mode 100644 index 0000000..7842cc9 --- /dev/null +++ b/include/vki/vki-xen.h @@ -0,0 +1,8 @@ +#ifndef __VKI_XEN_H +#define __VKI_XEN_H + +#endif // __VKI_XEN_H + +/*--------------------------------------------------------------------*/ +/*--- end ---*/ +/*--------------------------------------------------------------------*/ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2010-Jul-30 14:21 UTC
[Xen-devel] Re: [PATCH 0 of 3] xl: free allocations made at top level
Ian Campbell writes ("[Xen-devel] Re: [PATCH 0 of 3] xl: free allocations
made at top level"):> This seems to work. I''m not entirely sure about it though --
it''s not
> clear what happens if the thread which calls xc_interface_close is not
> the final thread to exit and whether we would still leak the buffer from
> the thread which does the final exit() call.
Yers. We don''t impose a requirement that the thread that calls
xc_interface_open has to be the one which calls _close, so you can
still leak the buffer from the original thread.
OTOH if you don''t want memory leaks you do need to do
xc_interface_close. So perhaps we should have a reference count or
something ? Is it even possible to destroy thread-specific data for
another thread ?
Also, while I was reading this code I noticed this:
pthread_once(&hcall_buf_pkey_once, _xc_init_hcall_buf);
and:
static void _xc_init_hcall_buf(void)
{
pthread_key_create(&hcall_buf_pkey, _xc_clean_hcall_buf);
}
It seems to me that this is possibly racy, if two threads call
hcall_buf_prep simultaneously for the first time (which is not at all
implausible in some possible scenarios). pthread_once doesn''t seem to
guarantee that the first two calls arrive at once, the 2nd call
doesn''t return before the first call has even entered the user
function. So the thread which loses the race can read an
uninitialised hcall_buf_pkey.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Campbell
2010-Jul-30 14:48 UTC
Re: [Xen-devel] Re: [PATCH 0 of 3] xl: free allocations made at top level
On Fri, 2010-07-30 at 15:21 +0100, Ian Jackson wrote:> Ian Campbell writes ("[Xen-devel] Re: [PATCH 0 of 3] xl: free allocations made at top level"): > > This seems to work. I''m not entirely sure about it though -- it''s not > > clear what happens if the thread which calls xc_interface_close is not > > the final thread to exit and whether we would still leak the buffer from > > the thread which does the final exit() call. > > Yers. We don''t impose a requirement that the thread that calls > xc_interface_open has to be the one which calls _close, so you can > still leak the buffer from the original thread.Unless that exists with pthread_exit, I suppose. TBH I think we needn''t worry too much about this particular leak -- for threads which do come and go they data will be freed, and for the last thread which exists it''s not really a worry. The patch is enough to make a single threaded user like xl leak free which is useful for the purposes of spotting any more serious leaks.> OTOH if you don''t want memory leaks you do need to do > xc_interface_close. So perhaps we should have a reference count or > something ?As long as the handle is either opened once and closed on exit or each thread opens and closes its own handle I think everything is ok.> Is it even possible to destroy thread-specific data for > another thread ?I don''t think so, but I don''t think its a concern in this case since all threads will either clean up after themselves when they are done or the whole process is exiting anyway. It''s also fine to free the thread-specific data too eagerly, worst case the next attempts at a hypercall will have to reallocate it.> Also, while I was reading this code I noticed this: > > pthread_once(&hcall_buf_pkey_once, _xc_init_hcall_buf); > > and: > > static void _xc_init_hcall_buf(void) > { > pthread_key_create(&hcall_buf_pkey, _xc_clean_hcall_buf); > } > > It seems to me that this is possibly racy, if two threads call > hcall_buf_prep simultaneously for the first time (which is not at all > implausible in some possible scenarios). pthread_once doesn''t seem to > guarantee that the first two calls arrive at once, the 2nd call > doesn''t return before the first call has even entered the user > function. So the thread which loses the race can read an > uninitialised hcall_buf_pkey.Looking at the glibc source code it looks like the loser of the race will wait, judging from the comment "/* Somebody else got here first. Wait. */" and the subsequent sys_futex call in the assembly (without studying it too hard). I''m not sure pthread_once would be at all useful without these semantics. I guess that doesn''t mean those aren''t the specified semantics though ;-) Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2010-Jul-30 14:53 UTC
Re: [Xen-devel] Re: [PATCH 0 of 3] xl: free allocations made at top level
Ian Campbell writes ("Re: [Xen-devel] Re: [PATCH 0 of 3] xl: free
allocations made at top level"):> The patch is enough to make a single threaded user like xl leak free
> which is useful for the purposes of spotting any more serious leaks.
I think this is the right way to think about it, so I guess we should
apply your patch. But I''ll wait for an opinion from Stefano in case
he has a different perspective.
So:
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> I''m not sure pthread_once would be at all useful without these
> semantics. I guess that doesn''t mean those aren''t the
specified
> semantics though ;-)
Quite :-). But in fact SuSv3 says
| On return from pthread_once(), init_routine shall have completed
So it''s OK.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Jul-30 14:57 UTC
Re: [Xen-devel] Re: [PATCH 0 of 3] xl: free allocations made at top level
On Fri, 30 Jul 2010, Ian Jackson wrote:> Ian Campbell writes ("Re: [Xen-devel] Re: [PATCH 0 of 3] xl: free allocations made at top level"): > > The patch is enough to make a single threaded user like xl leak free > > which is useful for the purposes of spotting any more serious leaks. > > I think this is the right way to think about it, so I guess we should > apply your patch. But I''ll wait for an opinion from Stefano in case > he has a different perspective. > > So: > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>I think this patch series should be applied. Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel