search

mikelangelo-project / capstan

Capstan, a tool for packaging and running your application on OSv.
http://osv.io/capstan/
Other
19 stars 7 forks source link

Attach volume on `capstan run` #69

Closed miha-plesko closed 6 years ago

miha-plesko commented 6 years ago

Until now we were not able to attach volume when using Capstan. With this commit we bring basic support for it by introducing following parameter to the command:

$ capstan run demo --volume ./test1.img --volume ./volume2.img --volume ./volume3.img

At the moment we only support volumes for QEMU hypervisor and print a warning if --volume option is used with any other hypervisor.

miha-plesko commented 6 years ago

@gberginc I'm opening this PR just to discuss approach with you, no need to look at the code yet. Please just comment on functionality description.

I'm thinking if it was perhaps better instead of current volume string:

--volume path/to/volume.img:raw

to go with

--volume path/to/volume.img:format=raw:driver=virtio-blk:...

Saying it out loud it just got obvious that the second suggestion would be better, since then I could introduce something like :devide=cdrom to even support cloud-init mount. Thanks for listening! 😄

miha-plesko commented 6 years ago

@gberginc I kindly ask for a review.

gberginc commented 6 years ago

Saying it out loud it just got obvious that the second suggestion would be better, since then I could introduce something like :devide=cdrom to even support cloud-init mount. Thanks for listening!

I am such a good listener :smile:

Doing the first pass right now.

gberginc commented 6 years ago

Thank you @miha-plesko! I am fine with this PR, but have another thing I would like to discuss: this is now supported only for QEMU hypervisor. Would it be possible to include a message for others that volumes are not yet supported there? We can also open an issue for this and get this PR merged.

I prefer the latter, but let me know if you could squeeze at least the warning into this one.

miha-plesko commented 6 years ago

@gberginc done. I've also added a note regarding the limitation in volumes.md. Yeah, the whole packaging part of Capstan is currently limited to QEMU-only. We need to decide if supporting other hypervisor (e.g. vbox) should be of greater priority; currently I think we're not planning this, right?