Linux-HyperV Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 2/4] hv_utils: Support host-initiated restart request
@ 2020-01-13  6:30 Dexuan Cui
  2020-01-22 17:16 ` Michael Kelley
  0 siblings, 1 reply; 3+ messages in thread
From: Dexuan Cui @ 2020-01-13  6:30 UTC (permalink / raw)
  To: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, sashal,
	Sasha Levin, linux-hyperv, Michael Kelley, vkuznets,
	linux-kernel


To test the code, run this command on the host:

Restart-VM $vm -Type Reboot

Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
 drivers/hv/hv_util.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 766bd8457346..fe3a316380c2 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -24,6 +24,8 @@
 
 #define SD_MAJOR	3
 #define SD_MINOR	0
+#define SD_MINOR_1	1
+#define SD_VERSION_3_1	(SD_MAJOR << 16 | SD_MINOR_1)
 #define SD_VERSION	(SD_MAJOR << 16 | SD_MINOR)
 
 #define SD_MAJOR_1	1
@@ -50,8 +52,9 @@ static int sd_srv_version;
 static int ts_srv_version;
 static int hb_srv_version;
 
-#define SD_VER_COUNT 2
+#define SD_VER_COUNT 3
 static const int sd_versions[] = {
+	SD_VERSION_3_1,
 	SD_VERSION,
 	SD_VERSION_1
 };
@@ -118,11 +121,21 @@ static void perform_shutdown(struct work_struct *dummy)
 	orderly_poweroff(true);
 }
 
+static void perform_restart(struct work_struct *dummy)
+{
+	orderly_reboot();
+}
+
 /*
  * Perform the shutdown operation in a thread context.
  */
 static DECLARE_WORK(shutdown_work, perform_shutdown);
 
+/*
+ * Perform the restart operation in a thread context.
+ */
+static DECLARE_WORK(restart_work, perform_restart);
+
 static void shutdown_onchannelcallback(void *context)
 {
 	struct vmbus_channel *channel = context;
@@ -166,6 +179,14 @@ static void shutdown_onchannelcallback(void *context)
 				pr_info("Shutdown request received -"
 					    " graceful shutdown initiated\n");
 				break;
+			case 2:
+			case 3:
+				pr_info("Restart request received -"
+					    " graceful restart initiated\n");
+				icmsghdrp->status = HV_S_OK;
+
+				schedule_work(&restart_work);
+				break;
 			default:
 				icmsghdrp->status = HV_E_FAIL;
 				execute_shutdown = false;
-- 
2.19.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH v2 2/4] hv_utils: Support host-initiated restart request
  2020-01-13  6:30 [PATCH v2 2/4] hv_utils: Support host-initiated restart request Dexuan Cui
@ 2020-01-22 17:16 ` Michael Kelley
  2020-01-23  8:11   ` Dexuan Cui
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Kelley @ 2020-01-22 17:16 UTC (permalink / raw)
  To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	sashal, Sasha Levin, linux-hyperv, vkuznets, linux-kernel

From: Dexuan Cui <decui@microsoft.com> Sent: Sunday, January 12, 2020 10:31 PM
> 
> 
> To test the code, run this command on the host:
> 
> Restart-VM $vm -Type Reboot
>

Need a better commit message here.  How about:

The hv_util driver currently supports a "shutdown" operation initiated from the
Hyper-V host.  Newer versions of Hyper-V also support a "restart" operation.  So
add support for the updated protocol version that has "restart" support, and
perform a clean reboot when such a message is received from Hyper-V.

To test the restart functionality, run this PowerShell command on the Hyper-V host:

Restart-VM  <vmname>  -Type Reboot

> 
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>  drivers/hv/hv_util.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index 766bd8457346..fe3a316380c2 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -24,6 +24,8 @@
> 
>  #define SD_MAJOR	3
>  #define SD_MINOR	0
> +#define SD_MINOR_1	1
> +#define SD_VERSION_3_1	(SD_MAJOR << 16 | SD_MINOR_1)
>  #define SD_VERSION	(SD_MAJOR << 16 | SD_MINOR)
> 
>  #define SD_MAJOR_1	1
> @@ -50,8 +52,9 @@ static int sd_srv_version;
>  static int ts_srv_version;
>  static int hb_srv_version;
> 
> -#define SD_VER_COUNT 2
> +#define SD_VER_COUNT 3
>  static const int sd_versions[] = {
> +	SD_VERSION_3_1,
>  	SD_VERSION,
>  	SD_VERSION_1
>  };
> @@ -118,11 +121,21 @@ static void perform_shutdown(struct work_struct *dummy)
>  	orderly_poweroff(true);
>  }
> 
> +static void perform_restart(struct work_struct *dummy)
> +{
> +	orderly_reboot();
> +}
> +
>  /*
>   * Perform the shutdown operation in a thread context.
>   */
>  static DECLARE_WORK(shutdown_work, perform_shutdown);
> 
> +/*
> + * Perform the restart operation in a thread context.
> + */
> +static DECLARE_WORK(restart_work, perform_restart);
> +
>  static void shutdown_onchannelcallback(void *context)
>  {
>  	struct vmbus_channel *channel = context;
> @@ -166,6 +179,14 @@ static void shutdown_onchannelcallback(void *context)
>  				pr_info("Shutdown request received -"
>  					    " graceful shutdown initiated\n");
>  				break;
> +			case 2:
> +			case 3:

How are the flags values 0, 1, 2, and 3 interpreted?  Perhaps a short comment
would be helpful.

> +				pr_info("Restart request received -"
> +					    " graceful restart initiated\n");
> +				icmsghdrp->status = HV_S_OK;
> +
> +				schedule_work(&restart_work);
> +				break;

For case 0 and 1 (shutdown), the schedule_work() call is performed only
after the response packet has been sent to the host.  Is there a reason the
new code for case 2 and 3 (restart) is doing it in the opposite order?

>  			default:
>  				icmsghdrp->status = HV_E_FAIL;
>  				execute_shutdown = false;
> --
> 2.19.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH v2 2/4] hv_utils: Support host-initiated restart request
  2020-01-22 17:16 ` Michael Kelley
@ 2020-01-23  8:11   ` Dexuan Cui
  0 siblings, 0 replies; 3+ messages in thread
From: Dexuan Cui @ 2020-01-23  8:11 UTC (permalink / raw)
  To: Michael Kelley, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	sashal, Sasha Levin, linux-hyperv, vkuznets, linux-kernel

> From: Michael Kelley <mikelley@microsoft.com>
> Sent: Wednesday, January 22, 2020 9:16 AM
> >
> > To test the code, run this command on the host:
> >
> > Restart-VM $vm -Type Reboot
> 
> Need a better commit message here.  How about:
> 
> The hv_util driver currently supports a "shutdown" operation initiated from the
> Hyper-V host.  Newer versions of Hyper-V also support a "restart" operation.
> So add support for the updated protocol version that has "restart" support, and
> perform a clean reboot when such a message is received from Hyper-V.
> 
> To test the restart functionality, run this PowerShell command on the Hyper-V
> host:
> 
> Restart-VM  <vmname>  -Type Reboot

Thanks a lot! I'll use this version. 

> > @@ -166,6 +179,14 @@ static void shutdown_onchannelcallback(void
> *context)
> >  				pr_info("Shutdown request received -"
> >  					    " graceful shutdown initiated\n");
> >  				break;
> > +			case 2:
> > +			case 3:
> 
> How are the flags values 0, 1, 2, and 3 interpreted?  Perhaps a short comment
> would be helpful.

If bit 0 is 1, it means a flag of "doing the operation by force".  IMO we'd like to
always perform the operation by force, even if the host doesn't set the flag -- this
is what the existing shutdown handling code does.

I'll add a comment.

> 
> > +				pr_info("Restart request received -"
> > +					    " graceful restart initiated\n");
> > +				icmsghdrp->status = HV_S_OK;
> > +
> > +				schedule_work(&restart_work);
> > +				break;
> 
> For case 0 and 1 (shutdown), the schedule_work() call is performed only
> after the response packet has been sent to the host.  Is there a reason the
> new code for case 2 and 3 (restart) is doing it in the opposite order?

The channel callback runs in a tasklet context, and an active tasklet handler can
not be cancelled, so even if the "work" starts to run on another CPU immediately,
I'm sure the channel callback will send the response packet and finish normally.

This way we can save a local bool variable "execute_reboot". :-)

But, let me change the patch to follow the shutdown handling code for
consistency.

Thanks,
-- Dexuan

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-13  6:30 [PATCH v2 2/4] hv_utils: Support host-initiated restart request Dexuan Cui
2020-01-22 17:16 ` Michael Kelley
2020-01-23  8:11   ` Dexuan Cui

Linux-HyperV Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-hyperv/0 linux-hyperv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-hyperv linux-hyperv/ https://lore.kernel.org/linux-hyperv \
		linux-hyperv@vger.kernel.org
	public-inbox-index linux-hyperv

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-hyperv


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git