Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

XOP-311: Modifications for VM Shutdown #843

Merged
merged 5 commits into from Apr 26, 2013

Conversation

Projects
None yet
7 participants
Contributor

siddharthv commented Aug 22, 2012

First perform a clean_shutdown on a VM, if that fails then perform a hard_shutdown.

This patch also resolves SCTX-942 and CA-88975.

Collaborator

johnelse commented Aug 22, 2012

@xen-git check

Owner

xen-git commented Aug 22, 2012

siddharthv/xen-api@5eee6fb293b5d9: Build succeeded. Can merge pull request.

Owner

robhoes commented Aug 23, 2012

Cool, thanks for the patch!

I believe @djs55 once planned to add a call that is simply called "VM.shutdown" that would do something like this (try shutting down cleanly first, and hard if that fails), since it is what most people would like to use. It is probably worth asking him for feedback.

Collaborator

djs55 commented Oct 8, 2012

Sorry for the delay looking at this.
Could you rename "VM.smart_shutdown" to "VM.shutdown"? As @robhoes says, the plan was to have both "VM_appliance.shutdown" and "VM.shutdown" do what people perceive to be the correct thing i.e.: try to perform a clean shutdown first but then fall back to a hard shutdown.

Member

zli commented Oct 8, 2012

If a clean shutdown fail, should we leave a trail by creating a system message (rather than just debug info) before attempting the hard shutdown? otherwise the system administrator has no way to tell that a hard shutdown (which will likely cause some unclean status inside the VM) had happened.

Collaborator

djs55 commented Oct 8, 2012

Good idea. Do you think an audit log entry would be appropriate?

On Monday, October 8, 2012, Zheng Li wrote:

If a clean shutdown fail, should we leave a trail by creating a system
message (rather than just debug info) before attempting the hard shutdown?
otherwise the system administrator has no way to tell that a hard shutdown
(which will likely cause some unclean status inside the VM) had happened.


Reply to this email directly or view it on GitHubhttps://github.com/xen-org/xen-api/pull/843#issuecomment-9226399.

Dave Scott

Member

zli commented Oct 8, 2012

Good idea. Do you think an audit log entry would be appropriate?

Which "audit" did you refer to? I think the word "audit" was overloaded in XenServer :-(

It depends on who should see it I guess. If we want the system administrator to be aware of that (via XenCenter), we should probably use xapi alerts (at some non-critical level); if we only want some evidences for later debugging purpose, then any kind of logging would do.

Owner

xen-git commented Nov 26, 2012

siddharthv/xen-api@a4255891bfdaaf: Build succeeded. Can merge pull request.

Contributor

siddharthv commented Mar 7, 2013

I have updated this patch with the modifications. Can anyone verify this ??

Owner

jonludlam commented Mar 27, 2013

As was commented, I think we need a bit more logging in the case of falling back to hard_shutdown - at least a 'warn' level log of the exception raised.

@ghost ghost assigned jonludlam Apr 22, 2013

siddharthv added some commits Aug 22, 2012

XOP-311, SCTX-942, CA-88975: Modifications for VM Shutdown
	First attempt to perform a clean_shutdown
	If clean_shutdown fails, then perform a hard_shutdown

Signed-off-by: Siddharth Vinothkumar <siddharth.vinothkumar@citrix.com>
Adding warn level logging when unable to perform clean_shutdown
Signed-off-by: Siddharth Vinothkumar <siddharth.vinothkumar@citrix.com>
Contributor

siddharthv commented Apr 24, 2013

I've now added a warn level log to indicate the failure of a clean_shutdown.
Would it be more appropriate to raise an exception to indicate the failure of clean_shutdown ??

@ghost ghost assigned zli Apr 24, 2013

Owner

jonludlam commented Apr 24, 2013

Assigning to @zli for review

in_product_since should be the first release this feature get in, i.e. clearwater or something newer?

It's better to update_vbd/vif_operations before generating the message (which is a much less urgent job) to reduce the time gap of potential inconsistency.

It's better to also print the exception e in the line of the warning message, otherwise we lose information on why clean shutdown fails.

XOP-311: Printing exception along with warn level log.
Also set in_product_since to clearwater.

Signed-off-by: Siddharth Vinothkumar <siddharth.vinothkumar@citrix.com>
Member

zli commented Apr 26, 2013

@siddharthv, could you also update the message_forwarding.ml wrt my comment above? It's better to call update_vbd_operations and update_vif_operations right after with_vm_operation and leave the less urgent message creation to later.

XOP-311: Moving up update_vbd_operations and update_vif_operations in
message_forwarding.ml
Signed-off-by: Siddharth Vinothkumar <siddharth.vinothkumar@citrix.com>
Contributor

siddharthv commented Apr 26, 2013

@zli : I have made the changes you've suggested. Could you have another check at the pull request ??

Merge failed, missing a semicolon here and an redundant one at the end.

Correcting merge build errors
Signed-off-by: Siddharth Vinothkumar <siddharth.vinothkumar@citrix.com>
Contributor

siddharthv commented Apr 26, 2013

@zli: I was able to build this within the chroot environment!!

Member

zli commented Apr 26, 2013

Looks good now, I'll merge it.

zli added a commit that referenced this pull request Apr 26, 2013

Merge pull request #843 from siddharthv/SmartShutdown
XOP-311: Modifications for VM Shutdown

@zli zli merged commit bdf44ec into xapi-project:master Apr 26, 2013

1 check passed

default Merged build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment