Re: [Qemu-block] [Qemu-devel] [PATCH 1/1 V2] Add vhost-pci-blk driver

[Kevin Wolf]

Am 09.11.2018 um 02:48 hat Dongli Zhang geschrieben:
> 
> 
> On 11/09/2018 12:47 AM, Michael S. Tsirkin wrote:
> > On Thu, Nov 08, 2018 at 10:09:00PM +0800, Dongli Zhang wrote:
> >> It looks the kernel space vhost-blk can only process raw image.
> >>
> >> How about to verify that only raw image is used in the drive command line 
> >> when
> >> vhost-blk-pci is paired with it?
> >>
> >> Otherwise, vhost-blk-pci might be working with qcow2 image without any 
> >> warning
> >> on qemu side.
> >>
> >> Dongli Zhang
> > 
> > raw being raw can you really verify that?
> 
> I meant to verify the property 'format=' of '-drive', e.g., to check if
> BlockBackend->root->bs->drv->format_name is raw?

I don't like string comparisons for this, but at least check for "file"
and "host_device" then, because "raw" is a format driver that can be
used above any protocol driver, not just for local files.

Though rather than comparing strings, I'd check bs->drv == &bdrv_file.

> We allow the user to erroneously give a qcow2 file with 'format=raw'.
> However, if 'format=qcow2' is set explicitly, vhots-blk-pci would exit
> with error as only raw is supported in kernel space.

Please note that the QEMU block layer offers various options even for
file-posix images, and silently ignoring them is probably not very nice.
So just checking the driver is not enough, you also need to check that
no feature is enabled that vhost can't provide.

At first sight, this includes at least detect-zeroes, copy-on-read and
cache.no-flush. There may be more, but this just needs some work to find
all of them. More interesting question: How will we make sure that the
list is updated when we add new options?

Is it actually a good idea to use the block layer at all for this, or
would just taking a filename string be a better idea for this device?
Of course, if you want to do automatic switchover as Stefan suggested,
havin a block node is required and we need to figure out the exact
conditions when we can switch and a way to keep them updated when we
change things.

Kevin