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

GuestMemory::read_from and GuestMemory::write_to have "exact" semantics #171

Open
alindima opened this issue Sep 15, 2021 · 3 comments
Open

Comments

@alindima
Copy link
Contributor

alindima commented Sep 15, 2021

The GuestMemory implementation of the read_from function of the Bytes trait is equivalent to read_exact_from.
In analogy, the write_to implementation is equivalent to write_all_to.

This is because both functions rely on a call to try_access, which will run until an irrecoverable error occurs (not ErrorKind::Interrupted) or count bytes are transferred.
This semantic is correct for read_exact_from and write_all_to, assuming they have similar meaning to the read_exact and write_all functions of the std::io::Read and std::io::Write traits.

For the "non-exact" counterparts, only one read/write should occur.

@alindima alindima changed the title GuestMemory::read_from and GuestMemory GuestMemory::read_from and GuestMemory::write_to have "exact" semantics Sep 15, 2021
@alindima
Copy link
Contributor Author

An example where this can be problematic:

When reading from a non-blocking socket you can't know the length of the data available on the socket, so one may use a maximum length of N bytes for the read_from call.

If there are only M bytes available (where M<N), the read_from call will return a WouldBlock io error. In reality, the first read attempt will succeed with M bytes, but the second read (called under the hood) will fail with WouldBlock.
The caller will have no idea of how many bytes have actually been transferred.

I believe we should better define the semantics of these functions. For example, the Read trait clearly defines that the read function performs at most one read.

@bonzini
Copy link
Member

bonzini commented Sep 27, 2021

the Read trait clearly defines that the read function performs at most one read.

That would be doable, though it would incur some code duplication.

One difference between read_from/write_to and std::io::Read::read/std::io::Write::write is that the latter do not handle the Interrupted error. However, doing so might break client code of vm-memory though, so I'm a bit wary of doing that.

@alindima
Copy link
Contributor Author

One difference between read_from/write_to and std::io::Read::read/std::io::Write::write is that the latter do not handle the Interrupted error. However, doing so might break client code of vm-memory though, so I'm a bit wary of doing that.

Indeed. Another difference that may break client applications is that we would only call read once (since they could be relying on this behaviour).
Maybe we can document that the methods perform a retry only if an Interrupted error is returned. WDYT?

The workaround we're using in Firecracker is to use the read_from/write_to from the GuestRegionMmap implementation of Bytes, which actually handles the Interrupted error but otherwise only calls read once

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

No branches or pull requests

2 participants