Skip to content

feat(control): expose VT management over DDE control socket#3

Merged
zccrs merged 1 commit into
linuxdeepin:dde/0.9.3from
zccrs:vt-control-socket-refactor
Jun 3, 2026
Merged

feat(control): expose VT management over DDE control socket#3
zccrs merged 1 commit into
linuxdeepin:dde/0.9.3from
zccrs:vt-control-socket-refactor

Conversation

@zccrs
Copy link
Copy Markdown
Member

@zccrs zccrs commented Jun 2, 2026

Summary

  • add control-socket operations for active VT query, free VT query, and VT switching
  • reuse the existing seat VT state machine for grouped and unmanaged switches
  • prepare ddm to drop direct VT ioctls and route VT control through dde-seatd

Test

  • meson compile -C build

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zccrs

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@zccrs zccrs force-pushed the vt-control-socket-refactor branch from a8bd4a5 to e9fcf19 Compare June 2, 2026 06:34
@zccrs zccrs requested a review from Copilot June 2, 2026 06:38
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

EN: This PR extends dde-seatd’s control socket protocol to expose basic VT management (query active VT, query a free VT, and request a VT switch), reusing the seat VT state machine so VT switching can be routed through dde-seatd instead of direct ioctls in downstream components (e.g. ddm).

中文: 该 PR 扩展了 dde-seatd 的 control socket 协议,新增 VT 管理能力(查询当前活动 VT、查询空闲 VT、请求切换 VT),并复用现有 seat 的 VT 状态机,从而让下游组件(如 ddm)可以通过 dde-seatd 统一进行 VT 控制而不是直接 ioctl。

Changes / 变更:

  • Add seat-level APIs for active VT query, free VT query, and VT switch request. / 新增 seat 层接口:获取活动 VT、查找空闲 VT、请求切换 VT。
  • Extend control socket opcodes and implement request handlers + responses (CONTROL_OK, VT state events). / 扩展控制协议 opcode,并实现请求处理与响应(CONTROL_OK、VT 状态事件)。
  • Update public headers for the new APIs and protocol structs. / 更新公共头文件以暴露新接口与协议结构体。

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
seatd/seat.c Adds VT query/switch entry points in the seat layer.
seatd/control.c Implements new control-socket operations and response helpers for VT management.
include/seat.h Exposes new seat VT APIs in the public header.
include/control.h Adds new control protocol opcodes and request/response structs.

Comment thread seatd/seat.c
Comment thread seatd/seat.c Outdated
@zccrs zccrs force-pushed the vt-control-socket-refactor branch from e9fcf19 to b76bafb Compare June 2, 2026 09:20
@zccrs zccrs requested a review from Copilot June 2, 2026 10:48
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comment thread seatd/seat.c
Comment thread seatd/seat.c
Comment thread seatd/seat.c
@zccrs zccrs force-pushed the vt-control-socket-refactor branch from b76bafb to 2d6f203 Compare June 2, 2026 11:58
@zccrs
Copy link
Copy Markdown
Member Author

zccrs commented Jun 2, 2026

Addressed the valuable review findings in this branch:

  • keep seat->cur_vt = -1 on seat_update_vt() failure so VT lookup failures continue to use the existing sentinel consistently
  • preserve the original errno in seat_find_available_vt() so close() does not clobber the control-socket error reported to callers

I also checked the other Copilot point about vt_switch() / terminal_switch_vt() failure handling. That part is already handled in the current branch: vt_switch() checks terminal_switch_vt() and seat_switch_vt() clears pending_vt_switch on failure, so no extra change was needed there.

Local validation: ninja -C build

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comment thread seatd/seat.c
Comment thread seatd/seat.c
Comment thread seatd/seat.c
Add active-vt, free-vt, and switch-vt control operations so DDM can stop issuing VT ioctls directly.

添加 active-vt、free-vt 和 switch-vt control 操作,使DDM不再直接发起VT ioctl。

Reuse the existing seat VT state machine for grouped and unmanaged switches.

复用现有 seat VT 状态机处理 grouped 与非托管切换。

Log: 为control socket补齐VT管理能力

Influence: DDM可通过dde-seatd统一查询和切换VT,减少双写状态机。
@zccrs zccrs force-pushed the vt-control-socket-refactor branch from 2d6f203 to 2855aa0 Compare June 2, 2026 13:17
@zccrs
Copy link
Copy Markdown
Member Author

zccrs commented Jun 2, 2026

@copilot review

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次修改主要为seatd添加了获取活动VT、查找空闲VT以及切换VT的控制接口,并对VT状态更新的错误处理进行了增强。

整体来看,代码结构清晰,对系统调用的错误处理(如保存errno防止被close覆盖)做得很到位。但为了进一步提升代码的语法逻辑、质量、性能和安全性,我提出以下改进意见:

1. 语法与逻辑问题

  • seat_update_vt 中的 errno 逻辑存在隐患:
    seat.cseat_update_vt 函数中,你使用了 int saved_errno = errno;,并在 vt <= 0 时设置 errno = saved_errno != 0 ? saved_errno : ENOENT;
    问题:在C语言中,如果 terminal_current_vt 成功执行并返回了一个正值,它不一定会将 errno 重置为 0(C标准只规定函数出错时设置errno,成功时不强制清零)。如果 terminal_current_vt 成功但 errno 恰好残留了之前的错误码,这里会误判。
    建议:在调用可能失败的函数前,主动将 errno 置 0,或者仅依赖函数的返回值来判断是否出错,出错时再去读 errno
  • seat_find_available_vt 中不必要的 errno 保存:
    seat.cseat_find_available_vt 中,close(tty0fd) 不会影响 terminal_find_available 产生的错误码,因为如果 vt == -1,你直接返回了 -1,此时 errno 本来就是 terminal_find_available 设置的值,无需额外保存和恢复。
  • seat_switch_vt 中冗余的 seat_update_vt 调用:
    seat_switch_vt 中,为了判断 seat->cur_vt == vt,你调用了 seat_update_vt(seat)。但在多进程/多线程环境中,获取当前VT和切换VT并不是原子操作(TOCTOU问题)。而且,内核的 VT_ACTIVATE 本身就能处理重复切换到当前VT的情况。
    建议:如果仅仅是为了避免不必要的切换,这个逻辑可以保留,但需意识到其非原子性;如果是为了性能,可以考虑去掉这次更新,直接交给底层内核处理。

2. 代码质量与可读性

  • 硬编码的 "seat0" 字符串:
    control.chandle_get_active_vthandle_find_free_vthandle_switch_vt 中,都使用了 server_get_seat(client->server, "seat0")
    建议:将 "seat0" 定义为宏或常量字符串(如 #define DEFAULT_SEAT_NAME "seat0"),或者考虑是否应该让客户端在请求中指定 seat 名称,以提高代码的扩展性。
  • control_send_ok 发送空载荷的规范性:
    control_send_okheader.size = 0,并只发送了 header。这在协议设计上没问题,但需确保 control_client_handle_opcode 中对 CONTROL_OK 的解析端也严格认为其 size 为 0,否则容易出现读写阻塞。

3. 代码性能

  • 频繁打开和关闭终端设备文件:
    seat_update_vtseat_find_available_vtvt_switch 每次被调用都会执行 terminal_open(0)close(tty0fd)。系统调用 openclose 开销较大,尤其是在高频查询或切换VT时。
    建议:如果业务场景允许,可以考虑在 seat 结构体中缓存 tty0 的文件描述符(FD),在 seat 生命周期内保持打开,或者使用一个全局的 FD,避免频繁的 open/close 系统调用。

4. 代码安全

  • 输入校验:对 request.vt 缺乏上界检查:
    handle_switch_vt 中,从网络/IPC接收到的 request.vt 仅在 seat_switch_vt 中检查了 vt <= 0。恶意或异常客户端可能会发送一个非常大的值(如 INT32_MAX)。
    建议:在 seat_switch_vt 中增加 VT 的上界检查。通常 Linux 的 VT 数量是有限的(一般最大为 63 或 12,取决于 MAX_NR_CONSOLES 配置)。应当拒绝超出合理范围的 VT 号。
  • seat_update_vtvt <= 0 的判断:
    terminal_current_vt 返回 0 的情况在某些边缘场景(如无活动控制台)可能发生,返回负值通常表示错误。当前逻辑将 vt == 0 也视为错误并覆盖 errnoENOENT,这是合理的防御性编程,建议保留,但最好增加注释说明为何 0 也被视为无效。

改进后的代码示例

针对上述意见,以下是修改后的关键代码片段:

seat.c 改进:

static int seat_update_vt(struct seat *seat) {
	int tty0fd = terminal_open(0);
	if (tty0fd == -1) {
		log_errorf("Could not open tty0 to update VT: %s", strerror(errno));
		return -1;
	}

    // 建议:调用前清空errno,防止残留值干扰成功返回时的判断
	errno = 0; 
	int vt = terminal_current_vt(tty0fd);
	int saved_errno = errno; // 如果terminal_current_vt出错,这里捕获
	close(tty0fd);
	
	if (vt <= 0) {
        // 如果vt<=0且errno未被设置,则默认返回ENOENT
		errno = saved_errno != 0 ? saved_errno : ENOENT;
		return -1;
	}

	seat->cur_vt = vt;
	return 0;
}

int seat_find_available_vt(struct seat *seat) {
	if (!seat->vt_bound) {
		errno = EINVAL;
		return -1;
	}

	int tty0fd = terminal_open(0);
	if (tty0fd == -1) {
		return -1;
	}

	int vt = terminal_find_available(tty0fd);
	close(tty0fd); // 直接关闭,如果vt==-1,errno不受close影响
    
    // 移除了冗余的saved_errno逻辑
	return vt;
}

// 增加 VT 合法范围限制
#define MAX_VT_NUMBER 63 // 根据实际系统内核配置调整

int seat_switch_vt(struct seat *seat, int vt) {
	if (!seat->vt_bound) {
		errno = EINVAL;
		return -1;
	}
    // 增加 vt 上界检查,防止恶意输入
	if (vt <= 0 || vt > MAX_VT_NUMBER) {
		errno = EINVAL;
		return -1;
	}

    // 保留当前VT检查逻辑,但需注意非原子性
	if (seat_update_vt(seat) == -1) {
		return -1;
	}
	if (seat->cur_vt == vt) {
		return 0;
	}

	if (seat->pending_vt_switch > 0) {
		errno = EBUSY;
		return -1;
	}

	seat->pending_vt_switch = vt;
	if (vt_switch(seat, vt) == -1) {
		seat->pending_vt_switch = 0;
		return -1;
	}
	return 0;
}

control.c 改进:

#define DEFAULT_SEAT_NAME "seat0"

static int handle_get_active_vt(struct control_client *client) {
    // 使用宏替代硬编码
	struct seat *seat = server_get_seat(client->server, DEFAULT_SEAT_NAME);
	if (seat == NULL) {
		return control_send_error(client, ENOENT);
	}

	int vt = seat_get_active_vt(seat);
	if (vt == -1) {
		return control_send_error(client, errno);
	}
	return control_send_vt_state(client, CONTROL_ACTIVE_VT, vt);
}

// handle_find_free_vt 和 handle_switch_vt 同样替换 "seat0" 为 DEFAULT_SEAT_NAME

希望这些审查意见和代码改进能对你有所帮助!如果有任何疑问,欢迎随时提问。

@zccrs zccrs requested a review from Copilot June 2, 2026 13:24
@zccrs zccrs requested review from Copilot and removed request for Copilot June 2, 2026 13:29
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread seatd/seat.c
Comment thread seatd/seat.c
@zccrs zccrs merged commit 0dffb8e into linuxdeepin:dde/0.9.3 Jun 3, 2026
10 checks passed
@zccrs zccrs deleted the vt-control-socket-refactor branch June 3, 2026 02:35
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