Skip to content

fix: stop SMGC service on SR detach to prevent orphaned systemd units - #770

Merged
MarkSymsCtx merged 1 commit into
xapi-project:masterfrom
xcp-ng:upstream-cleanup-SMGC-units
Sep 22, 2025
Merged

fix: stop SMGC service on SR detach to prevent orphaned systemd units#770
MarkSymsCtx merged 1 commit into
xapi-project:masterfrom
xcp-ng:upstream-cleanup-SMGC-units

Conversation

@Millefeuille42

Copy link
Copy Markdown
Contributor

This is not done on every and each implementation of SR but only on ones that calls cleanup.start_gc_service (like FileSR) and on the classes that inherits from them and don't call super on detach.

This is to prevent useless errors logs like Failed to stop xxx.service: Unit xxx.service not loaded.

@MarkSymsCtx MarkSymsCtx left a comment

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.

Can be significantly simplified by making cleanup.abort perform this operation

Comment thread libs/sm/drivers/LVHDSR.py Outdated
# However, we should still delete lock files on slaves as it is the
# only place to do so.
self._cleanup(self.isMaster)
cleanup.stop_gc_service(sr_uuid)

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.

This should replace the cleanup.abort() at line 589, or alternatively make cleanup.abort() stop the service.

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.

Ok, made it so cleanup.stop_gc_service is called in cleanup.abort

@Millefeuille42
Millefeuille42 force-pushed the upstream-cleanup-SMGC-units branch 2 times, most recently from 2387e7c to 9f07b80 Compare August 21, 2025 14:54
Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
@Millefeuille42
Millefeuille42 force-pushed the upstream-cleanup-SMGC-units branch from 9f07b80 to 30ea702 Compare August 21, 2025 15:34
@stormi

stormi commented Sep 22, 2025

Copy link
Copy Markdown
Contributor

There were two approvals last month. Are we getting close to a merge?

@MarkSymsCtx
MarkSymsCtx merged commit f80184e into xapi-project:master Sep 22, 2025
1 check 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.

4 participants