Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CA-391859: Failed to stop varstord-guard #5578

Merged
merged 1 commit into from Apr 23, 2024

Conversation

liulinC
Copy link
Collaborator

@liulinC liulinC commented Apr 22, 2024

varstord-guard has following configuration in the service file

After=message-switch.service syslog.target

This means varstored-guard needs to be stopped before message-switch, otherwise, it will hung and finally be killed by systemd during xe-toolstack-restart

@liulinC
Copy link
Collaborator Author

liulinC commented Apr 22, 2024

Test done:

  • Manual test on XS8 and XS9
  • XS9 ring3 BST: 197392 (Dev Run), some unrelated errors
  • XS8 ring3 BST: 197400 (Dev Run), In progress

Copy link
Member

@robhoes robhoes left a comment

Choose a reason for hiding this comment

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

I was confused by this change initially, but then I saw the loop that reverses SERVICES :/

@liulinC
Copy link
Collaborator Author

liulinC commented Apr 22, 2024

I was confused by this change initially, but then I saw the loop that reverses SERVICES :/

Yes, I should mention that in the comments to clarify that 😸

Copy link
Contributor

@Vincent-lau Vincent-lau left a comment

Choose a reason for hiding this comment

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

I was confused by this change initially, but then I saw the loop that reverses SERVICES :/
needs a List.rev 😄

@edwintorok
Copy link
Contributor

This would be useful for master too

@@ -30,7 +30,7 @@ if [ $POOLCONF == "master" ]; then MPATHALERT="mpathalert"; else MPATHALERT="";
SERVICES="perfmon v6d xenopsd xenopsd-xc xenopsd-xenlight
xenopsd-simulator xenopsd-libvirt xcp-rrdd-iostat xcp-rrdd-squeezed
xcp-rrdd-xenpm xcp-rrdd-gpumon xcp-rrdd xcp-networkd squeezed forkexecd
$MPATHALERT xapi-storage-script xapi-clusterd varstored-guard message-switch"
$MPATHALERT xapi-storage-script xapi-clusterd message-switch varstored-guard"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we put message-switch to the very beginning? Does message-switch have any dependencies of its own?

Copy link
Member

Choose a reason for hiding this comment

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

I expect all the toolstack daemons to depend on it, so it should be shut down last

Copy link
Member

Choose a reason for hiding this comment

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

Yes, let's make that change, and target the master branch.

@liulinC liulinC force-pushed the private/linl/xs9 branch 2 times, most recently from e2f9aa5 to 1e39bb7 Compare April 23, 2024 01:59
@liulinC liulinC changed the base branch from feature/xs9 to master April 23, 2024 01:59
varstord-guard has following configuration in the service file

After=message-switch.service syslog.target

This means varstored-guard needs to be stopped before
message-switch, otherwise, it will hung and finally be killed by
systemd during xe-toolstack-restart

Move message-switch to the begining of the list to shutdown last
as all other daemons depend on it

Note: the list order is reversed by `for svc in $SERVICES`

Signed-off-by: Lin Liu <lin.liu@citrix.com>
@liulinC
Copy link
Collaborator Author

liulinC commented Apr 23, 2024

git diff

 POOLCONF=`cat @ETCXENDIR@/pool.conf`
 if [ $POOLCONF == "master" ]; then MPATHALERT="mpathalert"; else MPATHALERT=""; fi
-SERVICES="perfmon v6d xenopsd xenopsd-xc xenopsd-xenlight
+SERVICES="message-switch perfmon v6d xenopsd xenopsd-xc xenopsd-xenlight
   xenopsd-simulator xenopsd-libvirt xcp-rrdd-iostat xcp-rrdd-squeezed
   xcp-rrdd-xenpm xcp-rrdd-gpumon xcp-rrdd xcp-networkd squeezed forkexecd
-  $MPATHALERT xapi-storage-script xapi-clusterd varstored-guard message-switch"
+  $MPATHALERT xapi-storage-script xapi-clusterd varstored-guard"
 
 tmp_file=$(mktemp --suffix="xe-toolstack-restart")

Manual test on XS8 and XS9.
ring3 bst can not be triggered due to XenRT Issue, merge considering the low risk. (and I tested the original PR on XS8 and XS9 Ring3 BST, see the first comments)

@liulinC liulinC merged commit bbefd9a into xapi-project:master Apr 23, 2024
14 checks passed
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.

None yet

5 participants