Roger Pau Monne
2012-Apr-24 18:05 UTC
[PATCH v2] libxl: prevent xl from running if xend is running.
Prevent xl from doing any operation if xend daemon is running. That
prevents bugs that happened when xl and xend raced to close a domain.
Changes since v1:
* Add documentation to xl man page.
* Permit the execution of commands that don''t modify anything.
* Indent error message.
Cc: george.dunlap@eu.citrix.com
Cc: ian.jackson@eu.citrix.com
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
docs/man/xl.pod.1 | 6 ++
tools/libxl/xl.c | 22 +++++++-
tools/libxl/xl.h | 1 +
tools/libxl/xl_cmdimpl.c | 4 +-
tools/libxl/xl_cmdtable.c | 132 ++++++++++++++++++++++----------------------
5 files changed, 96 insertions(+), 69 deletions(-)
diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index e5324fb..e829697 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -75,6 +75,12 @@ Verbose.
Dry run: do not actually execute the command.
+=item B<-f>
+
+Force execution: xl will refuse to run some commands if it detects that xend is
+also running, this option will force the execution of those commands, even
+though it is unsafe.
+
=back
=head1 DOMAIN SUBCOMMANDS
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 2b14814..720bb66 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -32,8 +32,11 @@
#include "libxlutil.h"
#include "xl.h"
+#define XEND_LOCK { "/var/lock/subsys/xend",
"/var/lock/xend" }
+
xentoollog_logger_stdiostream *logger;
int dryrun_only;
+int force_execution;
int autoballoon = 1;
char *lockfile;
char *default_vifscript = NULL;
@@ -103,8 +106,9 @@ int main(int argc, char **argv)
char *config_file;
void *config_data = 0;
int config_len = 0;
+ const char *locks[] = XEND_LOCK;
- while ((opt = getopt(argc, argv, "+vN")) >= 0) {
+ while ((opt = getopt(argc, argv, "+vfN")) >= 0) {
switch (opt) {
case ''v'':
if (minmsglevel > 0) minmsglevel--;
@@ -112,6 +116,9 @@ int main(int argc, char **argv)
case ''N'':
dryrun_only = 1;
break;
+ case ''f'':
+ force_execution = 1;
+ break;
default:
fprintf(stderr, "unknown global option\n");
exit(2);
@@ -162,6 +169,19 @@ int main(int argc, char **argv)
ret = 1;
goto xit;
}
+ if (cspec->modifies) {
+ for (int i = 0; i < sizeof(locks)/sizeof(locks[0]); i++) {
+ if (!access(locks[i], F_OK) && !force_execution) {
+ fprintf(stderr,
+"xend is running, which prevents xl from working correctly.\n"
+"If you still want to force the execution of xl please use the -f\n"
+"option.\n"
+ );
+ ret = 1;
+ goto xit;
+ }
+ }
+ }
ret = cspec->cmd_impl(argc, argv);
} else if (!strcmp(cmd, "help")) {
help(argv[1]);
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 0a3d628..5ddd2da 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -21,6 +21,7 @@ struct cmd_spec {
char *cmd_name;
int (*cmd_impl)(int argc, char **argv);
int can_dryrun;
+ int modifies;
char *cmd_desc;
char *cmd_usage;
char *cmd_option;
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 6f4dd09..65bc6d6 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1909,7 +1909,7 @@ void help(const char *command)
struct cmd_spec *cmd;
if (!command || !strcmp(command, "help")) {
- printf("Usage xl [-vN] <subcommand> [args]\n\n");
+ printf("Usage xl [-vfN] <subcommand> [args]\n\n");
printf("xl full list of subcommands:\n\n");
for (i = 0; i < cmdtable_len; i++) {
printf(" %-19s ", cmd_table[i].cmd_name);
@@ -1920,7 +1920,7 @@ void help(const char *command)
} else {
cmd = cmdtable_lookup(command);
if (cmd) {
- printf("Usage: xl [-v%s] %s %s\n\n%s.\n\n",
+ printf("Usage: xl [-vf%s] %s %s\n\n%s.\n\n",
cmd->can_dryrun ? "N" : "",
cmd->cmd_name,
cmd->cmd_usage,
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index f461a2a..5509126 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -19,7 +19,7 @@
struct cmd_spec cmd_table[] = {
{ "create",
- &main_create, 1,
+ &main_create, 1, 1,
"Create a domain from config file <filename>",
"<ConfigFile> [options] [vars]",
"-h Print this help.\n"
@@ -33,7 +33,7 @@ struct cmd_spec cmd_table[] = {
"-e Do not wait in the background for the death
of the domain."
},
{ "config-update",
- &main_config_update, 1,
+ &main_config_update, 1, 1,
"Update a running domain''s saved configuration, used when
rebuilding "
"the domain after reboot",
"<Domain> <ConfigFile> [options] [vars]",
@@ -42,7 +42,7 @@ struct cmd_spec cmd_table[] = {
"-d Enable debug messages.\n"
},
{ "list",
- &main_list, 0,
+ &main_list, 0, 0,
"List information about all/some domains",
"[options] [Domain]\n",
"-l, --long Output all VM details\n"
@@ -50,12 +50,12 @@ struct cmd_spec cmd_table[] = {
"-Z, --context Prints out security context"
},
{ "destroy",
- &main_destroy, 0,
+ &main_destroy, 0, 1,
"Terminate a domain immediately",
"<Domain>",
},
{ "shutdown",
- &main_shutdown, 0,
+ &main_shutdown, 0, 1,
"Issue a shutdown signal to a domain",
"[options] <Domain>",
"-h Print this help.\n"
@@ -64,7 +64,7 @@ struct cmd_spec cmd_table[] = {
"-w Wait for guest to shutdown.\n"
},
{ "reboot",
- &main_reboot, 0,
+ &main_reboot, 0, 1,
"Issue a reboot signal to a domain",
"[options] <Domain>",
"-h Print this help.\n"
@@ -72,44 +72,44 @@ struct cmd_spec cmd_table[] = {
" no PV drivers.\n"
},
{ "pci-attach",
- &main_pciattach, 0,
+ &main_pciattach, 0, 1,
"Insert a new pass-through pci device",
"<Domain> <BDF> [Virtual Slot]",
},
{ "pci-detach",
- &main_pcidetach, 0,
+ &main_pcidetach, 0, 1,
"Remove a domain''s pass-through pci device",
"<Domain> <BDF>",
},
{ "pci-list",
- &main_pcilist, 0,
+ &main_pcilist, 0, 0,
"List pass-through pci devices for a domain",
"<Domain>",
},
{ "pci-list-assignable-devices",
- &main_pcilist_assignable, 0,
+ &main_pcilist_assignable, 0, 0,
"List all the assignable pci devices",
"",
},
{ "pause",
- &main_pause, 0,
+ &main_pause, 0, 1,
"Pause execution of a domain",
"<Domain>",
},
{ "unpause",
- &main_unpause, 0,
+ &main_unpause, 0, 1,
"Unpause a paused domain",
"<Domain>",
},
{ "console",
- &main_console, 0,
+ &main_console, 0, 0,
"Attach to domain''s console",
"[options] <Domain>\n"
"-t <type> console type, pv or serial\n"
"-n <number> console number"
},
{ "vncviewer",
- &main_vncviewer, 0,
+ &main_vncviewer, 0, 0,
"Attach to domain''s VNC server.",
"[options] <Domain>\n"
"--autopass Pass VNC password to viewer via stdin
and\n"
@@ -117,14 +117,14 @@ struct cmd_spec cmd_table[] = {
"--vncviewer-autopass (consistency alias for --autopass)"
},
{ "save",
- &main_save, 0,
+ &main_save, 0, 1,
"Save a domain state to restore later",
"[options] <Domain> <CheckpointFile>
[<ConfigFile>]",
"-h Print this help.\n"
"-c Leave domain running after creating the snapshot."
},
{ "migrate",
- &main_migrate, 0,
+ &main_migrate, 0, 1,
"Save a domain state to restore later",
"[options] <Domain> <host>",
"-h Print this help.\n"
@@ -136,12 +136,12 @@ struct cmd_spec cmd_table[] = {
" of the domain."
},
{ "dump-core",
- &main_dump_core, 0,
+ &main_dump_core, 0, 1,
"Core dump a domain",
"<Domain> <filename>"
},
{ "restore",
- &main_restore, 0,
+ &main_restore, 0, 1,
"Restore a domain from a saved state",
"[options] [<ConfigFile>] <CheckpointFile>",
"-h Print this help.\n"
@@ -150,68 +150,68 @@ struct cmd_spec cmd_table[] = {
"-d Enable debug messages."
},
{ "migrate-receive",
- &main_migrate_receive, 0,
+ &main_migrate_receive, 0, 1,
"Restore a domain from a saved state",
"- for internal use only",
},
{ "cd-insert",
- &main_cd_insert, 0,
+ &main_cd_insert, 0, 1,
"Insert a cdrom into a guest''s cd drive",
"<Domain> <VirtualDevice> <type:path>",
},
{ "cd-eject",
- &main_cd_eject, 0,
+ &main_cd_eject, 0, 1,
"Eject a cdrom from a guest''s cd drive",
"<Domain> <VirtualDevice>",
},
{ "mem-max",
- &main_memmax, 0,
+ &main_memmax, 0, 1,
"Set the maximum amount reservation for a domain",
"<Domain>
<MemMB[''b''[bytes]|''k''[KB]|''m''[MB]|''g''[GB]|''t''[TB]]>",
},
{ "mem-set",
- &main_memset, 0,
+ &main_memset, 0, 1,
"Set the current memory usage for a domain",
"<Domain>
<MemMB[''b''[bytes]|''k''[KB]|''m''[MB]|''g''[GB]|''t''[TB]]>",
},
{ "button-press",
- &main_button_press, 0,
+ &main_button_press, 0, 1,
"Indicate an ACPI button press to the domain",
"<Domain> <Button>",
"<Button> may be ''power'' or
''sleep''."
},
{ "vcpu-list",
- &main_vcpulist, 0,
+ &main_vcpulist, 0, 0,
"List the VCPUs for all/some domains",
"[Domain, ...]",
},
{ "vcpu-pin",
- &main_vcpupin, 0,
+ &main_vcpupin, 0, 1,
"Set which CPUs a VCPU can use",
"<Domain> <VCPU|all> <CPUs|all>",
},
{ "vcpu-set",
- &main_vcpuset, 0,
+ &main_vcpuset, 0, 1,
"Set the number of active VCPUs allowed for the domain",
"<Domain> <vCPUs>",
},
{ "list-vm",
- &main_list_vm, 0,
+ &main_list_vm, 0, 0,
"List the VMs,without DOM0",
"",
},
{ "info",
- &main_info, 0,
+ &main_info, 0, 0,
"Get information about Xen host",
"-n, --numa List host NUMA topology information",
},
{ "sharing",
- &main_sharing, 0,
+ &main_sharing, 0, 0,
"Get information about page sharing",
"[Domain]",
},
{ "sched-credit",
- &main_sched_credit, 0,
+ &main_sched_credit, 0, 1,
"Get/set credit scheduler parameters",
"[-d <Domain> [-w[=WEIGHT]|-c[=CAP]]] [-s [-t TSLICE] [-r
RATELIMIT]] [-p CPUPOOL]",
"-d DOMAIN, --domain=DOMAIN Domain to modify\n"
@@ -223,7 +223,7 @@ struct cmd_spec cmd_table[] = {
"-p CPUPOOL, --cpupool=CPUPOOL Restrict output to CPUPOOL"
},
{ "sched-credit2",
- &main_sched_credit2, 0,
+ &main_sched_credit2, 0, 1,
"Get/set credit2 scheduler parameters",
"[-d <Domain> [-w[=WEIGHT]]] [-p CPUPOOL]",
"-d DOMAIN, --domain=DOMAIN Domain to modify\n"
@@ -231,7 +231,7 @@ struct cmd_spec cmd_table[] = {
"-p CPUPOOL, --cpupool=CPUPOOL Restrict output to CPUPOOL"
},
{ "sched-sedf",
- &main_sched_sedf, 0,
+ &main_sched_sedf, 0, 1,
"Get/set sedf scheduler parameters",
"[options]",
"-d DOMAIN, --domain=DOMAIN Domain to modify\n"
@@ -247,109 +247,109 @@ struct cmd_spec cmd_table[] = {
"-c CPUPOOL, --cpupool=CPUPOOL Restrict output to CPUPOOL"
},
{ "domid",
- &main_domid, 0,
+ &main_domid, 0, 0,
"Convert a domain name to domain id",
"<DomainName>",
},
{ "domname",
- &main_domname, 0,
+ &main_domname, 0, 0,
"Convert a domain id to domain name",
"<DomainId>",
},
{ "rename",
- &main_rename, 0,
+ &main_rename, 0, 1,
"Rename a domain",
"<Domain> <NewDomainName>",
},
{ "trigger",
- &main_trigger, 0,
+ &main_trigger, 0, 1,
"Send a trigger to a domain",
"<Domain> <nmi|reset|init|power|sleep|s3resume>
[<VCPU>]",
},
{ "sysrq",
- &main_sysrq, 0,
+ &main_sysrq, 0, 1,
"Send a sysrq to a domain",
"<Domain> <letter>",
},
{ "debug-keys",
- &main_debug_keys, 0,
+ &main_debug_keys, 0, 1,
"Send debug keys to Xen",
"<Keys>",
},
{ "dmesg",
- &main_dmesg, 0,
+ &main_dmesg, 0, 0,
"Read and/or clear dmesg buffer",
"[-c]",
" -c Clear dmesg buffer as well as printing
it",
},
{ "top",
- &main_top, 0,
+ &main_top, 0, 0,
"Monitor a host and the domains in real time",
"",
},
{ "network-attach",
- &main_networkattach, 0,
+ &main_networkattach, 0, 1,
"Create a new virtual network device",
"<Domain> [type=<type>] [mac=<mac>]
[bridge=<bridge>] "
"[ip=<ip>] [script=<script>]
[backend=<BackDomain>] [vifname=<name>] "
"[rate=<rate>] [model=<model>]
[accel=<accel>]",
},
{ "network-list",
- &main_networklist, 0,
+ &main_networklist, 0, 0,
"List virtual network interfaces for a domain",
"<Domain(s)>",
},
{ "network-detach",
- &main_networkdetach, 0,
+ &main_networkdetach, 0, 1,
"Destroy a domain''s virtual network device",
"<Domain> <DevId|mac>",
},
{ "block-attach",
- &main_blockattach, 1,
+ &main_blockattach, 1, 1,
"Create a new virtual block device",
"<Domain> <disk-spec-component(s)>...",
},
{ "block-list",
- &main_blocklist, 0,
+ &main_blocklist, 0, 0,
"List virtual block devices for a domain",
"<Domain(s)>",
},
{ "block-detach",
- &main_blockdetach, 0,
+ &main_blockdetach, 0, 1,
"Destroy a domain''s virtual block device",
"<Domain> <DevId>",
},
{ "uptime",
- &main_uptime, 0,
+ &main_uptime, 0, 0,
"Print uptime for all/some domains",
"[-s] [Domain]",
},
{ "tmem-list",
- &main_tmem_list, 0,
+ &main_tmem_list, 0, 0,
"List tmem pools",
"[-l] [<Domain>|-a]",
" -l List tmem stats",
},
{ "tmem-freeze",
- &main_tmem_freeze, 0,
+ &main_tmem_freeze, 0, 1,
"Freeze tmem pools",
"[<Domain>|-a]",
" -a Freeze all tmem",
},
{ "tmem-destroy",
- &main_tmem_destroy, 0,
+ &main_tmem_destroy, 0, 1,
"Destroy tmem pools",
"[<Domain>|-a]",
" -a Destroy all tmem",
},
{ "tmem-thaw",
- &main_tmem_thaw, 0,
+ &main_tmem_thaw, 0, 1,
"Thaw tmem pools",
"[<Domain>|-a]",
" -a Thaw all tmem",
},
{ "tmem-set",
- &main_tmem_set, 0,
+ &main_tmem_set, 0, 1,
"Change tmem settings",
"[<Domain>|-a] [-w[=WEIGHT]|-c[=CAP]|-p[=COMPRESS]]",
" -a Operate on all tmem\n"
@@ -358,7 +358,7 @@ struct cmd_spec cmd_table[] = {
" -p COMPRESS Compress (int)",
},
{ "tmem-shared-auth",
- &main_tmem_shared_auth, 0,
+ &main_tmem_shared_auth, 0, 1,
"De/authenticate shared tmem pool",
"[<Domain>|-a] [-u[=UUID] [-A[=AUTH]",
" -a Authenticate for all tmem
pools\n"
@@ -367,12 +367,12 @@ struct cmd_spec cmd_table[] = {
" -A AUTH 0=auth,1=deauth",
},
{ "tmem-freeable",
- &main_tmem_freeable, 0,
+ &main_tmem_freeable, 0, 0,
"Get information about how much freeable memory (MB) is in-use by
tmem",
"",
},
{ "cpupool-create",
- &main_cpupoolcreate, 1,
+ &main_cpupoolcreate, 1, 1,
"Create a CPU pool based an ConfigFile",
"[options] <ConfigFile> [vars]",
"-h, --help Print this help.\n"
@@ -381,53 +381,53 @@ struct cmd_spec cmd_table[] = {
" (deprecated in favour of global -N
option)."
},
{ "cpupool-list",
- &main_cpupoollist, 0,
+ &main_cpupoollist, 0, 0,
"List CPU pools on host",
"[-c|--cpus] [<CPU Pool>]",
"-c, --cpus Output list of CPUs used by a
pool"
},
{ "cpupool-destroy",
- &main_cpupooldestroy, 0,
+ &main_cpupooldestroy, 0, 1,
"Deactivates a CPU pool",
"<CPU Pool>",
},
{ "cpupool-rename",
- &main_cpupoolrename, 0,
+ &main_cpupoolrename, 0, 1,
"Renames a CPU pool",
"<CPU Pool> <new name>",
},
{ "cpupool-cpu-add",
- &main_cpupoolcpuadd, 0,
+ &main_cpupoolcpuadd, 0, 1,
"Adds a CPU to a CPU pool",
"<CPU Pool> <CPU nr>|node:<node nr>",
},
{ "cpupool-cpu-remove",
- &main_cpupoolcpuremove, 0,
+ &main_cpupoolcpuremove, 0, 1,
"Removes a CPU from a CPU pool",
"<CPU Pool> <CPU nr>|node:<node nr>",
},
{ "cpupool-migrate",
- &main_cpupoolmigrate, 0,
+ &main_cpupoolmigrate, 0, 1,
"Moves a domain into a CPU pool",
"<Domain> <CPU Pool>",
},
{ "cpupool-numa-split",
- &main_cpupoolnumasplit, 0,
+ &main_cpupoolnumasplit, 0, 1,
"Splits up the machine into one CPU pool per NUMA node",
"",
},
{ "getenforce",
- &main_getenforce, 0,
+ &main_getenforce, 0, 0,
"Returns the current enforcing mode of the Flask Xen security
module",
"",
},
{ "setenforce",
- &main_setenforce, 0,
+ &main_setenforce, 0, 1,
"Sets the current enforcing mode of the Flask Xen security
module",
"<1|0|Enforcing|Permissive>",
},
{ "loadpolicy",
- &main_loadpolicy, 0,
+ &main_loadpolicy, 0, 1,
"Loads a new policy int the Flask Xen security module",
"<policy file>",
},
--
1.7.7.5 (Apple Git-26)
Ian Jackson
2012-Apr-24 18:08 UTC
Re: [PATCH v2] libxl: prevent xl from running if xend is running.
Roger Pau Monne writes ("[PATCH v2] libxl: prevent xl from running if xend
is running."):> Prevent xl from doing any operation if xend daemon is running. That
> prevents bugs that happened when xl and xend raced to close a domain.
Thanks, but I''m afraid this needs a refresh. I recommend you wait
until tomorrow. I''m nearly at the end of my huge queue...
Ian.
Roger Pau Monne
2012-Apr-25 08:36 UTC
Re: [PATCH v2] libxl: prevent xl from running if xend is running.
Ian Jackson escribió:> Roger Pau Monne writes ("[PATCH v2] libxl: prevent xl from running if xend is running."): >> Prevent xl from doing any operation if xend daemon is running. That >> prevents bugs that happened when xl and xend raced to close a domain. > > Thanks, but I''m afraid this needs a refresh. I recommend you wait > until tomorrow. I''m nearly at the end of my huge queue...Do you mean that the code doesn''t apply cleanly, or a refresh in the commit message? In the lists of changes I''ve added the fact that now you are able to run commands that doesn''t modify anything. Thanks, Roger.
Ian Campbell
2012-Apr-25 09:22 UTC
Re: [PATCH v2] libxl: prevent xl from running if xend is running.
On Tue, 2012-04-24 at 19:05 +0100, Roger Pau Monne wrote:> Prevent xl from doing any operation if xend daemon is running. That > prevents bugs that happened when xl and xend raced to close a domain. > > Changes since v1: > > * Add documentation to xl man page. > > * Permit the execution of commands that don''t modify anything. > > * Indent error message. > > Cc: george.dunlap@eu.citrix.com > Cc: ian.jackson@eu.citrix.com > Signed-off-by: Roger Pau Monne <roger.pau@citrix.com> > --- > docs/man/xl.pod.1 | 6 ++ > tools/libxl/xl.c | 22 +++++++- > tools/libxl/xl.h | 1 + > tools/libxl/xl_cmdimpl.c | 4 +- > tools/libxl/xl_cmdtable.c | 132 ++++++++++++++++++++++---------------------- > 5 files changed, 96 insertions(+), 69 deletions(-) > > diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1 > index e5324fb..e829697 100644 > --- a/docs/man/xl.pod.1 > +++ b/docs/man/xl.pod.1 > @@ -75,6 +75,12 @@ Verbose. > > Dry run: do not actually execute the command. > > +=item B<-f> > + > +Force execution: xl will refuse to run some commands if it detects that xend is > +also running, this option will force the execution of those commands, even > +though it is unsafe. > + > =back > > =head1 DOMAIN SUBCOMMANDS > diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c > index 2b14814..720bb66 100644 > --- a/tools/libxl/xl.c > +++ b/tools/libxl/xl.c > @@ -32,8 +32,11 @@ > #include "libxlutil.h" > #include "xl.h" > > +#define XEND_LOCK { "/var/lock/subsys/xend", "/var/lock/xend" } > + > xentoollog_logger_stdiostream *logger; > int dryrun_only; > +int force_execution; > int autoballoon = 1; > char *lockfile; > char *default_vifscript = NULL; > @@ -103,8 +106,9 @@ int main(int argc, char **argv) > char *config_file; > void *config_data = 0; > int config_len = 0; > + const char *locks[] = XEND_LOCK; > > - while ((opt = getopt(argc, argv, "+vN")) >= 0) { > + while ((opt = getopt(argc, argv, "+vfN")) >= 0) { > switch (opt) { > case ''v'': > if (minmsglevel > 0) minmsglevel--; > @@ -112,6 +116,9 @@ int main(int argc, char **argv) > case ''N'': > dryrun_only = 1; > break; > + case ''f'': > + force_execution = 1; > + break; > default: > fprintf(stderr, "unknown global option\n"); > exit(2); > @@ -162,6 +169,19 @@ int main(int argc, char **argv) > ret = 1; > goto xit; > } > + if (cspec->modifies) {Need to handle -N (dry-run) too? That will turn a modifying command into a non-modifying one. Do we now have 3 classes of commands which could be represented with an enum rather than a pair of bools? * Purely introspective commands, only query data, do not change any state, -N and -f mean nothing. * Commands which modify state, but which have a "dry-run" mode. -N activates dry-run, -f only means something if !dry-run * Commands which modify state, but which do not support a dry-run mode, -N is not available, -f does what you''d expect.> + for (int i = 0; i < sizeof(locks)/sizeof(locks[0]); i++) { > + if (!access(locks[i], F_OK) && !force_execution) { > + fprintf(stderr, > +"xend is running, which prevents xl from working correctly.\n" > +"If you still want to force the execution of xl please use the -f\n" > +"option.\n" > + ); > + ret = 1; > + goto xit; > + } > + } > + } > ret = cspec->cmd_impl(argc, argv); > } else if (!strcmp(cmd, "help")) { > help(argv[1]); > diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h > index 0a3d628..5ddd2da 100644 > --- a/tools/libxl/xl.h > +++ b/tools/libxl/xl.h > @@ -21,6 +21,7 @@ struct cmd_spec { > char *cmd_name; > int (*cmd_impl)(int argc, char **argv); > int can_dryrun; > + int modifies; > char *cmd_desc; > char *cmd_usage; > char *cmd_option; > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 6f4dd09..65bc6d6 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -1909,7 +1909,7 @@ void help(const char *command) > struct cmd_spec *cmd; > > if (!command || !strcmp(command, "help")) { > - printf("Usage xl [-vN] <subcommand> [args]\n\n"); > + printf("Usage xl [-vfN] <subcommand> [args]\n\n"); > printf("xl full list of subcommands:\n\n"); > for (i = 0; i < cmdtable_len; i++) { > printf(" %-19s ", cmd_table[i].cmd_name); > @@ -1920,7 +1920,7 @@ void help(const char *command) > } else { > cmd = cmdtable_lookup(command); > if (cmd) { > - printf("Usage: xl [-v%s] %s %s\n\n%s.\n\n", > + printf("Usage: xl [-vf%s] %s %s\n\n%s.\n\n", > cmd->can_dryrun ? "N" : "", > cmd->cmd_name, > cmd->cmd_usage, > diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c > index f461a2a..5509126 100644 > --- a/tools/libxl/xl_cmdtable.c > +++ b/tools/libxl/xl_cmdtable.c > @@ -19,7 +19,7 @@ > > struct cmd_spec cmd_table[] = { > { "create", > - &main_create, 1, > + &main_create, 1, 1, > "Create a domain from config file <filename>", > "<ConfigFile> [options] [vars]", > "-h Print this help.\n" > @@ -33,7 +33,7 @@ struct cmd_spec cmd_table[] = { > "-e Do not wait in the background for the death of the domain." > }, > { "config-update", > - &main_config_update, 1, > + &main_config_update, 1, 1, > "Update a running domain''s saved configuration, used when rebuilding " > "the domain after reboot", > "<Domain> <ConfigFile> [options] [vars]", > @@ -42,7 +42,7 @@ struct cmd_spec cmd_table[] = { > "-d Enable debug messages.\n" > }, > { "list", > - &main_list, 0, > + &main_list, 0, 0, > "List information about all/some domains", > "[options] [Domain]\n", > "-l, --long Output all VM details\n" > @@ -50,12 +50,12 @@ struct cmd_spec cmd_table[] = { > "-Z, --context Prints out security context" > }, > { "destroy", > - &main_destroy, 0, > + &main_destroy, 0, 1, > "Terminate a domain immediately", > "<Domain>", > }, > { "shutdown", > - &main_shutdown, 0, > + &main_shutdown, 0, 1, > "Issue a shutdown signal to a domain", > "[options] <Domain>", > "-h Print this help.\n" > @@ -64,7 +64,7 @@ struct cmd_spec cmd_table[] = { > "-w Wait for guest to shutdown.\n" > }, > { "reboot", > - &main_reboot, 0, > + &main_reboot, 0, 1, > "Issue a reboot signal to a domain", > "[options] <Domain>", > "-h Print this help.\n" > @@ -72,44 +72,44 @@ struct cmd_spec cmd_table[] = { > " no PV drivers.\n" > }, > { "pci-attach", > - &main_pciattach, 0, > + &main_pciattach, 0, 1, > "Insert a new pass-through pci device", > "<Domain> <BDF> [Virtual Slot]", > }, > { "pci-detach", > - &main_pcidetach, 0, > + &main_pcidetach, 0, 1, > "Remove a domain''s pass-through pci device", > "<Domain> <BDF>", > }, > { "pci-list", > - &main_pcilist, 0, > + &main_pcilist, 0, 0, > "List pass-through pci devices for a domain", > "<Domain>", > }, > { "pci-list-assignable-devices", > - &main_pcilist_assignable, 0, > + &main_pcilist_assignable, 0, 0, > "List all the assignable pci devices", > "", > }, > { "pause", > - &main_pause, 0, > + &main_pause, 0, 1, > "Pause execution of a domain", > "<Domain>", > }, > { "unpause", > - &main_unpause, 0, > + &main_unpause, 0, 1, > "Unpause a paused domain", > "<Domain>", > }, > { "console", > - &main_console, 0, > + &main_console, 0, 0, > "Attach to domain''s console", > "[options] <Domain>\n" > "-t <type> console type, pv or serial\n" > "-n <number> console number" > }, > { "vncviewer", > - &main_vncviewer, 0, > + &main_vncviewer, 0, 0, > "Attach to domain''s VNC server.", > "[options] <Domain>\n" > "--autopass Pass VNC password to viewer via stdin and\n" > @@ -117,14 +117,14 @@ struct cmd_spec cmd_table[] = { > "--vncviewer-autopass (consistency alias for --autopass)" > }, > { "save", > - &main_save, 0, > + &main_save, 0, 1, > "Save a domain state to restore later", > "[options] <Domain> <CheckpointFile> [<ConfigFile>]", > "-h Print this help.\n" > "-c Leave domain running after creating the snapshot." > }, > { "migrate", > - &main_migrate, 0, > + &main_migrate, 0, 1, > "Save a domain state to restore later", > "[options] <Domain> <host>", > "-h Print this help.\n" > @@ -136,12 +136,12 @@ struct cmd_spec cmd_table[] = { > " of the domain." > }, > { "dump-core", > - &main_dump_core, 0, > + &main_dump_core, 0, 1, > "Core dump a domain", > "<Domain> <filename>" > }, > { "restore", > - &main_restore, 0, > + &main_restore, 0, 1, > "Restore a domain from a saved state", > "[options] [<ConfigFile>] <CheckpointFile>", > "-h Print this help.\n" > @@ -150,68 +150,68 @@ struct cmd_spec cmd_table[] = { > "-d Enable debug messages." > }, > { "migrate-receive", > - &main_migrate_receive, 0, > + &main_migrate_receive, 0, 1, > "Restore a domain from a saved state", > "- for internal use only", > }, > { "cd-insert", > - &main_cd_insert, 0, > + &main_cd_insert, 0, 1, > "Insert a cdrom into a guest''s cd drive", > "<Domain> <VirtualDevice> <type:path>", > }, > { "cd-eject", > - &main_cd_eject, 0, > + &main_cd_eject, 0, 1, > "Eject a cdrom from a guest''s cd drive", > "<Domain> <VirtualDevice>", > }, > { "mem-max", > - &main_memmax, 0, > + &main_memmax, 0, 1, > "Set the maximum amount reservation for a domain", > "<Domain> <MemMB[''b''[bytes]|''k''[KB]|''m''[MB]|''g''[GB]|''t''[TB]]>", > }, > { "mem-set", > - &main_memset, 0, > + &main_memset, 0, 1, > "Set the current memory usage for a domain", > "<Domain> <MemMB[''b''[bytes]|''k''[KB]|''m''[MB]|''g''[GB]|''t''[TB]]>", > }, > { "button-press", > - &main_button_press, 0, > + &main_button_press, 0, 1, > "Indicate an ACPI button press to the domain", > "<Domain> <Button>", > "<Button> may be ''power'' or ''sleep''." > }, > { "vcpu-list", > - &main_vcpulist, 0, > + &main_vcpulist, 0, 0, > "List the VCPUs for all/some domains", > "[Domain, ...]", > }, > { "vcpu-pin", > - &main_vcpupin, 0, > + &main_vcpupin, 0, 1, > "Set which CPUs a VCPU can use", > "<Domain> <VCPU|all> <CPUs|all>", > }, > { "vcpu-set", > - &main_vcpuset, 0, > + &main_vcpuset, 0, 1, > "Set the number of active VCPUs allowed for the domain", > "<Domain> <vCPUs>", > }, > { "list-vm", > - &main_list_vm, 0, > + &main_list_vm, 0, 0, > "List the VMs,without DOM0", > "", > }, > { "info", > - &main_info, 0, > + &main_info, 0, 0, > "Get information about Xen host", > "-n, --numa List host NUMA topology information", > }, > { "sharing", > - &main_sharing, 0, > + &main_sharing, 0, 0, > "Get information about page sharing", > "[Domain]", > }, > { "sched-credit", > - &main_sched_credit, 0, > + &main_sched_credit, 0, 1, > "Get/set credit scheduler parameters", > "[-d <Domain> [-w[=WEIGHT]|-c[=CAP]]] [-s [-t TSLICE] [-r RATELIMIT]] [-p CPUPOOL]", > "-d DOMAIN, --domain=DOMAIN Domain to modify\n" > @@ -223,7 +223,7 @@ struct cmd_spec cmd_table[] = { > "-p CPUPOOL, --cpupool=CPUPOOL Restrict output to CPUPOOL" > }, > { "sched-credit2", > - &main_sched_credit2, 0, > + &main_sched_credit2, 0, 1, > "Get/set credit2 scheduler parameters", > "[-d <Domain> [-w[=WEIGHT]]] [-p CPUPOOL]", > "-d DOMAIN, --domain=DOMAIN Domain to modify\n" > @@ -231,7 +231,7 @@ struct cmd_spec cmd_table[] = { > "-p CPUPOOL, --cpupool=CPUPOOL Restrict output to CPUPOOL" > }, > { "sched-sedf", > - &main_sched_sedf, 0, > + &main_sched_sedf, 0, 1, > "Get/set sedf scheduler parameters", > "[options]", > "-d DOMAIN, --domain=DOMAIN Domain to modify\n" > @@ -247,109 +247,109 @@ struct cmd_spec cmd_table[] = { > "-c CPUPOOL, --cpupool=CPUPOOL Restrict output to CPUPOOL" > }, > { "domid", > - &main_domid, 0, > + &main_domid, 0, 0, > "Convert a domain name to domain id", > "<DomainName>", > }, > { "domname", > - &main_domname, 0, > + &main_domname, 0, 0, > "Convert a domain id to domain name", > "<DomainId>", > }, > { "rename", > - &main_rename, 0, > + &main_rename, 0, 1, > "Rename a domain", > "<Domain> <NewDomainName>", > }, > { "trigger", > - &main_trigger, 0, > + &main_trigger, 0, 1, > "Send a trigger to a domain", > "<Domain> <nmi|reset|init|power|sleep|s3resume> [<VCPU>]", > }, > { "sysrq", > - &main_sysrq, 0, > + &main_sysrq, 0, 1, > "Send a sysrq to a domain", > "<Domain> <letter>", > }, > { "debug-keys", > - &main_debug_keys, 0, > + &main_debug_keys, 0, 1, > "Send debug keys to Xen", > "<Keys>", > }, > { "dmesg", > - &main_dmesg, 0, > + &main_dmesg, 0, 0, > "Read and/or clear dmesg buffer", > "[-c]", > " -c Clear dmesg buffer as well as printing it", > }, > { "top", > - &main_top, 0, > + &main_top, 0, 0, > "Monitor a host and the domains in real time", > "", > }, > { "network-attach", > - &main_networkattach, 0, > + &main_networkattach, 0, 1, > "Create a new virtual network device", > "<Domain> [type=<type>] [mac=<mac>] [bridge=<bridge>] " > "[ip=<ip>] [script=<script>] [backend=<BackDomain>] [vifname=<name>] " > "[rate=<rate>] [model=<model>] [accel=<accel>]", > }, > { "network-list", > - &main_networklist, 0, > + &main_networklist, 0, 0, > "List virtual network interfaces for a domain", > "<Domain(s)>", > }, > { "network-detach", > - &main_networkdetach, 0, > + &main_networkdetach, 0, 1, > "Destroy a domain''s virtual network device", > "<Domain> <DevId|mac>", > }, > { "block-attach", > - &main_blockattach, 1, > + &main_blockattach, 1, 1, > "Create a new virtual block device", > "<Domain> <disk-spec-component(s)>...", > }, > { "block-list", > - &main_blocklist, 0, > + &main_blocklist, 0, 0, > "List virtual block devices for a domain", > "<Domain(s)>", > }, > { "block-detach", > - &main_blockdetach, 0, > + &main_blockdetach, 0, 1, > "Destroy a domain''s virtual block device", > "<Domain> <DevId>", > }, > { "uptime", > - &main_uptime, 0, > + &main_uptime, 0, 0, > "Print uptime for all/some domains", > "[-s] [Domain]", > }, > { "tmem-list", > - &main_tmem_list, 0, > + &main_tmem_list, 0, 0, > "List tmem pools", > "[-l] [<Domain>|-a]", > " -l List tmem stats", > }, > { "tmem-freeze", > - &main_tmem_freeze, 0, > + &main_tmem_freeze, 0, 1, > "Freeze tmem pools", > "[<Domain>|-a]", > " -a Freeze all tmem", > }, > { "tmem-destroy", > - &main_tmem_destroy, 0, > + &main_tmem_destroy, 0, 1, > "Destroy tmem pools", > "[<Domain>|-a]", > " -a Destroy all tmem", > }, > { "tmem-thaw", > - &main_tmem_thaw, 0, > + &main_tmem_thaw, 0, 1, > "Thaw tmem pools", > "[<Domain>|-a]", > " -a Thaw all tmem", > }, > { "tmem-set", > - &main_tmem_set, 0, > + &main_tmem_set, 0, 1, > "Change tmem settings", > "[<Domain>|-a] [-w[=WEIGHT]|-c[=CAP]|-p[=COMPRESS]]", > " -a Operate on all tmem\n" > @@ -358,7 +358,7 @@ struct cmd_spec cmd_table[] = { > " -p COMPRESS Compress (int)", > }, > { "tmem-shared-auth", > - &main_tmem_shared_auth, 0, > + &main_tmem_shared_auth, 0, 1, > "De/authenticate shared tmem pool", > "[<Domain>|-a] [-u[=UUID] [-A[=AUTH]", > " -a Authenticate for all tmem pools\n" > @@ -367,12 +367,12 @@ struct cmd_spec cmd_table[] = { > " -A AUTH 0=auth,1=deauth", > }, > { "tmem-freeable", > - &main_tmem_freeable, 0, > + &main_tmem_freeable, 0, 0, > "Get information about how much freeable memory (MB) is in-use by tmem", > "", > }, > { "cpupool-create", > - &main_cpupoolcreate, 1, > + &main_cpupoolcreate, 1, 1, > "Create a CPU pool based an ConfigFile", > "[options] <ConfigFile> [vars]", > "-h, --help Print this help.\n" > @@ -381,53 +381,53 @@ struct cmd_spec cmd_table[] = { > " (deprecated in favour of global -N option)." > }, > { "cpupool-list", > - &main_cpupoollist, 0, > + &main_cpupoollist, 0, 0, > "List CPU pools on host", > "[-c|--cpus] [<CPU Pool>]", > "-c, --cpus Output list of CPUs used by a pool" > }, > { "cpupool-destroy", > - &main_cpupooldestroy, 0, > + &main_cpupooldestroy, 0, 1, > "Deactivates a CPU pool", > "<CPU Pool>", > }, > { "cpupool-rename", > - &main_cpupoolrename, 0, > + &main_cpupoolrename, 0, 1, > "Renames a CPU pool", > "<CPU Pool> <new name>", > }, > { "cpupool-cpu-add", > - &main_cpupoolcpuadd, 0, > + &main_cpupoolcpuadd, 0, 1, > "Adds a CPU to a CPU pool", > "<CPU Pool> <CPU nr>|node:<node nr>", > }, > { "cpupool-cpu-remove", > - &main_cpupoolcpuremove, 0, > + &main_cpupoolcpuremove, 0, 1, > "Removes a CPU from a CPU pool", > "<CPU Pool> <CPU nr>|node:<node nr>", > }, > { "cpupool-migrate", > - &main_cpupoolmigrate, 0, > + &main_cpupoolmigrate, 0, 1, > "Moves a domain into a CPU pool", > "<Domain> <CPU Pool>", > }, > { "cpupool-numa-split", > - &main_cpupoolnumasplit, 0, > + &main_cpupoolnumasplit, 0, 1, > "Splits up the machine into one CPU pool per NUMA node", > "", > }, > { "getenforce", > - &main_getenforce, 0, > + &main_getenforce, 0, 0, > "Returns the current enforcing mode of the Flask Xen security module", > "", > }, > { "setenforce", > - &main_setenforce, 0, > + &main_setenforce, 0, 1, > "Sets the current enforcing mode of the Flask Xen security module", > "<1|0|Enforcing|Permissive>", > }, > { "loadpolicy", > - &main_loadpolicy, 0, > + &main_loadpolicy, 0, 1, > "Loads a new policy int the Flask Xen security module", > "<policy file>", > }, > -- > 1.7.7.5 (Apple Git-26) > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Ian Jackson
2012-Apr-25 10:25 UTC
Re: [PATCH v2] libxl: prevent xl from running if xend is running.
Roger Pau Monne writes ("Re: [PATCH v2] libxl: prevent xl from running if
xend is running."):> Ian Jackson escribió:
...> > Thanks, but I''m afraid this needs a refresh. I recommend you
wait
> > until tomorrow. I''m nearly at the end of my huge queue...
>
> Do you mean that the code doesn''t apply cleanly, or a refresh in
the
> commit message?
The former.
> In the lists of changes I''ve added the fact that now you are able
to run
> commands that doesn''t modify anything.
Right, yes, that''s good.
Thanks,
Ian.
Ian Jackson
2012-Apr-25 10:29 UTC
Re: [PATCH v2] libxl: prevent xl from running if xend is running.
Ian Campbell writes ("Re: [Xen-devel] [PATCH v2] libxl: prevent xl from
running if xend is running."):> On Tue, 2012-04-24 at 19:05 +0100, Roger Pau Monne wrote:
> > Prevent xl from doing any operation if xend daemon is running. That
> > prevents bugs that happened when xl and xend raced to close a domain.
...> Do we now have 3 classes of commands which could be represented with an
> enum rather than a pair of bools?
No, because hopefully eventually every command will support dryrun and
then we can abolish that flag.
Ian.
Roger Pau Monne
2012-Apr-25 10:32 UTC
Re: [PATCH v2] libxl: prevent xl from running if xend is running.
Ian Jackson escribió:> Roger Pau Monne writes ("Re: [PATCH v2] libxl: prevent xl from running if xend is running."): >> Ian Jackson escribió: > ... >>> Thanks, but I''m afraid this needs a refresh. I recommend you wait >>> until tomorrow. I''m nearly at the end of my huge queue... >> Do you mean that the code doesn''t apply cleanly, or a refresh in the >> commit message? > > The former.Anyway I''m going to make the change requested by Ian Campbell, and I''m going to add an enum to define the type of commands, currently I have the following names: STATIC: doesn''t modify anything DRY_RUN: modifies, but has a dry_run option MODIFIES: modifies, but has no dry_run option The names are crap, so if you have a better naming scheme I''m open to suggestions.> >> In the lists of changes I''ve added the fact that now you are able to run >> commands that doesn''t modify anything. > > Right, yes, that''s good. > > Thanks, > Ian.
Ian Jackson
2012-Apr-25 10:34 UTC
Re: [PATCH v2] libxl: prevent xl from running if xend is running.
Roger Pau Monne writes ("Re: [PATCH v2] libxl: prevent xl from running if
xend is running."):> Ian Jackson escribió:
> > The former.
>
> Anyway I''m going to make the change requested by Ian Campbell, and
I''m
> going to add an enum to define the type of commands, currently I have
> the following names:
Please don''t. I don''t agree with him, as I said...
Thanks,
Ian.
Ian Campbell
2012-Apr-25 11:04 UTC
Re: [PATCH v2] libxl: prevent xl from running if xend is running.
On Wed, 2012-04-25 at 11:29 +0100, Ian Jackson wrote:> Ian Campbell writes ("Re: [Xen-devel] [PATCH v2] libxl: prevent xl from running if xend is running."): > > On Tue, 2012-04-24 at 19:05 +0100, Roger Pau Monne wrote: > > > Prevent xl from doing any operation if xend daemon is running. That > > > prevents bugs that happened when xl and xend raced to close a domain. > ... > > Do we now have 3 classes of commands which could be represented with an > > enum rather than a pair of bools? > > No, because hopefully eventually every command will support dryrun and > then we can abolish that flag.Makes sense.