<apache2>
what I think happens here is that the FAT implementation allocates a 4096 byte (`alloc 4096`) Cstruct.t, then tries to read a sector
<apache2>
the solo5 implementation only allows reading 512 bytes at a time
<apache2>
mirage-block-solo5 uses the cstruct len of the page to do the read, which means it tries to do a read of 4096 bytes, which fails
<apache2>
I'm not who is misbehaving politically speaking (if it's the FAT implementation not using the block device's sector size, but can't see how it should be able to tell the page size of a Mirage_block_lwt.S, so I would say it's mirage-block-solo5
<apache2>
but in any case it seems to me like you can fix it by patching mirage-block-solo5 so that `write` and `read` writes in increments of the block size (512) instead of assuming that the consumer somehow knows only to pass 512 byte cstructs
<hannes>
whom to blame / what to specify for read&write is open for discussion -- I'd take a look and the users and implementors of mirage-block-lwt (-xen, -unix, fat and tar as users) and find the common scheme there
<apache2>
I couldn't find docs, but I have a feeling there might be an undocumented invariant that block size is 4096
<hannes>
then revise (at least) the documentation of mirage-block to be crystal clear what to expect in there, and adjust the implementations. I think/hope g2p and others are already on that path, but I don't know for sure
<apache2>
if that's the case, patching mirage-block-solo5 to read four sectors (= 4096 bytes) seems reasonable to me
mort___ has quit [Quit: Leaving.]
<apache2>
ideally though we'd expose sector size in the mirage_block_solo5 api... since 4096 is kind of small too
<apache2>
in that case I guess mirage-fat should pull that info and use that
<hannes>
i feel that the "single sector read/write" in block-solo5 is a limitation and should be improved, but imho best to investigate the current practises first, and revise the documentation (and maybe signatures if they're not convenient)
<hannes>
(maybe including prototyping and testing of performance if you restrict it to single buffer operations vs larger ones (a page, 4086 -- a huge page, 2MB), discuss tradeoffs and find a suitable solution?
<apache2>
mirage-block-lwt should probably have a "read multiple sectors" function where the implementation can decide what to do (read sequentially in a loop or request a larger range when that is possible)
<hannes>
that's basically what we did recently with mirage-kv, and I also did with the network stack API
<apache2>
I agree @ single-sector read/write in solo5, but I don't think it's a showstopper. things that are currently not working (like mirage-fat) on solo5 could easily work with single-sector i/o, just with a performance hit compared to the other implementations
<apache2>
and then we could adapt when/if solo5 makes api changes upstream
<apache2>
ehirdoy1: ok, so to sum it up this is a slightly neglected area of mirage, if you want a hotfix it will be to make mirage-fat use `B.get_info device >>= fun info ->` and make it `alloc info.sector_size`instead of `alloc 4096`
<ehirdoy1>
So is the current solution to issue multiple read in mirage-block until it fulfills a given buffer?
<apache2>
ehirdoy1: there are two sizes at play, one is the block device sector size; the other is the FAT sector size (used by the FAT filesystem). it's only the former you have to change
<apache2>
the latter is `bps` in this case
<apache2>
let buf = alloc (List.length xs * 512) in <-- this needs to have 512 replaced with the block devices sector_size from get_info
<apache2>
I would extend `type fs` to include the sector size so you don't have to call get_info for each read
<apache2>
but if you look at the implementation of `alloc` there's an implcit expectation about block device sector size there too..
<apache2>
let page = alloc 4096 in
<apache2>
B.read device (Int64.of_int sector') [ page ] >>= function
<apache2>
this is what is causing the problem
<apache2>
page.Cstruct.len (aka Cstruct.len page) needs to be = info.sector_size here
<apache2>
so I would leave the 512 alone I think...
<apache2>
and make `alloc` take an argument sector_size
<apache2>
and replace 4096 with sector_size and 4095 with (sector_size-1)
<apache2>
does that make sense?
<apache2>
(and then you also want to call `alloc` with sector_size instead of 4096, so you'd be looking at `alloc device device.sector_size` instead of `alloc 4096` and `alloc` would then pull device.sector_size for the rounding up etc
pie_ has quit [Ping timeout: 245 seconds]
Haudegen has quit [Remote host closed the connection]
Haudegen has joined #mirage
mort___ has quit [Quit: Leaving.]
Haudegen has quit [Remote host closed the connection]
zsmithnyc has quit [Read error: Connection reset by peer]
zsmithnyc has joined #mirage
jnavila has joined #mirage
mort___ has joined #mirage
jnavila has quit [Ping timeout: 252 seconds]
jnavila has joined #mirage
jnavila has quit [Ping timeout: 246 seconds]
jnavila has joined #mirage
jnavila has quit [Ping timeout: 252 seconds]
Haudegen has joined #mirage
mort___ has quit [Quit: Leaving.]
mort___ has joined #mirage
mort___ has quit [Quit: Leaving.]
Haudegen has quit [Remote host closed the connection]