diskfs / go-diskfs

MIT License
501 stars 112 forks source link

Close file on InitDisk if its a block device #166

Closed Itxaka closed 1 year ago

Itxaka commented 1 year ago

Looks like if its a block device, on initDisk we re-open the block device to get the size, but unfortunately we never close it. As the life of that os.File is very short, we should release it as soon as possible in case other process may be trying to access the same block device

Signed-off-by: Itxaka itxaka@spectrocloud.com

Itxaka commented 1 year ago

cc @davidcassany I think this is what was causing the issues with the metadata in linuxkit :eyes:

davidcassany commented 1 year ago

thanks! interesting, I did a similar patch in https://github.com/davidcassany/linuxkit/commit/d63c4383b4c247ec7365c7fa69cd7b6cf69872c5 but I could not manage to fix the issue. Then stopped debugging as the cdrom provider was no longer being used 🤷🏽‍♂️ So the I think I never actually made a PR for that.

Itxaka commented 1 year ago

thanks! interesting, I did a similar patch in davidcassany/linuxkit@d63c438 but I could not manage to fix the issue. Then stopped debugging as the cdrom provider was no longer being used 🤷🏽‍♂️ So the I think I never actually made a PR for that.

I think your patch didnt work because the diskfs lib re-opens the block device to check for size! and that opened block device is never returned to the linuxkit call so it hangs in there until the metadata process is done!

deitch commented 1 year ago

And in. If you want to open a PR on linuxkit to get the latest, so it fixes the metadata issue, @ cc me so I can approve and merge it in.

davidcassany commented 1 year ago

I think your patch didnt work because the diskfs lib re-opens the block device to check for size! and that opened block device is never returned to the linuxkit call so it hangs in there until the metadata process is done!

Good catch :+1:

Itxaka commented 1 year ago

thanks @deitch !