Hello Roger,
I want to know what about these patch, Please let me know if you have
any questions.
If you think these patch are fine, I want to your checking-in plan.
Thanks!
-邮件原件-
发件人: Liuyingdong
发送时间: 2017年2月2日 21:15
收件人: 'freebsd xen'
抄送: freebsd-xen@freebsd.org; Suoben ; Zhaojun (Euler)
; Wanglinkai ; chuzhaosong
; Wangchunfeng (Ivan)
主题: Re: [PATCH]netfront: need release all resources after adding and removing
NICs time and again
Hello Roger,
Thank you for the time and patience you devoted to reading my messages
and e-mails. I really appreciate that.
Depend on your reply, I submit those against current HEAD
(origin/master).
1. 0001 patch: introduce suspend_cancel mechanism for frontend devices.
1.1 On a cancelled suspend, xen frontend devices need know their state is
invariant.
1.2 On a cancelled suspend the vcpu_info location does not change (it's still
in the per-cpu area registered by xen_hvm_cpu_init()). So do not call
xen_hvm_init_shared_info_page() which would make the kernel think its back in
the shared info. With the wrong vcpu_info, events cannot be received and the
domain will hang after a cancelled suspend.
I declare a global suspend_cancelled variable in xen/xenbus/xenbusvar.h and
export that to all the frontends that need it. It should also be a bool type
instead of int. I merge together with a _set_suspend_cancelled() function.
2. 0002 patch: a userspace application need wait until stop_all_proc has
finished during live migration.
If there is a user process which maybe often reads and writes xenstore, the
application has been suspended after stop_all_proc is called. It held
xs.request_mutex lock but in the xctrl_suspend() function xs_write and
xs_suspend will not obtain this lock. So the VM will hang. In order to prevent
this from happening, this process need wait until stop_all_proc has finished
during live migration.
3. 0003 patch: wrong order of device resume causes virtual machine to become
unresponsive after live migration.
Because wrong order of device resume, VM will hang after live migration. attach
the Xen PV timer to the nexus directly as it was done before.
In my laboratory, FreeBSD VMs under no load and high load(cpu 80% and
memory 80%) were alive after made the live migration back and forth 1000 times.
I want to know what about these patch, Please let me know if you have
any questions. Thanks!
-邮件原件-
发件人: freebsd xen [mailto:roger@citrix.com]
发送时间: 2017年1月6日 14:44
收件人: Liuyingdong
抄送: freebsd-xen@freebsd.org; Zhaojun (Euler) ; Suoben
; Ouyangzhaowei (Charles) ;
chuzhaosong ; Wanglinkai
主题: Re: [PATCH]netfront: need release all resources after adding and removing
NICs time and again
On Thu, Dec 15, 2016 at 02:00:53AM +, Liuyingdong wrote:
> Hello Roger,
> Thank you for the time and patience you devoted to reading my messages
> and e-mails. I really appreciate that.
> I can't use git send-email so I attach the patches directly. In the
> 0001 patch I introduce suspend_cancel mechanism for frontend devices and in
> the 0002 patch I release all resources after hot unplug NICs.
>
> Note: These two patches is on the base of the origin/release/10.2.0
> branch and the 0002 patch is made after the 0001 patch.
Hello,
Thanks for the patches, now they look fine. You will however need to submit
those against current HEAD (origin/master), and then I will do the backport to
stable/10 and stable/11. I cannot apply a patch directly to stable branches
without it being in HEAD first unless there's a good reason for it.
Replying to both patches here inline.
---8<---
> From fc85ac7eba55a5f14f5f7d81f0e1fc7fbf6a7447 Mon Sep 17 00:00:00 2001
> From: Yingdong Liu
> Date: Tue, 13 Dec 2016 21:53:25 +0800
> Subject: [PATCH] introduce suspend cancel mechanism for frontend
> devices
>
Can you elaborate a little bit more on the commit message here? What issues are
you seeing without this patch applied? What is the result after applying the
patch?
Commit messages are very important in order to know why a change is needed,
specially when you look at them in for example 3 years time.
> ---
> sys/dev/xen/blkfront/blkfront.c | 13 +++
> sys/dev/xen/control/control.c | 9 ++-
> sys/dev/xen/netfront/netfront.c | 52
> ++---
> sys/x86/xen/hvm.c | 16 -
> sys/xen/xenbus/xenbusb.c| 45 ++-
> sys/xen/xenbus/xenbusvar.h | 4
> 6 files changed, 102 insertions(+), 37 deletions(-)
>
> diff --git a/sys/dev/xen/blkfront/blkfront.c
> b/sys/dev/xen/blkfront/blkfront.c index a71251d..8d7c32a 100644
> --- a/sys/dev/xen/blkfront/blkfront.c
> +++ b/sys/dev/xen/blkfront/blkfront.c
> @@ -68,6 +68,8 @@ __FBSDID("$FreeBSD$");
>
> #include "xenbus_if.h"
>
> +static int blkfront_suspend_cancelled = 0;
I see that you add one of those to each frontend y