search

mirage / ocaml-tar

Pure OCaml library to read and write tar files
ISC License
54 stars 34 forks source link

mirage-kv 2.0.0 #65

Closed hannesm closed 5 years ago

hannesm commented 5 years ago

this upgrades tar-mirage to the mirage-kv 2.0.0 API :D

hannesm commented 5 years ago

while doing this port, I noticed that this is unlikely to work on Solo5, where block_read expects at most one sector to be read -- while in tar-mirage, BLOCK.read gets a single huge buffer passed. do other block backends (unix/xen) have the same semantics as solo5-block? .oO(feels like a revision of mirage-block needs to be done /cc @g2p @samoht @mato)

I also noticed that while sector_size is available, there are various instances of 4095 and 4096 hardcoded in tar-mirage, also 512. in addition, the code looks a bit suspicious in terms of integer overflows (+ and add are used without checking, lots ot Int64.to_int without checks)

mato commented 5 years ago

feels like a revision of mirage-block needs to be done /cc @g2p @samoht @mato

See https://github.com/Solo5/solo5/issues/325#issuecomment-465483894 for what needs to be done in Solo5 to support block operations in multiples of the block size.

hannesm commented 5 years ago

ok, thanks. the block read/write did not change in this PR, and should be addressed in other PRs. IMHO this is ready to be merged + released to unbreak tar-mirage with mirage 3.5.0 (which uses mirage-kv 2.0.0). any concerns about merging this PR?

samoht commented 5 years ago

LGTM

hannesm commented 5 years ago

how can we move forward here? tar-mirage is atm not compatible with mirage 3.5.0 due to the changes in mirage-kv. IMHO it'd be great to have this merged and released //cc @djs55

djs55 commented 5 years ago

@samoht thanks for taking care of this!