mirror of
https://github.com/genodelabs/genode.git
synced 2025-01-29 15:44:02 +00:00
fs servers: handle result propagation better
This patch removes the notion of partial writes from the file-system servers. Since write operations are asynchronously submitted, they are expected to succeed completely, except for I/O errors. I/O errors are propagated with the write acknowledgement but those are usually handled out of band at the client side. Partial writes must never occur because they would go undetected by clients, which usually don't wait for the completion of each single write operation. Until now, most file-system servers returned the number of written bytes in the acknowledgement packet. If a server managed to write a part of the request only, it issued the acknowledgement immediately where it should have cared about writing the remaining part first. The patch detects such misbehaving server-side code. If partial writes unexpectedly occur, it prints a message and leaves the corresponding request unacknowdleged. Issue #2672
This commit is contained in:
parent
f4e9c94bf2
commit
accc7e7521
@ -251,7 +251,12 @@ class Rump_fs::Directory : public Node
|
||||
void *current, *end;
|
||||
for (current = buf, end = &buf[bytes];
|
||||
current < end;
|
||||
current = _DIRENT_NEXT((dirent *)current), count++) { }
|
||||
current = _DIRENT_NEXT((dirent *)current))
|
||||
{
|
||||
struct ::dirent *d = (dirent*)current;
|
||||
if (strcmp(".", d->d_name) && strcmp("..", d->d_name))
|
||||
count++;
|
||||
}
|
||||
} while(bytes);
|
||||
|
||||
return count;
|
||||
|
@ -70,17 +70,34 @@ class Rump_fs::Session_component : public Session_rpc_object
|
||||
|
||||
/* resulting length */
|
||||
size_t res_length = 0;
|
||||
bool succeeded = false;
|
||||
|
||||
switch (packet.operation()) {
|
||||
|
||||
case Packet_descriptor::READ:
|
||||
if (content && (packet.length() <= packet.size()))
|
||||
res_length = open_node.node().read((char *)content, length, packet.position());
|
||||
if (content && (packet.length() <= packet.size())) {
|
||||
res_length = open_node.node().read((char *)content, length,
|
||||
packet.position());
|
||||
|
||||
/* read data or EOF is a success */
|
||||
succeeded = res_length || (packet.position() >= open_node.node().status().size);
|
||||
}
|
||||
break;
|
||||
|
||||
case Packet_descriptor::WRITE:
|
||||
if (content && (packet.length() <= packet.size()))
|
||||
res_length = open_node.node().write((char const *)content, length, packet.position());
|
||||
if (content && (packet.length() <= packet.size())) {
|
||||
res_length = open_node.node().write((char const *)content,
|
||||
length,
|
||||
packet.position());
|
||||
/* File system session can't handle partial writes */
|
||||
if (res_length != length) {
|
||||
Genode::error("partial write detected ",
|
||||
res_length, " vs ", length);
|
||||
/* don't acknowledge */
|
||||
return;
|
||||
}
|
||||
succeeded = true;
|
||||
}
|
||||
break;
|
||||
|
||||
case Packet_descriptor::CONTENT_CHANGED:
|
||||
@ -92,15 +109,17 @@ class Rump_fs::Session_component : public Session_rpc_object
|
||||
|
||||
case Packet_descriptor::READ_READY:
|
||||
/* not supported */
|
||||
succeeded = true;
|
||||
break;
|
||||
|
||||
case Packet_descriptor::SYNC:
|
||||
rump_sys_sync();
|
||||
succeeded = true;
|
||||
break;
|
||||
}
|
||||
|
||||
packet.length(res_length);
|
||||
packet.succeeded(res_length > 0);
|
||||
packet.succeeded(succeeded);
|
||||
tx_sink()->acknowledge_packet(packet);
|
||||
}
|
||||
|
||||
|
@ -47,7 +47,7 @@ class Rump_fs::Symlink : public Node
|
||||
Node::name(basename(path));
|
||||
}
|
||||
|
||||
size_t write(char const *src, size_t len, seek_off_t seek_offset) override
|
||||
size_t write(char const *src, size_t const len, seek_off_t seek_offset) override
|
||||
{
|
||||
/* Ideal symlink operations are atomic. */
|
||||
if (!_create || seek_offset)
|
||||
@ -57,7 +57,7 @@ class Rump_fs::Symlink : public Node
|
||||
Genode::String<MAX_PATH_LEN> target(Genode::Cstring(src, len));
|
||||
|
||||
int ret = rump_sys_symlink(target.string(), _path.base());
|
||||
return ret == -1 ? 0 : ret;
|
||||
return ret == -1 ? 0 : len;
|
||||
}
|
||||
|
||||
size_t read(char *dst, size_t len, seek_off_t seek_offset) override
|
||||
|
@ -94,7 +94,7 @@ append_if $use_ahci_drv config {
|
||||
<resource name="RAM" quantum="2M"/>
|
||||
<provides> <service name="Block"/> </provides>
|
||||
<config>
|
||||
<policy label_prefix="fs" device="0" />
|
||||
<policy label_prefix="fs" device="0" writeable="yes" />
|
||||
</config>
|
||||
</start>
|
||||
}
|
||||
@ -161,7 +161,7 @@ append qemu_args " -nographic "
|
||||
append_if $use_ahci_drv qemu_args " -drive id=disk,file=$disk_image,format=raw,if=none -device ahci,id=ahci -device ide-drive,drive=disk,bus=ahci.0 -boot d"
|
||||
append_if $use_sd_card_drv qemu_args " -drive file=$disk_image,format=raw,if=sd,cache=writeback "
|
||||
|
||||
run_genode_until {.*child "test-libc_vfs" exited with exit value 0.*} 60
|
||||
run_genode_until {.*child "test-libc_vfs" exited with exit value 0.*} 120
|
||||
|
||||
exec rm -f $disk_image
|
||||
|
||||
|
@ -82,40 +82,63 @@ class Fatfs_fs::Session_component : public Session_rpc_object
|
||||
size_t const length = packet.length();
|
||||
seek_off_t const offset = packet.position();
|
||||
|
||||
if (!content || (packet.length() > packet.size())) {
|
||||
packet.succeeded(false);
|
||||
return;
|
||||
}
|
||||
|
||||
/* resulting length */
|
||||
size_t res_length = 0;
|
||||
bool succeeded = false;
|
||||
|
||||
switch (packet.operation()) {
|
||||
|
||||
case Packet_descriptor::READ:
|
||||
res_length = open_node.node().read((char *)content, length, offset);
|
||||
break;
|
||||
case Packet_descriptor::READ:
|
||||
if (content && (packet.length() <= packet.size())) {
|
||||
res_length = open_node.node().read((char *)content, length,
|
||||
offset);
|
||||
|
||||
case Packet_descriptor::WRITE:
|
||||
res_length = open_node.node().write((char const *)content, length, offset);
|
||||
break;
|
||||
/* read data or EOF is a success */
|
||||
succeeded = res_length > 0;
|
||||
if (!succeeded) {
|
||||
File * file = dynamic_cast<File *>(&open_node.node());
|
||||
if (file)
|
||||
succeeded = f_eof((file->fatfs_fil()));
|
||||
}
|
||||
}
|
||||
break;
|
||||
|
||||
case Packet_descriptor::CONTENT_CHANGED:
|
||||
open_node.register_notify(*tx_sink());
|
||||
open_node.node().notify_listeners();
|
||||
return;
|
||||
case Packet_descriptor::WRITE:
|
||||
if (content && (packet.length() <= packet.size())) {
|
||||
res_length = open_node.node().write((char const *)content,
|
||||
length, offset);
|
||||
|
||||
case Packet_descriptor::READ_READY:
|
||||
/* not supported */
|
||||
break;
|
||||
/* File system session can't handle partial writes */
|
||||
if (res_length != length) {
|
||||
Genode::error("partial write detected ",
|
||||
res_length, " vs ", length);
|
||||
/* don't acknowledge */
|
||||
return;
|
||||
}
|
||||
succeeded = true;
|
||||
}
|
||||
break;
|
||||
|
||||
case Packet_descriptor::SYNC:
|
||||
/* not supported */
|
||||
break;
|
||||
case Packet_descriptor::CONTENT_CHANGED:
|
||||
open_node.register_notify(*tx_sink());
|
||||
open_node.node().notify_listeners();
|
||||
return;
|
||||
|
||||
case Packet_descriptor::READ_READY:
|
||||
succeeded = true;
|
||||
/* not supported */
|
||||
break;
|
||||
|
||||
case Packet_descriptor::SYNC:
|
||||
succeeded = true;
|
||||
/* not supported */
|
||||
break;
|
||||
}
|
||||
|
||||
packet.length(res_length);
|
||||
packet.succeeded(res_length > 0);
|
||||
packet.succeeded(succeeded);
|
||||
|
||||
tx_sink()->acknowledge_packet(packet);
|
||||
}
|
||||
|
||||
void _process_packet()
|
||||
@ -134,12 +157,6 @@ class Fatfs_fs::Session_component : public Session_rpc_object
|
||||
} catch (Id_space<File_system::Node>::Unknown_id const &) {
|
||||
Genode::error("Invalid_handle");
|
||||
}
|
||||
|
||||
/*
|
||||
* The 'acknowledge_packet' function cannot block because we
|
||||
* checked for 'ready_to_ack' in '_process_packets'.
|
||||
*/
|
||||
tx_sink()->acknowledge_packet(packet);
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -72,17 +72,32 @@ class Fuse_fs::Session_component : public Session_rpc_object
|
||||
|
||||
/* resulting length */
|
||||
size_t res_length = 0;
|
||||
bool succeeded = false;
|
||||
|
||||
switch (packet.operation()) {
|
||||
|
||||
case Packet_descriptor::READ:
|
||||
if (content && (packet.length() <= packet.size()))
|
||||
res_length = open_node.node().read((char *)content, length, packet.position());
|
||||
if (content && (packet.length() <= packet.size())) {
|
||||
res_length = open_node.node().read((char *)content, length,
|
||||
packet.position());
|
||||
|
||||
succeeded = res_length > 0;
|
||||
}
|
||||
break;
|
||||
|
||||
case Packet_descriptor::WRITE:
|
||||
if (content && (packet.length() <= packet.size()))
|
||||
if (content && (packet.length() <= packet.size())) {
|
||||
res_length = open_node.node().write((char const *)content, length, packet.position());
|
||||
|
||||
/* File system session can't handle partial writes */
|
||||
if (res_length != length) {
|
||||
Genode::error("partial write detected ",
|
||||
res_length, " vs ", length);
|
||||
/* don't acknowledge */
|
||||
return;
|
||||
}
|
||||
succeeded = true;
|
||||
}
|
||||
break;
|
||||
|
||||
case Packet_descriptor::CONTENT_CHANGED:
|
||||
@ -93,16 +108,18 @@ class Fuse_fs::Session_component : public Session_rpc_object
|
||||
return;
|
||||
|
||||
case Packet_descriptor::READ_READY:
|
||||
succeeded = true;
|
||||
/* not supported */
|
||||
break;
|
||||
|
||||
case Packet_descriptor::SYNC:
|
||||
Fuse::sync_fs();
|
||||
succeeded = true;
|
||||
break;
|
||||
}
|
||||
|
||||
packet.length(res_length);
|
||||
packet.succeeded(res_length > 0);
|
||||
packet.succeeded(succeeded);
|
||||
tx_sink()->acknowledge_packet(packet);
|
||||
}
|
||||
|
||||
|
@ -67,17 +67,35 @@ class Lx_fs::Session_component : public Session_rpc_object
|
||||
|
||||
/* resulting length */
|
||||
size_t res_length = 0;
|
||||
bool succeeded = false;
|
||||
|
||||
switch (packet.operation()) {
|
||||
|
||||
case Packet_descriptor::READ:
|
||||
if (content && (packet.length() <= packet.size()))
|
||||
res_length = open_node.node().read((char *)content, length, packet.position());
|
||||
if (content && (packet.length() <= packet.size())) {
|
||||
res_length = open_node.node().read((char *)content, length,
|
||||
packet.position());
|
||||
|
||||
/* read data or EOF is a success */
|
||||
succeeded = res_length || (packet.position() >= open_node.node().status().size);
|
||||
}
|
||||
break;
|
||||
|
||||
case Packet_descriptor::WRITE:
|
||||
if (content && (packet.length() <= packet.size()))
|
||||
res_length = open_node.node().write((char const *)content, length, packet.position());
|
||||
if (content && (packet.length() <= packet.size())) {
|
||||
res_length = open_node.node().write((char const *)content,
|
||||
length,
|
||||
packet.position());
|
||||
|
||||
/* File system session can't handle partial writes */
|
||||
if (res_length != length) {
|
||||
Genode::error("partial write detected ",
|
||||
res_length, " vs ", length);
|
||||
/* don't acknowledge */
|
||||
return;
|
||||
}
|
||||
succeeded = true;
|
||||
}
|
||||
break;
|
||||
|
||||
case Packet_descriptor::CONTENT_CHANGED:
|
||||
@ -88,6 +106,7 @@ class Lx_fs::Session_component : public Session_rpc_object
|
||||
return;
|
||||
|
||||
case Packet_descriptor::READ_READY:
|
||||
succeeded = true;
|
||||
/* not supported */
|
||||
break;
|
||||
|
||||
@ -99,11 +118,12 @@ class Lx_fs::Session_component : public Session_rpc_object
|
||||
* use-case.
|
||||
*/
|
||||
Genode::warning("SYNC not implemented!");
|
||||
succeeded = true;
|
||||
break;
|
||||
}
|
||||
|
||||
packet.length(res_length);
|
||||
packet.succeeded(res_length > 0);
|
||||
packet.succeeded(succeeded);
|
||||
tx_sink()->acknowledge_packet(packet);
|
||||
}
|
||||
|
||||
|
@ -72,6 +72,7 @@ class Ram_fs::Session_component : public File_system::Session_rpc_object
|
||||
|
||||
/* resulting length */
|
||||
size_t res_length = 0;
|
||||
bool succeeded = false;
|
||||
|
||||
switch (packet.operation()) {
|
||||
|
||||
@ -82,6 +83,9 @@ class Ram_fs::Session_component : public File_system::Session_rpc_object
|
||||
break;
|
||||
res_length = node->read((char *)content, length,
|
||||
packet.position());
|
||||
|
||||
/* read data or EOF is a success */
|
||||
succeeded = res_length || (packet.position() >= node->status().size);
|
||||
}
|
||||
break;
|
||||
|
||||
@ -92,17 +96,26 @@ class Ram_fs::Session_component : public File_system::Session_rpc_object
|
||||
break;
|
||||
res_length = node->write((char const *)content, length,
|
||||
packet.position());
|
||||
|
||||
/* File system session can't handle partial writes */
|
||||
if (res_length != length) {
|
||||
Genode::error("partial write detected ",
|
||||
res_length, " vs ", length);
|
||||
/* don't acknowledge */
|
||||
return;
|
||||
}
|
||||
succeeded = true;
|
||||
}
|
||||
open_node.mark_as_written();
|
||||
break;
|
||||
|
||||
case Packet_descriptor::CONTENT_CHANGED: {
|
||||
case Packet_descriptor::CONTENT_CHANGED:
|
||||
Genode::error("CONTENT_CHANGED packets from clients have no effect");
|
||||
return;
|
||||
}
|
||||
|
||||
case Packet_descriptor::READ_READY:
|
||||
/* not supported */
|
||||
succeeded = true;
|
||||
break;
|
||||
|
||||
case Packet_descriptor::SYNC: {
|
||||
@ -110,12 +123,13 @@ class Ram_fs::Session_component : public File_system::Session_rpc_object
|
||||
if (!node.valid())
|
||||
break;
|
||||
node->notify_listeners();
|
||||
succeeded = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
packet.length(res_length);
|
||||
packet.succeeded(res_length > 0);
|
||||
packet.succeeded(succeeded);
|
||||
tx_sink()->acknowledge_packet(packet);
|
||||
}
|
||||
|
||||
|
@ -40,6 +40,8 @@ class Ram_fs::Symlink : public Node
|
||||
|
||||
size_t write(char const *src, size_t len, seek_off_t seek_offset) override
|
||||
{
|
||||
size_t const consumed_len = len;
|
||||
|
||||
/* Ideal symlink operations are atomic. */
|
||||
if (seek_offset) return 0;
|
||||
|
||||
@ -59,7 +61,8 @@ class Ram_fs::Symlink : public Node
|
||||
|
||||
Genode::memcpy(_link_to, src, len);
|
||||
_len = len;
|
||||
return len;
|
||||
|
||||
return consumed_len;
|
||||
}
|
||||
|
||||
Status status() override
|
||||
|
@ -654,6 +654,7 @@ class Trace_fs::Session_component : public Session_rpc_object
|
||||
|
||||
/* resulting length */
|
||||
size_t res_length = 0;
|
||||
bool succeeded = false;
|
||||
|
||||
switch (packet.operation()) {
|
||||
|
||||
@ -663,6 +664,7 @@ class Trace_fs::Session_component : public Session_rpc_object
|
||||
if (!node.valid())
|
||||
break;
|
||||
res_length = node->read((char *)content, length, packet.position());
|
||||
succeeded = res_length > 0;
|
||||
}
|
||||
break;
|
||||
|
||||
@ -672,6 +674,15 @@ class Trace_fs::Session_component : public Session_rpc_object
|
||||
if (!node.valid())
|
||||
break;
|
||||
res_length = node->write((char const *)content, length, packet.position());
|
||||
|
||||
/* File system session can't handle partial writes */
|
||||
if (res_length != length) {
|
||||
Genode::error("partial write detected ",
|
||||
res_length, " vs ", length);
|
||||
/* don't acknowledge */
|
||||
return;
|
||||
}
|
||||
succeeded = true;
|
||||
}
|
||||
break;
|
||||
|
||||
@ -689,16 +700,18 @@ class Trace_fs::Session_component : public Session_rpc_object
|
||||
}
|
||||
|
||||
case Packet_descriptor::READ_READY:
|
||||
succeeded = true;
|
||||
/* not supported */
|
||||
break;
|
||||
|
||||
case Packet_descriptor::SYNC:
|
||||
succeeded = true;
|
||||
/* not supported */
|
||||
break;
|
||||
}
|
||||
|
||||
packet.length(res_length);
|
||||
packet.succeeded(res_length > 0);
|
||||
packet.succeeded(succeeded);
|
||||
tx_sink()->acknowledge_packet(packet);
|
||||
}
|
||||
|
||||
|
@ -148,6 +148,7 @@ class Vfs_server::Session_component : public File_system::Session_rpc_object,
|
||||
|
||||
/* resulting length */
|
||||
size_t res_length = 0;
|
||||
bool succeeded = false;
|
||||
|
||||
switch (packet.operation()) {
|
||||
|
||||
@ -160,8 +161,12 @@ class Vfs_server::Session_component : public File_system::Session_rpc_object,
|
||||
throw Not_ready();
|
||||
}
|
||||
|
||||
if (node.mode() & READ_ONLY)
|
||||
if (node.mode() & READ_ONLY) {
|
||||
res_length = node.read((char *)content, length, seek);
|
||||
/* no way to distinguish EOF from unsuccessful
|
||||
reads, both have res_length == 0 */
|
||||
succeeded = true;
|
||||
}
|
||||
});
|
||||
}
|
||||
catch (Not_ready) { throw; }
|
||||
@ -174,8 +179,18 @@ class Vfs_server::Session_component : public File_system::Session_rpc_object,
|
||||
|
||||
try {
|
||||
_apply(packet.handle(), [&] (Node &node) {
|
||||
if (node.mode() & WRITE_ONLY)
|
||||
if (node.mode() & WRITE_ONLY) {
|
||||
res_length = node.write((char const *)content, length, seek);
|
||||
|
||||
/* File system session can't handle partial writes */
|
||||
if (res_length != length) {
|
||||
Genode::error("partial write detected ",
|
||||
res_length, " vs ", length);
|
||||
/* don't acknowledge */
|
||||
throw Dont_ack();
|
||||
}
|
||||
succeeded = true;
|
||||
}
|
||||
});
|
||||
} catch (Operation_incomplete) {
|
||||
throw Not_ready();
|
||||
@ -191,6 +206,7 @@ class Vfs_server::Session_component : public File_system::Session_rpc_object,
|
||||
throw Dont_ack();
|
||||
}
|
||||
});
|
||||
succeeded = true;
|
||||
}
|
||||
catch (Dont_ack) { throw; }
|
||||
catch (...) { }
|
||||
@ -208,6 +224,7 @@ class Vfs_server::Session_component : public File_system::Session_rpc_object,
|
||||
try {
|
||||
_apply(packet.handle(), [&] (Node &node) {
|
||||
node.sync();
|
||||
succeeded = true;
|
||||
});
|
||||
} catch (Operation_incomplete) {
|
||||
throw Not_ready();
|
||||
@ -216,7 +233,7 @@ class Vfs_server::Session_component : public File_system::Session_rpc_object,
|
||||
}
|
||||
|
||||
packet.length(res_length);
|
||||
packet.succeeded(!!res_length);
|
||||
packet.succeeded(succeeded);
|
||||
}
|
||||
|
||||
bool _try_process_packet_op(Packet_descriptor &packet)
|
||||
|
@ -287,7 +287,7 @@ struct Vfs_server::Symlink : Node
|
||||
return _read(dst, len, 0);
|
||||
}
|
||||
|
||||
size_t write(char const *src, size_t len, seek_off_t)
|
||||
size_t write(char const *src, size_t const len, seek_off_t)
|
||||
{
|
||||
/*
|
||||
* if the symlink target is too long return a short result
|
||||
@ -311,7 +311,7 @@ struct Vfs_server::Symlink : Node
|
||||
|
||||
mark_as_updated();
|
||||
notify_listeners();
|
||||
return out_count;
|
||||
return len;
|
||||
}
|
||||
};
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user