This patch fixes the bug that "xm reboot <domid>" can not reboot
vmx
domain.
In xm reboot process, control panel will wait until DomU notified it.
Unlike DomU, vmx domain can not passively do that, since it runs
unmodified OS. This patch adds the missing logic in control panel.
Any comment?
Signed-off-by: Xu Dan <dan.d.xu@intel.com >
Yu Ke <ke.yu@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ewan Mellor
2005-Nov-10 11:17 UTC
Re: [Xen-devel] [PATCH][RESEND] xm reboot for vmx domain
On Thu, Nov 10, 2005 at 06:08:52PM +0800, Yu, Ke wrote:> This patch fixes the bug that "xm reboot <domid>" can not reboot vmx > domain. > > In xm reboot process, control panel will wait until DomU notified it. > Unlike DomU, vmx domain can not passively do that, since it runs > unmodified OS. This patch adds the missing logic in control panel. > Any comment? > > Signed-off-by: Xu Dan <dan.d.xu@intel.com > > Yu Ke <ke.yu@intel.com>Content-Description: reboot1.patch> [-- mutt.octet.filter file type: "''diff'' output text" --] > > [-- Statistics (lines words chars): 29 118 1176 /tmp/reboot1.patch --] > > diff -r 0240a69e1f97 tools/python/xen/xend/XendDomainInfo.py > --- a/tools/python/xen/xend/XendDomainInfo.py Wed Oct 26 13:59:29 2005 > +++ b/tools/python/xen/xend/XendDomainInfo.py Thu Oct 27 17:03:08 2005 > @@ -680,6 +680,14 @@ > """Get this domain''s target memory size, in KiB.""" > return self.info[''memory_KiB''] > > + def vmx_get_xeninfo(self, xeninfo): > + """Get vmx xeninfo""" > + shutdown_reason = self.readDom(''control/shutdown'') > + if shutdown_reason: > + xeninfo[''shutdown''] = 1 > + for x in shutdown_reasons.keys(): > + if shutdown_reasons[x] == shutdown_reason: > + xeninfo[''shutdown_reason''] = x > > def refreshShutdown(self, xeninfo = None): > # If set at the end of this method, a restart is required, with the > @@ -701,6 +709,10 @@ > # machine. > self.cleanupDomain() > return > + > + if self.image: > + if self.image.ostype == "vmx": > + self.vmx_get_xeninfo(xeninfo) > > if xeninfo[''dying'']: > # Dying means that a domain has been destroyed, but has notThanks for your resend, and sorry for not replying sooner. The reason I did not apply this immediately is that I am concerned by the arrangement here. Up until now, all VMX-related code has been isolated in image.py, but your patch brings VMX code directly into XendDomainInfo.py, and I would like to avoid doing that if possible. I am also concerned about race conditions. The control/shutdown node is used to tell the domain to shut down, but you are using it also as a flag to tell Xend that the domain has actually shut down. This means that if refreshShutdown is called before the VMX guest has had time to see a new control/shutdown entry, then Xend will think that the domain has shut down even before it has started to do so. I think that this patch needs some thought before it goes in. Perhaps we could discuss my concerns and move forward from there. Thanks, Ewan. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ewan Mellor wrote:> > Thanks for your resend, and sorry for not replying sooner. > > The reason I did not apply this immediately is that I am concerned by > the arrangement here. Up until now, all VMX-related code has been > isolated in image.py, but your patch brings VMX code directly into > XendDomainInfo.py, and I would like to avoid doing that if possible.Thanks for your comments. I truly understand your concerns. Yes, it is better to isolate change within image.py. However, I actually did not find a good place in image.py to place these code, so finally I choose XendDomainInfo.py. Please allow me to explain the reason, which comes from the difference between DomU and Vmx reboot. it is a bit longer, thank you for the patience in advance :) DomU reboot: Step1. XendDomainInfo.shutdown write "control/shutdown" node Step2. in guest linux, a xenstore watch is triggered by "control/shutdown" writing, the watch handler __shutdown_handler() will finally execute "/sbin/reboot" Step3. guest "/sbin/reboot" will do normal reboot cleanup and call HYPERVISOR_sched_op(SCHEDOP_shutdown, SHUTDOWN_reboot), Step4. hypervisor will set d->domain_flags _DOMF_shutdown bit, and set shutdown_code=reason. so now XendDomainInfo.xeninfo[''shutdown'']=1 and XendDomainInfo.xeninfo[''shutdown_reason''] =''reboot'' Step5. a daemon in control panel periodically call XendDomainInfo.refreshShutdown(). it will notice the change of XendDomainInfo.xeninfo, and finally call XendDomainInfo.maybeRestart to do the actual restart. Vmx domain reboot: Step1. same as domU Step2-4: will not happend, since vmx domain is not para-virtualized. and has no xenstore support in guest kernel. Step5: due to the timeout, XendDomainInfo.refreshShutdown() in the daemon will call XendDomainInfo.destroy(), and return withour further action. so vmx domain will only be destroyed, instead of restarted. the reason is that XendDomainInfo.xeninfo is not changed during the reboot process. And, I did not find a good way to change XendDomainInfo.xeninfo in image.py, since image.py has no reference back to XendInfo. Anyway, I totally agree it will be better if XendDomainInfo.xeninfo can be changed in image.py. Do you have idea on this?> > I am also concerned about race conditions. The control/shutdown node > is used to tell the domain to shut down, but you are using it also as > a flag to tell Xend that the domain has actually shut down. This > means that if refreshShutdown is called before the VMX guest has had > time to see a new control/shutdown entry, then Xend will think that > the domain has shut down even before it has started to do so.since vmx guest has no idea of xenstore, there will be no race condition.> > I think that this patch needs some thought before it goes in. > Perhaps we could discuss my concerns and move forward from there. > > Thanks, > > Ewan. > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel