| Message ID | 20220314092115.48309-1-raphning@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | kexec-xen: Allow xen_kexec_exec() to return in case of Live Update | expand |
On Mon, Mar 14, 2022 at 09:21:15AM +0000, Raphael Ning wrote: > From: Raphael Ning <raphning@amazon.com> > > Currently, my_exec() does not expect the Xen KEXEC_CMD_kexec hypercall > to return on success, because it assumes that the hypercall always > triggers an immediate reboot. However, for Live Update, the hypercall > merely schedules the kexec operation and returns; the actual reboot > happens asynchronously. [1] > > Therefore, rework the Xen code path of my_exec() such that it does not > treat a successfully processed Live Update request as an error. Also, > rephrase the comment above the function to remove ambiguity. > > [1] https://lists.xen.org/archives/html/xen-devel/2021-05/msg00286.html > > Signed-off-by: Raphael Ning <raphning@amazon.com> Hi Raphael, thanks for your patch. Overall I think this is good. Unfortunately I am seeing a build failure with this patch applied. ../../kexec/kexec-xen.c:292:6: error: conflicting types for ‘xen_kexec_exec’ 292 | void xen_kexec_exec(uint64_t kexec_flags) | ^~~~~~~~~~~~~~ In file included from ../../kexec/kexec-xen.c:6: ../../kexec/kexec.h:327:5: note: previous declaration of ‘xen_kexec_exec’ was here 327 | int xen_kexec_exec(uint64_t kexec_flags); | ^~~~~~~~~~~~~~ make[1]: *** [Makefile:124: kexec/kexec-xen.o] Error 1 make[1]: *** Waiting for unfinished jobs.... See: https://github.com/horms/kexec-tools/runs/5661629877?check_suite_focus=true
On 23/03/2022 14:08, Simon Horman wrote: > Hi Raphael, > thanks for your patch. Overall I think this is good. > > Unfortunately I am seeing a build failure with this patch applied. > > ../../kexec/kexec-xen.c:292:6: error: conflicting types for ‘xen_kexec_exec’ > 292 | void xen_kexec_exec(uint64_t kexec_flags) > | ^~~~~~~~~~~~~~ > In file included from ../../kexec/kexec-xen.c:6: > ../../kexec/kexec.h:327:5: note: previous declaration of ‘xen_kexec_exec’ was here > 327 | int xen_kexec_exec(uint64_t kexec_flags); > | ^~~~~~~~~~~~~~ > make[1]: *** [Makefile:124: kexec/kexec-xen.o] Error 1 > make[1]: *** Waiting for unfinished jobs.... > > See: https://github.com/horms/kexec-tools/runs/5661629877?check_suite_focus=true Hi Simon, Thanks for the review. The conflicting declaration is for the --without-xen build, which I didn't test. Let me fix that and post a new revision. Raphael
On Wed, Mar 23, 2022 at 02:20:52PM +0000, Raphael Ning wrote: > > On 23/03/2022 14:08, Simon Horman wrote: > > Hi Raphael, > > thanks for your patch. Overall I think this is good. > > > > Unfortunately I am seeing a build failure with this patch applied. > > > > ../../kexec/kexec-xen.c:292:6: error: conflicting types for ‘xen_kexec_exec’ > > 292 | void xen_kexec_exec(uint64_t kexec_flags) > > | ^~~~~~~~~~~~~~ > > In file included from ../../kexec/kexec-xen.c:6: > > ../../kexec/kexec.h:327:5: note: previous declaration of ‘xen_kexec_exec’ was here > > 327 | int xen_kexec_exec(uint64_t kexec_flags); > > | ^~~~~~~~~~~~~~ > > make[1]: *** [Makefile:124: kexec/kexec-xen.o] Error 1 > > make[1]: *** Waiting for unfinished jobs.... > > > > See: https://github.com/horms/kexec-tools/runs/5661629877?check_suite_focus=true > > > Hi Simon, > > > Thanks for the review. The conflicting declaration is for the --without-xen build, which I didn't test. Let me fix that and post a new revision. Thanks Raphael, much appreciated.
diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c index 47da3da466f0..44c64d99c566 100644 --- a/kexec/kexec-xen.c +++ b/kexec/kexec-xen.c @@ -247,21 +247,24 @@ int xen_kexec_status(uint64_t kexec_flags) return ret; } -void xen_kexec_exec(uint64_t kexec_flags) +int xen_kexec_exec(uint64_t kexec_flags) { xc_interface *xch; uint8_t type = KEXEC_TYPE_DEFAULT; + int ret; xch = xc_interface_open(NULL, NULL, 0); if (!xch) - return; + return -1; if (kexec_flags & KEXEC_LIVE_UPDATE) type = KEXEC_TYPE_LIVE_UPDATE; - xc_kexec_exec(xch, type); + ret = xc_kexec_exec(xch, type); xc_interface_close(xch); + + return ret; } #else /* ! HAVE_LIBXENCTRL */ diff --git a/kexec/kexec.c b/kexec/kexec.c index 7e4787bc8211..e7861049bbea 100644 --- a/kexec/kexec.c +++ b/kexec/kexec.c @@ -902,13 +902,28 @@ static int my_shutdown(void) } /* - * Exec the new kernel (reboot) + * Exec the new kernel. If successful, this triggers an immediate reboot + * and does not return, but Xen Live Update is an exception (more on this + * below). */ static int my_exec(void) { - if (xen_present()) - xen_kexec_exec(kexec_flags); - else + if (xen_present()) { + int ret; + + /* + * There are two cases in which the Xen hypercall may return: + * 1) An error occurred, e.g. the kexec image was not loaded. + * The exact error is indicated by errno. + * 2) Live Update was successfully scheduled. Note that unlike + * a normal kexec, Live Update happens asynchronously, i.e. + * the hypercall merely schedules the kexec operation and + * returns immediately. + */ + ret = xen_kexec_exec(kexec_flags); + if ((kexec_flags & KEXEC_LIVE_UPDATE) && !ret) + return 0; + } else reboot(LINUX_REBOOT_CMD_KEXEC); /* I have failed if I make it here */ fprintf(stderr, "kexec failed: %s\n", diff --git a/kexec/kexec.h b/kexec/kexec.h index 595dd681db6d..0f97a974cb8a 100644 --- a/kexec/kexec.h +++ b/kexec/kexec.h @@ -324,7 +324,7 @@ void cmdline_add_liveupdate(char **base); int xen_present(void); int xen_kexec_load(struct kexec_info *info); int xen_kexec_unload(uint64_t kexec_flags); -void xen_kexec_exec(uint64_t kexec_flags); +int xen_kexec_exec(uint64_t kexec_flags); int xen_kexec_status(uint64_t kexec_flags); extern unsigned long long get_kernel_sym(const char *text);