Skip to content

Conversation

@liuw
Copy link
Member

@liuw liuw commented Dec 21, 2024

The original code relied on the default read_vectored or write_vectored implementations from the standard library.

The default implementation of those functions only uses the first non-empty buffer. That's not correct when there are more than one buffers.

Fixes: #6876

This should be backported.

The original code relied on the default `read_vectored` or
`write_vectored` implementations from the standard library.

The default implementation of those functions only uses the first
non-empty buffer. That's not correct when there are more than one
buffers.

Fixes: cloud-hypervisor#6876
Signed-off-by: Wei Liu <[email protected]>
@liuw liuw requested a review from a team as a code owner December 21, 2024 00:24
.map_err(AsyncIoError::ReadVectored)?
let mut r = 0;
for b in slices.iter_mut() {
r += file.read(b).map_err(AsyncIoError::ReadVectored)?;
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to think about using libc::readv to minimise the syscalls.

Copy link
Member Author

Choose a reason for hiding this comment

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

That cannot be done easily. VHDX and QCOW2 disks are not regular files. Their read and write implementation does a bunch of calculation before reading and writing the actual file on disk.

@rbradford rbradford added this pull request to the merge queue Dec 21, 2024
Merged via the queue into cloud-hypervisor:main with commit b2f40af Dec 21, 2024
38 checks passed
@liuw liuw deleted the fix-async-adaptor branch December 21, 2024 20:18
@likebreath likebreath added the bug-fix Bug fix to include in release notes label Jan 1, 2025
@likebreath
Copy link
Member

@liuw Thank you for fixing the issue. Please add a label bug-fix for such case, particularly if a PR needs to be backported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix Bug fix to include in release notes

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

VIRTIO_BLK_F_SEG_MAX incompatible with qcow2 and vhdx?

4 participants