Skip to content

Commit 9cabed9

Browse files
committed
Fixed pread-related bug
pread may or may not read the requested size. This was not taken into account for. If the data is large, we probably can't read it, in one go.
1 parent 03a4eaa commit 9cabed9

File tree

2 files changed

+26
-7
lines changed

2 files changed

+26
-7
lines changed

src/PersistentCheckpointing.cc

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,26 @@ WriteVmConfig::WriteVmConfig(Task* clone_leader, const char* data_dir,
6767
ASSERT(clone_leader, proc_pagemap_fd.is_open())
6868
<< "Serializing VM for " << clone_leader->rec_tid
6969
<< " failed. Couldn't open " << proc_pagemap_fd;
70-
buffer = { .ptr = ::mmap(nullptr, buffer_size, PROT_READ | PROT_WRITE,
71-
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0),
70+
buffer = { .ptr =
71+
(uint8_t*)::mmap(nullptr, buffer_size, PROT_READ | PROT_WRITE,
72+
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0),
7273
.size = buffer_size };
7374
ASSERT(clone_leader, buffer.ptr != MAP_FAILED)
7475
<< "Failed to mmap buffer with capacity " << buffer_size;
7576
}
7677

78+
ssize_t WriteVmConfig::pread(ssize_t bytes_read,
79+
const KernelMapping& km) const {
80+
DEBUG_ASSERT(bytes_read != 1 &&
81+
"you've passed in the 'invalid pread result' as bytes_read");
82+
const auto current_read =
83+
::pread(proc_mem_fd, buffer.ptr + bytes_read, km.size() - bytes_read,
84+
km.start().as_int() + bytes_read);
85+
if (current_read == -1)
86+
return current_read;
87+
return bytes_read + current_read;
88+
}
89+
7790
std::string checkpoints_index_file(const std::string& trace_dir) {
7891
return trace_dir + "/checkpoints";
7992
}
@@ -151,10 +164,14 @@ static void write_map(const WriteVmConfig& cfg,
151164
FILE_OP_FATAL(file) << "couldn't truncate file to size "
152165
<< map.map.size();
153166

154-
const auto bytes_read = ::pread(cfg.proc_mem_fd, cfg.buffer.ptr,
155-
map.map.size(), map.map.start().as_int());
156-
if (bytes_read == -1)
157-
FILE_OP_FATAL(file) << " couldn't read contents of " << map.map.str();
167+
auto bytes_read = 0;
168+
while (static_cast<size_t>(bytes_read) < map.map.size()) {
169+
const auto current_read = cfg.pread(bytes_read, map.map);
170+
if (current_read == -1)
171+
FILE_OP_FATAL(file) << " couldn't read contents of " << map.map.str();
172+
bytes_read = current_read;
173+
}
174+
158175
ASSERT(cfg.clone_leader,
159176
static_cast<unsigned long>(bytes_read) == map.map.size())
160177
<< " data read from /proc/" << cfg.clone_leader->tid

src/PersistentCheckpointing.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@ class WriteVmConfig {
2828
const char* cp_data_dir;
2929

3030
struct {
31-
void* ptr;
31+
uint8_t* ptr;
3232
size_t size;
3333
} buffer;
34+
35+
ssize_t pread(ssize_t bytes_read, const KernelMapping& km) const;
3436
};
3537

3638
/* Writes capture `state` to state builder `sb`. */

0 commit comments

Comments
 (0)