Skip to content

fix: add UI error handling for direct IO copy operations#3564

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Johnson-zs:master
Feb 5, 2026
Merged

fix: add UI error handling for direct IO copy operations#3564
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Johnson-zs:master

Conversation

@Johnson-zs
Copy link
Contributor

@Johnson-zs Johnson-zs commented Feb 5, 2026

  1. Removed redundant error message parameters from doHandleErrorAndWait calls
  2. Added immediate errno saving to prevent value corruption
  3. Implemented system error to job error type mapping function
  4. Added UI dialog support for read/write errors during direct IO copy
  5. Implemented retry mechanism for failed operations with proper file seeking

Log: Fixed direct IO copy operations to show proper error dialogs instead of silently failing

Influence:

  1. Test direct IO copy with insufficient disk space scenarios
  2. Verify permission denied errors trigger proper UI dialogs
  3. Test retry functionality for transient I/O errors
  4. Validate error mapping for various system error codes
  5. Check file position handling during retry operations

fix: 为直接IO拷贝操作添加UI错误处理

  1. 从doHandleErrorAndWait调用中移除冗余的错误消息参数
  2. 添加立即保存errno值以防止值被破坏
  3. 实现系统错误到作业错误类型的映射函数
  4. 为直接IO拷贝过程中的读写错误添加UI对话框支持
  5. 实现失败操作的重试机制,包含正确的文件定位处理

Log: 修复直接IO拷贝操作,现在会显示正确的错误对话框而不是静默失败

Influence:

  1. 测试磁盘空间不足场景下的直接IO拷贝
  2. 验证权限拒绝错误是否正确触发UI对话框
  3. 测试瞬时I/O错误的重试功能
  4. 验证各种系统错误代码的错误映射
  5. 检查重试操作期间的文件位置处理

Summary by Sourcery

Handle direct I/O copy read/write failures with proper UI error reporting and retry support.

Bug Fixes:

  • Ensure direct I/O copy read and write errors surface as job errors instead of silently failing.
  • Preserve errno values during direct I/O operations to avoid incorrect error handling.
  • Correctly restore file positions when retrying failed read or write chunks in direct I/O copies.

Enhancements:

  • Map system errno values to high-level job error types for consistent UI error dialogs during direct I/O copies.
  • Simplify error handling calls by removing redundant error message parameters from direct I/O copy operations.

1. Removed redundant error message parameters from doHandleErrorAndWait
calls
2. Added immediate errno saving to prevent value corruption
3. Implemented system error to job error type mapping function
4. Added UI dialog support for read/write errors during direct IO copy
5. Implemented retry mechanism for failed operations with proper file
seeking

Log: Fixed direct IO copy operations to show proper error dialogs
instead of silently failing

Influence:
1. Test direct IO copy with insufficient disk space scenarios
2. Verify permission denied errors trigger proper UI dialogs
3. Test retry functionality for transient I/O errors
4. Validate error mapping for various system error codes
5. Check file position handling during retry operations

fix: 为直接IO拷贝操作添加UI错误处理

1. 从doHandleErrorAndWait调用中移除冗余的错误消息参数
2. 添加立即保存errno值以防止值被破坏
3. 实现系统错误到作业错误类型的映射函数
4. 为直接IO拷贝过程中的读写错误添加UI对话框支持
5. 实现失败操作的重试机制,包含正确的文件定位处理

Log: 修复直接IO拷贝操作,现在会显示正确的错误对话框而不是静默失败

Influence:
1. 测试磁盘空间不足场景下的直接IO拷贝
2. 验证权限拒绝错误是否正确触发UI对话框
3. 测试瞬时I/O错误的重试功能
4. 验证各种系统错误代码的错误映射
5. 检查重试操作期间的文件位置处理
@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Johnson-zs

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

@sourcery-ai
Copy link

sourcery-ai bot commented Feb 5, 2026

Reviewer's Guide

Adds robust UI-visible error handling and retry support for direct I/O copy operations by mapping system errno values to job error types, saving errno early, and wiring read/write failures into the existing job handler dialog flow.

Sequence diagram for direct IO copy error handling and retry

sequenceDiagram
    actor User
    participant DoCopyFileWorker
    participant AbstractJobHandler
    participant FileSystem

    User->>DoCopyFileWorker: doCopyFileWithDirectIO
    DoCopyFileWorker->>FileSystem: read chunk with O_DIRECT
    FileSystem-->>DoCopyFileWorker: error, sets errno

    alt read_error
        DoCopyFileWorker->>DoCopyFileWorker: savedErrno = errno
        DoCopyFileWorker->>DoCopyFileWorker: jobError = mapSystemErrorToJobError(savedErrno, false)
        DoCopyFileWorker->>AbstractJobHandler: doHandleErrorAndWait(jobError, isWriteError false)
        AbstractJobHandler-->>DoCopyFileWorker: action (Retry or Skip or Cancel)

        alt action is Retry
            DoCopyFileWorker->>FileSystem: lseek(srcFd, copied, SEEK_SET)
            FileSystem-->>DoCopyFileWorker: seek result
            DoCopyFileWorker->>FileSystem: retry read chunk
            FileSystem-->>DoCopyFileWorker: data or error
        else action is Skip or Cancel
            DoCopyFileWorker->>DoCopyFileWorker: success = false
            DoCopyFileWorker-->>User: operation skipped or cancelled
        end
    end

    alt write_error
        DoCopyFileWorker->>FileSystem: write chunk with O_DIRECT
        FileSystem-->>DoCopyFileWorker: error, sets errno
        DoCopyFileWorker->>DoCopyFileWorker: savedErrno = errno

        opt O_DIRECT_invalid
            DoCopyFileWorker->>FileSystem: fcntl remove O_DIRECT
            FileSystem-->>DoCopyFileWorker: flags updated
            DoCopyFileWorker->>FileSystem: retry write without O_DIRECT
            FileSystem-->>DoCopyFileWorker: success or error
        end

        DoCopyFileWorker->>DoCopyFileWorker: jobError = mapSystemErrorToJobError(savedErrno, true)
        DoCopyFileWorker->>AbstractJobHandler: doHandleErrorAndWait(jobError, isWriteError true)
        AbstractJobHandler-->>DoCopyFileWorker: action (Retry or Skip or Cancel)

        alt action is Retry
            DoCopyFileWorker->>FileSystem: lseek(writer.fd, copied + bytesWritten, SEEK_SET)
            FileSystem-->>DoCopyFileWorker: seek result
            DoCopyFileWorker->>FileSystem: retry write chunk
            FileSystem-->>DoCopyFileWorker: success or error
        else action is Skip or Cancel
            DoCopyFileWorker->>DoCopyFileWorker: success = false
            DoCopyFileWorker-->>User: operation skipped or cancelled
        end
    end
Loading

Class diagram for DoCopyFileWorker error mapping and handling changes

classDiagram
    class DoCopyFileWorker {
        +doCopyFileWithDirectIO(fromInfo, toInfo, skip) NextDo
        -handlePauseResume(writer, dest, skip) bool
        -actionToNextDo(action, size, skip) NextDo
        -shouldFallbackFromCopyFileRange(errorCode) bool
        -mapSystemErrorToJobError(systemErrno, isWriteError) JobErrorType
    }

    class AbstractJobHandler {
        <<interface>> AbstractJobHandler
        +doHandleErrorAndWait(fromUri, toUri, jobError, isWriteError) SupportAction
    }

    class JobErrorType {
        <<enumeration>> JobErrorType
        kOpenError
        kNotEnoughSpaceError
        kPermissionDeniedError
        kWriteError
        kReadError
        kProrogramError
        kNonexistenceError
    }

    class SupportAction {
        <<enumeration>> SupportAction
        kRetryAction
        kSkipAction
        kCancelAction
    }

    DoCopyFileWorker ..> AbstractJobHandler : uses
    DoCopyFileWorker ..> JobErrorType : returns
    DoCopyFileWorker ..> SupportAction : converts_to
    AbstractJobHandler ..> JobErrorType : consumes
    AbstractJobHandler ..> SupportAction : returns
    AbstractJobHandler o-- JobErrorType
    AbstractJobHandler o-- SupportAction
Loading

File-Level Changes

Change Details Files
Wire O_DIRECT read errors into the UI error dialog flow with retry support and correct file seeking.
  • On non-EINTR read failures, save errno immediately to a local variable before logging or other calls.
  • Map the saved errno to a JobErrorType via a new helper, then call doHandleErrorAndWait without a custom message.
  • If the user chooses Retry, lseek the source fd back to the current copied offset and retry the read; otherwise mark the operation as failed.
src/plugins/common/dfmplugin-fileoperations/fileoperations/fileoperationutils/docopyfileworker.cpp
Wire O_DIRECT write errors into the UI error dialog flow, preserving errno and supporting retry with proper seeking.
  • Save errno to a local variable before any handling, and use that for all subsequent decisions and logging.
  • Reuse the EINVAL/O_DIRECT fallback but keyed off the saved errno instead of errno directly.
  • After a write error, map the saved errno to a JobErrorType, call doHandleErrorAndWait, and on Retry seek the writer fd to copied + bytesWritten before retrying; otherwise fail the operation.
src/plugins/common/dfmplugin-fileoperations/fileoperations/fileoperationutils/docopyfileworker.cpp
Simplify error handler calls for open failures and introduce a reusable errno-to-job-error mapping helper.
  • Remove redundant formatted error-message QString parameters from doHandleErrorAndWait calls for source/destination open failures, relying on JobErrorType instead.
  • Add a new private helper mapSystemErrorToJobError(int systemErrno, bool isWriteError) with explicit mappings for ENOSPC, EACCES/EPERM, EIO, EBADF/EINVAL, ENOENT, and a read/write-oriented default.
  • Declare the new helper in DoCopyFileWorker’s private interface in the header.
src/plugins/common/dfmplugin-fileoperations/fileoperations/fileoperationutils/docopyfileworker.cpp
src/plugins/common/dfmplugin-fileoperations/fileoperations/fileoperationutils/docopyfileworker.h

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

这份代码 diff 主要针对 DoCopyFileWorker 类中的文件复制逻辑进行了改进,特别是在处理 Direct I/O 模式下的错误处理和重试机制上。以下是对代码的详细审查意见,分为语法逻辑、代码质量、代码性能和代码安全四个方面:

1. 语法逻辑

  • 错误处理的改进(优点)

    • 代码引入了 mapSystemErrorToJobError 函数,将底层的 errno 映射为业务层的 JobErrorType。这使得错误处理更加统一和结构化,避免了在错误发生处硬编码错误类型。
    • 在读取和写入失败时,增加了 kRetryAction 的处理逻辑。通过 lseek 将文件指针回退到正确的位置后 continue,实现了对特定错误的自动重试,逻辑清晰且符合预期。
  • 潜在逻辑问题(建议)

    • lseek 的原子性问题:在 kRetryAction 分支中,代码先执行 lseek,如果失败则 break。如果在高并发或复杂的文件系统环境下,lseek 和随后的 read/write 之间如果文件描述符被其他线程修改(虽然在此类 Worker 模型中可能性较小,但在共享文件描述符时需注意),可能会导致重试位置偏移。不过考虑到这是单线程的 Worker 模型,此风险较低。
    • EINVAL 的特殊处理:在写入错误处理中,保留了针对 EINVAL 的 O_DIRECT 降级逻辑。这是一个很好的回退机制。但需要注意的是,mapSystemErrorToJobError 函数中 EINVAL 被映射为 kProrogramError(注意拼写:Program 拼写错误,见下文)。如果在 O_DIRECT 降级逻辑中捕获了 EINVAL,它会先尝试降级,如果降级失败(fcntl 失败)才会走到下面的 doHandleErrorAndWait。这里的逻辑是正确的,即优先尝试技术性恢复(降级),失败后再报错给用户。

2. 代码质量

  • 拼写错误(必须修复)

    • mapSystemErrorToJobError 函数中,case EINVAL: 返回的是 AbstractJobHandler::JobErrorType::kProrogramError;
    • 问题Prorogram 是拼写错误,应为 Program
    • 建议:请修正为 kProgramError,否则可能导致编译错误或运行时无法匹配到正确的错误枚举值。
  • 错误信息的保留(优点)

    • 在调用 strerror(errno) 之前,使用 savedErrno = errno 保存了错误码。这是非常必要的,因为 strerror 或其他日志函数本身可能会修改 errno,导致后续逻辑判断错误。这是一个很好的实践。
  • 代码可读性(优点)

    • 通过提取 mapSystemErrorToJobError 函数,消除了重复的 switch-case 逻辑,使主流程代码更加简洁。
    • 注释清晰,解释了为什么需要保存 errno 以及重试时的 lseek 操作。

3. 代码性能

  • 系统调用开销(中立)

    • 引入重试机制意味着在发生 I/O 错误时,会增加额外的 lseek 系统调用。这是为了容错性必须付出的代价。
    • mapSystemErrorToJobError 是纯函数,开销极小,对性能无负面影响。
  • Direct I/O 降级(优点)

    • 保留了针对 EINVAL 的 O_DIRECT 降级逻辑。这是性能优化的关键点,允许在不支持 Direct I/O 的文件系统上自动回退到缓冲 I/O,保证操作的通用性。

4. 代码安全

  • 资源泄漏风险(低风险)

    • doCopyFileWithDirectIO 函数中,如果 readwrite 发生错误并进入 kRetryAction 流程,代码通过 continue 回到了循环顶部。此时 srcFdwriter.fd 依然是打开状态,这是符合预期的(因为要重试)。
    • 如果 lseek 失败,代码执行 success = false; break;。随后函数会跳出循环,最终到达函数末尾或返回点。根据上下文(diff 未完全展示),通常 writer 对象会在析构时自动关闭 fd(RAII 习惯),或者有显式的 close 调用。如果 writer 不是 RAII 管理的,需要确保在 break 出循环后有资源清理逻辑。从代码风格看,FileWriter 很可能封装了 close 操作,因此风险较低。
  • 错误码覆盖(已解决)

    • 前文提到的 savedErrno 解决了安全风险,防止了错误码在日志输出过程中被覆盖。

总结与建议

  1. 必须修改:修正 mapSystemErrorToJobError 函数中的拼写错误,将 kProrogramError 改为 kProgramError
  2. 逻辑确认:确认 FileWriter 类在析构或销毁时会自动关闭文件描述符 fd,以确保在 lseek 失败导致 break 时不会发生文件描述符泄漏。
  3. 代码审查通过:除了拼写错误外,该代码改进显著增强了 Direct I/O 复制操作的健壮性,错误处理更加完善,重试机制设计合理。

修正后的代码片段建议:

AbstractJobHandler::JobErrorType DoCopyFileWorker::mapSystemErrorToJobError(int systemErrno, bool isWriteError)
{
    switch (systemErrno) {
    case ENOSPC:   // No space left on device
        return AbstractJobHandler::JobErrorType::kNotEnoughSpaceError;
    case EACCES:   // Permission denied
    case EPERM:   // Operation not permitted
        return AbstractJobHandler::JobErrorType::kPermissionDeniedError;
    case EIO:   // I/O error
        return isWriteError ? AbstractJobHandler::JobErrorType::kWriteError
                            : AbstractJobHandler::JobErrorType::kReadError;
    case EBADF:   // Bad file descriptor
    case EINVAL:   // Invalid argument
        return AbstractJobHandler::JobErrorType::kProgramError; // 修正拼写
    case ENOENT:   // No such file or directory
        return AbstractJobHandler::JobErrorType::kNonexistenceError;
    default:
        // Default to read/write error based on operation type
        return isWriteError ? AbstractJobHandler::JobErrorType::kWriteError
                            : AbstractJobHandler::JobErrorType::kReadError;
    }
}

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `src/plugins/common/dfmplugin-fileoperations/fileoperations/fileoperationutils/docopyfileworker.cpp:380` </location>
<code_context>
+
+                if (action == AbstractJobHandler::SupportAction::kRetryAction) {
+                    // Seek to retry writing this chunk
+                    if (lseek(writer.fd, copied + bytesWritten, SEEK_SET) < 0) {
+                        success = false;
+                        break;
</code_context>

<issue_to_address>
**issue (bug_risk):** Using `copied + bytesWritten` for the retry seek after a write error is incorrect because `bytesWritten` is negative on error.

On error `bytesWritten` is `< 0`, so `lseek(writer.fd, copied + bytesWritten, SEEK_SET)` seeks *before* the last confirmed offset and can corrupt the destination. For a retry you should seek to the last known good offset (e.g. `copied` or `copied + totalWrittenForThisChunk`), not use the failing `bytesWritten` value. Track the intended write offset separately and use that for `lseek`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Johnson-zs
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Feb 5, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 6ce4b89 into linuxdeepin:master Feb 5, 2026
21 checks 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.

2 participants