[PATCH] kexec-xen: Allow xen_kexec_exec() to return in case of Live Update

Raphael Ning posted 1 patch 2 years, 2 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
kexec/kexec-xen.c |  9 ++++++---
kexec/kexec.c     | 23 +++++++++++++++++++----
kexec/kexec.h     |  2 +-
3 files changed, 26 insertions(+), 8 deletions(-)
[PATCH] kexec-xen: Allow xen_kexec_exec() to return in case of Live Update
Posted by Raphael Ning 2 years, 2 months ago
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>
---
 kexec/kexec-xen.c |  9 ++++++---
 kexec/kexec.c     | 23 +++++++++++++++++++----
 kexec/kexec.h     |  2 +-
 3 files changed, 26 insertions(+), 8 deletions(-)

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);
-- 
2.32.0
Re: [PATCH] kexec-xen: Allow xen_kexec_exec() to return in case of Live Update
Posted by Simon Horman 2 years, 1 month ago
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

Re: [PATCH] kexec-xen: Allow xen_kexec_exec() to return in case of Live Update
Posted by Raphael Ning 2 years, 1 month ago
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


Re: [PATCH] kexec-xen: Allow xen_kexec_exec() to return in case of Live Update
Posted by Simon Horman 2 years, 1 month ago
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.