On 3/4/20 9:28 am, Samuel Thibault wrote: > Concerning the link line, I don't understand why hardcoding everything? > > - For a start, are the _pic.a versions really needed? The .a versions > should just work.
I think the _pic.a versions are required. I could not get rump to work with .a That is why I made rumpkernel package install them. > About IOC macros in block-rump.c, is rump not providing a header that > defines them, instead of hardcoding them here? I can put them in my own header, I could not find them exposed in rump. > Concerning translate_name, instead of malloc+snprintf, you can use > asprintf. That being said, since it doesn't actually need to be kept > allocated, better just make this a function that takes the output > buffer, which you can allocate on the stack in device_open. OK I will fix this. > Take care of indentation, the GNU Coding Style is... OK I will fix this. > There are a couple mach_print debugging calls left. I think they should remain, they only occur in severe error. > Please add a /* FIXME: make multithreaded */ in device_write/read. The > way it is currently shaped is completely synchronous: device_read will > be stuck inside rump_sys_read() until the data is retrieved from the > disk. To fix this, we need to add a machdev_multithread_server function > in libmachdev, which calls ports_manage_port_operations_multithread > instead of ports_manage_port_operations_one_thread, and use that here > instead of machdev_server. That way there will be one thread per > device_read/write request, not blocking each other. That will however > pose a problem with the lseek-then-read/write way currently used in > device_read/write: two concurrent device_read calls with call lseek > concurrently before calling read concurrently. You can already fix this > now (even before moving to ports_manage_port_operations_multithread) by > calling rump_sys_pread/pwrite instead. ACK > That being said, on even longer run, we may probably not want > to keep one thread per device_read/write request. We'll want to > call rump_sys_aio_read/write instead and return MIG_NO_REPLY from > device_read/write, and send the mig reply once the aio request has > completed. That way, only the aio request will be kept in rumpdisk > memory instead of a whole thread structure. Again, please add this as a > FIXME note, but for much later. Our current linux glue disk driver does > not even do that currently :) ACK > On rump_sys_read error, you need to unmap buf. ACK Damien
