Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nvme/056: Add check for empty ddp capabilities #159

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

nuclearcat
Copy link
Contributor

Fixes a shell syntax error that occurs when trying to perform bitwise AND operation on empty ddp capabilities.
Now explicitly checks if ddp_caps hw returns empty string before proceeding with capability check.
Improves error handling for non-ddp capable devices like qemu virtio-net.

Without this patch qemu with virtio-net returns error:

tests/nvme/056: line 222: & 3: syntax error: operand expected (error token is "& 3")

With patch:

nvme/056 (tr=tcp) (enable zero copy offload and run rw traffic) [not run]
    runtime    ...  0.150s
    No ddp capabilities found for enp0s6

Fixes a shell syntax error that occurs when trying to perform bitwise
AND operation on empty ddp capabilities.
Now explicitly checks if ddp_caps hw returns empty string before
proceeding with capability check.
Improves error handling for non-ddp capable devices like qemu virtio-net.

Without this patch qemu with virtio-net returns error:
```
tests/nvme/056: line 222: & 3: syntax error: operand expected (error token is "& 3")
```

With patch:
```
nvme/056 (tr=tcp) (enable zero copy offload and run rw traffic) [not run]
    runtime    ...  0.150s
    No ddp capabilities found for enp0s6
```

Signed-off-by: Denys Fedoryshchenko <[email protected]>
@kawasaki
Copy link
Collaborator

@nuclearcat Thanks for the fix. It looks good to me. I'll wait a few more days before merge just in case @aaptel has any comments.

@aaptel FYI.

@kawasaki kawasaki merged commit 8bec47b into osandov:master Feb 18, 2025
5 checks passed
@kawasaki
Copy link
Collaborator

I have merged this PR. Thanks!

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