Skip to content

[host] Host shutdown idle function#45

Merged
jandryuk merged 1 commit intoOpenXT:masterfrom
crogers1:idle-shutdown
Oct 25, 2022
Merged

[host] Host shutdown idle function#45
jandryuk merged 1 commit intoOpenXT:masterfrom
crogers1:idle-shutdown

Conversation

@crogers1
Copy link
Copy Markdown
Contributor

@crogers1 crogers1 commented Oct 7, 2022

to isolate when this case is called. This allows for a configurable
option to force the host to be shut down after the set amount of time
has passed while it is attempting to do a regular shutdown.

Signed-off-by: Chris Rogers crogers122@gmail.com

Copy link
Copy Markdown
Contributor

@jandryuk jandryuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a corresponding xenmgr change, or the build would break, I think?

Comment thread interfaces/xenmgr_host.xml Outdated
<tp:docstring>Shutdown the host device.</tp:docstring>
</method>
<method name="shutdown_idle">
<tp:docstring>Called when idle period is determined to have passed</tp:docstring>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've only peeked at the xcpmd changes, and this is "Called when idle period is determined to have passed". That reads to me like this method triggers the shutdown_idle action. However it's use in xcpmd looks more like "returns a boolean stating whether or not Shutdown-on-Idle is enabled." I haven't seen the xenmgr piece, but I think the docstring may need clarification.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the naming could definitely be better, but in this case shutdown_idle is the RPC function call. shutdown_idle is also a xcpmd variable that lives in the db, defined in minutes, of how long the system must be idle before calling this function. shutdown_on_idle is the boolean yes/no that tells xcpmd whether to care about the idle timeout.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I've looked more at the xcpmd changes, I see that shutdown_idle method is called. So the docstring is:
<tp:docstring>Shutdown the host device. Called when idle period is determined to have passed. </tp:docstring>

How is this different than a plain shutdown?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shutdown_idle adds a timeout that will forcefully poweroff the box after a set period. The thinking is, no user input for the specified time should shut off the box. First, we should try to do a graceful shutdown and let all the guest domains clean up, but if a VM gets stuck for some reason, the box won't completely turn off. The extra timeout ensures that the box will be poweroff no matter what, after gracefully trying to shutdown.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Makes sense to force shutdown guests.

So the docstring should be something like:
Shutdown the host devices. VMs which fail to gracefully shutdown are forced off.?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I can update the docstring to reflect that.

@crogers1
Copy link
Copy Markdown
Contributor Author

This needs a corresponding xenmgr change, or the build would break, I think?

You're right. Xenmgr wasn't rebuilt for me locally so I missed it. I can decouple the idle shutdown stuff and make a PR for that.

  to isolate when this case is called. This allows for a configurable
  option to force the host to be shut down after the set amount of time
  has passed while it is attempting to do a regular shutdown.

Signed-off-by: Chris Rogers <crogers122@gmail.com>
@crogers1
Copy link
Copy Markdown
Contributor Author

Updated the docstring in latest push.

Copy link
Copy Markdown
Contributor

@jandryuk jandryuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

Needs XenMgr PR before it can be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants