* [PATCH v2] libxl: prevent xl from running if xend is running. @ 2012-04-24 18:05 Roger Pau Monne 2012-04-24 18:08 ` Ian Jackson 2012-04-25 9:22 ` Ian Campbell 0 siblings, 2 replies; 9+ messages in thread From: Roger Pau Monne @ 2012-04-24 18:05 UTC (permalink / raw) To: xen-devel; +Cc: george.dunlap, ian.jackson, Roger Pau Monne 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) ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] libxl: prevent xl from running if xend is running. 2012-04-24 18:05 [PATCH v2] libxl: prevent xl from running if xend is running Roger Pau Monne @ 2012-04-24 18:08 ` Ian Jackson 2012-04-25 8:36 ` Roger Pau Monne 2012-04-25 9:22 ` Ian Campbell 1 sibling, 1 reply; 9+ messages in thread From: Ian Jackson @ 2012-04-24 18:08 UTC (permalink / raw) To: Roger Pau Monne; +Cc: George Dunlap, xen-devel@lists.xen.org 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] libxl: prevent xl from running if xend is running. 2012-04-24 18:08 ` Ian Jackson @ 2012-04-25 8:36 ` Roger Pau Monne 2012-04-25 10:25 ` Ian Jackson 0 siblings, 1 reply; 9+ messages in thread From: Roger Pau Monne @ 2012-04-25 8:36 UTC (permalink / raw) To: Ian Jackson; +Cc: George Dunlap, xen-devel@lists.xen.org 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] libxl: prevent xl from running if xend is running. 2012-04-25 8:36 ` Roger Pau Monne @ 2012-04-25 10:25 ` Ian Jackson 2012-04-25 10:32 ` Roger Pau Monne 0 siblings, 1 reply; 9+ messages in thread From: Ian Jackson @ 2012-04-25 10:25 UTC (permalink / raw) To: Roger Pau Monne; +Cc: George Dunlap, xen-devel@lists.xen.org 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] libxl: prevent xl from running if xend is running. 2012-04-25 10:25 ` Ian Jackson @ 2012-04-25 10:32 ` Roger Pau Monne 2012-04-25 10:34 ` Ian Jackson 0 siblings, 1 reply; 9+ messages in thread From: Roger Pau Monne @ 2012-04-25 10:32 UTC (permalink / raw) To: Ian Jackson; +Cc: George Dunlap, xen-devel@lists.xen.org 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] libxl: prevent xl from running if xend is running. 2012-04-25 10:32 ` Roger Pau Monne @ 2012-04-25 10:34 ` Ian Jackson 0 siblings, 0 replies; 9+ messages in thread From: Ian Jackson @ 2012-04-25 10:34 UTC (permalink / raw) To: Roger Pau Monne; +Cc: George Dunlap, xen-devel@lists.xen.org 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] libxl: prevent xl from running if xend is running. 2012-04-24 18:05 [PATCH v2] libxl: prevent xl from running if xend is running Roger Pau Monne 2012-04-24 18:08 ` Ian Jackson @ 2012-04-25 9:22 ` Ian Campbell 2012-04-25 10:29 ` Ian Jackson 1 sibling, 1 reply; 9+ messages in thread From: Ian Campbell @ 2012-04-25 9:22 UTC (permalink / raw) To: Roger Pau Monne; +Cc: George Dunlap, Ian Jackson, xen-devel@lists.xen.org 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] libxl: prevent xl from running if xend is running. 2012-04-25 9:22 ` Ian Campbell @ 2012-04-25 10:29 ` Ian Jackson 2012-04-25 11:04 ` Ian Campbell 0 siblings, 1 reply; 9+ messages in thread From: Ian Jackson @ 2012-04-25 10:29 UTC (permalink / raw) To: Ian Campbell; +Cc: George Dunlap, xen-devel@lists.xen.org, Roger Pau Monne 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] libxl: prevent xl from running if xend is running. 2012-04-25 10:29 ` Ian Jackson @ 2012-04-25 11:04 ` Ian Campbell 0 siblings, 0 replies; 9+ messages in thread From: Ian Campbell @ 2012-04-25 11:04 UTC (permalink / raw) To: Ian Jackson; +Cc: George Dunlap, xen-devel@lists.xen.org, Roger Pau Monne 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-04-25 11:04 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-04-24 18:05 [PATCH v2] libxl: prevent xl from running if xend is running Roger Pau Monne 2012-04-24 18:08 ` Ian Jackson 2012-04-25 8:36 ` Roger Pau Monne 2012-04-25 10:25 ` Ian Jackson 2012-04-25 10:32 ` Roger Pau Monne 2012-04-25 10:34 ` Ian Jackson 2012-04-25 9:22 ` Ian Campbell 2012-04-25 10:29 ` Ian Jackson 2012-04-25 11:04 ` Ian Campbell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).