fix: Fix incorrect network selection after disabling and re-enabling network interface#518
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Mar 9, 2026
Merged
Conversation
Reviewer's GuidePersists the last successfully activated connection per device in a config file and uses it to restore the correct network after re‑enabling a network interface, while removing now‑redundant per‑device DBus enable callbacks and auto‑reconnect logic. Sequence diagram for device re-enable reconnection using stored connection UUIDsequenceDiagram
actor User
participant NetworkManagerDevice as NetworkManagerDevice
participant NetworkThread as NetworkThread
participant NetworkEnabledConfig as NetworkEnabledConfig
participant NetworkManager as NetworkManager
User->>NetworkManagerDevice: disable interface
NetworkManagerDevice-->>NetworkThread: onDevicestateChanged(newState, oldState)
NetworkThread->>NetworkEnabledConfig: deviceEnabled(dev->uni())
NetworkEnabledConfig-->>NetworkThread: enabled = false
NetworkThread->>NetworkManagerDevice: disconnectInterface() (if state Preparing..Activated)
User->>NetworkManagerDevice: enable interface
NetworkManagerDevice-->>NetworkThread: onDevicestateChanged(newState, oldState)
NetworkThread->>NetworkEnabledConfig: deviceEnabled(dev->uni())
NetworkEnabledConfig-->>NetworkThread: enabled = true
alt state == Activated
NetworkThread->>NetworkManagerDevice: activeConnection()
NetworkManagerDevice-->>NetworkThread: ActiveConnection
NetworkThread->>NetworkEnabledConfig: setConnectionInfo(interfaceName, connection->uuid())
NetworkThread->>NetworkEnabledConfig: saveConfig()
end
User->>NetworkThread: enableDevice(device)
NetworkThread->>NetworkEnabledConfig: connectionUuid(device->interfaceName())
NetworkEnabledConfig-->>NetworkThread: last_uuid or empty
alt last_uuid not empty and autoconnect
NetworkThread->>NetworkManager: findConnectionByUuid(last_uuid)
NetworkManager-->>NetworkThread: Connection
NetworkThread->>NetworkManager: activateConnection(connPath0, device->uni(), "")
else no valid stored connection
NetworkThread->>NetworkManagerDevice: availableConnections()
NetworkManagerDevice-->>NetworkThread: ConnectionList
NetworkThread->>NetworkThread: pick autoconnect connection with max timestamp
NetworkThread->>NetworkManager: activateConnection(connPath0, device->uni(), "")
end
ER diagram for NetworkEnabledConfig JSON structure with Connections maperDiagram
NetworkEnabledConfig {
string filePath
}
DeviceEntry {
string deviceUni
boolean enabled
}
ConnectionEntry {
string interfaceName
string connectionUuid
}
NetworkEnabledConfig ||--o{ DeviceEntry : has
NetworkEnabledConfig ||--o{ ConnectionEntry : has
Class diagram for NetworkEnabledConfig, NetworkThread, and device manager classesclassDiagram
class NetworkEnabledConfig {
- QMap~QString, QVariant~ m_map
+ NetworkEnabledConfig()
+ void loadConfig()
+ QString saveConfig()
+ void setConnectionInfo(QString dev, QString uuid)
+ QString connectionUuid(QString dev) const
+ bool deviceEnabled(QString devUni) const
+ void setDeviceEnabled(QString devUni, bool enabled)
+ bool vpnEnabled() const
+ void setVpnEnabled(bool enabled)
}
class NetworkThread {
- NetworkEnabledConfig* m_networkConfig
+ void onDevicestateChanged(Device_State newState, Device_State oldState, NetworkManager_Device_Ptr dev)
+ QString enableDevice(NetworkManager_Device_Ptr device)
+ QString setPropVpnEnabled(bool enabled)
}
class DeviceManagerRealize {
+ DeviceManagerRealize(QObject* parent)
+ ~DeviceManagerRealize()
+ void setEnabled(bool enabled)
+ void disconnectNetwork() = 0
+ void addConnection(NetworkManager_Connection_Ptr connection) = 0
+ void removeConnection(QString connection) = 0
+ void onActiveConnectionChanged() = 0
+ void deviceEnabledChanged(bool enabled)
}
class WiredDeviceManagerRealize {
+ WiredDeviceManagerRealize(QObject* parent)
+ ~WiredDeviceManagerRealize()
+ void disconnectNetwork()
+ void addConnection(NetworkManager_Connection_Ptr connection)
+ void removeConnection(QString connectionUni)
+ void onActiveConnectionChanged()
+ QString usingHwAdr() const
+ bool carrier() const
}
class WirelessDeviceManagerRealize {
+ WirelessDeviceManagerRealize(QObject* parent)
+ ~WirelessDeviceManagerRealize()
+ void disconnectNetwork()
+ void addConnection(NetworkManager_Connection_Ptr connection)
+ void removeConnection(QString connectionUni)
+ void onActiveConnectionChanged()
+ DeviceStatus deviceStatus() const
+ void deviceEnabledChanged(bool enabled)
+ QString usingHwAdr() const
}
NetworkThread --> NetworkEnabledConfig : uses
WiredDeviceManagerRealize --|> DeviceManagerRealize
WirelessDeviceManagerRealize --|> DeviceManagerRealize
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 - I've found 1 issue, and left some high level feedback:
- In
NetworkThread::onDevicestateChanged, consider guardingactiveConnection->connection()before dereferencing to get the UUID, asActiveConnection::Ptrmay be valid while its associatedconnection()is null in some transitional states. - In
NetworkThread::enableDevice, the newqDebug()call for the successful connection is inconsistent with the surrounding logging (qCDebug(DSM)); using the same debug macro and category would keep log output uniform and easier to filter.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `NetworkThread::onDevicestateChanged`, consider guarding `activeConnection->connection()` before dereferencing to get the UUID, as `ActiveConnection::Ptr` may be valid while its associated `connection()` is null in some transitional states.
- In `NetworkThread::enableDevice`, the new `qDebug()` call for the successful connection is inconsistent with the surrounding logging (`qCDebug(DSM)`); using the same debug macro and category would keep log output uniform and easier to filter.
## Individual Comments
### Comment 1
<location path="network-service-plugin/src/system/networkthread.cpp" line_range="313-315" />
<code_context>
+ }
}
}
+ if (!connPath0.isEmpty()) {
+ NetworkManager::activateConnection(connPath0, device->uni(), QString());
+ qDebug() << "connected:" << connPath0;
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Connection activation now occurs before checking/enabling global networking state.
This changes the previous behavior where activation only occurred after confirming/enabling `NetworkManager::isNetworkingEnabled()`. Now `NetworkManager::activateConnection` can be called while networking is disabled, which may cause failures or different semantics. Please either restore activation to after the networking-enabled check or explicitly handle/document the disabled-networking case when calling `activateConnection`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+313
to
+315
| if (!connPath0.isEmpty()) { | ||
| NetworkManager::activateConnection(connPath0, device->uni(), QString()); | ||
| qDebug() << "connected:" << connPath0; |
There was a problem hiding this comment.
issue (bug_risk): Connection activation now occurs before checking/enabling global networking state.
This changes the previous behavior where activation only occurred after confirming/enabling NetworkManager::isNetworkingEnabled(). Now NetworkManager::activateConnection can be called while networking is disabled, which may cause failures or different semantics. Please either restore activation to after the networking-enabled check or explicitly handle/document the disabled-networking case when calling activateConnection.
…network interface After a successful connection, the latest connection time failed to save correctly, leading to errors when manually retrieving connections. The solution involves saving the UUID of the currently successful connection to a configuration file. When the network interface is re-enabled, the connection configuration is read directly from this configuration file and actively reconnected. Log: Fix reconnection error after enabling network interface BUG: PMS-350785 fix: 修复关闭再开启后选择连接网络错误 在连接成功后，最新的连接的时间保存失败，手动获取的连接错误，修改为保存当前连接成功的连接的uuid到配置文件，开启网卡后直接从配置文件读取连接配置并主动连接 Log: 修复开启网卡后回连错误的问题
caixr23
approved these changes
Mar 9, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23, ut003640 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 |
deepin pr auto review代码审查报告总体概述这次代码变更主要是为了改进网络设备的连接管理逻辑，特别是保存和恢复网络设备的连接信息。主要变更包括添加了连接信息的持久化存储、修改了设备启用时的连接逻辑，以及删除了一些不再需要的代码。 语法和逻辑分析1. NetworkEnabledConfig 类变更优点：
问题：
改进建议：// 在 NetworkEnabledConfig 类中添加私有方法
private:
QVariantMap loadConnections(const QJsonObject &jsonObj) {
QJsonObject connObj = jsonObj.value("Connections").toObject();
QVariantMap conn_map;
for (const QString &conn_key : connObj.keys()) {
QString curr_obj_value = connObj.value(conn_key).toString();
conn_map.insert(conn_key, curr_obj_value);
}
return conn_map;
}
QJsonObject saveConnections(const QVariantMap &conn_map) {
QJsonObject connectionObj;
for (auto it = conn_map.constBegin(); it != conn_map.constEnd(); it++) {
connectionObj.insert(it.key(), it.value().toString());
}
return connectionObj;
}
// 修改 setConnectionInfo 方法，添加参数检查
void NetworkEnabledConfig::setConnectionInfo(const QString &dev, const QString &uuid)
{
if (dev.isEmpty() || uuid.isEmpty()) {
qCWarning(DSM()) << "Invalid parameters for setConnectionInfo";
return;
}
QVariant conn = m_map["Connections"];
QVariantMap conn_map = conn.value<QVariantMap>();
conn_map[dev] = uuid;
m_map["Connections"] = conn_map;
}
// 修改 connectionUuid 方法，添加参数检查
QString NetworkEnabledConfig::connectionUuid(const QString &dev) const
{
if (dev.isEmpty()) {
qCWarning(DSM()) << "Invalid parameter for connectionUuid";
return QString();
}
QVariant conn = m_map["Connections"];
QVariantMap conn_map = conn.value<QVariantMap>();
if (conn_map.contains(dev))
return conn_map.value(dev).toString();
return QString();
}2. NetworkThread 类变更优点：
问题：
改进建议：// 修改 onDevicestateChanged 方法
void NetworkThread::onDevicestateChanged(NetworkManager::Device::State newState, NetworkManager::Device::State oldState, NetworkManager::Device::StateChangeReason reason)
{
// ... 现有代码 ...
Device::State state = dev->state();
if (enabled) {
if (state == Device::Activated) {
qCDebug(DSM) << "device connection success";
NetworkManager::ActiveConnection::Ptr activeConnection = dev->activeConnection();
if (activeConnection && activeConnection->connection()) {
m_networkConfig->setConnectionInfo(dev->interfaceName(), activeConnection->connection()->uuid());
// 考虑使用定时器延迟保存，避免频繁I/O
m_saveConfigTimer.start(1000); // 1秒后保存
}
}
} else {
// ... 现有代码 ...
}
}
// 修改 enableDevice 方法
QString NetworkThread::enableDevice(NetworkManager::Device::Ptr device)
{
QString connPath0;
const QString &device_uuid = m_networkConfig->connectionUuid(device->interfaceName());
if (!device_uuid.isEmpty()) {
NetworkManager::Connection::Ptr current_connection = NetworkManager::findConnectionByUuid(device_uuid);
if (current_connection && current_connection->settings()->autoconnect()) {
connPath0 = current_connection->path();
}
}
if (connPath0.isEmpty()) {
// ... 现有代码 ...
}
// 添加错误检查
if (!connPath0.isEmpty()) {
NetworkManager::Connection::Ptr conn = NetworkManager::findConnection(connPath0);
if (!conn) {
qCWarning(DSM()) << "Connection not found:" << connPath0;
return QString();
}
NetworkManager::activateConnection(connPath0, device->uni(), QString());
qDebug() << "connected:" << connPath0;
}
// ... 现有代码 ...
}3. DeviceManagerRealize 类变更优点：
问题：
改进建议：void DeviceManagerRealize::setEnabled(bool enabled)
{
qCDebug(DNC) << QString("set Device %1, enabled: %2").arg(m_device->uni()).arg(enabled ? "true" : "false");
QDBusInterface dbusInter(SYS_NETWORK_SERVICE, SYS_NETWORK_PATH, SYS_NETWORK_INTER, QDBusConnection::systemBus());
QDBusReply<QDBusObjectPath> reply = dbusInter.call("EnableDevice", m_device->uni(), enabled);
if (!reply.isValid()) {
qCWarning(DNC) << "Failed to enable device:" << reply.error().message();
return;
}
// 如果启用设备，确保自动连接逻辑在服务端处理
if (enabled) {
qCDebug(DNC) << "Device enabled successfully";
}
}代码质量评估
安全性评估
总结与建议
这些改进将有助于提高代码的质量、可靠性和可维护性。 |
Contributor
Author
|
/merge |
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.
After a successful connection, the latest connection time failed to save correctly, leading to errors when manually retrieving connections. The solution involves saving the UUID of the currently successful connection to a configuration file. When the network interface is re-enabled, the connection configuration is read directly from this configuration file and actively reconnected.
Log: Fix reconnection error after enabling network interface
BUG: PMS-350785
fix: 修复关闭再开启后选择连接网络错误
在连接成功后，最新的连接的时间保存失败，手动获取的连接错误，修改为保存当前连接成功的连接的uuid到配置文件，开启网卡后直接从配置文件读取连接配置并主动连接
Log: 修复开启网卡后回连错误的问题
Summary by Sourcery
Persist and reuse the last successful network connection per device to ensure correct reconnection after disabling and re-enabling network interfaces.
Bug Fixes:
Enhancements: