breaking change: virtio-blk,format= is required

[nirs]

Upstream krunkit requires the vrtio-blk,format= option. Existing users or programs using krunkit will break when the new code will be released.

Example command working with current brew release (0.1.4):

$ target/release/krunkit --memory=1024 --cpus=1 --device=virtio-blk,path=disk.img --krun-log-level=3
error: invalid value 'virtio-blk,path=disk.img' for '--device <DEVICES>': expected --virtio-blk argument to have 2 comma-separated sub-arguments, found 1

For more information, try '--help'.

Issues:

• Breaking change, should have a good reason to break existing users
• The error message is not good enough. It should not complain about the number of arguments, but about the missing argument, for example:

  error: invalid value 'virtio-blk,path=disk.img' for '--device <DEVICES>': --virtio-blk "format" sub-argument is required
  

I guess the reason the option is required is that want to avoid probing the image format. I don't think this is a good reason, there is no reason why probing qcow2 image need to be unsafe. But even if we really cannot probe the image, we can be safe and backward compatible by using this logic:

1. Make format=raw default
2. If format was not specified, we try to use the image as raw image. If the user provides a qcow2 image, the vm will not boot (safe).
3. To use qcow2 image user will have to specify the format (safe)

To allow automatic probing, we can add format=auto. When we don't probe image format, users will use qemu-img info to probe the image, so we did not make the user flow safer, just made the experience worse.

[jakecorrenti]

Hi @nirs!

I agree the error message isn't great. It seems the error message comes from the crate we use, but I can look into finding a way to improve the message.

I'm ok with making the default format Raw.

With regards to probing, we aren't going to add any form of it. Any probing that is necessary must be done by the user, but should be avoided at all costs. You can see the (lengthy) discussion about it in this PR: containers/libkrun#237

[nirs]

I agree the error message isn't great. It seems the error message comes from the crate we use, but I can look into finding a way to improve the message.

We can report an issue for the library then.

With regards to probing, we aren't going to add any form of it. Any probing that is necessary must be done by the user, but should be avoided at all costs. You can see the (lengthy) discussion about it in this PR: containers/libkrun#237

Guest being able to modify the image to look like a qcow2 image is good point, we should not probe based on guest controlled data.

[jakecorrenti]

I agree the error message isn't great. It seems the error message comes from the crate we use, but I can look into finding a way to improve the message.

We can report an issue for the library then.

I was mistaken, and that error message is coming from krunkit. Putting together a fix right now.

With regards to probing, we aren't going to add any form of it. Any probing that is necessary must be done by the user, but should be avoided at all costs. You can see the (lengthy) discussion about it in this PR: containers/libkrun#237

Guest being able to modify the image to look like a qcow2 image is good point, we should not probe based on guest controlled data.