* [PATCH 1 of 5] libxl: propagate user supplied values into event for_user field
2012-10-15 10:09 [PATCH 0 of 5] xl shutdown compatibility with xm Ian Campbell
@ 2012-10-15 10:09 ` Ian Campbell
2012-10-17 15:58 ` Ian Jackson
2012-10-15 10:09 ` [PATCH 2 of 5] xl: Introduce xl shutdown --all support for xm compatibility Ian Campbell
` (5 subsequent siblings)
6 siblings, 1 reply; 31+ messages in thread
From: Ian Campbell @ 2012-10-15 10:09 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, linux
# HG changeset patch
# User Ian Campbell <ijc@hellion.org.uk>
# Date 1350290590 -3600
# Node ID 65e301c71ca41af8cf0aaea653cd9e8d08bca2f7
# Parent de18f0577d3ff521822dd3760fbfe5e08a771f9d
libxl: propagate user supplied values into event for_user field.
This was ommited in the majority of cases. Add as a parameter to
libxl__event_new and the NEW_EVENT wrapper to help prevent it being
forgotten in the future.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
This should be backported to 4.2.
diff -r de18f0577d3f -r 65e301c71ca4 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c Fri Oct 12 15:58:22 2012 +0100
+++ b/tools/libxl/libxl.c Mon Oct 15 09:43:10 2012 +0100
@@ -955,7 +955,7 @@ static void domain_death_occurred(libxl_
libxl_evgen_domain_death *evg_next = LIBXL_TAILQ_NEXT(evg, entry);
*evg_upd = evg_next;
- libxl_event *ev = NEW_EVENT(egc, DOMAIN_DEATH, evg->domid);
+ libxl_event *ev = NEW_EVENT(egc, DOMAIN_DEATH, evg->domid, evg->user);
libxl__event_occurred(egc, ev);
@@ -1041,8 +1041,9 @@ static void domain_death_xswatch_callbac
if (!evg->shutdown_reported &&
(got->flags & XEN_DOMINF_shutdown)) {
- libxl_event *ev = NEW_EVENT(egc, DOMAIN_SHUTDOWN, got->domain);
-
+ libxl_event *ev = NEW_EVENT(egc, DOMAIN_SHUTDOWN,
+ got->domain, evg->user);
+
LIBXL__LOG(CTX, LIBXL__LOG_DEBUG, " shutdown reporting");
ev->u.domain_shutdown.shutdown_reason =
@@ -1141,7 +1142,7 @@ static void disk_eject_xswatch_callback(
return;
}
- libxl_event *ev = NEW_EVENT(egc, DISK_EJECT, evg->domid);
+ libxl_event *ev = NEW_EVENT(egc, DISK_EJECT, evg->domid, evg->user);
libxl_device_disk *disk = &ev->u.disk_eject.disk;
backend = libxl__xs_read(gc, XBT_NULL,
diff -r de18f0577d3f -r 65e301c71ca4 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Fri Oct 12 15:58:22 2012 +0100
+++ b/tools/libxl/libxl_create.c Mon Oct 15 09:43:10 2012 +0100
@@ -705,7 +705,8 @@ static void domcreate_console_available(
libxl__domain_create_state *dcs) {
libxl__ao_progress_report(egc, dcs->ao, &dcs->aop_console_how,
NEW_EVENT(egc, DOMAIN_CREATE_CONSOLE_AVAILABLE,
- dcs->guest_domid));
+ dcs->guest_domid,
+ dcs->aop_console_how.for_event));
}
static void domcreate_bootloader_done(libxl__egc *egc,
diff -r de18f0577d3f -r 65e301c71ca4 tools/libxl/libxl_event.c
--- a/tools/libxl/libxl_event.c Fri Oct 12 15:58:22 2012 +0100
+++ b/tools/libxl/libxl_event.c Mon Oct 15 09:43:10 2012 +0100
@@ -1157,7 +1157,8 @@ void libxl_event_free(libxl_ctx *ctx, li
}
libxl_event *libxl__event_new(libxl__egc *egc,
- libxl_event_type type, uint32_t domid)
+ libxl_event_type type, uint32_t domid,
+ libxl_ev_user for_user)
{
EGC_GC;
libxl_event *ev;
@@ -1168,6 +1169,7 @@ libxl_event *libxl__event_new(libxl__egc
libxl_event_init_type(ev, type);
ev->domid = domid;
+ ev->for_user = for_user;
return ev;
}
@@ -1528,9 +1530,8 @@ void libxl__ao_complete_check_progress_r
LIBXL_TAILQ_INSERT_TAIL(&egc->aos_for_callback, ao, entry_for_callback);
} else {
libxl_event *ev;
- ev = NEW_EVENT(egc, OPERATION_COMPLETE, ao->domid);
+ ev = NEW_EVENT(egc, OPERATION_COMPLETE, ao->domid, ao->how.u.for_event);
if (ev) {
- ev->for_user = ao->how.u.for_event;
ev->u.operation_complete.rc = ao->rc;
libxl__event_occurred(egc, ev);
}
@@ -1662,7 +1663,6 @@ void libxl__ao_progress_report(libxl__eg
const libxl_asyncprogress_how *how, libxl_event *ev)
{
AO_GC;
- ev->for_user = how->for_event;
if (how->callback == dummy_asyncprogress_callback_ignore) {
LOG(DEBUG,"ao %p: progress report: ignored",ao);
libxl_event_free(CTX,ev);
diff -r de18f0577d3f -r 65e301c71ca4 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h Fri Oct 12 15:58:22 2012 +0100
+++ b/tools/libxl/libxl_internal.h Mon Oct 15 09:43:10 2012 +0100
@@ -783,13 +783,14 @@ _hidden void libxl__event_occurred(libxl
* event should be suitable for passing to libxl_event_free. */
_hidden libxl_event *libxl__event_new(libxl__egc*, libxl_event_type,
- uint32_t domid);
+ uint32_t domid,
+ libxl_ev_user for_user);
/* Convenience function.
* Allocates a new libxl_event, fills in domid and type.
* Cannot fail. */
-#define NEW_EVENT(egc, type, domid) \
- libxl__event_new((egc), LIBXL_EVENT_TYPE_##type, (domid))
+#define NEW_EVENT(egc, type, domid, user) \
+ libxl__event_new((egc), LIBXL_EVENT_TYPE_##type, (domid), (user))
/* Convenience macro. */
/*
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 2 of 5] xl: Introduce xl shutdown --all support for xm compatibility
2012-10-15 10:09 [PATCH 0 of 5] xl shutdown compatibility with xm Ian Campbell
2012-10-15 10:09 ` [PATCH 1 of 5] libxl: propagate user supplied values into event for_user field Ian Campbell
@ 2012-10-15 10:09 ` Ian Campbell
2012-10-17 16:03 ` Ian Jackson
2012-10-15 10:09 ` [PATCH 3 of 5] xl: Add --wait and --all to xl reboot Ian Campbell
` (4 subsequent siblings)
6 siblings, 1 reply; 31+ messages in thread
From: Ian Campbell @ 2012-10-15 10:09 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, linux
# HG changeset patch
# User Sander Eikelenboom <linux@eikelenboom.it>
# Date 1346960487 -7200
# Node ID 2329dca4ef449979b1403c4bb002fcb70c578b35
# Parent 65e301c71ca41af8cf0aaea653cd9e8d08bca2f7
xl: Introduce xl shutdown --all support for xm compatibility
Based on a patch by Sander Eikelenboom
Signed-off-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
Changed since v3: (ijc)
* Rebased. -w already implemented.
* Shutdown domains in //.
Changed since v2:
* fix error occuring when using both -a and -w options
* Due to mixing local and global domid variable
Changed since v1:
* address review comments.
* Change shutdown_domain to take domid instead of domname
* Docs: Make it more clear -a only shuts down GUEST domains
diff -r 65e301c71ca4 -r 2329dca4ef44 docs/man/xl.pod.1
--- a/docs/man/xl.pod.1 Mon Oct 15 09:43:10 2012 +0100
+++ b/docs/man/xl.pod.1 Thu Sep 06 21:41:27 2012 +0200
@@ -527,7 +527,7 @@ List specifically for that domain. Other
=back
-=item B<shutdown> [I<OPTIONS>] I<domain-id>
+=item B<shutdown> [I<OPTIONS>] I<-a|domain-id>
Gracefully shuts down a domain. This coordinates with the domain OS
to perform graceful shutdown, so there is no guarantee that it will
@@ -550,6 +550,11 @@ B<OPTIONS>
=over 4
+=item B<-a>, B<--all>
+
+Shutdown all guest domains. Often used when doing a complete shutdown
+of a Xen system.
+
=item B<-w>, B<--wait>
Wait for the domain to complete shutdown before returning.
diff -r 65e301c71ca4 -r 2329dca4ef44 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Mon Oct 15 09:43:10 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Thu Sep 06 21:41:27 2012 +0200
@@ -2713,12 +2713,46 @@ static void destroy_domain(uint32_t domi
if (rc) { fprintf(stderr,"destroy failed (rc=%d)\n",rc); exit(-1); }
}
-static void shutdown_domain(uint32_t domid, int wait_for_it,
+static void wait_for_domain_deaths(libxl_evgen_domain_death **deathws, int nr)
+{
+ int rc, count = 0;
+ LOG("Waiting for %d domains to shutdown", nr);
+ while(1 && count < nr) {
+ libxl_event *event;
+ rc = libxl_event_wait(ctx, &event, LIBXL_EVENTMASK_ALL, 0,0);
+ if (rc) {
+ LOG("Failed to get event, quitting (rc=%d)", rc);
+ exit(-1);
+ }
+
+ switch (event->type) {
+ case LIBXL_EVENT_TYPE_DOMAIN_DEATH:
+ LOG("Domain %d has been destroyed", event->domid);
+ libxl_evdisable_domain_death(ctx, deathws[event->for_user]);
+ count++;
+ break;
+ case LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN:
+ LOG("Domain %d has been shut down, reason code %d",
+ event->domid, event->u.domain_shutdown.shutdown_reason);
+ libxl_evdisable_domain_death(ctx, deathws[event->for_user]);
+ count++;
+ break;
+ default:
+ LOG("Unexpected event type %d", event->type);
+ break;
+ }
+ libxl_event_free(ctx, event);
+ }
+}
+
+static void shutdown_domain(uint32_t domid,
+ libxl_evgen_domain_death **deathw,
+ libxl_ev_user for_user,
int fallback_trigger)
{
int rc;
- libxl_event *event;
-
+
+ fprintf(stderr, "Shutting down domain %d\n", domid);
rc=libxl_domain_shutdown(ctx, domid);
if (rc == ERROR_NOPARAVIRT) {
if (fallback_trigger) {
@@ -2731,44 +2765,19 @@ static void shutdown_domain(uint32_t dom
fprintf(stderr, "Use \"-F\" to fallback to ACPI power event.\n");
}
}
+
if (rc) {
fprintf(stderr,"shutdown failed (rc=%d)\n",rc);exit(-1);
}
- if (wait_for_it) {
- libxl_evgen_domain_death *deathw;
-
- rc = libxl_evenable_domain_death(ctx, domid, 0, &deathw);
+ if (deathw) {
+ rc = libxl_evenable_domain_death(ctx, domid, for_user, deathw);
if (rc) {
fprintf(stderr,"wait for death failed (evgen, rc=%d)\n",rc);
exit(-1);
}
-
- for (;;) {
- rc = domain_wait_event(domid, &event);
- if (rc) exit(-1);
-
- switch (event->type) {
-
- case LIBXL_EVENT_TYPE_DOMAIN_DEATH:
- LOG("Domain %d has been destroyed", domid);
- goto done;
-
- case LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN:
- LOG("Domain %d has been shut down, reason code %d %x", domid,
- event->u.domain_shutdown.shutdown_reason,
- event->u.domain_shutdown.shutdown_reason);
- goto done;
-
- default:
- LOG("Unexpected event type %d", event->type);
- break;
- }
- libxl_event_free(ctx, event);
- }
- done:
- libxl_event_free(ctx, event);
- libxl_evdisable_domain_death(ctx, deathw);
+ printf("Waiting for domain %d death %p %"PRIx64"\n",
+ domid, *deathw, for_user);
}
}
@@ -3706,18 +3715,21 @@ int main_destroy(int argc, char **argv)
int main_shutdown(int argc, char **argv)
{
- int opt;
- int wait_for_it = 0;
+ int opt, i, nb_domain;
+ int wait_for_it = 0, all =0;
int fallback_trigger = 0;
static struct option long_options[] = {
+ {"all", 0, 0, 'a'},
{"wait", 0, 0, 'w'},
{0, 0, 0, 0}
};
- while ((opt = getopt_long(argc, argv, "wF", long_options, NULL)) != -1) {
+ while ((opt = getopt_long(argc, argv, "awF", long_options, NULL)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
+ case 'a':
+ all = 1;
case 'w':
wait_for_it = 1;
break;
@@ -3727,7 +3739,44 @@ int main_shutdown(int argc, char **argv)
}
}
- shutdown_domain(find_domain(argv[optind]), wait_for_it, fallback_trigger);
+ if (!argv[optind] && !all) {
+ fprintf(stderr, "You must specify -a or a domain id.\n\n");
+ return opt;
+ }
+
+ if (all) {
+ libxl_dominfo *dominfo;
+ libxl_evgen_domain_death **deathws = NULL;
+ if (!(dominfo = libxl_list_domain(ctx, &nb_domain))) {
+ fprintf(stderr, "libxl_list_domain failed.\n");
+ return -1;
+ }
+
+ if (wait_for_it)
+ deathws = calloc(nb_domain, sizeof(deathws));
+
+ for (i = 0; i<nb_domain; i++) {
+ if (dominfo[i].domid == 0)
+ continue;
+ shutdown_domain(dominfo[i].domid,
+ deathws ? &deathws[i] : NULL, i,
+ fallback_trigger);
+ }
+
+ wait_for_domain_deaths(deathws, nb_domain - 1 /* not dom 0 */);
+ libxl_dominfo_list_free(dominfo, nb_domain);
+ } else {
+ libxl_evgen_domain_death *deathw = NULL;
+ uint32_t domid = find_domain(argv[optind]);
+
+ shutdown_domain(domid, wait_for_it ? &deathw : NULL, 0,
+ fallback_trigger);
+
+ if (wait_for_it)
+ wait_for_domain_deaths(&deathw, 1);
+ }
+
+
return 0;
}
diff -r 65e301c71ca4 -r 2329dca4ef44 tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c Mon Oct 15 09:43:10 2012 +0100
+++ b/tools/libxl/xl_cmdtable.c Thu Sep 06 21:41:27 2012 +0200
@@ -60,11 +60,12 @@ struct cmd_spec cmd_table[] = {
{ "shutdown",
&main_shutdown, 0, 1,
"Issue a shutdown signal to a domain",
- "[options] <Domain>",
+ "[options] <-a|Domain>",
+ "-a, --all Shutdown all guest domains.\n"
"-h Print this help.\n"
"-F Fallback to ACPI power event for HVM guests with\n"
" no PV drivers.\n"
- "-w, --wait Wait for guest to shutdown.\n"
+ "-w, --wait Wait for guest(s) to shutdown.\n"
},
{ "reboot",
&main_reboot, 0, 1,
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 2 of 5] xl: Introduce xl shutdown --all support for xm compatibility
2012-10-15 10:09 ` [PATCH 2 of 5] xl: Introduce xl shutdown --all support for xm compatibility Ian Campbell
@ 2012-10-17 16:03 ` Ian Jackson
2012-10-18 8:24 ` Ian Campbell
0 siblings, 1 reply; 31+ messages in thread
From: Ian Jackson @ 2012-10-17 16:03 UTC (permalink / raw)
To: Ian Campbell; +Cc: linux@eikelenboom.it, xen-devel@lists.xen.org
Ian Campbell writes ("[PATCH 2 of 5] xl: Introduce xl shutdown --all support for xm compatibility"):
> xl: Introduce xl shutdown --all support for xm compatibility
...>
> Based on a patch by Sander Eikelenboom
...
> int main_shutdown(int argc, char **argv)
...
> + if (wait_for_it)
> + deathws = calloc(nb_domain, sizeof(deathws));
This should read sizeof(*deathws). It's a shame we don't have a macro
to avoid this broken pattern in xl. In libxl we have GCNEW_ARRAY of
course. (In fact in this particular case the two things are both
pointers so the sizeof() is the same and the bug is theoretical.)
Apart from that,
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 2 of 5] xl: Introduce xl shutdown --all support for xm compatibility
2012-10-17 16:03 ` Ian Jackson
@ 2012-10-18 8:24 ` Ian Campbell
0 siblings, 0 replies; 31+ messages in thread
From: Ian Campbell @ 2012-10-18 8:24 UTC (permalink / raw)
To: Ian Jackson; +Cc: linux@eikelenboom.it, xen-devel@lists.xen.org
On Wed, 2012-10-17 at 17:03 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH 2 of 5] xl: Introduce xl shutdown --all support for xm compatibility"):
> > xl: Introduce xl shutdown --all support for xm compatibility
> ...>
> > Based on a patch by Sander Eikelenboom
> ...
> > int main_shutdown(int argc, char **argv)
> ...
> > + if (wait_for_it)
> > + deathws = calloc(nb_domain, sizeof(deathws));
>
> This should read sizeof(*deathws). It's a shame we don't have a macro
> to avoid this broken pattern in xl. In libxl we have GCNEW_ARRAY of
> course. (In fact in this particular case the two things are both
> pointers so the sizeof() is the same and the bug is theoretical.)
Oops, will fix upon commit, thanks.
> Apart from that,
>
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
>
> Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3 of 5] xl: Add --wait and --all to xl reboot
2012-10-15 10:09 [PATCH 0 of 5] xl shutdown compatibility with xm Ian Campbell
2012-10-15 10:09 ` [PATCH 1 of 5] libxl: propagate user supplied values into event for_user field Ian Campbell
2012-10-15 10:09 ` [PATCH 2 of 5] xl: Introduce xl shutdown --all support for xm compatibility Ian Campbell
@ 2012-10-15 10:09 ` Ian Campbell
2012-10-17 16:04 ` Ian Jackson
2012-10-15 10:09 ` [PATCH 4 of 5] xl: allow def_getopt to handle long options Ian Campbell
` (3 subsequent siblings)
6 siblings, 1 reply; 31+ messages in thread
From: Ian Campbell @ 2012-10-15 10:09 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, linux
# HG changeset patch
# User Ian Campbell <ijc@hellion.org.uk>
# Date 1350295718 -3600
# Node ID 5a24315f39c035e5e5b773ebca676d5248a41252
# Parent 2329dca4ef449979b1403c4bb002fcb70c578b35
xl: Add --wait and --all to xl reboot.
Inspired by a patch by Sander Eikelenboom.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r 2329dca4ef44 -r 5a24315f39c0 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Thu Sep 06 21:41:27 2012 +0200
+++ b/tools/libxl/xl_cmdimpl.c Mon Oct 15 11:08:38 2012 +0100
@@ -2716,7 +2716,7 @@ static void destroy_domain(uint32_t domi
static void wait_for_domain_deaths(libxl_evgen_domain_death **deathws, int nr)
{
int rc, count = 0;
- LOG("Waiting for %d domains to shutdown", nr);
+ LOG("Waiting for %d domains", nr);
while(1 && count < nr) {
libxl_event *event;
rc = libxl_event_wait(ctx, &event, LIBXL_EVENTMASK_ALL, 0,0);
@@ -2776,15 +2776,15 @@ static void shutdown_domain(uint32_t dom
fprintf(stderr,"wait for death failed (evgen, rc=%d)\n",rc);
exit(-1);
}
- printf("Waiting for domain %d death %p %"PRIx64"\n",
- domid, *deathw, for_user);
- }
-}
-
-static void reboot_domain(uint32_t domid, int fallback_trigger)
+ }
+}
+
+static void reboot_domain(uint32_t domid, libxl_evgen_domain_death **deathw,
+ libxl_ev_user for_user, int fallback_trigger)
{
int rc;
+ fprintf(stderr, "Rebooting domain %d\n", domid);
rc=libxl_domain_reboot(ctx, domid);
if (rc == ERROR_NOPARAVIRT) {
if (fallback_trigger) {
@@ -2800,6 +2800,14 @@ static void reboot_domain(uint32_t domid
if (rc) {
fprintf(stderr,"reboot failed (rc=%d)\n",rc);exit(-1);
}
+
+ if (deathw) {
+ rc = libxl_evenable_domain_death(ctx, domid, for_user, deathw);
+ if (rc) {
+ fprintf(stderr,"wait for death failed (evgen, rc=%d)\n",rc);
+ exit(-1);
+ }
+ }
}
static void list_domains_details(const libxl_dominfo *info, int nb_domain)
@@ -3713,8 +3721,11 @@ int main_destroy(int argc, char **argv)
return 0;
}
-int main_shutdown(int argc, char **argv)
-{
+static int main_shutdown_or_reboot(int reboot, int argc, char **argv)
+{
+ void (*fn)(uint32_t domid,
+ libxl_evgen_domain_death **, libxl_ev_user, int) =
+ reboot ? &reboot_domain : &shutdown_domain;
int opt, i, nb_domain;
int wait_for_it = 0, all =0;
int fallback_trigger = 0;
@@ -3730,6 +3741,7 @@ int main_shutdown(int argc, char **argv)
return opt;
case 'a':
all = 1;
+ break;
case 'w':
wait_for_it = 1;
break;
@@ -3758,9 +3770,8 @@ int main_shutdown(int argc, char **argv)
for (i = 0; i<nb_domain; i++) {
if (dominfo[i].domid == 0)
continue;
- shutdown_domain(dominfo[i].domid,
- deathws ? &deathws[i] : NULL, i,
- fallback_trigger);
+ fn(dominfo[i].domid, deathws ? &deathws[i] : NULL, i,
+ fallback_trigger);
}
wait_for_domain_deaths(deathws, nb_domain - 1 /* not dom 0 */);
@@ -3769,8 +3780,7 @@ int main_shutdown(int argc, char **argv)
libxl_evgen_domain_death *deathw = NULL;
uint32_t domid = find_domain(argv[optind]);
- shutdown_domain(domid, wait_for_it ? &deathw : NULL, 0,
- fallback_trigger);
+ fn(domid, wait_for_it ? &deathw : NULL, 0, fallback_trigger);
if (wait_for_it)
wait_for_domain_deaths(&deathw, 1);
@@ -3780,23 +3790,14 @@ int main_shutdown(int argc, char **argv)
return 0;
}
+int main_shutdown(int argc, char **argv)
+{
+ return main_shutdown_or_reboot(0, argc, argv);
+}
+
int main_reboot(int argc, char **argv)
{
- int opt;
- int fallback_trigger = 0;
-
- while ((opt = def_getopt(argc, argv, "F", "reboot", 1)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'F':
- fallback_trigger = 1;
- break;
- }
- }
-
- reboot_domain(find_domain(argv[optind]), fallback_trigger);
- return 0;
+ return main_shutdown_or_reboot(1, argc, argv);
}
int main_list(int argc, char **argv)
diff -r 2329dca4ef44 -r 5a24315f39c0 tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c Thu Sep 06 21:41:27 2012 +0200
+++ b/tools/libxl/xl_cmdtable.c Mon Oct 15 11:08:38 2012 +0100
@@ -70,10 +70,12 @@ struct cmd_spec cmd_table[] = {
{ "reboot",
&main_reboot, 0, 1,
"Issue a reboot signal to a domain",
- "[options] <Domain>",
+ "[options] <-a|Domain>",
+ "-a, --all Shutdown all guest domains.\n"
"-h Print this help.\n"
"-F Fallback to ACPI reset event for HVM guests with\n"
" no PV drivers.\n"
+ "-w, --wait Wait for guest(s) to reboot.\n"
},
{ "pci-attach",
&main_pciattach, 0, 1,
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 4 of 5] xl: allow def_getopt to handle long options
2012-10-15 10:09 [PATCH 0 of 5] xl shutdown compatibility with xm Ian Campbell
` (2 preceding siblings ...)
2012-10-15 10:09 ` [PATCH 3 of 5] xl: Add --wait and --all to xl reboot Ian Campbell
@ 2012-10-15 10:09 ` Ian Campbell
2012-10-17 16:05 ` Ian Jackson
2012-10-15 10:09 ` [PATCH 5 of 5] xl: Introduce helper macro for option parsing Ian Campbell
` (2 subsequent siblings)
6 siblings, 1 reply; 31+ messages in thread
From: Ian Campbell @ 2012-10-15 10:09 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, linux
# HG changeset patch
# User Ian Campbell <ijc@hellion.org.uk>
# Date 1350295719 -3600
# Node ID bbb839ceb966cc265e5d2ca530ba0f2bf823176a
# Parent 5a24315f39c035e5e5b773ebca676d5248a41252
xl: allow def_getopt to handle long options
Improves consistency of option parsing and error handling.
Consistently support --help for all options.
Many users of getopt_long were needlessly passing an option_index
pointer which was not used.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r 5a24315f39c0 -r bbb839ceb966 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Mon Oct 15 11:08:38 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Mon Oct 15 11:08:39 2012 +0100
@@ -2241,19 +2241,34 @@ static int64_t parse_mem_size_kb(const c
return kbytes;
}
-static int def_getopt(int argc, char * const argv[], const char *optstring,
+#define COMMON_LONG_OPTS {"help", 0, 0, 'h'}
+
+static int def_getopt(int argc, char * const argv[],
+ const char *optstring,
+ const struct option *longopts,
const char* helpstr, int reqargs)
{
int opt;
+ const struct option def_options[] = {
+ COMMON_LONG_OPTS,
+ {0, 0, 0, 0}
+ };
+
+ if (!longopts)
+ longopts = def_options;
opterr = 0;
- while ((opt = getopt(argc, argv, optstring)) == '?') {
+ while ((opt = getopt_long(argc, argv, optstring, longopts, NULL)) == '?') {
if (optopt == 'h') {
help(helpstr);
return 0;
}
fprintf(stderr, "option `%c' not supported.\n", optopt);
}
+ if (opt == 'h') {
+ help(helpstr);
+ return 0;
+ }
if (opt != -1)
return opt;
@@ -2289,7 +2304,7 @@ int main_memmax(int argc, char **argv)
char *mem;
int rc;
- if ((opt = def_getopt(argc, argv, "", "mem-max", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "mem-max", 2)) != -1)
return opt;
domid = find_domain(argv[optind]);
@@ -2323,7 +2338,7 @@ int main_memset(int argc, char **argv)
int opt = 0;
const char *mem;
- if ((opt = def_getopt(argc, argv, "", "mem-set", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "mem-set", 2)) != -1)
return opt;
domid = find_domain(argv[optind]);
@@ -2362,7 +2377,7 @@ int main_cd_eject(int argc, char **argv)
int opt = 0;
const char *virtdev;
- if ((opt = def_getopt(argc, argv, "", "cd-eject", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "cd-eject", 2)) != -1)
return opt;
domid = find_domain(argv[optind]);
@@ -2379,7 +2394,7 @@ int main_cd_insert(int argc, char **argv
const char *virtdev;
char *file = NULL; /* modified by cd_insert tokenising it */
- if ((opt = def_getopt(argc, argv, "", "cd-insert", 3)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "cd-insert", 3)) != -1)
return opt;
domid = find_domain(argv[optind]);
@@ -2396,7 +2411,7 @@ int main_console(int argc, char **argv)
int opt = 0, num = 0;
libxl_console_type type = 0;
- while ((opt = def_getopt(argc, argv, "n:t:", "console", 1)) != -1) {
+ while ((opt = def_getopt(argc, argv, "n:t:", NULL, "console", 1)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -2427,36 +2442,23 @@ int main_console(int argc, char **argv)
int main_vncviewer(int argc, char **argv)
{
- static const struct option long_options[] = {
+ static const struct option opts[] = {
{"autopass", 0, 0, 'a'},
{"vncviewer-autopass", 0, 0, 'a'},
- {"help", 0, 0, 'h'},
+ COMMON_LONG_OPTS,
{0, 0, 0, 0}
};
uint32_t domid;
int opt, autopass = 0;
- while (1) {
- opt = getopt_long(argc, argv, "ah", long_options, NULL);
- if (opt == -1)
- break;
-
+ while ((opt = def_getopt(argc, argv, "ah", opts, "vncviewer", 1)) != -1) {
switch (opt) {
+ case 0: case 2:
+ return opt;
case 'a':
autopass = 1;
break;
- case 'h':
- help("vncviewer");
- return 0;
- default:
- fprintf(stderr, "option `%c' not supported.\n", optopt);
- break;
- }
- }
-
- if (argc - optind != 1) {
- help("vncviewer");
- return 2;
+ }
}
domid = find_domain(argv[optind]);
@@ -2489,7 +2491,7 @@ int main_pcilist(int argc, char **argv)
uint32_t domid;
int opt;
- if ((opt = def_getopt(argc, argv, "", "pci-list", 1)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "pci-list", 1)) != -1)
return opt;
domid = find_domain(argv[optind]);
@@ -2528,7 +2530,7 @@ int main_pcidetach(int argc, char **argv
int force = 0;
const char *bdf = NULL;
- while ((opt = def_getopt(argc, argv, "f", "pci-detach", 2)) != -1) {
+ while ((opt = def_getopt(argc, argv, "f", NULL, "pci-detach", 2)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -2570,7 +2572,7 @@ int main_pciattach(int argc, char **argv
int opt;
const char *bdf = NULL, *vs = NULL;
- if ((opt = def_getopt(argc, argv, "", "pci-attach", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "pci-attach", 2)) != -1)
return opt;
domid = find_domain(argv[optind]);
@@ -2604,7 +2606,7 @@ int main_pciassignable_list(int argc, ch
{
int opt;
- if ((opt = def_getopt(argc, argv, "", "pci-assignable-list", 0)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "pci-assignable-list", 0)) != -1)
return opt;
pciassignable_list();
@@ -2636,7 +2638,7 @@ int main_pciassignable_add(int argc, cha
int opt;
const char *bdf = NULL;
- while ((opt = def_getopt(argc, argv, "", "pci-assignable-add", 1)) != -1) {
+ while ((opt = def_getopt(argc, argv, "", NULL, "pci-assignable-add", 1)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -2675,7 +2677,7 @@ int main_pciassignable_remove(int argc,
const char *bdf = NULL;
int rebind = 0;
- while ((opt = def_getopt(argc, argv, "r", "pci-assignable-remove", 1)) != -1) {
+ while ((opt = def_getopt(argc, argv, "r", NULL, "pci-assignable-remove", 1)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -3486,24 +3488,18 @@ int main_restore(int argc, char **argv)
int paused = 0, debug = 0, daemonize = 1, monitor = 1,
console_autoconnect = 0, vnc = 0, vncautopass = 0;
int opt, rc;
- int option_index = 0;
- static struct option long_options[] = {
+ static struct option opts[] = {
{"vncviewer", 0, 0, 'V'},
{"vncviewer-autopass", 0, 0, 'A'},
+ COMMON_LONG_OPTS,
{0, 0, 0, 0}
};
- while (1) {
- opt = getopt_long(argc, argv, "FhcpdeVA", long_options, &option_index);
- if (opt == -1)
- break;
-
+ while ((opt = def_getopt(argc, argv, "FhcpdeVA",
+ opts, "restore", 1)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
- case 'h':
- help("restore");
- return 2;
case 'c':
console_autoconnect = 1;
break;
@@ -3563,7 +3559,7 @@ int main_migrate_receive(int argc, char
int debug = 0, daemonize = 1, monitor = 1, remus = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "Fedr", "migrate-receive", 0)) != -1) {
+ while ((opt = def_getopt(argc, argv, "Fedr", NULL, "migrate-receive", 0)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -3602,7 +3598,7 @@ int main_save(int argc, char **argv)
int checkpoint = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "c", "save", 2)) != -1) {
+ while ((opt = def_getopt(argc, argv, "c", NULL, "save", 2)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -3635,7 +3631,7 @@ int main_migrate(int argc, char **argv)
char *host;
int opt, daemonize = 1, monitor = 1, debug = 0;
- while ((opt = def_getopt(argc, argv, "FC:s:ed", "migrate", 2)) != -1) {
+ while ((opt = def_getopt(argc, argv, "FC:s:ed", NULL, "migrate", 2)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -3679,7 +3675,7 @@ int main_dump_core(int argc, char **argv
{
int opt;
- if ((opt = def_getopt(argc, argv, "", "dump-core", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "dump-core", 2)) != -1)
return opt;
core_dump_domain(find_domain(argv[optind]), argv[optind + 1]);
@@ -3690,7 +3686,7 @@ int main_pause(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", "pause", 1)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "pause", 1)) != -1)
return opt;
pause_domain(find_domain(argv[optind]));
@@ -3702,7 +3698,7 @@ int main_unpause(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", "unpause", 1)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "unpause", 1)) != -1)
return opt;
unpause_domain(find_domain(argv[optind]));
@@ -3714,7 +3710,7 @@ int main_destroy(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", "destroy", 1)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "destroy", 1)) != -1)
return opt;
destroy_domain(find_domain(argv[optind]));
@@ -3723,19 +3719,21 @@ int main_destroy(int argc, char **argv)
static int main_shutdown_or_reboot(int reboot, int argc, char **argv)
{
+ const char *what = reboot ? "reboot" : "shutdown";
void (*fn)(uint32_t domid,
libxl_evgen_domain_death **, libxl_ev_user, int) =
reboot ? &reboot_domain : &shutdown_domain;
int opt, i, nb_domain;
int wait_for_it = 0, all =0;
int fallback_trigger = 0;
- static struct option long_options[] = {
+ static struct option opts[] = {
{"all", 0, 0, 'a'},
{"wait", 0, 0, 'w'},
+ COMMON_LONG_OPTS,
{0, 0, 0, 0}
};
- while ((opt = getopt_long(argc, argv, "awF", long_options, NULL)) != -1) {
+ while ((opt = def_getopt(argc, argv, "awF", opts, what, 0)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -3805,12 +3803,11 @@ int main_list(int argc, char **argv)
int opt, verbose = 0;
int context = 0;
int details = 0;
- int option_index = 0;
- static struct option long_options[] = {
+ static struct option opts[] = {
{"long", 0, 0, 'l'},
- {"help", 0, 0, 'h'},
{"verbose", 0, 0, 'v'},
{"context", 0, 0, 'Z'},
+ COMMON_LONG_OPTS,
{0, 0, 0, 0}
};
@@ -3818,12 +3815,10 @@ int main_list(int argc, char **argv)
libxl_dominfo *info, *info_free=0;
int nb_domain, rc;
- while (1) {
- opt = getopt_long(argc, argv, "lvhZ", long_options, &option_index);
- if (opt == -1)
- break;
-
+ while ((opt = def_getopt(argc, argv, "lvhZ", opts, "list", 0)) != -1) {
switch (opt) {
+ case 0: case 2:
+ return opt;
case 'l':
details = 1;
break;
@@ -3836,9 +3831,6 @@ int main_list(int argc, char **argv)
case 'Z':
context = 1;
break;
- default:
- fprintf(stderr, "option `%c' not supported.\n", optopt);
- break;
}
}
@@ -3885,7 +3877,7 @@ int main_vm_list(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", "vm-list", 0)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "vm-list", 0)) != -1)
return opt;
list_vm();
@@ -3901,14 +3893,13 @@ int main_create(int argc, char **argv)
int paused = 0, debug = 0, daemonize = 1, console_autoconnect = 0,
quiet = 0, monitor = 1, vnc = 0, vncautopass = 0;
int opt, rc;
- int option_index = 0;
- static struct option long_options[] = {
+ static struct option opts[] = {
{"dryrun", 0, 0, 'n'},
{"quiet", 0, 0, 'q'},
- {"help", 0, 0, 'h'},
{"defconfig", 1, 0, 'f'},
{"vncviewer", 0, 0, 'V'},
{"vncviewer-autopass", 0, 0, 'A'},
+ COMMON_LONG_OPTS,
{0, 0, 0, 0}
};
@@ -3917,12 +3908,10 @@ int main_create(int argc, char **argv)
argc--; argv++;
}
- while (1) {
- opt = getopt_long(argc, argv, "Fhnqf:pcdeVA", long_options, &option_index);
- if (opt == -1)
- break;
-
+ while ((opt = def_getopt(argc, argv, "Fhnqf:pcdeVA", opts, "create", 0)) != -1) {
switch (opt) {
+ case 0: case 2:
+ return opt;
case 'f':
filename = optarg;
break;
@@ -3957,9 +3946,6 @@ int main_create(int argc, char **argv)
case 'A':
vnc = vncautopass = 1;
break;
- default:
- fprintf(stderr, "option `%c' not supported.\n", optopt);
- break;
}
}
@@ -4007,11 +3993,10 @@ int main_config_update(int argc, char **
int config_len = 0;
libxl_domain_config d_config;
int opt, rc;
- int option_index = 0;
int debug = 0;
- static struct option long_options[] = {
- {"help", 0, 0, 'h'},
+ static struct option opts[] = {
{"defconfig", 1, 0, 'f'},
+ COMMON_LONG_OPTS,
{0, 0, 0, 0}
};
@@ -4029,24 +4014,16 @@ int main_config_update(int argc, char **
argc--; argv++;
}
- while (1) {
- opt = getopt_long(argc, argv, "dhqf:", long_options, &option_index);
- if (opt == -1)
- break;
-
+ while ((opt = def_getopt(argc, argv, "dhqf:", opts, "config_update", 0)) != -1) {
switch (opt) {
+ case 0: case 2:
+ return opt;
case 'd':
debug = 1;
break;
case 'f':
filename = optarg;
break;
- case 'h':
- help("create");
- return 0;
- default:
- fprintf(stderr, "option `%c' not supported.\n", optopt);
- break;
}
}
@@ -4135,7 +4112,7 @@ int main_button_press(int argc, char **a
fprintf(stderr, "WARNING: \"button-press\" is deprecated. "
"Please use \"trigger\"\n");
- if ((opt = def_getopt(argc, argv, "", "button-press", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "button-press", 2)) != -1)
return opt;
button_press(find_domain(argv[optind]), argv[optind + 1]);
@@ -4276,7 +4253,7 @@ int main_vcpulist(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", "cpu-list", 0)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "cpu-list", 0)) != -1)
return opt;
vcpulist(argc - optind, argv + optind);
@@ -4337,7 +4314,7 @@ int main_vcpupin(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", "vcpu-pin", 3)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "vcpu-pin", 3)) != -1)
return opt;
vcpupin(find_domain(argv[optind]), argv[optind+1] , argv[optind+2]);
@@ -4373,7 +4350,7 @@ int main_vcpuset(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", "vcpu-set", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "vcpu-set", 2)) != -1)
return opt;
vcpuset(find_domain(argv[optind]), argv[optind+1]);
@@ -4549,25 +4526,20 @@ static void print_info(int numa)
int main_info(int argc, char **argv)
{
int opt;
- int option_index = 0;
- static struct option long_options[] = {
- {"help", 0, 0, 'h'},
+ static struct option opts[] = {
{"numa", 0, 0, 'n'},
+ COMMON_LONG_OPTS,
{0, 0, 0, 0}
};
int numa = 0;
- while ((opt = getopt_long(argc, argv, "hn", long_options, &option_index)) != -1) {
+ while ((opt = def_getopt(argc, argv, "hn", opts, "info", 0)) != -1) {
switch (opt) {
- case 'h':
- help("info");
- return 0;
+ case 0: case 2:
+ return opt;
case 'n':
numa = 1;
break;
- default:
- fprintf(stderr, "option `%c' not supported.\n", optopt);
- break;
}
}
@@ -4602,7 +4574,7 @@ int main_sharing(int argc, char **argv)
libxl_dominfo *info, *info_free = NULL;
int nb_domain, rc;
- if ((opt = def_getopt(argc, argv, "", "sharing", 0)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "sharing", 0)) != -1)
return opt;
if (optind >= argc) {
@@ -4871,8 +4843,7 @@ int main_sched_credit(int argc, char **a
int opt_s = 0;
int tslice = 0, opt_t = 0, ratelimit = 0, opt_r = 0;
int opt, rc;
- int option_index = 0;
- static struct option long_options[] = {
+ static struct option opts[] = {
{"domain", 1, 0, 'd'},
{"weight", 1, 0, 'w'},
{"cap", 1, 0, 'c'},
@@ -4880,15 +4851,11 @@ int main_sched_credit(int argc, char **a
{"tslice_ms", 1, 0, 't'},
{"ratelimit_us", 1, 0, 'r'},
{"cpupool", 1, 0, 'p'},
- {"help", 0, 0, 'h'},
+ COMMON_LONG_OPTS,
{0, 0, 0, 0}
};
- while (1) {
- opt = getopt_long(argc, argv, "d:w:c:p:t:r:hs", long_options,
- &option_index);
- if (opt == -1)
- break;
+ while ((opt = def_getopt(argc, argv, "d:w:c:p:t:r:hs", opts, "sched-credit", 0)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -4917,9 +4884,6 @@ int main_sched_credit(int argc, char **a
case 'p':
cpupool = optarg;
break;
- case 'h':
- help("sched-credit");
- return 0;
}
}
@@ -5002,19 +4966,15 @@ int main_sched_credit2(int argc, char **
const char *cpupool = NULL;
int weight = 256, opt_w = 0;
int opt, rc;
- int option_index = 0;
- static struct option long_options[] = {
+ static struct option opts[] = {
{"domain", 1, 0, 'd'},
{"weight", 1, 0, 'w'},
{"cpupool", 1, 0, 'p'},
- {"help", 0, 0, 'h'},
+ COMMON_LONG_OPTS,
{0, 0, 0, 0}
};
- while (1) {
- opt = getopt_long(argc, argv, "d:w:p:h", long_options, &option_index);
- if (opt == -1)
- break;
+ while ((opt = def_getopt(argc, argv, "d:w:p:h", opts, "sched-credit2", 0)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -5028,9 +4988,6 @@ int main_sched_credit2(int argc, char **
case 'p':
cpupool = optarg;
break;
- case 'h':
- help("sched-credit");
- return 0;
}
}
@@ -5081,23 +5038,18 @@ int main_sched_sedf(int argc, char **arg
int extra = 0, opt_e = 0;
int weight = 0, opt_w = 0;
int opt, rc;
- int option_index = 0;
- static struct option long_options[] = {
+ static struct option opts[] = {
{"period", 1, 0, 'p'},
{"slice", 1, 0, 's'},
{"latency", 1, 0, 'l'},
{"extra", 1, 0, 'e'},
{"weight", 1, 0, 'w'},
{"cpupool", 1, 0, 'c'},
- {"help", 0, 0, 'h'},
+ COMMON_LONG_OPTS,
{0, 0, 0, 0}
};
- while (1) {
- opt = getopt_long(argc, argv, "d:p:s:l:e:w:c:h", long_options,
- &option_index);
- if (opt == -1)
- break;
+ while ((opt = def_getopt(argc, argv, "d:p:s:l:e:w:c:h", opts, "sched-sedf", 0)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -5127,9 +5079,6 @@ int main_sched_sedf(int argc, char **arg
case 'c':
cpupool = optarg;
break;
- case 'h':
- help("sched-sedf");
- return 0;
}
}
@@ -5197,7 +5146,7 @@ int main_domid(int argc, char **argv)
int opt;
const char *domname = NULL;
- if ((opt = def_getopt(argc, argv, "", "domid", 1)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "domid", 1)) != -1)
return opt;
domname = argv[optind];
@@ -5219,7 +5168,7 @@ int main_domname(int argc, char **argv)
char *domname = NULL;
char *endptr = NULL;
- if ((opt = def_getopt(argc, argv, "", "domname", 1)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "domname", 1)) != -1)
return opt;
domid = strtol(argv[optind], &endptr, 10);
@@ -5247,7 +5196,7 @@ int main_rename(int argc, char **argv)
int opt;
const char *dom, *new_name;
- if ((opt = def_getopt(argc, argv, "", "rename", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "rename", 2)) != -1)
return opt;
dom = argv[optind++];
@@ -5271,7 +5220,7 @@ int main_trigger(int argc, char **argv)
const char *trigger_name = NULL;
libxl_trigger trigger;
- if ((opt = def_getopt(argc, argv, "", "trigger", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "trigger", 2)) != -1)
return opt;
domid = find_domain(argv[optind++]);
@@ -5301,7 +5250,7 @@ int main_sysrq(int argc, char **argv)
int opt;
const char *sysrq = NULL;
- if ((opt = def_getopt(argc, argv, "", "sysrq", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "sysrq", 2)) != -1)
return opt;
domid = find_domain(argv[optind++]);
@@ -5324,7 +5273,7 @@ int main_debug_keys(int argc, char **arg
int opt;
char *keys;
- if ((opt = def_getopt(argc, argv, "", "debug-keys", 1)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "debug-keys", 1)) != -1)
return opt;
keys = argv[optind];
@@ -5344,7 +5293,7 @@ int main_dmesg(int argc, char **argv)
char *line;
int opt, ret = 1;
- while ((opt = def_getopt(argc, argv, "c", "dmesg", 0)) != -1) {
+ while ((opt = def_getopt(argc, argv, "c", NULL, "dmesg", 0)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -5370,7 +5319,7 @@ int main_top(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", "top", 0)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "top", 0)) != -1)
return opt;
return system("xentop");
@@ -5387,7 +5336,7 @@ int main_networkattach(int argc, char **
int i;
unsigned int val;
- if ((opt = def_getopt(argc, argv, "", "network-attach", 1)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "network-attach", 1)) != -1)
return opt;
if (argc-optind > 11) {
@@ -5474,7 +5423,7 @@ int main_networklist(int argc, char **ar
libxl_nicinfo nicinfo;
int nb, i;
- if ((opt = def_getopt(argc, argv, "", "network-list", 1)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "network-list", 1)) != -1)
return opt;
/* Idx BE MAC Hdl Sta evch txr/rxr BE-path */
@@ -5511,7 +5460,7 @@ int main_networkdetach(int argc, char **
int opt;
libxl_device_nic nic;
- if ((opt = def_getopt(argc, argv, "", "network-detach", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "network-detach", 2)) != -1)
return opt;
domid = find_domain(argv[optind]);
@@ -5542,7 +5491,7 @@ int main_blockattach(int argc, char **ar
libxl_device_disk disk = { 0 };
XLU_Config *config = 0;
- if ((opt = def_getopt(argc, argv, "", "block-attach", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "block-attach", 2)) != -1)
return opt;
if (domain_qualifier_to_domid(argv[optind], &fe_domid, 0) < 0) {
@@ -5577,7 +5526,7 @@ int main_blocklist(int argc, char **argv
libxl_device_disk *disks;
libxl_diskinfo diskinfo;
- if ((opt = def_getopt(argc, argv, "", "block-list", 1)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "block-list", 1)) != -1)
return opt;
printf("%-5s %-3s %-6s %-5s %-6s %-8s %-30s\n",
@@ -5613,7 +5562,7 @@ int main_blockdetach(int argc, char **ar
int opt, rc = 0;
libxl_device_disk disk;
- if ((opt = def_getopt(argc, argv, "", "block-detach", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "block-detach", 2)) != -1)
return opt;
domid = find_domain(argv[optind]);
@@ -5797,7 +5746,7 @@ int main_uptime(int argc, char **argv)
int nb_doms = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "s", "uptime", 1)) != -1) {
+ while ((opt = def_getopt(argc, argv, "s", NULL, "uptime", 1)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -5824,7 +5773,7 @@ int main_tmem_list(int argc, char **argv
int all = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "al", "tmem-list", 0)) != -1) {
+ while ((opt = def_getopt(argc, argv, "al", NULL, "tmem-list", 0)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -5865,7 +5814,7 @@ int main_tmem_freeze(int argc, char **ar
int all = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "a", "tmem-freeze", 0)) != -1) {
+ while ((opt = def_getopt(argc, argv, "a", NULL, "tmem-freeze", 0)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -5898,7 +5847,7 @@ int main_tmem_thaw(int argc, char **argv
int all = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "a", "tmem-thaw", 0)) != -1) {
+ while ((opt = def_getopt(argc, argv, "a", NULL, "tmem-thaw", 0)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -5933,7 +5882,7 @@ int main_tmem_set(int argc, char **argv)
int all = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "aw:c:p:", "tmem-set", 0)) != -1) {
+ while ((opt = def_getopt(argc, argv, "aw:c:p:", NULL, "tmem-set", 0)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -5994,7 +5943,7 @@ int main_tmem_shared_auth(int argc, char
int all = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "au:A:", "tmem-shared-auth", 0)) != -1) {
+ while ((opt = def_getopt(argc, argv, "au:A:", NULL, "tmem-shared-auth", 0)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
@@ -6044,7 +5993,7 @@ int main_tmem_freeable(int argc, char **
int opt;
int mb;
- if ((opt = def_getopt(argc, argv, "", "tmem-freeable", 0)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "tmem-freeable", 0)) != -1)
return opt;
mb = libxl_tmem_freeable(ctx);
@@ -6061,11 +6010,10 @@ int main_cpupoolcreate(int argc, char **
const char *p;
char extra_config[1024];
int opt;
- int option_index = 0;
- static struct option long_options[] = {
- {"help", 0, 0, 'h'},
+ static struct option opts[] = {
{"defconfig", 1, 0, 'f'},
{"dryrun", 0, 0, 'n'},
+ COMMON_LONG_OPTS,
{0, 0, 0, 0}
};
int ret;
@@ -6083,26 +6031,18 @@ int main_cpupoolcreate(int argc, char **
libxl_bitmap cpumap;
libxl_uuid uuid;
libxl_cputopology *topology;
- int rc = -ERROR_FAIL;
-
- while (1) {
- opt = getopt_long(argc, argv, "hnf:", long_options, &option_index);
- if (opt == -1)
- break;
-
+ int rc = -ERROR_FAIL;
+
+ while ((opt = def_getopt(argc, argv, "hnf:", opts, "cpupool-create", 0)) != -1) {
switch (opt) {
+ case 0: case 2:
+ return opt;
case 'f':
filename = optarg;
break;
- case 'h':
- help("cpupool-create");
- return 0;
case 'n':
dryrun_only = 1;
break;
- default:
- fprintf(stderr, "option `%c' not supported.\n", optopt);
- break;
}
}
@@ -6265,10 +6205,9 @@ out:
int main_cpupoollist(int argc, char **argv)
{
int opt;
- int option_index = 0;
- static struct option long_options[] = {
- {"help", 0, 0, 'h'},
+ static struct option opts[] = {
{"cpus", 0, 0, 'c'},
+ COMMON_LONG_OPTS,
{0, 0, 0, 0}
};
int opt_cpus = 0;
@@ -6279,28 +6218,16 @@ int main_cpupoollist(int argc, char **ar
char *name;
int ret = 0;
- while (1) {
- opt = getopt_long(argc, argv, "hc", long_options, &option_index);
- if (opt == -1)
+ while ((opt = def_getopt(argc, argv, "hc", opts, "cpupool-list", 1)) != -1) {
+ switch (opt) {
+ case 0: case 2:
break;
-
- switch (opt) {
- case 'h':
- help("cpupool-list");
- return 0;
case 'c':
opt_cpus = 1;
break;
- default:
- fprintf(stderr, "option `%c' not supported.\n", optopt);
- break;
- }
- }
-
- if ((optind + 1) < argc) {
- help("cpupool-list");
- return -ERROR_FAIL;
- }
+ }
+ }
+
if (optind < argc) {
pool = argv[optind];
if (libxl_name_to_cpupoolid(ctx, pool, &poolid)) {
@@ -6358,7 +6285,7 @@ int main_cpupooldestroy(int argc, char *
const char *pool;
uint32_t poolid;
- if ((opt = def_getopt(argc, argv, "", "cpupool-destroy", 1)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "cpupool-destroy", 1)) != -1)
return opt;
pool = argv[optind];
@@ -6379,7 +6306,7 @@ int main_cpupoolrename(int argc, char **
const char *new_name;
uint32_t poolid;
- if ((opt = def_getopt(argc, argv, "", "cpupool-rename", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "cpupool-rename", 2)) != -1)
return opt;
pool = argv[optind++];
@@ -6409,7 +6336,7 @@ int main_cpupoolcpuadd(int argc, char **
int node;
int n;
- if ((opt = def_getopt(argc, argv, "", "cpupool-cpu-add", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "cpupool-cpu-add", 2)) != -1)
return opt;
pool = argv[optind++];
@@ -6453,7 +6380,7 @@ int main_cpupoolcpuremove(int argc, char
int node;
int n;
- if ((opt = def_getopt(argc, argv, "", "cpupool-cpu-remove", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "cpupool-cpu-remove", 2)) != -1)
return opt;
pool = argv[optind++];
@@ -6496,7 +6423,7 @@ int main_cpupoolmigrate(int argc, char *
const char *dom;
uint32_t domid;
- if ((opt = def_getopt(argc, argv, "", "cpupool-migrate", 2)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "cpupool-migrate", 2)) != -1)
return opt;
dom = argv[optind++];
@@ -6536,7 +6463,7 @@ int main_cpupoolnumasplit(int argc, char
libxl_cputopology *topology;
libxl_dominfo info;
- if ((opt = def_getopt(argc, argv, "", "cpupool-numa-split", 0)) != -1)
+ if ((opt = def_getopt(argc, argv, "", NULL, "cpupool-numa-split", 0)) != -1)
return opt;
ret = 0;
@@ -6789,7 +6716,7 @@ int main_remus(int argc, char **argv)
r_info.blackhole = 0;
r_info.compression = 1;
- while ((opt = def_getopt(argc, argv, "bui:s:e", "remus", 2)) != -1) {
+ while ((opt = def_getopt(argc, argv, "bui:s:e", NULL, "remus", 2)) != -1) {
switch (opt) {
case 0: case 2:
return opt;
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 5 of 5] xl: Introduce helper macro for option parsing
2012-10-15 10:09 [PATCH 0 of 5] xl shutdown compatibility with xm Ian Campbell
` (3 preceding siblings ...)
2012-10-15 10:09 ` [PATCH 4 of 5] xl: allow def_getopt to handle long options Ian Campbell
@ 2012-10-15 10:09 ` Ian Campbell
2012-10-17 16:13 ` Ian Jackson
2012-10-15 10:32 ` [PATCH 0 of 5] xl shutdown compatibility with xm Sander Eikelenboom
2012-10-18 8:35 ` Ian Campbell
6 siblings, 1 reply; 31+ messages in thread
From: Ian Campbell @ 2012-10-15 10:09 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, linux
# HG changeset patch
# User Ian Campbell <ijc@hellion.org.uk>
# Date 1350295720 -3600
# Node ID 22688187afa84747de9b820cb29addbd65385042
# Parent bbb839ceb966cc265e5d2ca530ba0f2bf823176a
xl: Introduce helper macro for option parsing.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r bbb839ceb966 -r 22688187afa8 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Mon Oct 15 11:08:39 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Mon Oct 15 11:08:40 2012 +0100
@@ -2281,6 +2281,11 @@ static int def_getopt(int argc, char * c
return -1;
}
+#define FOREACH_OPT(_opt, _opts, _lopts, _help, _req) \
+ while (((_opt) = def_getopt(argc, argv, (_opts), (_lopts), \
+ (_help), (_req))) != -1) \
+ switch (opt)
+
static int set_memory_max(uint32_t domid, const char *mem)
{
int64_t memorykb;
@@ -2304,8 +2309,10 @@ int main_memmax(int argc, char **argv)
char *mem;
int rc;
- if ((opt = def_getopt(argc, argv, "", NULL, "mem-max", 2)) != -1)
+ FOREACH_OPT(opt, "", NULL, "mem-max", 2) {
+ case 0: case 2:
return opt;
+ }
domid = find_domain(argv[optind]);
mem = argv[optind + 1];
@@ -2338,8 +2345,10 @@ int main_memset(int argc, char **argv)
int opt = 0;
const char *mem;
- if ((opt = def_getopt(argc, argv, "", NULL, "mem-set", 2)) != -1)
+ FOREACH_OPT(opt, "", NULL, "mem-set", 2) {
+ case 0: case 2:
return opt;
+ }
domid = find_domain(argv[optind]);
mem = argv[optind + 1];
@@ -2377,8 +2386,10 @@ int main_cd_eject(int argc, char **argv)
int opt = 0;
const char *virtdev;
- if ((opt = def_getopt(argc, argv, "", NULL, "cd-eject", 2)) != -1)
- return opt;
+ FOREACH_OPT(opt, "", NULL, "cd-eject", 2) {
+ case 0: case 2:
+ return opt;
+ }
domid = find_domain(argv[optind]);
virtdev = argv[optind + 1];
@@ -2394,8 +2405,10 @@ int main_cd_insert(int argc, char **argv
const char *virtdev;
char *file = NULL; /* modified by cd_insert tokenising it */
- if ((opt = def_getopt(argc, argv, "", NULL, "cd-insert", 3)) != -1)
+ FOREACH_OPT(opt, "", NULL, "cd-insert", 3) {
+ case 0: case 2:
return opt;
+ }
domid = find_domain(argv[optind]);
virtdev = argv[optind + 1];
@@ -2411,24 +2424,22 @@ int main_console(int argc, char **argv)
int opt = 0, num = 0;
libxl_console_type type = 0;
- while ((opt = def_getopt(argc, argv, "n:t:", NULL, "console", 1)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 't':
- if (!strcmp(optarg, "pv"))
- type = LIBXL_CONSOLE_TYPE_PV;
- else if (!strcmp(optarg, "serial"))
- type = LIBXL_CONSOLE_TYPE_SERIAL;
- else {
- fprintf(stderr, "console type supported are: pv, serial\n");
- return 2;
- }
- break;
- case 'n':
- num = atoi(optarg);
- break;
+ FOREACH_OPT(opt, "n:t:", NULL, "console", 1) {
+ case 0: case 2:
+ return opt;
+ case 't':
+ if (!strcmp(optarg, "pv"))
+ type = LIBXL_CONSOLE_TYPE_PV;
+ else if (!strcmp(optarg, "serial"))
+ type = LIBXL_CONSOLE_TYPE_SERIAL;
+ else {
+ fprintf(stderr, "console type supported are: pv, serial\n");
+ return 2;
}
+ break;
+ case 'n':
+ num = atoi(optarg);
+ break;
}
domid = find_domain(argv[optind]);
@@ -2451,14 +2462,12 @@ int main_vncviewer(int argc, char **argv
uint32_t domid;
int opt, autopass = 0;
- while ((opt = def_getopt(argc, argv, "ah", opts, "vncviewer", 1)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'a':
- autopass = 1;
- break;
- }
+ FOREACH_OPT(opt, "ah", opts, "vncviewer", 1) {
+ case 0: case 2:
+ return opt;
+ case 'a':
+ autopass = 1;
+ break;
}
domid = find_domain(argv[optind]);
@@ -2491,8 +2500,10 @@ int main_pcilist(int argc, char **argv)
uint32_t domid;
int opt;
- if ((opt = def_getopt(argc, argv, "", NULL, "pci-list", 1)) != -1)
+ FOREACH_OPT(opt, "", NULL, "pci-list", 1) {
+ case 0: case 2:
return opt;
+ }
domid = find_domain(argv[optind]);
@@ -2530,14 +2541,12 @@ int main_pcidetach(int argc, char **argv
int force = 0;
const char *bdf = NULL;
- while ((opt = def_getopt(argc, argv, "f", NULL, "pci-detach", 2)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'f':
- force = 1;
- break;
- }
+ FOREACH_OPT(opt, "f", NULL, "pci-detach", 2) {
+ case 0: case 2:
+ return opt;
+ case 'f':
+ force = 1;
+ break;
}
domid = find_domain(argv[optind]);
@@ -2572,8 +2581,10 @@ int main_pciattach(int argc, char **argv
int opt;
const char *bdf = NULL, *vs = NULL;
- if ((opt = def_getopt(argc, argv, "", NULL, "pci-attach", 2)) != -1)
+ FOREACH_OPT(opt, "", NULL, "pci-attach", 2) {
+ case 0: case 2:
return opt;
+ }
domid = find_domain(argv[optind]);
bdf = argv[optind + 1];
@@ -2606,8 +2617,10 @@ int main_pciassignable_list(int argc, ch
{
int opt;
- if ((opt = def_getopt(argc, argv, "", NULL, "pci-assignable-list", 0)) != -1)
+ FOREACH_OPT(opt, "", NULL, "pci-assignable-list", 0) {
+ case 0: case 2:
return opt;
+ }
pciassignable_list();
return 0;
@@ -2638,11 +2651,9 @@ int main_pciassignable_add(int argc, cha
int opt;
const char *bdf = NULL;
- while ((opt = def_getopt(argc, argv, "", NULL, "pci-assignable-add", 1)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- }
+ FOREACH_OPT(opt, "", NULL, "pci-assignable-add", 1) {
+ case 0: case 2:
+ return opt;
}
bdf = argv[optind];
@@ -2677,14 +2688,12 @@ int main_pciassignable_remove(int argc,
const char *bdf = NULL;
int rebind = 0;
- while ((opt = def_getopt(argc, argv, "r", NULL, "pci-assignable-remove", 1)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'r':
- rebind=1;
- break;
- }
+ FOREACH_OPT(opt, "r", NULL, "pci-assignable-remove", 1) {
+ case 0: case 2:
+ return opt;
+ case 'r':
+ rebind=1;
+ break;
}
bdf = argv[optind];
@@ -3495,34 +3504,31 @@ int main_restore(int argc, char **argv)
{0, 0, 0, 0}
};
- while ((opt = def_getopt(argc, argv, "FhcpdeVA",
- opts, "restore", 1)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'c':
- console_autoconnect = 1;
- break;
- case 'p':
- paused = 1;
- break;
- case 'd':
- debug = 1;
- break;
- case 'F':
- daemonize = 0;
- break;
- case 'e':
- daemonize = 0;
- monitor = 0;
- break;
- case 'V':
- vnc = 1;
- break;
- case 'A':
- vnc = vncautopass = 1;
- break;
- }
+ FOREACH_OPT(opt, "FhcpdeVA", opts, "restore", 1) {
+ case 0: case 2:
+ return opt;
+ case 'c':
+ console_autoconnect = 1;
+ break;
+ case 'p':
+ paused = 1;
+ break;
+ case 'd':
+ debug = 1;
+ break;
+ case 'F':
+ daemonize = 0;
+ break;
+ case 'e':
+ daemonize = 0;
+ monitor = 0;
+ break;
+ case 'V':
+ vnc = 1;
+ break;
+ case 'A':
+ vnc = vncautopass = 1;
+ break;
}
if (argc-optind == 1) {
@@ -3559,24 +3565,22 @@ int main_migrate_receive(int argc, char
int debug = 0, daemonize = 1, monitor = 1, remus = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "Fedr", NULL, "migrate-receive", 0)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'F':
- daemonize = 0;
- break;
- case 'e':
- daemonize = 0;
- monitor = 0;
- break;
- case 'd':
- debug = 1;
- break;
- case 'r':
- remus = 1;
- break;
- }
+ FOREACH_OPT(opt, "Fedr", NULL, "migrate-receive", 0) {
+ case 0: case 2:
+ return opt;
+ case 'F':
+ daemonize = 0;
+ break;
+ case 'e':
+ daemonize = 0;
+ monitor = 0;
+ break;
+ case 'd':
+ debug = 1;
+ break;
+ case 'r':
+ remus = 1;
+ break;
}
if (argc-optind != 0) {
@@ -3598,14 +3602,12 @@ int main_save(int argc, char **argv)
int checkpoint = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "c", NULL, "save", 2)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'c':
- checkpoint = 1;
- break;
- }
+ FOREACH_OPT(opt, "c", NULL, "save", 2) {
+ case 0: case 2:
+ return opt;
+ case 'c':
+ checkpoint = 1;
+ break;
}
if (argc-optind > 3) {
@@ -3631,27 +3633,25 @@ int main_migrate(int argc, char **argv)
char *host;
int opt, daemonize = 1, monitor = 1, debug = 0;
- while ((opt = def_getopt(argc, argv, "FC:s:ed", NULL, "migrate", 2)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'C':
- config_filename = optarg;
- break;
- case 's':
- ssh_command = optarg;
- break;
- case 'F':
- daemonize = 0;
- break;
- case 'e':
- daemonize = 0;
- monitor = 0;
- break;
- case 'd':
- debug = 1;
- break;
- }
+ FOREACH_OPT(opt, "FC:s:ed", NULL, "migrate", 2) {
+ case 0: case 2:
+ return opt;
+ case 'C':
+ config_filename = optarg;
+ break;
+ case 's':
+ ssh_command = optarg;
+ break;
+ case 'F':
+ daemonize = 0;
+ break;
+ case 'e':
+ daemonize = 0;
+ monitor = 0;
+ break;
+ case 'd':
+ debug = 1;
+ break;
}
domid = find_domain(argv[optind]);
@@ -3675,8 +3675,10 @@ int main_dump_core(int argc, char **argv
{
int opt;
- if ((opt = def_getopt(argc, argv, "", NULL, "dump-core", 2)) != -1)
+ FOREACH_OPT(opt, "", NULL, "dump-core", 2) {
+ case 0: case 2:
return opt;
+ }
core_dump_domain(find_domain(argv[optind]), argv[optind + 1]);
return 0;
@@ -3686,8 +3688,10 @@ int main_pause(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", NULL, "pause", 1)) != -1)
+ FOREACH_OPT(opt, "", NULL, "pause", 1) {
+ case 0: case 2:
return opt;
+ }
pause_domain(find_domain(argv[optind]));
@@ -3698,8 +3702,10 @@ int main_unpause(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", NULL, "unpause", 1)) != -1)
+ FOREACH_OPT(opt, "", NULL, "unpause", 1) {
+ case 0: case 2:
return opt;
+ }
unpause_domain(find_domain(argv[optind]));
@@ -3710,8 +3716,10 @@ int main_destroy(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", NULL, "destroy", 1)) != -1)
+ FOREACH_OPT(opt, "", NULL, "destroy", 1) {
+ case 0: case 2:
return opt;
+ }
destroy_domain(find_domain(argv[optind]));
return 0;
@@ -3815,23 +3823,18 @@ int main_list(int argc, char **argv)
libxl_dominfo *info, *info_free=0;
int nb_domain, rc;
- while ((opt = def_getopt(argc, argv, "lvhZ", opts, "list", 0)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'l':
- details = 1;
- break;
- case 'h':
- help("list");
- return 0;
- case 'v':
- verbose = 1;
- break;
- case 'Z':
- context = 1;
- break;
- }
+ FOREACH_OPT(opt, "lvhZ", opts, "list", 0) {
+ case 0: case 2:
+ return opt;
+ case 'l':
+ details = 1;
+ break;
+ case 'v':
+ verbose = 1;
+ break;
+ case 'Z':
+ context = 1;
+ break;
}
if (optind >= argc) {
@@ -3877,8 +3880,10 @@ int main_vm_list(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", NULL, "vm-list", 0)) != -1)
+ FOREACH_OPT(opt, "", NULL, "vm-list", 0) {
+ case 0: case 2:
return opt;
+ }
list_vm();
return 0;
@@ -3908,45 +3913,40 @@ int main_create(int argc, char **argv)
argc--; argv++;
}
- while ((opt = def_getopt(argc, argv, "Fhnqf:pcdeVA", opts, "create", 0)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'f':
- filename = optarg;
- break;
- case 'p':
- paused = 1;
- break;
- case 'c':
- console_autoconnect = 1;
- break;
- case 'd':
- debug = 1;
- break;
- case 'F':
- daemonize = 0;
- break;
- case 'e':
- daemonize = 0;
- monitor = 0;
- break;
- case 'h':
- help("create");
- return 0;
- case 'n':
- dryrun_only = 1;
- break;
- case 'q':
- quiet = 1;
- break;
- case 'V':
- vnc = 1;
- break;
- case 'A':
- vnc = vncautopass = 1;
- break;
- }
+ FOREACH_OPT(opt, "Fhnqf:pcdeVA", opts, "create", 0) {
+ case 0: case 2:
+ return opt;
+ case 'f':
+ filename = optarg;
+ break;
+ case 'p':
+ paused = 1;
+ break;
+ case 'c':
+ console_autoconnect = 1;
+ break;
+ case 'd':
+ debug = 1;
+ break;
+ case 'F':
+ daemonize = 0;
+ break;
+ case 'e':
+ daemonize = 0;
+ monitor = 0;
+ break;
+ case 'n':
+ dryrun_only = 1;
+ break;
+ case 'q':
+ quiet = 1;
+ break;
+ case 'V':
+ vnc = 1;
+ break;
+ case 'A':
+ vnc = vncautopass = 1;
+ break;
}
extra_config[0] = '\0';
@@ -4014,17 +4014,15 @@ int main_config_update(int argc, char **
argc--; argv++;
}
- while ((opt = def_getopt(argc, argv, "dhqf:", opts, "config_update", 0)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'd':
- debug = 1;
- break;
- case 'f':
- filename = optarg;
- break;
- }
+ FOREACH_OPT(opt, "dhqf:", opts, "config_update", 0) {
+ case 0: case 2:
+ return opt;
+ case 'd':
+ debug = 1;
+ break;
+ case 'f':
+ filename = optarg;
+ break;
}
extra_config[0] = '\0';
@@ -4112,8 +4110,11 @@ int main_button_press(int argc, char **a
fprintf(stderr, "WARNING: \"button-press\" is deprecated. "
"Please use \"trigger\"\n");
- if ((opt = def_getopt(argc, argv, "", NULL, "button-press", 2)) != -1)
+
+ FOREACH_OPT(opt, "", NULL, "button-press", 2) {
+ case 0: case 2:
return opt;
+ }
button_press(find_domain(argv[optind]), argv[optind + 1]);
@@ -4253,8 +4254,10 @@ int main_vcpulist(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", NULL, "cpu-list", 0)) != -1)
+ FOREACH_OPT(opt, "", NULL, "cpu-list", 0) {
+ case 0: case 2:
return opt;
+ }
vcpulist(argc - optind, argv + optind);
return 0;
@@ -4314,8 +4317,10 @@ int main_vcpupin(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", NULL, "vcpu-pin", 3)) != -1)
+ FOREACH_OPT(opt, "", NULL, "vcpu-pin", 3) {
+ case 0: case 2:
return opt;
+ }
vcpupin(find_domain(argv[optind]), argv[optind+1] , argv[optind+2]);
return 0;
@@ -4350,8 +4355,10 @@ int main_vcpuset(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", NULL, "vcpu-set", 2)) != -1)
+ FOREACH_OPT(opt, "", NULL, "vcpu-set", 2) {
+ case 0: case 2:
return opt;
+ }
vcpuset(find_domain(argv[optind]), argv[optind+1]);
return 0;
@@ -4533,14 +4540,12 @@ int main_info(int argc, char **argv)
};
int numa = 0;
- while ((opt = def_getopt(argc, argv, "hn", opts, "info", 0)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'n':
- numa = 1;
- break;
- }
+ FOREACH_OPT(opt, "hn", opts, "info", 0) {
+ case 0: case 2:
+ return opt;
+ case 'n':
+ numa = 1;
+ break;
}
print_info(numa);
@@ -4574,8 +4579,10 @@ int main_sharing(int argc, char **argv)
libxl_dominfo *info, *info_free = NULL;
int nb_domain, rc;
- if ((opt = def_getopt(argc, argv, "", NULL, "sharing", 0)) != -1)
+ FOREACH_OPT(opt, "", NULL, "sharing", 0) {
+ case 0: case 2:
return opt;
+ }
if (optind >= argc) {
info = libxl_list_domain(ctx, &nb_domain);
@@ -4855,36 +4862,34 @@ int main_sched_credit(int argc, char **a
{0, 0, 0, 0}
};
- while ((opt = def_getopt(argc, argv, "d:w:c:p:t:r:hs", opts, "sched-credit", 0)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'd':
- dom = optarg;
- break;
- case 'w':
- weight = strtol(optarg, NULL, 10);
- opt_w = 1;
- break;
- case 'c':
- cap = strtol(optarg, NULL, 10);
- opt_c = 1;
- break;
- case 't':
- tslice = strtol(optarg, NULL, 10);
- opt_t = 1;
- break;
- case 'r':
- ratelimit = strtol(optarg, NULL, 10);
- opt_r = 1;
- break;
- case 's':
- opt_s = 1;
- break;
- case 'p':
- cpupool = optarg;
- break;
- }
+ FOREACH_OPT(opt, "d:w:c:p:t:r:hs", opts, "sched-credit", 0) {
+ case 0: case 2:
+ return opt;
+ case 'd':
+ dom = optarg;
+ break;
+ case 'w':
+ weight = strtol(optarg, NULL, 10);
+ opt_w = 1;
+ break;
+ case 'c':
+ cap = strtol(optarg, NULL, 10);
+ opt_c = 1;
+ break;
+ case 't':
+ tslice = strtol(optarg, NULL, 10);
+ opt_t = 1;
+ break;
+ case 'r':
+ ratelimit = strtol(optarg, NULL, 10);
+ opt_r = 1;
+ break;
+ case 's':
+ opt_s = 1;
+ break;
+ case 'p':
+ cpupool = optarg;
+ break;
}
if ((cpupool || opt_s) && (dom || opt_w || opt_c)) {
@@ -4974,21 +4979,19 @@ int main_sched_credit2(int argc, char **
{0, 0, 0, 0}
};
- while ((opt = def_getopt(argc, argv, "d:w:p:h", opts, "sched-credit2", 0)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'd':
- dom = optarg;
- break;
- case 'w':
- weight = strtol(optarg, NULL, 10);
- opt_w = 1;
- break;
- case 'p':
- cpupool = optarg;
- break;
- }
+ FOREACH_OPT(opt, "d:w:p:h", opts, "sched-credit2", 0) {
+ case 0: case 2:
+ return opt;
+ case 'd':
+ dom = optarg;
+ break;
+ case 'w':
+ weight = strtol(optarg, NULL, 10);
+ opt_w = 1;
+ break;
+ case 'p':
+ cpupool = optarg;
+ break;
}
if (cpupool && (dom || opt_w)) {
@@ -5049,37 +5052,35 @@ int main_sched_sedf(int argc, char **arg
{0, 0, 0, 0}
};
- while ((opt = def_getopt(argc, argv, "d:p:s:l:e:w:c:h", opts, "sched-sedf", 0)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'd':
- dom = optarg;
- break;
- case 'p':
- period = strtol(optarg, NULL, 10);
- opt_p = 1;
- break;
- case 's':
- slice = strtol(optarg, NULL, 10);
- opt_s = 1;
- break;
- case 'l':
- latency = strtol(optarg, NULL, 10);
- opt_l = 1;
- break;
- case 'e':
- extra = strtol(optarg, NULL, 10);
- opt_e = 1;
- break;
- case 'w':
- weight = strtol(optarg, NULL, 10);
- opt_w = 1;
- break;
- case 'c':
- cpupool = optarg;
- break;
- }
+ FOREACH_OPT(opt, "d:p:s:l:e:w:c:h", opts, "sched-sedf", 0) {
+ case 0: case 2:
+ return opt;
+ case 'd':
+ dom = optarg;
+ break;
+ case 'p':
+ period = strtol(optarg, NULL, 10);
+ opt_p = 1;
+ break;
+ case 's':
+ slice = strtol(optarg, NULL, 10);
+ opt_s = 1;
+ break;
+ case 'l':
+ latency = strtol(optarg, NULL, 10);
+ opt_l = 1;
+ break;
+ case 'e':
+ extra = strtol(optarg, NULL, 10);
+ opt_e = 1;
+ break;
+ case 'w':
+ weight = strtol(optarg, NULL, 10);
+ opt_w = 1;
+ break;
+ case 'c':
+ cpupool = optarg;
+ break;
}
if (cpupool && (dom || opt_p || opt_s || opt_l || opt_e || opt_w)) {
@@ -5146,8 +5147,10 @@ int main_domid(int argc, char **argv)
int opt;
const char *domname = NULL;
- if ((opt = def_getopt(argc, argv, "", NULL, "domid", 1)) != -1)
+ FOREACH_OPT(opt, "", NULL, "domid", 1) {
+ case 0: case 2:
return opt;
+ }
domname = argv[optind];
@@ -5168,8 +5171,10 @@ int main_domname(int argc, char **argv)
char *domname = NULL;
char *endptr = NULL;
- if ((opt = def_getopt(argc, argv, "", NULL, "domname", 1)) != -1)
+ FOREACH_OPT(opt, "", NULL, "domname", 1) {
+ case 0: case 2:
return opt;
+ }
domid = strtol(argv[optind], &endptr, 10);
if (domid == 0 && !strcmp(endptr, argv[optind])) {
@@ -5196,8 +5201,10 @@ int main_rename(int argc, char **argv)
int opt;
const char *dom, *new_name;
- if ((opt = def_getopt(argc, argv, "", NULL, "rename", 2)) != -1)
+ FOREACH_OPT(opt, "", NULL, "rename", 2) {
+ case 0: case 2:
return opt;
+ }
dom = argv[optind++];
new_name = argv[optind];
@@ -5220,8 +5227,10 @@ int main_trigger(int argc, char **argv)
const char *trigger_name = NULL;
libxl_trigger trigger;
- if ((opt = def_getopt(argc, argv, "", NULL, "trigger", 2)) != -1)
+ FOREACH_OPT(opt, "", NULL, "trigger", 2) {
+ case 0: case 2:
return opt;
+ }
domid = find_domain(argv[optind++]);
@@ -5250,8 +5259,10 @@ int main_sysrq(int argc, char **argv)
int opt;
const char *sysrq = NULL;
- if ((opt = def_getopt(argc, argv, "", NULL, "sysrq", 2)) != -1)
+ FOREACH_OPT(opt, "", NULL, "sysrq", 2) {
+ case 0: case 2:
return opt;
+ }
domid = find_domain(argv[optind++]);
@@ -5273,8 +5284,10 @@ int main_debug_keys(int argc, char **arg
int opt;
char *keys;
- if ((opt = def_getopt(argc, argv, "", NULL, "debug-keys", 1)) != -1)
+ FOREACH_OPT(opt, "", NULL, "debug-keys", 1) {
+ case 0: case 2:
return opt;
+ }
keys = argv[optind];
@@ -5293,14 +5306,12 @@ int main_dmesg(int argc, char **argv)
char *line;
int opt, ret = 1;
- while ((opt = def_getopt(argc, argv, "c", NULL, "dmesg", 0)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'c':
- clear = 1;
- break;
- }
+ FOREACH_OPT(opt, "c", NULL, "dmesg", 0) {
+ case 0: case 2:
+ return opt;
+ case 'c':
+ clear = 1;
+ break;
}
cr = libxl_xen_console_read_start(ctx, clear);
@@ -5319,8 +5330,10 @@ int main_top(int argc, char **argv)
{
int opt;
- if ((opt = def_getopt(argc, argv, "", NULL, "top", 0)) != -1)
+ FOREACH_OPT(opt, "", NULL, "top", 0) {
+ case 0: case 2:
return opt;
+ }
return system("xentop");
}
@@ -5336,8 +5349,10 @@ int main_networkattach(int argc, char **
int i;
unsigned int val;
- if ((opt = def_getopt(argc, argv, "", NULL, "network-attach", 1)) != -1)
+ FOREACH_OPT(opt, "", NULL, "network-attach", 1) {
+ case 0: case 2:
return opt;
+ }
if (argc-optind > 11) {
help("network-attach");
@@ -5423,8 +5438,10 @@ int main_networklist(int argc, char **ar
libxl_nicinfo nicinfo;
int nb, i;
- if ((opt = def_getopt(argc, argv, "", NULL, "network-list", 1)) != -1)
+ FOREACH_OPT(opt, "", NULL, "network-list", 1) {
+ case 0: case 2:
return opt;
+ }
/* Idx BE MAC Hdl Sta evch txr/rxr BE-path */
printf("%-3s %-2s %-17s %-6s %-5s %-6s %5s/%-5s %-30s\n",
@@ -5460,8 +5477,10 @@ int main_networkdetach(int argc, char **
int opt;
libxl_device_nic nic;
- if ((opt = def_getopt(argc, argv, "", NULL, "network-detach", 2)) != -1)
+ FOREACH_OPT(opt, "", NULL, "network-detach", 2) {
+ case 0: case 2:
return opt;
+ }
domid = find_domain(argv[optind]);
@@ -5491,8 +5510,10 @@ int main_blockattach(int argc, char **ar
libxl_device_disk disk = { 0 };
XLU_Config *config = 0;
- if ((opt = def_getopt(argc, argv, "", NULL, "block-attach", 2)) != -1)
+ FOREACH_OPT(opt, "", NULL, "block-attach", 2) {
+ case 0: case 2:
return opt;
+ }
if (domain_qualifier_to_domid(argv[optind], &fe_domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
@@ -5526,8 +5547,10 @@ int main_blocklist(int argc, char **argv
libxl_device_disk *disks;
libxl_diskinfo diskinfo;
- if ((opt = def_getopt(argc, argv, "", NULL, "block-list", 1)) != -1)
+ FOREACH_OPT(opt, "", NULL, "block-list", 1) {
+ case 0: case 2:
return opt;
+ }
printf("%-5s %-3s %-6s %-5s %-6s %-8s %-30s\n",
"Vdev", "BE", "handle", "state", "evt-ch", "ring-ref", "BE-path");
@@ -5562,8 +5585,10 @@ int main_blockdetach(int argc, char **ar
int opt, rc = 0;
libxl_device_disk disk;
- if ((opt = def_getopt(argc, argv, "", NULL, "block-detach", 2)) != -1)
+ FOREACH_OPT(opt, "", NULL, "block-detach", 2) {
+ case 0: case 2:
return opt;
+ }
domid = find_domain(argv[optind]);
@@ -5746,14 +5771,12 @@ int main_uptime(int argc, char **argv)
int nb_doms = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "s", NULL, "uptime", 1)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 's':
- short_mode = 1;
- break;
- }
+ FOREACH_OPT(opt, "s", NULL, "uptime", 1) {
+ case 0: case 2:
+ return opt;
+ case 's':
+ short_mode = 1;
+ break;
}
for (;(dom = argv[optind]) != NULL; nb_doms++,optind++)
@@ -5773,17 +5796,15 @@ int main_tmem_list(int argc, char **argv
int all = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "al", NULL, "tmem-list", 0)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'l':
- use_long = 1;
- break;
- case 'a':
- all = 1;
- break;
- }
+ FOREACH_OPT(opt, "al", NULL, "tmem-list", 0) {
+ case 0: case 2:
+ return opt;
+ case 'l':
+ use_long = 1;
+ break;
+ case 'a':
+ all = 1;
+ break;
}
dom = argv[optind];
@@ -5814,14 +5835,12 @@ int main_tmem_freeze(int argc, char **ar
int all = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "a", NULL, "tmem-freeze", 0)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'a':
- all = 1;
- break;
- }
+ FOREACH_OPT(opt, "a", NULL, "tmem-freeze", 0) {
+ case 0: case 2:
+ return opt;
+ case 'a':
+ all = 1;
+ break;
}
dom = argv[optind];
@@ -5847,14 +5866,12 @@ int main_tmem_thaw(int argc, char **argv
int all = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "a", NULL, "tmem-thaw", 0)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'a':
- all = 1;
- break;
- }
+ FOREACH_OPT(opt, "a", NULL, "tmem-thaw", 0) {
+ case 0: case 2:
+ return opt;
+ case 'a':
+ all = 1;
+ break;
}
dom = argv[optind];
@@ -5882,26 +5899,24 @@ int main_tmem_set(int argc, char **argv)
int all = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "aw:c:p:", NULL, "tmem-set", 0)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'a':
- all = 1;
- break;
- case 'w':
- weight = strtol(optarg, NULL, 10);
- opt_w = 1;
- break;
- case 'c':
- cap = strtol(optarg, NULL, 10);
- opt_c = 1;
- break;
- case 'p':
- compress = strtol(optarg, NULL, 10);
- opt_p = 1;
- break;
- }
+ FOREACH_OPT(opt, "aw:c:p:", NULL, "tmem-set", 0) {
+ case 0: case 2:
+ return opt;
+ case 'a':
+ all = 1;
+ break;
+ case 'w':
+ weight = strtol(optarg, NULL, 10);
+ opt_w = 1;
+ break;
+ case 'c':
+ cap = strtol(optarg, NULL, 10);
+ opt_c = 1;
+ break;
+ case 'p':
+ compress = strtol(optarg, NULL, 10);
+ opt_p = 1;
+ break;
}
dom = argv[optind];
@@ -5943,20 +5958,18 @@ int main_tmem_shared_auth(int argc, char
int all = 0;
int opt;
- while ((opt = def_getopt(argc, argv, "au:A:", NULL, "tmem-shared-auth", 0)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'a':
- all = 1;
- break;
- case 'u':
- uuid = optarg;
- break;
- case 'A':
- autharg = optarg;
- break;
- }
+ FOREACH_OPT(opt, "au:A:", NULL, "tmem-shared-auth", 0) {
+ case 0: case 2:
+ return opt;
+ case 'a':
+ all = 1;
+ break;
+ case 'u':
+ uuid = optarg;
+ break;
+ case 'A':
+ autharg = optarg;
+ break;
}
dom = argv[optind];
@@ -5993,8 +6006,10 @@ int main_tmem_freeable(int argc, char **
int opt;
int mb;
- if ((opt = def_getopt(argc, argv, "", NULL, "tmem-freeable", 0)) != -1)
+ FOREACH_OPT(opt, "", NULL, "tmem-freeale", 0) {
+ case 0: case 2:
return opt;
+ }
mb = libxl_tmem_freeable(ctx);
if (mb == -1)
@@ -6033,17 +6048,15 @@ int main_cpupoolcreate(int argc, char **
libxl_cputopology *topology;
int rc = -ERROR_FAIL;
- while ((opt = def_getopt(argc, argv, "hnf:", opts, "cpupool-create", 0)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
- case 'f':
- filename = optarg;
- break;
- case 'n':
- dryrun_only = 1;
- break;
- }
+ FOREACH_OPT(opt, "hnf:", opts, "cpupool-create", 0) {
+ case 0: case 2:
+ return opt;
+ case 'f':
+ filename = optarg;
+ break;
+ case 'n':
+ dryrun_only = 1;
+ break;
}
memset(extra_config, 0, sizeof(extra_config));
@@ -6218,14 +6231,12 @@ int main_cpupoollist(int argc, char **ar
char *name;
int ret = 0;
- while ((opt = def_getopt(argc, argv, "hc", opts, "cpupool-list", 1)) != -1) {
- switch (opt) {
- case 0: case 2:
- break;
- case 'c':
- opt_cpus = 1;
- break;
- }
+ FOREACH_OPT(opt, "hc", opts, "cpupool-list", 1) {
+ case 0: case 2:
+ break;
+ case 'c':
+ opt_cpus = 1;
+ break;
}
if (optind < argc) {
@@ -6285,8 +6296,10 @@ int main_cpupooldestroy(int argc, char *
const char *pool;
uint32_t poolid;
- if ((opt = def_getopt(argc, argv, "", NULL, "cpupool-destroy", 1)) != -1)
+ FOREACH_OPT(opt, "", NULL, "cpupool-destroy", 1) {
+ case 0: case 2:
return opt;
+ }
pool = argv[optind];
@@ -6306,8 +6319,10 @@ int main_cpupoolrename(int argc, char **
const char *new_name;
uint32_t poolid;
- if ((opt = def_getopt(argc, argv, "", NULL, "cpupool-rename", 2)) != -1)
+ FOREACH_OPT(opt, "", NULL, "cpupool-rename", 2) {
+ case 0: case 2:
return opt;
+ }
pool = argv[optind++];
@@ -6336,8 +6351,10 @@ int main_cpupoolcpuadd(int argc, char **
int node;
int n;
- if ((opt = def_getopt(argc, argv, "", NULL, "cpupool-cpu-add", 2)) != -1)
+ FOREACH_OPT(opt, "", NULL, "cpupool-cpu-add", 2) {
+ case 0: case 2:
return opt;
+ }
pool = argv[optind++];
node = -1;
@@ -6380,8 +6397,10 @@ int main_cpupoolcpuremove(int argc, char
int node;
int n;
- if ((opt = def_getopt(argc, argv, "", NULL, "cpupool-cpu-remove", 2)) != -1)
+ FOREACH_OPT(opt, "", NULL, "cpupool-cpu-remove", 2) {
+ case 0: case 2:
return opt;
+ }
pool = argv[optind++];
node = -1;
@@ -6423,8 +6442,10 @@ int main_cpupoolmigrate(int argc, char *
const char *dom;
uint32_t domid;
- if ((opt = def_getopt(argc, argv, "", NULL, "cpupool-migrate", 2)) != -1)
+ FOREACH_OPT(opt, "", NULL, "cpupool-migrate", 2) {
+ case 0: case 2:
return opt;
+ }
dom = argv[optind++];
pool = argv[optind];
@@ -6463,8 +6484,11 @@ int main_cpupoolnumasplit(int argc, char
libxl_cputopology *topology;
libxl_dominfo info;
- if ((opt = def_getopt(argc, argv, "", NULL, "cpupool-numa-split", 0)) != -1)
+ FOREACH_OPT(opt, "", NULL, "cpupool-numa-split", 0) {
+ case 0: case 2:
return opt;
+ }
+
ret = 0;
poolinfo = libxl_list_cpupool(ctx, &n_pools);
@@ -6716,27 +6740,24 @@ int main_remus(int argc, char **argv)
r_info.blackhole = 0;
r_info.compression = 1;
- while ((opt = def_getopt(argc, argv, "bui:s:e", NULL, "remus", 2)) != -1) {
- switch (opt) {
- case 0: case 2:
- return opt;
-
- case 'i':
- r_info.interval = atoi(optarg);
- break;
- case 'b':
- r_info.blackhole = 1;
- break;
- case 'u':
- r_info.compression = 0;
- break;
- case 's':
- ssh_command = optarg;
- break;
- case 'e':
- daemonize = 0;
- break;
- }
+ FOREACH_OPT(opt, "bui:s:e", NULL, "remus", 2) {
+ case 0: case 2:
+ return opt;
+ case 'i':
+ r_info.interval = atoi(optarg);
+ break;
+ case 'b':
+ r_info.blackhole = 1;
+ break;
+ case 'u':
+ r_info.compression = 0;
+ break;
+ case 's':
+ ssh_command = optarg;
+ break;
+ case 'e':
+ daemonize = 0;
+ break;
}
domid = find_domain(argv[optind]);
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 5 of 5] xl: Introduce helper macro for option parsing
2012-10-15 10:09 ` [PATCH 5 of 5] xl: Introduce helper macro for option parsing Ian Campbell
@ 2012-10-17 16:13 ` Ian Jackson
2012-10-18 7:42 ` Ian Campbell
0 siblings, 1 reply; 31+ messages in thread
From: Ian Jackson @ 2012-10-17 16:13 UTC (permalink / raw)
To: Ian Campbell; +Cc: linux@eikelenboom.it, xen-devel@lists.xen.org
Ian Campbell writes ("[PATCH 5 of 5] xl: Introduce helper macro for option parsing"):
> xl: Introduce helper macro for option parsing.
The idea is a reasonable one, but I have some quibbles.
> +#define FOREACH_OPT(_opt, _opts, _lopts, _help, _req) \
> + while (((_opt) = def_getopt(argc, argv, (_opts), (_lopts), \
> + (_help), (_req))) != -1) \
> + switch (opt)
This macro name should have the word SWITCH in it too so you know it
has to take cases and that "break" doesn't do what you think.
FOREACH_SWITCH_OPT maybe.
> @@ -2304,8 +2309,10 @@ int main_memmax(int argc, char **argv)
> char *mem;
> int rc;
>
> - if ((opt = def_getopt(argc, argv, "", NULL, "mem-max", 2)) != -1)
> + FOREACH_OPT(opt, "", NULL, "mem-max", 2) {
> + case 0: case 2:
> return opt;
> + }
This error handling behaviour (?) is rather opaque. I RTFM
getopt_long and AFAICT it returns 0 only for certain long options and
it doesn't seem to mention returning 2.
Perhaps this would all be clearer if FOREACH_OPT had a big comment
explaining how to use it.
Thanks,
Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 5 of 5] xl: Introduce helper macro for option parsing
2012-10-17 16:13 ` Ian Jackson
@ 2012-10-18 7:42 ` Ian Campbell
0 siblings, 0 replies; 31+ messages in thread
From: Ian Campbell @ 2012-10-18 7:42 UTC (permalink / raw)
To: Ian Jackson; +Cc: linux@eikelenboom.it, xen-devel@lists.xen.org
On Wed, 2012-10-17 at 17:13 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH 5 of 5] xl: Introduce helper macro for option parsing"):
> > xl: Introduce helper macro for option parsing.
>
> The idea is a reasonable one, but I have some quibbles.
>
> > +#define FOREACH_OPT(_opt, _opts, _lopts, _help, _req) \
> > + while (((_opt) = def_getopt(argc, argv, (_opts), (_lopts), \
> > + (_help), (_req))) != -1) \
> > + switch (opt)
>
> This macro name should have the word SWITCH in it too so you know it
> has to take cases and that "break" doesn't do what you think.
> FOREACH_SWITCH_OPT maybe.
Good idea.
>
> > @@ -2304,8 +2309,10 @@ int main_memmax(int argc, char **argv)
> > char *mem;
> > int rc;
> >
> > - if ((opt = def_getopt(argc, argv, "", NULL, "mem-max", 2)) != -1)
> > + FOREACH_OPT(opt, "", NULL, "mem-max", 2) {
> > + case 0: case 2:
> > return opt;
> > + }
>
> This error handling behaviour (?) is rather opaque. I RTFM
> getopt_long and AFAICT it returns 0 only for certain long options and
> it doesn't seem to mention returning 2.
2 comes from our existing implementation of def_getopt, which returns it
in the "not enough args" case. I don't think any callers differentiate
between that and 0 which is the other error indication though.
> Perhaps this would all be clearer if FOREACH_OPT had a big comment
> explaining how to use it.
Yes, will do.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-15 10:09 [PATCH 0 of 5] xl shutdown compatibility with xm Ian Campbell
` (4 preceding siblings ...)
2012-10-15 10:09 ` [PATCH 5 of 5] xl: Introduce helper macro for option parsing Ian Campbell
@ 2012-10-15 10:32 ` Sander Eikelenboom
2012-10-15 10:37 ` Ian Campbell
2012-10-18 8:35 ` Ian Campbell
6 siblings, 1 reply; 31+ messages in thread
From: Sander Eikelenboom @ 2012-10-15 10:32 UTC (permalink / raw)
To: Ian Campbell; +Cc: ian.jackson, xen-devel
Hi Ian,
Great thanks !
Only thing i was wondering about:
Shouldn't the "-F" option be dropped in favour of always trying the "acpi fallback" when the pv shutdown fails.
This because the shutdown scripts still don't work for domains without pv shutdown and i don't see a down side to just trying that as fallback.
--
Sander
Monday, October 15, 2012, 12:09:49 PM, you wrote:
> The following implements xm compatibility for the xl shutdown command
> and a prerequisite bug fix.
> In particular add --all and --wait which are used by the xendomains
> initscript. It also adds the same options to xl reboot.
> Lastly it cleans up and simplifies option parsing.
> The xl shutdown/reboot patches should go into 4.2.1, the option
> parsing cleanups should not.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-15 10:32 ` [PATCH 0 of 5] xl shutdown compatibility with xm Sander Eikelenboom
@ 2012-10-15 10:37 ` Ian Campbell
2012-10-15 11:21 ` Sander Eikelenboom
0 siblings, 1 reply; 31+ messages in thread
From: Ian Campbell @ 2012-10-15 10:37 UTC (permalink / raw)
To: Sander Eikelenboom; +Cc: Ian Jackson, xen-devel@lists.xen.org
On Mon, 2012-10-15 at 11:32 +0100, Sander Eikelenboom wrote:
> Hi Ian,
>
> Great thanks !
> Only thing i was wondering about:
>
> Shouldn't the "-F" option be dropped in favour of always trying the
> "acpi fallback" when the pv shutdown fails.
>
> This because the shutdown scripts still don't work for domains without
> pv shutdown and i don't see a down side to just trying that as
> fallback.
It is guess OS dependent what the ACPI button press event does, it can
reboot, shutdown or hibernate etc depending on the OS type and its
configuration. (in theory I suppose it is completely arbitrary e.g. it
could be configured to eject the CD-ROM or something equally random).
Therefore the user needs to be aware of when they can safely use it.
Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-15 10:37 ` Ian Campbell
@ 2012-10-15 11:21 ` Sander Eikelenboom
2012-10-15 11:45 ` Ian Campbell
0 siblings, 1 reply; 31+ messages in thread
From: Sander Eikelenboom @ 2012-10-15 11:21 UTC (permalink / raw)
To: Ian Campbell; +Cc: Ian Jackson, xen-devel@lists.xen.org
Monday, October 15, 2012, 12:37:57 PM, you wrote:
> On Mon, 2012-10-15 at 11:32 +0100, Sander Eikelenboom wrote:
>> Hi Ian,
>>
>> Great thanks !
>> Only thing i was wondering about:
>>
>> Shouldn't the "-F" option be dropped in favour of always trying the
>> "acpi fallback" when the pv shutdown fails.
>>
>> This because the shutdown scripts still don't work for domains without
>> pv shutdown and i don't see a down side to just trying that as
>> fallback.
> It is guess OS dependent what the ACPI button press event does, it can
> reboot, shutdown or hibernate etc depending on the OS type and its
> configuration. (in theory I suppose it is completely arbitrary e.g. it
> could be configured to eject the CD-ROM or something equally random).
> Therefore the user needs to be aware of when they can safely use it.
Well yes and no:
- can't remember having (to make) that choice with xm ?
- On shutdown with xl as toolstack and when the guest doesn't support pv shutdown, the init.d/xendomains script doesn't even attempt to shutdown this guest by acpi fallback.
- As a result when using xl as toolstack, the guest is terminated non gracefully when the whole machine finally shutsdown, which seems less desirable then at least *trying* to shut it down gracefully by using the acpi button.
> Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-15 11:21 ` Sander Eikelenboom
@ 2012-10-15 11:45 ` Ian Campbell
2012-10-16 8:59 ` Sander Eikelenboom
2012-10-19 10:21 ` Ian Jackson
0 siblings, 2 replies; 31+ messages in thread
From: Ian Campbell @ 2012-10-15 11:45 UTC (permalink / raw)
To: Sander Eikelenboom; +Cc: Ian Jackson, xen-devel@lists.xen.org
On Mon, 2012-10-15 at 12:21 +0100, Sander Eikelenboom wrote:
> Monday, October 15, 2012, 12:37:57 PM, you wrote:
>
> > On Mon, 2012-10-15 at 11:32 +0100, Sander Eikelenboom wrote:
> >> Hi Ian,
> >>
> >> Great thanks !
> >> Only thing i was wondering about:
> >>
> >> Shouldn't the "-F" option be dropped in favour of always trying the
> >> "acpi fallback" when the pv shutdown fails.
> >>
> >> This because the shutdown scripts still don't work for domains without
> >> pv shutdown and i don't see a down side to just trying that as
> >> fallback.
>
> > It is guess OS dependent what the ACPI button press event does, it can
> > reboot, shutdown or hibernate etc depending on the OS type and its
> > configuration. (in theory I suppose it is completely arbitrary e.g. it
> > could be configured to eject the CD-ROM or something equally random).
>
> > Therefore the user needs to be aware of when they can safely use it.
>
> Well yes and no:
> - can't remember having (to make) that choice with xm ?
Looks like xend uses SCHEDOP_remote_shutdown. It's unclear to me why
this is better than just shooting the domain with destroy...
> - On shutdown with xl as toolstack and when the guest doesn't
> support pv shutdown, the init.d/xendomains script doesn't even attempt
> to shutdown this guest by acpi fallback.
> - As a result when using xl as toolstack, the guest is terminated
> non gracefully when the whole machine finally shutsdown, which seems
> less desirable then at least *trying* to shut it down gracefully by
> using the acpi button.
Using the ACPI fallback is a decision which can only be made locally
with full knowledge of the configuration of the guests.
Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-15 11:45 ` Ian Campbell
@ 2012-10-16 8:59 ` Sander Eikelenboom
2012-10-16 9:43 ` Ian Campbell
2012-10-19 10:21 ` Ian Jackson
1 sibling, 1 reply; 31+ messages in thread
From: Sander Eikelenboom @ 2012-10-16 8:59 UTC (permalink / raw)
To: Ian Campbell; +Cc: Ian Jackson, xen-devel@lists.xen.org
Monday, October 15, 2012, 1:45:33 PM, you wrote:
> On Mon, 2012-10-15 at 12:21 +0100, Sander Eikelenboom wrote:
>> Monday, October 15, 2012, 12:37:57 PM, you wrote:
>>
>> > On Mon, 2012-10-15 at 11:32 +0100, Sander Eikelenboom wrote:
>> >> Hi Ian,
>> >>
>> >> Great thanks !
>> >> Only thing i was wondering about:
>> >>
>> >> Shouldn't the "-F" option be dropped in favour of always trying the
>> >> "acpi fallback" when the pv shutdown fails.
>> >>
>> >> This because the shutdown scripts still don't work for domains without
>> >> pv shutdown and i don't see a down side to just trying that as
>> >> fallback.
>>
>> > It is guess OS dependent what the ACPI button press event does, it can
>> > reboot, shutdown or hibernate etc depending on the OS type and its
>> > configuration. (in theory I suppose it is completely arbitrary e.g. it
>> > could be configured to eject the CD-ROM or something equally random).
>>
>> > Therefore the user needs to be aware of when they can safely use it.
>>
>> Well yes and no:
>> - can't remember having (to make) that choice with xm ?
> Looks like xend uses SCHEDOP_remote_shutdown. It's unclear to me why
> this is better than just shooting the domain with destroy...
>> - On shutdown with xl as toolstack and when the guest doesn't
>> support pv shutdown, the init.d/xendomains script doesn't even attempt
>> to shutdown this guest by acpi fallback.
>> - As a result when using xl as toolstack, the guest is terminated
>> non gracefully when the whole machine finally shutsdown, which seems
>> less desirable then at least *trying* to shut it down gracefully by
>> using the acpi button.
> Using the ACPI fallback is a decision which can only be made locally
> with full knowledge of the configuration of the guests.
I'm not totally convinced (yet):
- In case of a system shutdown, it seems better to at least *try* instead of just halting the system without shutting such a guest down.
For the shutdown scripts the problems is that there is no way to give it the "full knowledge" of how to shutdown a particular guest.
It would be possible to use the -F flag in the sysconfig xendomains file, since the pv shutdown is always tried first.
- Not everyone seems to be aware, say an IanJ ;-)
Under the assumption that the guests in the xen-test-harness do react in the right way to a acpi powerbutton event,
it seems the "never passes" in "Guest-stop" row of http://www.chiark.greenend.org.uk/~xensrcts/logs/13967/
Will probably pass when the -F option will be used, see http://www.chiark.greenend.org.uk/~xensrcts/logs/13967/test-amd64-i386-xl-win7-amd64/13.ts-guest-stop.log
--
Sander
> Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-16 8:59 ` Sander Eikelenboom
@ 2012-10-16 9:43 ` Ian Campbell
2012-10-16 10:06 ` Sander Eikelenboom
0 siblings, 1 reply; 31+ messages in thread
From: Ian Campbell @ 2012-10-16 9:43 UTC (permalink / raw)
To: Sander Eikelenboom; +Cc: Ian Jackson, xen-devel@lists.xen.org
On Tue, 2012-10-16 at 09:59 +0100, Sander Eikelenboom wrote:
> Monday, October 15, 2012, 1:45:33 PM, you wrote:
>
> > On Mon, 2012-10-15 at 12:21 +0100, Sander Eikelenboom wrote:
> >> Monday, October 15, 2012, 12:37:57 PM, you wrote:
> >>
> >> > On Mon, 2012-10-15 at 11:32 +0100, Sander Eikelenboom wrote:
> >> >> Hi Ian,
> >> >>
> >> >> Great thanks !
> >> >> Only thing i was wondering about:
> >> >>
> >> >> Shouldn't the "-F" option be dropped in favour of always trying the
> >> >> "acpi fallback" when the pv shutdown fails.
> >> >>
> >> >> This because the shutdown scripts still don't work for domains without
> >> >> pv shutdown and i don't see a down side to just trying that as
> >> >> fallback.
> >>
> >> > It is guess OS dependent what the ACPI button press event does, it can
> >> > reboot, shutdown or hibernate etc depending on the OS type and its
> >> > configuration. (in theory I suppose it is completely arbitrary e.g. it
> >> > could be configured to eject the CD-ROM or something equally random).
> >>
> >> > Therefore the user needs to be aware of when they can safely use it.
> >>
> >> Well yes and no:
> >> - can't remember having (to make) that choice with xm ?
>
> > Looks like xend uses SCHEDOP_remote_shutdown. It's unclear to me why
> > this is better than just shooting the domain with destroy...
>
> >> - On shutdown with xl as toolstack and when the guest doesn't
> >> support pv shutdown, the init.d/xendomains script doesn't even attempt
> >> to shutdown this guest by acpi fallback.
> >> - As a result when using xl as toolstack, the guest is terminated
> >> non gracefully when the whole machine finally shutsdown, which seems
> >> less desirable then at least *trying* to shut it down gracefully by
> >> using the acpi button.
>
> > Using the ACPI fallback is a decision which can only be made locally
> > with full knowledge of the configuration of the guests.
>
> I'm not totally convinced (yet):
> - In case of a system shutdown, it seems better to at least *try* instead of just halting the system without shutting such a guest down.
You might as well do xl shutdown, wait a bit, xl destroy in the
initscript. "xm shutdown" is effectively "xm destroy" for guests without
PV drivers.
> For the shutdown scripts the problems is that there is no way to give it the "full knowledge" of how to shutdown a particular guest.
> It would be possible to use the -F flag in the sysconfig xendomains file, since the pv shutdown is always tried first.
It would be possible for an individual administrator to add -F if that
is compatible with their configuration. It would be inappropriate for
upstream to do this by default because we *don't know* what -F does to
an arbitrary guest.
> - Not everyone seems to be aware, say an IanJ ;-)
> Under the assumption that the guests in the xen-test-harness do react in the right way to a acpi powerbutton event,
> it seems the "never passes" in "Guest-stop" row of http://www.chiark.greenend.org.uk/~xensrcts/logs/13967/
> Will probably pass when the -F option will be used, see http://www.chiark.greenend.org.uk/~xensrcts/logs/13967/test-amd64-i386-xl-win7-amd64/13.ts-guest-stop.log
Ian J is well aware that -F could help here (this is the main reason I
implemented this option). It would be appropriate here because we can
control the guest OS configuration in the harness (and if we can't we
shouldn't use -F).
He just hasn't implemented it in the harness yet (TBH I suspect he has
forgotten ;-)).
Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-16 9:43 ` Ian Campbell
@ 2012-10-16 10:06 ` Sander Eikelenboom
0 siblings, 0 replies; 31+ messages in thread
From: Sander Eikelenboom @ 2012-10-16 10:06 UTC (permalink / raw)
To: Ian Campbell; +Cc: Ian Jackson, xen-devel@lists.xen.org
Tuesday, October 16, 2012, 11:43:20 AM, you wrote:
> On Tue, 2012-10-16 at 09:59 +0100, Sander Eikelenboom wrote:
>> Monday, October 15, 2012, 1:45:33 PM, you wrote:
>>
>> > On Mon, 2012-10-15 at 12:21 +0100, Sander Eikelenboom wrote:
>> >> Monday, October 15, 2012, 12:37:57 PM, you wrote:
>> >>
>> >> > On Mon, 2012-10-15 at 11:32 +0100, Sander Eikelenboom wrote:
>> >> >> Hi Ian,
>> >> >>
>> >> >> Great thanks !
>> >> >> Only thing i was wondering about:
>> >> >>
>> >> >> Shouldn't the "-F" option be dropped in favour of always trying the
>> >> >> "acpi fallback" when the pv shutdown fails.
>> >> >>
>> >> >> This because the shutdown scripts still don't work for domains without
>> >> >> pv shutdown and i don't see a down side to just trying that as
>> >> >> fallback.
>> >>
>> >> > It is guess OS dependent what the ACPI button press event does, it can
>> >> > reboot, shutdown or hibernate etc depending on the OS type and its
>> >> > configuration. (in theory I suppose it is completely arbitrary e.g. it
>> >> > could be configured to eject the CD-ROM or something equally random).
>> >>
>> >> > Therefore the user needs to be aware of when they can safely use it.
>> >>
>> >> Well yes and no:
>> >> - can't remember having (to make) that choice with xm ?
>>
>> > Looks like xend uses SCHEDOP_remote_shutdown. It's unclear to me why
>> > this is better than just shooting the domain with destroy...
>>
>> >> - On shutdown with xl as toolstack and when the guest doesn't
>> >> support pv shutdown, the init.d/xendomains script doesn't even attempt
>> >> to shutdown this guest by acpi fallback.
>> >> - As a result when using xl as toolstack, the guest is terminated
>> >> non gracefully when the whole machine finally shutsdown, which seems
>> >> less desirable then at least *trying* to shut it down gracefully by
>> >> using the acpi button.
>>
>> > Using the ACPI fallback is a decision which can only be made locally
>> > with full knowledge of the configuration of the guests.
>>
>> I'm not totally convinced (yet):
>> - In case of a system shutdown, it seems better to at least *try* instead of just halting the system without shutting such a guest down.
> You might as well do xl shutdown, wait a bit, xl destroy in the
> initscript. "xm shutdown" is effectively "xm destroy" for guests without
> PV drivers.
>> For the shutdown scripts the problems is that there is no way to give it the "full knowledge" of how to shutdown a particular guest.
>> It would be possible to use the -F flag in the sysconfig xendomains file, since the pv shutdown is always tried first.
> It would be possible for an individual administrator to add -F if that
> is compatible with their configuration. It would be inappropriate for
> upstream to do this by default because we *don't know* what -F does to
> an arbitrary guest.
It's just that i think upstream should use sensible defaults that work in the majority of cases (unless it will have a devastating effect in de minority of cases).
In this case both hvm guests with fairly old windows and linux install seem to respond correctly to the acpi powerbutton, so perhaps the majority of cases.
In the minority of cases it will perhaps do something else, but could/will that really be more devastating than the pulling the rug from under a guests feet when the machine powers down without the guest being shutdown ?
But any how, since it's not acceptable to upstream i will keep it in my private scripts and shut up :-)
>> - Not everyone seems to be aware, say an IanJ ;-)
>> Under the assumption that the guests in the xen-test-harness do react in the right way to a acpi powerbutton event,
>> it seems the "never passes" in "Guest-stop" row of http://www.chiark.greenend.org.uk/~xensrcts/logs/13967/
>> Will probably pass when the -F option will be used, see http://www.chiark.greenend.org.uk/~xensrcts/logs/13967/test-amd64-i386-xl-win7-amd64/13.ts-guest-stop.log
> Ian J is well aware that -F could help here (this is the main reason I
> implemented this option). It would be appropriate here because we can
> control the guest OS configuration in the harness (and if we can't we
> shouldn't use -F).
> He just hasn't implemented it in the harness yet (TBH I suspect he has
> forgotten ;-)).
> Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-15 11:45 ` Ian Campbell
2012-10-16 8:59 ` Sander Eikelenboom
@ 2012-10-19 10:21 ` Ian Jackson
2012-10-25 15:18 ` Sander Eikelenboom
1 sibling, 1 reply; 31+ messages in thread
From: Ian Jackson @ 2012-10-19 10:21 UTC (permalink / raw)
To: Ian Campbell; +Cc: Sander Eikelenboom, xen-devel@lists.xen.org
Ian Campbell writes ("Re: [PATCH 0 of 5] xl shutdown compatibility with xm"):
> On Mon, 2012-10-15 at 12:21 +0100, Sander Eikelenboom wrote:
> > - On shutdown with xl as toolstack and when the guest doesn't
> > support pv shutdown, the init.d/xendomains script doesn't even attempt
> > to shutdown this guest by acpi fallback.
> > - As a result when using xl as toolstack, the guest is terminated
> > non gracefully when the whole machine finally shutsdown, which seems
> > less desirable then at least *trying* to shut it down gracefully by
> > using the acpi button.
>
> Using the ACPI fallback is a decision which can only be made locally
> with full knowledge of the configuration of the guests.
I'm still with Sander on this, I'm afraid.
Certainly in the init scripts the default should be to try
xl shutdown -F
and only do destroy if that didn't seem to work.
Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-19 10:21 ` Ian Jackson
@ 2012-10-25 15:18 ` Sander Eikelenboom
2012-10-25 15:23 ` Ian Jackson
0 siblings, 1 reply; 31+ messages in thread
From: Sander Eikelenboom @ 2012-10-25 15:18 UTC (permalink / raw)
To: Ian Jackson; +Cc: Ian Campbell, xen-devel@lists.xen.org
Friday, October 19, 2012, 12:21:45 PM, you wrote:
> Ian Campbell writes ("Re: [PATCH 0 of 5] xl shutdown compatibility with xm"):
>> On Mon, 2012-10-15 at 12:21 +0100, Sander Eikelenboom wrote:
>> > - On shutdown with xl as toolstack and when the guest doesn't
>> > support pv shutdown, the init.d/xendomains script doesn't even attempt
>> > to shutdown this guest by acpi fallback.
>> > - As a result when using xl as toolstack, the guest is terminated
>> > non gracefully when the whole machine finally shutsdown, which seems
>> > less desirable then at least *trying* to shut it down gracefully by
>> > using the acpi button.
>>
>> Using the ACPI fallback is a decision which can only be made locally
>> with full knowledge of the configuration of the guests.
> I'm still with Sander on this, I'm afraid.
> Certainly in the init scripts the default should be to try
> xl shutdown -F
> and only do destroy if that didn't seem to work.
So Ian, what would your prefer ?
A) only fix the xendomains init script, since it's automated and a administator can not intervene, for manual usage of xl shutdown keep the current behaviour
B) Drop the -F option and let xl shutdown always try the acpi fallback. In this case you can very well turn around IanC's argumentation:
An administrator who knows that a domain can't be shutdown with either the pv or acpi fallback just shouldn't try to use xl shutdown manually.
(and if he does probably nothing devastating will happen)
C) Invert the -F option, to NOT try to use the apci fallback
I think option B is acceptable and preferable:
- It reduces config options
- Will work out of the box in most cases (for domains without PV drivers), since most of the time the acpi power button event is wired correctly.
- In most cases nothing devastating will happen when it doesn't work.
- Only in very special cases, probably setup explicitly, a admin should shutdown the domain by other means, but he has that knowledge in those cases
and shouldn't use xl shutdown on that domain, and shut the domain down before shutting down the machine (and running the xendomains init script)
--
Sander
> Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-25 15:18 ` Sander Eikelenboom
@ 2012-10-25 15:23 ` Ian Jackson
2012-10-25 15:32 ` Sander Eikelenboom
2012-10-25 15:34 ` Ian Campbell
0 siblings, 2 replies; 31+ messages in thread
From: Ian Jackson @ 2012-10-25 15:23 UTC (permalink / raw)
To: Sander Eikelenboom; +Cc: Ian Campbell, xen-devel@lists.xen.org
Sander Eikelenboom writes ("Re: [PATCH 0 of 5] xl shutdown compatibility with xm"):
>
> So Ian, what would your prefer ?
>
> A) only fix the xendomains init script, since it's automated and a administator can not intervene, for manual usage of xl shutdown keep the current behaviour
> B) Drop the -F option and let xl shutdown always try the acpi fallback. In this case you can very well turn around IanC's argumentation:
> An administrator who knows that a domain can't be shutdown with either the pv or acpi fallback just shouldn't try to use xl shutdown manually.
> (and if he does probably nothing devastating will happen)
> C) Invert the -F option, to NOT try to use the apci fallback
>
> I think option B is acceptable and preferable:
I agree with you. But I think we need to convince Ian C.
Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-25 15:23 ` Ian Jackson
@ 2012-10-25 15:32 ` Sander Eikelenboom
2012-10-25 15:47 ` Ian Campbell
2012-10-25 15:34 ` Ian Campbell
1 sibling, 1 reply; 31+ messages in thread
From: Sander Eikelenboom @ 2012-10-25 15:32 UTC (permalink / raw)
To: Ian Jackson; +Cc: Ian Campbell, xen-devel@lists.xen.org
Thursday, October 25, 2012, 5:23:17 PM, you wrote:
> Sander Eikelenboom writes ("Re: [PATCH 0 of 5] xl shutdown compatibility with xm"):
>>
>> So Ian, what would your prefer ?
>>
>> A) only fix the xendomains init script, since it's automated and a administator can not intervene, for manual usage of xl shutdown keep the current behaviour
>> B) Drop the -F option and let xl shutdown always try the acpi fallback. In this case you can very well turn around IanC's argumentation:
>> An administrator who knows that a domain can't be shutdown with either the pv or acpi fallback just shouldn't try to use xl shutdown manually.
>> (and if he does probably nothing devastating will happen)
>> C) Invert the -F option, to NOT try to use the apci fallback
>>
>> I think option B is acceptable and preferable:
> I agree with you. But I think we need to convince Ian C.
Ok I tried with the reasoning above.
I agree with his argumentation that for domains that do not properly shutdown with pv and acpi fallback intervention by a admin was and is required.
But i try to explain that for this special case it doesn't matter if xl shutdown tries to do the acpi fallback automatically, since this admin shouldn't use xl shutdown on this domain anyway.
> Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-25 15:32 ` Sander Eikelenboom
@ 2012-10-25 15:47 ` Ian Campbell
2012-10-25 15:55 ` Sander Eikelenboom
2012-10-25 15:56 ` Ian Jackson
0 siblings, 2 replies; 31+ messages in thread
From: Ian Campbell @ 2012-10-25 15:47 UTC (permalink / raw)
To: Sander Eikelenboom; +Cc: Ian Jackson, xen-devel@lists.xen.org
On Thu, 2012-10-25 at 16:32 +0100, Sander Eikelenboom wrote:
> Thursday, October 25, 2012, 5:23:17 PM, you wrote:
>
> > Sander Eikelenboom writes ("Re: [PATCH 0 of 5] xl shutdown compatibility with xm"):
> >>
> >> So Ian, what would your prefer ?
> >>
> >> A) only fix the xendomains init script, since it's automated and a administator can not intervene, for manual usage of xl shutdown keep the current behaviour
> >> B) Drop the -F option and let xl shutdown always try the acpi fallback. In this case you can very well turn around IanC's argumentation:
> >> An administrator who knows that a domain can't be shutdown with either the pv or acpi fallback just shouldn't try to use xl shutdown manually.
> >> (and if he does probably nothing devastating will happen)
> >> C) Invert the -F option, to NOT try to use the apci fallback
> >>
> >> I think option B is acceptable and preferable:
>
> > I agree with you. But I think we need to convince Ian C.
>
> Ok I tried with the reasoning above.
> I agree with his argumentation that for domains that do not properly
> shutdown with pv and acpi fallback intervention by a admin was and is
> required.
ACPI fallback != shutdown. It might just as likely be a hibernate or
(more likely) a reboot.
Are we going to add code to xl which forces on_reboot = destroy when xl
shutdown -F is invoked on a domain?
> But i try to explain that for this special case it doesn't matter if
> xl shutdown tries to do the acpi fallback automatically, since this
> admin shouldn't use xl shutdown on this domain anyway.
Whether they should or not the xendomains script is going to magically
do it for them.
Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-25 15:47 ` Ian Campbell
@ 2012-10-25 15:55 ` Sander Eikelenboom
2012-10-25 15:56 ` Ian Jackson
1 sibling, 0 replies; 31+ messages in thread
From: Sander Eikelenboom @ 2012-10-25 15:55 UTC (permalink / raw)
To: Ian Campbell; +Cc: Ian Jackson, xen-devel@lists.xen.org
Thursday, October 25, 2012, 5:47:29 PM, you wrote:
> On Thu, 2012-10-25 at 16:32 +0100, Sander Eikelenboom wrote:
>> Thursday, October 25, 2012, 5:23:17 PM, you wrote:
>>
>> > Sander Eikelenboom writes ("Re: [PATCH 0 of 5] xl shutdown compatibility with xm"):
>> >>
>> >> So Ian, what would your prefer ?
>> >>
>> >> A) only fix the xendomains init script, since it's automated and a administator can not intervene, for manual usage of xl shutdown keep the current behaviour
>> >> B) Drop the -F option and let xl shutdown always try the acpi fallback. In this case you can very well turn around IanC's argumentation:
>> >> An administrator who knows that a domain can't be shutdown with either the pv or acpi fallback just shouldn't try to use xl shutdown manually.
>> >> (and if he does probably nothing devastating will happen)
>> >> C) Invert the -F option, to NOT try to use the apci fallback
>> >>
>> >> I think option B is acceptable and preferable:
>>
>> > I agree with you. But I think we need to convince Ian C.
>>
>> Ok I tried with the reasoning above.
>> I agree with his argumentation that for domains that do not properly
>> shutdown with pv and acpi fallback intervention by a admin was and is
>> required.
> ACPI fallback != shutdown. It might just as likely be a hibernate or
> (more likely) a reboot.
> Are we going to add code to xl which forces on_reboot = destroy when xl
> shutdown -F is invoked on a domain?
If you don't want your guests to be shutdown, you would presumably use save and restore instead of shutdown for the xendomains init scripts ?
>> But i try to explain that for this special case it doesn't matter if
>> xl shutdown tries to do the acpi fallback automatically, since this
>> admin shouldn't use xl shutdown on this domain anyway.
> Whether they should or not the xendomains script is going to magically
> do it for them.
Well at the moment the xl shutdown fails, the script continues and the hardware shutdown.
So the guest is also destroyed in the end anyhow.
So if a admin has a special guest that doesn't properly shutdown on either pv or acpi he should shut it down manually before invoking the xendomains init script.
That's the case at present (without the change) and would be the future case (when changing to try the fallback).
> Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-25 15:47 ` Ian Campbell
2012-10-25 15:55 ` Sander Eikelenboom
@ 2012-10-25 15:56 ` Ian Jackson
2012-10-25 17:27 ` Sander Eikelenboom
1 sibling, 1 reply; 31+ messages in thread
From: Ian Jackson @ 2012-10-25 15:56 UTC (permalink / raw)
To: Ian Campbell; +Cc: Sander Eikelenboom, xen-devel@lists.xen.org
Ian Campbell writes ("Re: [PATCH 0 of 5] xl shutdown compatibility with xm"):
> ACPI fallback != shutdown. It might just as likely be a hibernate or
> (more likely) a reboot.
>
> Are we going to add code to xl which forces on_reboot = destroy when xl
> shutdown -F is invoked on a domain?
We might need to do something along those lines.
> > But i try to explain that for this special case it doesn't matter if
> > xl shutdown tries to do the acpi fallback automatically, since this
> > admin shouldn't use xl shutdown on this domain anyway.
>
> Whether they should or not the xendomains script is going to magically
> do it for them.
Well, at the moment the xendomains script will try "xl shutdown" and
if that fails the guest will be shot in the head. Trying the acpi
fallback is surely better?
Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-25 15:56 ` Ian Jackson
@ 2012-10-25 17:27 ` Sander Eikelenboom
0 siblings, 0 replies; 31+ messages in thread
From: Sander Eikelenboom @ 2012-10-25 17:27 UTC (permalink / raw)
To: Ian Jackson; +Cc: Ian Campbell, xen-devel@lists.xen.org
Thursday, October 25, 2012, 5:56:00 PM, you wrote:
> Ian Campbell writes ("Re: [PATCH 0 of 5] xl shutdown compatibility with xm"):
>> ACPI fallback != shutdown. It might just as likely be a hibernate or
>> (more likely) a reboot.
>>
>> Are we going to add code to xl which forces on_reboot = destroy when xl
>> shutdown -F is invoked on a domain?
> We might need to do something along those lines.
>> > But i try to explain that for this special case it doesn't matter if
>> > xl shutdown tries to do the acpi fallback automatically, since this
>> > admin shouldn't use xl shutdown on this domain anyway.
>>
>> Whether they should or not the xendomains script is going to magically
>> do it for them.
> Well, at the moment the xendomains script will try "xl shutdown" and
> if that fails the guest will be shot in the head. Trying the acpi
> fallback is surely better?
Perhaps it helps to keep the 2 "use cases", 2 possible changes and their arguments apart:
1) xl shutdown invoked by the xendomains init script
2) xl shutdown invoked manually by a admin or admin-scripts
Possible variants:
Change A) Only change the xendomains init/sysconfig script, and add the -F option to the command.
Or
Change B) Change the xl shutdown command and remove the -F option, always try acpi fallback when pv fails.
1) xl shutdown invoked by the xendomains init script (also used in a non machine shutdown scenario)
At present:
- a guest which can handle the pv shutdown: Guest will shutdown properly.
- a guest which can't handle pv but would do *shutdown* on the acpi fallback: Xl shutdown will fail, guest will be shot through the head on hardware shutdown.
- a guest which can't handle pv but would do *something* on the acpi fallback: Xl shutdown will fail, guest will be shot through the head on hardware shutdown.
- a guest which can't handle pv and does nothing on acpi fallback: Xl shutdown will fail, guest will be shot through the head on hardware shutdown.
So at present if the domain doesn't react to pv it will be shot through the head anyway, so it could eat your data at present as well, (but in my case i seem to
be testing and relying on journalling filesystems for some time) :-)
After change (both A or B):
- a guest which can handle the pv shutdown: Guest will shutdown properly.
- a guest which can't handle pv but would do *shutdown* on the acpi fallback: Guest will shutdown properly.
- a guest which can't handle pv but would do *something* on the acpi fallback: Xl shutdown will fail, guest will do *somehting* AND be shot through the head on hardware shutdown.
- a guest which can't handle pv and does nothing on acpi fallback: Xl shutdown will fail, guest will be shot through the head on hardware shutdown.
So both change A or B seem to have no serious negative side effects unless the acpi fallback triggers something like a "rm -rf /" in the guest.
Such disastrous behavior seems unlikely, but if present, it would probably be known to the admin. In that case he should take special care of this guest anyway.
It does make a guest that supports properly shutting down on acpi power button to do just that, so in my opinion both change A or B would be a win in this use case.
2) xl shutdown invoked manually by a admin or admin-scripts (also used in a non machine shutdown scenario)
At present:
- a guest which can handle the pv shutdown: Guest will shutdown properly.
- a guest which can't handle pv but would do *shutdown* on the acpi fallback: Xl shutdown will fail, admin can supply -F after which the guest will shutdown properly.
- a guest which can't handle pv but would do *something* on the acpi fallback: Xl shutdown will fail, admin can supply -F after which the guest will do *something* but doesn't shutdown.
- a guest which can't handle pv and does nothing on acpi fallback: Xl shutdown will fail, guest will do nothing and doesn't shutdown.
After change (A)):
- Same as present
After change (B)):
- a guest which can handle the pv shutdown: Guest will shutdown properly.
- a guest which can't handle pv but would do *shutdown* on the acpi fallback: Guest will shutdown properly.
- a guest which can't handle pv but would do *something* on the acpi fallback: Guest will do *something* but doesn't shutdown.
- a guest which can't handle pv and does nothing on acpi fallback: Xl shutdown will fail, guest will do nothing and doesn't shutdown.
In this case the win is for the guests that support shutdown on the acpi fallback, it removes the need for a user intervention and supplying the -F option.
The downside would be a admin using xl shutdown on a domain that does *something* on acpi fallback, where *something* is some devastating action.
This seems unlikely, but if present, it would probably be known to the admin. In that case he shouldn't use xl shutdown, and probably won't use it anyway since it won't help him anyway.
Summary:
- change (A) looks quite uncontroversial to me
- change (B) has as additional benefit that it's one option and manual intervention less to care about (especially for scripts, since it's quite possible to forget the -F option there (see IanJ with the test harness scripts)
--
Sander
> Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-25 15:23 ` Ian Jackson
2012-10-25 15:32 ` Sander Eikelenboom
@ 2012-10-25 15:34 ` Ian Campbell
2012-10-25 15:41 ` Ian Jackson
1 sibling, 1 reply; 31+ messages in thread
From: Ian Campbell @ 2012-10-25 15:34 UTC (permalink / raw)
To: Ian Jackson; +Cc: Sander Eikelenboom, xen-devel@lists.xen.org
On Thu, 2012-10-25 at 16:23 +0100, Ian Jackson wrote:
> Sander Eikelenboom writes ("Re: [PATCH 0 of 5] xl shutdown compatibility with xm"):
> >
> > So Ian, what would your prefer ?
> >
> > A) only fix the xendomains init script, since it's automated and a administator can not intervene, for manual usage of xl shutdown keep the current behaviour
> > B) Drop the -F option and let xl shutdown always try the acpi fallback. In this case you can very well turn around IanC's argumentation:
> > An administrator who knows that a domain can't be shutdown with either the pv or acpi fallback just shouldn't try to use xl shutdown manually.
> > (and if he does probably nothing devastating will happen)
> > C) Invert the -F option, to NOT try to use the apci fallback
> >
> > I think option B is acceptable and preferable:
>
> I agree with you. But I think we need to convince Ian C.
My main concern is that triggering a hibernation or suspend is, AUIU, a
reasonably common out of the box configuration for some versions of
Windows, which combined with hibernation being (historically at least)
notoriously flakey on real hardware (and I would expect more so on a
virtualised platform) worries me. Also note that we don't actually
implement suspend to RAM -- the domain will quiesce itself and come to a
stop but AFAIK it won't actually die and the domain's RAM won't be saved
anywhere. Presumably this is roughly equivalent to your battery running
out while suspended and therefore not all that dangerous.
But anyway I seem to be in the minority. We can always revert it if it
starts eating peoples data.
Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-25 15:34 ` Ian Campbell
@ 2012-10-25 15:41 ` Ian Jackson
0 siblings, 0 replies; 31+ messages in thread
From: Ian Jackson @ 2012-10-25 15:41 UTC (permalink / raw)
To: Ian Campbell; +Cc: Sander Eikelenboom, xen-devel@lists.xen.org
Ian Campbell writes ("Re: [PATCH 0 of 5] xl shutdown compatibility with xm"):
> My main concern is that triggering a hibernation or suspend is, AUIU, a
> reasonably common out of the box configuration for some versions of
> Windows, which combined with hibernation being (historically at least)
> notoriously flakey on real hardware (and I would expect more so on a
> virtualised platform) worries me. Also note that we don't actually
> implement suspend to RAM -- the domain will quiesce itself and come to a
> stop but AFAIK it won't actually die and the domain's RAM won't be saved
> anywhere. Presumably this is roughly equivalent to your battery running
> out while suspended and therefore not all that dangerous.
If it triggers a suspend, the domain won't end up shutting down so if
we wait for it there will be no data loss. If we time out and do a
destroy then doing the suspend first was probably a big improvement.
If it triggers a hibernation, then if the hibernation is broken the
effect will be just as if the user pressed the power button and the
hibernation is broken: the machine will fail to come back and
effectively will have been crashed. This is probably worse than doing
nothing (in the case where the caller/user isn't going to time out and
do a destroy instead) but it is definitely better than a destroy.
> But anyway I seem to be in the minority. We can always revert it if it
> starts eating peoples data.
OK, I think that's a go-ahead. Thanks.
Ian.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0 of 5] xl shutdown compatibility with xm
2012-10-15 10:09 [PATCH 0 of 5] xl shutdown compatibility with xm Ian Campbell
` (5 preceding siblings ...)
2012-10-15 10:32 ` [PATCH 0 of 5] xl shutdown compatibility with xm Sander Eikelenboom
@ 2012-10-18 8:35 ` Ian Campbell
6 siblings, 0 replies; 31+ messages in thread
From: Ian Campbell @ 2012-10-18 8:35 UTC (permalink / raw)
To: xen-devel@lists.xen.org; +Cc: linux@eikelenboom.it, Ian Jackson
On Mon, 2012-10-15 at 11:09 +0100, Ian Campbell wrote:
> The following implements xm compatibility for the xl shutdown command
> and a prerequisite bug fix.
>
> In particular add --all and --wait which are used by the xendomains
> initscript. It also adds the same options to xl reboot.
Applied these bits (first three patches) with Ian's ack.
> Lastly it cleans up and simplifies option parsing.
Ian had comments on #5 and it logically belongs with #4 so I have
skipped both for now.
Thanks.
>
> The xl shutdown/reboot patches should go into 4.2.1, the option
> parsing cleanups should not.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 31+ messages in thread