refactor: remove PowerManager dependency from session manager#166
Merged
fly602 merged 1 commit intolinuxdeepin:masterfrom Oct 30, 2025
Merged
refactor: remove PowerManager dependency from session manager#166fly602 merged 1 commit intolinuxdeepin:masterfrom
fly602 merged 1 commit intolinuxdeepin:masterfrom
Conversation
1. Removed PowerManager D-Bus interface XML definition and generated adaptor files 2. Replaced PowerManager dependency with direct system checks for power operations 3. Added virtual machine detection using systemd-detect-virt 4. Modified CanHibernate() and CanSuspend() to check /sys/power/ mem_sleep directly 5. Removed m_powerInter member variable and related initialization The PowerManager service dependency was unnecessary for basic power state checks. The session manager can directly query system files and detect VM environments to determine available power operations, reducing service dependencies and improving reliability. Influence: 1. Test hibernation functionality on physical machines with hibernation support 2. Test suspend functionality on systems with different sleep states 3. Verify VM detection works correctly in virtualized environments 4. Test power operations are properly disabled in VMs 5. Check /sys/power/mem_sleep file access permissions 6. Verify no regression in shutdown and reboot functionality 重构：从会话管理器中移除 PowerManager 依赖 1. 删除了 PowerManager D-Bus 接口 XML 定义和生成的适配器文件 2. 使用直接的系统检查替换 PowerManager 依赖来获取电源操作状态 3. 添加了使用 systemd-detect-virt 的虚拟机检测功能 4. 修改 CanHibernate() 和 CanSuspend() 直接检查 /sys/power/mem_sleep 文件 5. 移除了 m_powerInter 成员变量和相关初始化 PowerManager 服务依赖对于基本的电源状态检查是不必要的。会话管理器可以直 接查询系统文件和检测 VM 环境来确定可用的电源操作，从而减少服务依赖并提高 可靠性。 Influence: 1. 在支持休眠的物理机器上测试休眠功能 2. 在不同睡眠状态的系统上测试挂起功能 3. 验证虚拟化环境中的 VM 检测功能正常工作 4. 测试 VM 中电源操作是否正确禁用 5. 检查 /sys/power/mem_sleep 文件访问权限 6. 验证关机和重启功能没有回归问题
deepin pr auto review我来分析一下这个代码变更：
优点：
需要改进的地方：
bool SessionManager::detectVirtualMachine() {
QProcess process;
process.start("/usr/bin/systemd-detect-virt", QStringList());
process.waitForFinished(-1);
if (process.exitCode() != 0) {
qWarning() << "Failed to detect virtual machine, error:" << process.errorString();
return false;
}
QString name = QString::fromUtf8(process.readAllStandardOutput()).trimmed();
return name != "none" && !name.isEmpty();
}建议改进：
改进版本： bool SessionManager::detectVirtualMachine() {
static bool isVM = false;
static bool detected = false;
if (detected) {
return isVM;
}
QProcess process;
process.start("/usr/bin/systemd-detect-virt", QStringList());
if (!process.waitForFinished(3000)) { // 3秒超时
qWarning() << "Virtual machine detection timeout";
process.kill();
return false;
}
if (process.exitCode() != 0) {
qWarning() << "Failed to detect virtual machine. Exit code:" << process.exitCode()
<< "Error:" << process.errorString();
return false;
}
QString name = QString::fromUtf8(process.readAllStandardOutput()).trimmed();
isVM = (name != "none" && !name.isEmpty());
detected = true;
qDebug() << "Virtual machine detection result:" << (isVM ? name : "physical machine");
return isVM;
}
if (!QFile::exists("/sys/power/mem_sleep")) {
return false;
}建议改进：
改进版本： bool SessionManager::CanSuspend()
{
if (QString(getenv("POWER_CAN_SLEEP")) == "0" || m_isVM) {
return false;
}
QFile file("/sys/power/mem_sleep");
if (!file.exists()) {
qWarning() << "Suspend not supported: mem_sleep interface not available";
return false;
}
if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
qWarning() << "Failed to read mem_sleep interface:" << file.errorString();
return false;
}
QString content = file.readAll().trimmed();
file.close();
// 检查是否支持 s2idle (suspend-to-idle)
if (!content.contains("s2idle")) {
qWarning() << "Suspend not supported: s2idle not available in mem_sleep states";
return false;
}
return true;
}
这些改进建议可以提高代码的可靠性、性能和可维护性。 |
Reviewer's GuideThis PR removes the external PowerManager D-Bus dependency by deleting its XML/adaptor definitions, drops the m_powerInter member and related CMake entries, introduces in-process VM detection via systemd-detect-virt, and refactors CanHibernate/CanSuspend to rely on environment variables and direct sysfs checks. Sequence diagram for CanHibernate/CanSuspend after PowerManager removalsequenceDiagram
participant "SessionManager"
participant "Environment"
participant "System Files"
participant "systemd-detect-virt"
"SessionManager"->>"Environment": Read POWER_CAN_SLEEP
alt POWER_CAN_SLEEP == "0"
"SessionManager"-->>"SessionManager": Return false
else
"SessionManager"->>"systemd-detect-virt": Check VM status
alt Is VM
"SessionManager"-->>"SessionManager": Return false
else
"SessionManager"->>"System Files": Check /sys/power/mem_sleep
alt File exists
"SessionManager"-->>"SessionManager": Return true
else
"SessionManager"-->>"SessionManager": Return false
end
end
end
Entity relationship diagram for power state checks after PowerManager removalerDiagram
SESSION_MANAGER {
bool CanHibernate
bool CanSuspend
bool detectVirtualMachine
bool m_isVM
}
SESSION_MANAGER ||--|| "sysfs:/sys/power/mem_sleep" : checks
SESSION_MANAGER ||--|| "env:POWER_CAN_SLEEP" : reads
SESSION_MANAGER ||--|| "systemd-detect-virt" : executes
Class diagram for refactored SessionManager (PowerManager dependency removed)classDiagram
class SessionManager {
-QString m_currentUid
-QString m_soundTheme
-int m_stage
-QDBusObjectPath m_currentSessionPath
-bool m_isVM
-org::deepin::dde::Audio1 *m_audioInter
-org::freedesktop::login1::Manager *m_login1ManagerInter
-org::freedesktop::login1::User *m_login1UserInter
-org::freedesktop::login1::Session *m_login1SessionInter
-org::freedesktop::systemd1::Manager *m_systemd1ManagerInter
-org::freedesktop::DBus *m_DBusInter
+bool CanHibernate()
+bool CanSuspend()
+bool detectVirtualMachine()
}
Flow diagram for VM detection in SessionManagerflowchart TD
A["SessionManager starts systemd-detect-virt"] --> B["systemd-detect-virt runs"]
B --> C["Read output"]
C --> D{Is output 'none'?}
D -- Yes --> E["Not a VM"]
D -- No --> F["Is a VM"]
F --> G["Set m_isVM = true"]
E --> H["Set m_isVM = false"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Add a timeout to QProcess.waitForFinished or explicitly check for the existence of systemd-detect-virt to prevent blocking if the tool is missing or hangs.
- Instead of solely checking /sys/power/mem_sleep existence, parse its contents to ensure the required sleep modes (e.g., "deep") are actually supported.
- Consider refactoring detectVirtualMachine into a standalone helper or service to improve reuse and make it easier to mock or test independently.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Add a timeout to QProcess.waitForFinished or explicitly check for the existence of systemd-detect-virt to prevent blocking if the tool is missing or hangs.
- Instead of solely checking /sys/power/mem_sleep existence, parse its contents to ensure the required sleep modes (e.g., "deep") are actually supported.
- Consider refactoring detectVirtualMachine into a standalone helper or service to improve reuse and make it easier to mock or test independently.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
mhduiy
approved these changes
Oct 30, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602, mhduiy The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The PowerManager service dependency was unnecessary for basic power state checks. The session manager can directly query system files and detect VM environments to determine available power operations, reducing service dependencies and improving reliability.
Influence:
重构：从会话管理器中移除 PowerManager 依赖
PowerManager 服务依赖对于基本的电源状态检查是不必要的。会话管理器可以直
接查询系统文件和检测 VM 环境来确定可用的电源操作，从而减少服务依赖并提高
可靠性。
Influence:
Summary by Sourcery
Refactor session manager to remove dependency on the PowerManager D-Bus interface and replace it with direct system checks and virtual machine detection
New Features:
Enhancements: