Page MenuHomeFreeBSD

xendomains rc script for xen-tools to allow automatic guest start/stop
Needs ReviewPublic

Authored by editor_callfortesting.org on Feb 9 2021, 5:31 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 13, 11:17 PM
Unknown Object (File)
Feb 8 2024, 12:48 AM
Unknown Object (File)
Feb 7 2024, 10:10 PM
Unknown Object (File)
Feb 2 2024, 8:56 AM
Unknown Object (File)
Jan 26 2024, 4:07 AM
Unknown Object (File)
Dec 10 2023, 7:55 PM
Unknown Object (File)
Nov 24 2023, 3:01 PM
Unknown Object (File)
Nov 24 2023, 1:27 PM
Subscribers

Details

Reviewers
royger
lwhsu
Summary

Question: What should it require?
Question: Should it echo -n ?
Note: No need to say "Parsing config" as xl will produce that, unless -q(iet)
Note: The awk syntax is from the NetBSD xendomains rc script

Test Plan

Tested on 13.0-BETA1 with FreeBSD and Windows Server 2019 guests.
Waiting on rc syntax feedback

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

lwhsu requested changes to this revision.Feb 9 2021, 6:04 AM
lwhsu added a subscriber: lwhsu.
lwhsu added inline comments.
sysutils/xen-tools/files/xendomains
20 ↗(On Diff #83582)

Please let them honor $PREFIX.

This revision now requires changes to proceed.Feb 9 2021, 6:04 AM

lwhsu@,
Looking at other examples in files, is this correct?

: ${xendomains_cmd:="%%PREFIX%%/sbin/xl}"
: ${xendomains_dir:="%%PREFIX%%/etc/xen/auto}"

lwhsu@,
Looking at other examples in files, is this correct?

: ${xendomains_cmd:="%%PREFIX%%/sbin/xl}"
: ${xendomains_dir:="%%PREFIX%%/etc/xen/auto}"

Yes, for other connections, this file needs to be renamed to xendomains.in and be added to USE_RC_SUBR= in Makefile.

Correcting PREFIX and adding USE_RC_SUBR

sysutils/xen-tools/Makefile
27

while here, I found this is not necessary. DOCS is "default by default" :)

sysutils/xen-tools/files/xendomains.in
23

Looking at the status command in /etc/rc.subr, it would be good to keep the behavior that if the domain is not running, return 1 or other non-zero value.

Let's see what royger@ says on those points. Thank you!

Please send the patch to xen-devel@lists.xenproject.org against the upstream Xen repository (http://xenbits.xen.org/gitweb/?p=xen.git;a=summary) (ie: place it in tools/hotplug/FreeBSD/rc.d/xendomains.in). It makes no sense to carry a custom script in the port when we can have it added upstream.

Note the %%PRERIX%% stuff is replaced by the Xen build system, hence using the .in file format to denote this is not the final script that gets installed.

Feedback from royger@: REQUIRE: XENCOMMONS (All caps?)

Guessing "xencommons" looking at other rc scripts.

just wanted to provide some comments, not mean to block this.

sysutils/xen-tools/files/xendomains.in
49

Does -F here mean force? I'm somehow nervous to see this and maybe we can only have this when force prefix is provided.

sysutils/xen-tools/files/xendomains.in
49

It's not really a 'force' option: by default 'xl shutdown' will only try to use the PV interface to shutdown the domain. By adding the -F option it will fallback to ACPI if the PV interface is not supported by the guest.

It should really be the default behaviour of shutdown to be honest.

just wanted to provide some comments, not mean to block this.

Having never contributed to ports, I assumed royger@'s block and welcome your block!

Removing DOCS mention, adding xencommons REQUIREment

(Not sure how to submit unsubmitted comments...)
Regarding "status": This relies entirely on xl(1) for status, and it just provides the facts. There is no daemon involved short of xencommons and the Xen kernel itself. I had to use xencomains_cmd because without it, the rc framework would look to see if xl was running (xl top in my case for debugging), and it would say "Hey, is this already running?" when you started domains. NetBSD opts for a "list" rather than status, and Linux does things I need to decrypt.

royger@: I am finding that Windows sometimes needs two shutdown attempts. Should this be left to the user to verify that the shutdown was successful, or should we consider some logic to wait, destroy, etc?

Here's a thought on VM tear-down:

  1. Have a user-overridable timeout for a second shutdown try. (I'm not sure why Windows Server wants two tries)
  1. Have a second user-overridable timeout for a destruction. If they have some RDBMS that needs 10 minutes to cleanly shut down, they can set that number of seconds.

Form the, the logic of testing every condition of a VM cleanly shutting down after N number of tries and a destroy could be rather complicated.

Note the re-use of the status function.

I'm guessing load_rc_config $name isn't needed?

Am I correct that xen_destroy_wait="120" could be set in /etc/rc.conf to override the embedded default?

If I have that wrong, please set me straight. Thanks!

royger@, Do you have any thoughts on how to elegantly check if a VM is running during start, to skip it? Will xl create generally do the right thing if a VM is already running?

Ignore the Dom0 reported in xl list! LOL!

Two people have confirmed that Windows often needs two ACPI nudges. It may be wise to take this approach.

When testing with fully-booted Windows Server 2019 and FreeBSD 13 guests, 10 second and 30 second waits work. (Destroy wait isn't reached)

Trying to clear Phabricator messages/

I have some comments, could you please send this upstream to xen-devel against xen.git? I don't plan to carry custom rc.d scripts in the package. If you don't feel comfortable sending it yourself I can pick it up.

sysutils/xen-tools/files/xendomains.in
19

${sbindir} here instead of hardcoding the path.

20

I think you will have to use @CONFIG_DIR@/auto

21

that might be too low, I think you want 60 here at least.

22

destroy doesn't really need a wait, when the 'xl destroy' command returns the domain can be considered as destroyed.

47

You could instead use:

xl shutdown -aF

Which will attempt to shutdown all domains. I think you likely want something like:

xl shutdown -aF
sleep 1
xl shutdown -aF

Because IIRC Windows will require two of those events in order to shut down.

59

The above chunk can be removed if you issue two shutdown calls as suggested in the previous comment.