Gihan Munasinghe
2010-May-07 23:36 UTC
[Xen-devel] [PATCH] new commands "xl reboot" & "xl shutdown"
Guys I patched xl to have "reboot" and "shutdown" commands. I tested this with hvm domains with and and without pv drivers seems to work, of course the os level reboot and shutdown will only happen if there are pv drivers with the dom U. Also the libxl_domain_shutdown was not working for hvm guests without pv drivers, I did a small patch to that as well. (same way xend shutdown works used || instead of a && ). I would appreciate if someone can test this more with hvm and pv domains. Let me know what you think -- Gihan Munasinghe R&D Team Leader Flexiant Ltd. www.flexiant.com _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-May-10 15:11 UTC
Re: [Xen-devel] [PATCH] new commands "xl reboot" & "xl shutdown"
On Sat, 8 May 2010, Gihan Munasinghe wrote:> Guys > > I patched xl to have "reboot" and "shutdown" commands. > I tested this with hvm domains with and and without pv drivers seems to > work, of course the os level reboot and shutdown will only happen if > there are pv drivers with the dom U. > > Also the libxl_domain_shutdown was not working for hvm guests without pv > drivers, I did a small patch to that as well. (same way xend shutdown > works used || instead of a && ). I would appreciate if someone can test > this more with hvm and pv domains. > > Let me know what you think >Thanks for the patch! It is mostly correct, however libxenlight changed quite a bit since xen 4.0 so could you please port your changes to xen-unstable?> diff -Naur vanila/xen-4.0.0/tools/libxl/libxl.c xen4patch/xen-4.0.0/tools/libxl/libxl.c > --- vanila/xen-4.0.0/tools/libxl/libxl.c 2010-04-07 17:12:04.000000000 +0100 > +++ xen4patch/xen-4.0.0/tools/libxl/libxl.c 2010-05-07 23:14:26.000000000 +0100 > @@ -400,12 +400,12 @@ > shutdown_path = libxl_sprintf(ctx, "%s/control/shutdown", dom_path); > > xs_write(ctx->xsh, XBT_NULL, shutdown_path, req_table[req], strlen(req_table[req])); > - if (/* hvm */ 0) { > + if (/* hvm */ 1) { > unsigned long acpi_s_state = 0; > unsigned long pvdriver = 0; > xc_get_hvm_param(ctx->xch, domid, HVM_PARAM_ACPI_S_STATE, &acpi_s_state); > xc_get_hvm_param(ctx->xch, domid, HVM_PARAM_CALLBACK_IRQ, &pvdriver); > - if (!pvdriver && acpi_s_state != 0) > + if (!pvdriver || acpi_s_state != 0) > xc_domain_shutdown(ctx->xch, domid, req); > } > return 0;this should be correct> diff -Naur vanila/xen-4.0.0/tools/libxl/xl.c xen4patch/xen-4.0.0/tools/libxl/xl.c > --- vanila/xen-4.0.0/tools/libxl/xl.c 2010-04-07 17:12:04.000000000 +0100 > +++ xen4patch/xen-4.0.0/tools/libxl/xl.c 2010-05-08 00:19:51.000000000 +0100 > @@ -702,9 +702,6 @@ > if (debug) > printf_info(&info1, &info2, disks, num_disks, vifs, num_vifs, pcidevs, num_pcidevs, vfbs, num_vfbs, vkbs, num_vkbs, &dm_info); > > -start: > - domid = 0; > - > if (libxl_ctx_init(&ctx, LIBXL_VERSION)) { > fprintf(stderr, "cannot init xl context\n"); > return; > @@ -712,6 +709,9 @@ > > libxl_ctx_set_log(&ctx, log_callback, NULL); > > +start: > + domid = 0; > + > ret = libxl_domain_make(&ctx, &info1, &domid); > if (ret) { > fprintf(stderr, "cannot make domain: %d\n", ret);this is probably not needed anymore> @@ -830,8 +830,9 @@ > libxl_free_waiter(w2); > free(w1); > free(w2); > - libxl_ctx_free(&ctx); > LOG("Done. Rebooting now"); > + sleep(2);/*Fix Me: The sleep is put here to slowdown the recreation of the domain > + If this sleep it not there, hvm_domain creation failes sometimes*/ > goto start; > } > LOG("Done. Exiting now");since we don''t free the ctx anymore here, it might be unnecessary. The other changes look OK but we have a command table now, so they need to be adapted. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gihan Munasinghe
2010-May-11 06:51 UTC
Re: [Xen-devel] [PATCH] new commands "xl reboot" & "xl shutdown"
Stefano Stabellini wrote:> On Sat, 8 May 2010, Gihan Munasinghe wrote: > >> Guys >> >> I patched xl to have "reboot" and "shutdown" commands. >> >> >> > > Thanks for the patch! > It is mostly correct, however libxenlight changed quite a bit since xen > 4.0 so could you please port your changes to xen-unstable? > >I''ll do the port and resubmit the patch Thanks Gihan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gihan Munasinghe
2010-May-12 15:57 UTC
Re: [Xen-devel] [PATCH] new commands "xl reboot" & "xl shutdown"
Stefano Stabellini wrote:> On Sat, 8 May 2010, Gihan Munasinghe wrote: > >> Guys >> >> I patched xl to have "reboot" and "shutdown" commands. >> I tested this with hvm domains with and and without pv drivers seems to >> work, of course the os level reboot and shutdown will only happen if >> there are pv drivers with the dom U. >> >> Also the libxl_domain_shutdown was not working for hvm guests without pv >> drivers, I did a small patch to that as well. (same way xend shutdown >> works used || instead of a && ). I would appreciate if someone can test >> this more with hvm and pv domains. >> >> Let me know what you think >> >> > > Thanks for the patch! > It is mostly correct, however libxenlight changed quite a bit since xen > 4.0 so could you please port your changes to xen-unstable? > > >The Patch is ported to xen-unstable see attached>> >> +start: >> + domid = 0; >> + >> ret = libxl_domain_make(&ctx, &info1, &domid); >> if (ret) { >> fprintf(stderr, "cannot make domain: %d\n", ret); >> > > this is probably not needed anymore > >Yes this fix is not needed anymore> >> LOG("Done. Rebooting now"); >> + sleep(2);/*Fix Me: The sleep is put here to slowdown the recreation of the domain >> + If this sleep it not there, hvm_domain creation failes sometimes*/ >> goto start; >> } >> LOG("Done. Exiting now"); >> > > since we don''t free the ctx anymore here, it might be unnecessary. > The other changes look OK but we have a command table now, so they need > to be adapted. > >The sleep is still needed, if not libxl_* calls fails sometimes. I''ll do more debugging with in the calls it self and see, but for now the sleep() seems to do the trick. Thanks Gihan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Vincent Hanquez
2010-May-12 16:34 UTC
Re: [Xen-devel] [PATCH] new commands "xl reboot" & "xl shutdown"
On 12/05/10 16:57, Gihan Munasinghe wrote:> The sleep is still needed, if not libxl_* calls fails sometimes. I''ll do > more debugging with in the calls it self and see, but for now the > sleep() seems to do the trick.diff -Naur libxl/libxl.c libxl-patch/libxl.c --- libxl/libxl.c 2010-05-11 09:37:50.000000000 +0100 +++ libxl-patch/libxl.c 2010-05-12 16:24:42.000000000 +0100 @@ -538,12 +538,12 @@ shutdown_path = libxl_sprintf(ctx, "%s/control/shutdown", dom_path); xs_write(ctx->xsh, XBT_NULL, shutdown_path, req_table[req], strlen(req_table[req])); - if (/* hvm */ 0) { + if (/* hvm */ 1) { unsigned long acpi_s_state = 0; unsigned long pvdriver = 0; you can''t just switch from 0 to 1, otherwise pv domains will just fails (although i see we''re not testing xc_get_hvm_param return values either). the if (/* hvm */ 0) is because the function never properly tested if the domain is hvm or not. nowadays you have is_hvm(domid) function that can handily replace the hardcoded value. -- Vincent _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gihan Munasinghe
2010-May-12 17:09 UTC
Re: [Xen-devel] [PATCH] new commands "xl reboot" & "xl shutdown"
Vincent Hanquez wrote:> On 12/05/10 16:57, Gihan Munasinghe wrote: >> The sleep is still needed, if not libxl_* calls fails sometimes. I''ll do >> more debugging with in the calls it self and see, but for now the >> sleep() seems to do the trick. > > diff -Naur libxl/libxl.c libxl-patch/libxl.c > --- libxl/libxl.c 2010-05-11 09:37:50.000000000 +0100 > +++ libxl-patch/libxl.c 2010-05-12 16:24:42.000000000 +0100 > @@ -538,12 +538,12 @@ > shutdown_path = libxl_sprintf(ctx, "%s/control/shutdown", dom_path); > > xs_write(ctx->xsh, XBT_NULL, shutdown_path, req_table[req], > strlen(req_table[req])); > - if (/* hvm */ 0) { > + if (/* hvm */ 1) { > unsigned long acpi_s_state = 0; > unsigned long pvdriver = 0; > > you can''t just switch from 0 to 1, otherwise pv domains will just > fails (although i see we''re not testing xc_get_hvm_param return values > either).I though PV domain would have been captured by.. xc_get_hvm_param(ctx->xch, domid, HVM_PARAM_CALLBACK_IRQ, &pvdriver); if (!pvdriver .. )> > the if (/* hvm */ 0) is because the function never properly tested if > the domain is hvm or not. nowadays you have is_hvm(domid) function > that can handily replace the hardcoded value. >Yes having is_hvm is much better than the hard coded values I have changed that bit of code see attached patch. I tested with hvm with and with out pv drivers. seems to work. Would be appreciate if someone can test in full pv domains. Thanks -- Gihan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-May-13 07:41 UTC
Re: [Xen-devel] [PATCH] new commands "xl reboot" & "xl shutdown"
On 12/05/2010 18:09, "Gihan Munasinghe" <GMunasinghe@flexiant.com> wrote:>> the if (/* hvm */ 0) is because the function never properly tested if >> the domain is hvm or not. nowadays you have is_hvm(domid) function >> that can handily replace the hardcoded value. >> > Yes having is_hvm is much better than the hard coded values I have > changed that bit of code see attached patch.Please port your patch to the tip of xen-unstable and re-post with changeset comment and signed-off-by-line. Thanks, Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gihan Munasinghe
2010-May-31 19:03 UTC
Re: [Xen-devel] [PATCH] new commands "xl reboot" & "xl shutdown"
Keir Fraser wrote:> On 12/05/2010 18:09, "Gihan Munasinghe" <GMunasinghe@flexiant.com> wrote: > > >>> the if (/* hvm */ 0) is because the function never properly tested if >>> the domain is hvm or not. nowadays you have is_hvm(domid) function >>> that can handily replace the hardcoded value. >>> >>> >> Yes having is_hvm is much better than the hard coded values I have >> changed that bit of code see attached patch. >> > > Please port your patch to the tip of xen-unstable and re-post with changeset > comment and signed-off-by-line. > > Thanks, > Keir > > > > >Sorry for the very long delay see updated patch xl: adds shutdown and reboot commands libxl : remote shutdown to work for pure hvm domains Signed-off-by: Gihan Munasinghe <GMunasinghe@flexiant.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Jun-01 06:09 UTC
Re: [Xen-devel] [PATCH] new commands "xl reboot" & "xl shutdown"
On 31/05/2010 20:03, "Gihan Munasinghe" <GMunasinghe@flexiant.com> wrote:> Sorry for the very long delay see updated patch > > > xl: adds shutdown and reboot commands > libxl : remote shutdown to work for pure hvm domains > > Signed-off-by: Gihan Munasinghe <GMunasinghe@flexiant.com>This is still against a very old version of xen-unstable. In your repo do: hg pull -u http://xenbits.xensource.com/staging/xen-unstable.hg Then re-base your patch to that. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gihan Munasinghe
2010-Jun-01 22:51 UTC
Re: [Xen-devel] [PATCH] new commands "xl reboot" & "xl shutdown"
> This is still against a very old version of xen-unstable. > > In your repo do: > hg pull -u http://xenbits.xensource.com/staging/xen-unstable.hg > > Then re-base your patch to that. > > -- Keir > > > > >Hi Re did the patch for the latest unstable release see attached xl: adds shutdown and reboot commands libxl : remote shutdown to work for pure hvm domains Signed-off-by: Gihan Munasinghe <GMunasinghe@flexiant.com> Thanks Gihan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel