Roger Pau Monne
2012-Apr-25 11:09 UTC
[PATCH v3] 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 v2: * Allow to dry run even if xend is running. * Refresh to apply to current tree. Changes since v1: * Add documentation to xl man page. * Permit the execution of commands that don''t modify anything. * Indent error message. 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 | 5 +- tools/libxl/xl_cmdtable.c | 132 ++++++++++++++++++++++---------------------- 5 files changed, 97 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 a6ffd25..51369fd 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; @@ -114,8 +117,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--; @@ -123,6 +127,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); @@ -173,6 +180,19 @@ int main(int argc, char **argv) ret = 1; goto xit; } + if (cspec->modifies && !dryrun_only) { + 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 7e258d5..8c0823f 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 5703512..085ec80 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -1905,7 +1905,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); @@ -1916,7 +1916,8 @@ 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 [-v%s%s] %s %s\n\n%s.\n\n", + cmd->modifies ? "f" : "", 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 736a836..36c047f 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, 1, + &main_networkattach, 1, 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-25 13:38 UTC
Re: [PATCH v3] libxl: prevent xl from running if xend is running.
Roger Pau Monne writes ("[PATCH v3] 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.Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian Jackson
2012-May-11 16:04 UTC
Re: [PATCH v3] libxl: prevent xl from running if xend is running.
Roger Pau Monne writes ("[Xen-devel] [PATCH v3] 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, Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> I tried to apply this but I''m afraid it needs a refresh. Ian.
Roger Pau Monne
2012-May-14 09:41 UTC
Re: [PATCH v3] libxl: prevent xl from running if xend is running.
Ian Jackson escribió:> Roger Pau Monne writes ("[Xen-devel] [PATCH v3] 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, > > Acked-by: Ian Jackson<ian.jackson@eu.citrix.com> > > I tried to apply this but I''m afraid it needs a refresh.Ok, I''ve updated it and resend, but I''m afraid I''ve forgot to add the Ack, sorry about that.