Skip to content

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

Merged
Millefeuille42 merged 1 commit into
3.2.12-8.3from
mlr-970-cleanup-SMGC-units
Oct 6, 2025
Merged

fix: stop SMGC service on SR detach to prevent orphaned systemd units#92
Millefeuille42 merged 1 commit into
3.2.12-8.3from
mlr-970-cleanup-SMGC-units

Conversation

@Millefeuille42

@Millefeuille42 Millefeuille42 commented Jul 22, 2025

Copy link
Copy Markdown

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 (like CephFSSR).

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

@Millefeuille42 Millefeuille42 self-assigned this Jul 22, 2025
Comment thread drivers/cleanup.py
Comment on lines +4055 to +4066
def stop_gc_service(sr_uuid):
"""
Stops the templated systemd service which runs GC on the given SR UUID.
"""
sr_uuid_esc = sr_uuid.replace("-", "\\x2d")
util.SMlog(f"Stopping SMGC@{sr_uuid}...")
cmd = ["/usr/bin/systemctl", "--quiet", "stop", f"SMGC@{sr_uuid_esc}"]
try:
subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, close_fds=True, check=True)
except subprocess.CalledProcessError as e:
util.SMlog(f"Failed to stop gc service `SMGC@{sr_uuid_esc}`: `{e.stderr.decode().strip()}`")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's cleaner to use util.doexec and compact the common code with start_gc_service.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed in 4b1d2d9

@Millefeuille42
Millefeuille42 force-pushed the mlr-970-cleanup-SMGC-units branch from f2d41c7 to 4b1d2d9 Compare August 20, 2025 10:46
Comment thread drivers/cleanup.py Outdated
Comment on lines +4041 to +4044
cmd=[ "/usr/bin/systemctl", "--quiet" ]
if extra_args:
cmd.extend(extra_args)
cmd += [action, f"SMGC@{sr_uuid_esc}"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Small difference in using arrays with and without spaces.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed in 95ed874

@Wescoeur

Copy link
Copy Markdown
Member

Before merge, it would be great to open a PR on the upstream side.

@Millefeuille42

Copy link
Copy Markdown
Author

Opened PR on upstream: xapi-project#770

@Millefeuille42
Millefeuille42 force-pushed the mlr-970-cleanup-SMGC-units branch 2 times, most recently from 23aaeb6 to 3c9feba Compare August 21, 2025 15:31
@Wescoeur
Wescoeur force-pushed the 3.2.12-8.3 branch 3 times, most recently from e7801da to ca5b52d Compare August 28, 2025 15:32
@Millefeuille42
Millefeuille42 force-pushed the mlr-970-cleanup-SMGC-units branch from 3c9feba to e2d8514 Compare September 1, 2025 15:04
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
@Millefeuille42
Millefeuille42 force-pushed the mlr-970-cleanup-SMGC-units branch from e2d8514 to 7af8b28 Compare September 22, 2025 13:02
@Millefeuille42

Copy link
Copy Markdown
Author

Upstream PR have been merged. Are we good to merge this one ?

@Wescoeur

Wescoeur commented Oct 2, 2025

Copy link
Copy Markdown
Member

@Nambrok Can you validate on your side?

@Millefeuille42
Millefeuille42 merged commit e3e8daf into 3.2.12-8.3 Oct 6, 2025
5 checks passed
@Wescoeur
Wescoeur deleted the mlr-970-cleanup-SMGC-units branch November 17, 2025 17:20
Wescoeur pushed a commit that referenced this pull request Dec 1, 2025
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Wescoeur pushed a commit that referenced this pull request Dec 1, 2025
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Wescoeur pushed a commit that referenced this pull request Dec 1, 2025
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Wescoeur pushed a commit that referenced this pull request Dec 9, 2025
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Wescoeur pushed a commit that referenced this pull request Dec 9, 2025
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Millefeuille42 added a commit that referenced this pull request Dec 10, 2025
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Wescoeur pushed a commit that referenced this pull request Feb 2, 2026
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Wescoeur pushed a commit that referenced this pull request Feb 3, 2026
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Wescoeur pushed a commit that referenced this pull request Feb 3, 2026
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Wescoeur pushed a commit that referenced this pull request Feb 3, 2026
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Wescoeur pushed a commit that referenced this pull request May 19, 2026
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Wescoeur pushed a commit that referenced this pull request May 19, 2026
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Wescoeur pushed a commit that referenced this pull request Jun 25, 2026
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Wescoeur pushed a commit that referenced this pull request Jun 25, 2026
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Wescoeur pushed a commit that referenced this pull request Jun 25, 2026
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Wescoeur pushed a commit that referenced this pull request Jun 25, 2026
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Wescoeur pushed a commit that referenced this pull request Jun 25, 2026
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.

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
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.

3 participants