Re: [Qemu-devel] [PATCH v2 2/5] Fix migration in case of scsi-generic

[Kevin Wolf <kwolf@redhat.com>]

Am 08.05.2015 um 19:47 hat Dimitris Aragiorgis geschrieben:
> During migration, QEMU uses fsync()/fdatasync() on the open file
> descriptor for read-write block devices to flush data just before
> stopping the VM.
> 
> However, fsync() on a scsi-generic device returns -EINVAL which
> causes the migration to fail. This patch skips flushing data in case
> of an SG device, since submitting SCSI commands directly via an SG
> character device (e.g. /dev/sg0) bypasses the page cache completely,
> anyway.

fsync() doesn't only flush the kernel page cache, but also the disk
cache. The full justification includes at least that the scsi-generic
device never sends flushes, and for migration it's assumed that the same
SCSI device is used by the destination host (rather than another way to
access the same backing storage). If this were not enough, we would have
to send a SCSI flush command.

On another note, I expect we still neglect to check bs_is_sg() in too
many places. Any monitor command that does normal I/O on such a BDS
should actually fail.

> Remove the bdrv_is_sg() test from iscsi_co_flush() since this is
> now redundant (we flush the underlying protocol at the end
> of bdrv_co_flush() which, with this patch, we never reach).
> 
> Signed-off-by: Dimitris Aragiorgis <dimara@arrikto.com>
> ---
>  block/io.c    |    2 +-
>  block/iscsi.c |    4 ----
>  2 files changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 1ce62c4..d248a4d 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -2231,7 +2231,7 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
>  {
>      int ret;
>  
> -    if (!bs || !bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
> +    if (!bs || !bdrv_is_inserted(bs) || bdrv_is_read_only(bs) || bdrv_is_sg(bs)) {

This line exceeds 80 characters.

Kevin