Re: [virtio] Re: [virtio-dev] Re: [virtio-comment] Problems with VIRTIO-4 and writeback only disks

[Paolo Bonzini <pbonzini@redhat.com>]

Il 01/10/2013 07:22, Rusty Russell ha scritto:
> Rusty Russell <rusty@au1.ibm.com> writes:
>> Thinking about this some more: why not make WCE the only option?
>>
>> (1) It's simple.
>> (2) We say flush SHOULD hit the disk.
> 
> OK, here's the actual patch.  It applies most of Paulo's patch,
> but to the legacy section, and remove VIRTIO_BLK_F_FLUSH.  It also
> documents what FLUSH should do (is more clarity required?)

Actually that's a pretty smart thing to do. :)  Just a couple comments
below.

> diff --git a/virtio-v1.0-wd01-part1-specification.txt b/virtio-v1.0-wd01-part1-specification.txt
> index 5efeba7..9b5c29a 100644
> --- a/virtio-v1.0-wd01-part1-specification.txt
> +++ b/virtio-v1.0-wd01-part1-specification.txt
> @@ -1817,8 +1817,6 @@ device except where noted.
>  
>    VIRTIO_BLK_F_BLK_SIZE (6) Block size of disk is in “blk_size”.
>  
> -  VIRTIO_BLK_F_FLUSH (9) Cache flush command support.
> -
>    VIRTIO_BLK_F_TOPOLOGY (10) Device exports information on optimal I/O
>      alignment.
>  
> @@ -1843,6 +1841,7 @@ device except where noted.
>  			u16 min_io_size;
>  			u32 opt_io_size;
>  		} topology;
> +		u8 reserved;
>  	};
>  
>  2.4.2.3.1 Legacy Interface: Feature bits
> @@ -1851,6 +1850,15 @@ device except where noted.
>  
>    VIRTIO_BLK_F_SCSI (7) Device supports scsi packet commands.
>  
> +  VIRTIO_BLK_F_FLUSH (9) Cache flush command support.
> +
> +  VIRTIO_BLK_F_CONFIG_WCE (11) Device can toggle its cache between writeback
> +    and writethrough modes.

Do you need to document where the wce flag is in the configuration?

> +VIRTIO_BLK_F_FLUSH was also called VIRTIO_BLK_F_WCE: Legacy drivers
> +should only negotiate this feature if they are capable of sending
> +VIRTIO_BLK_T_FLUSH commands.
> +
>  2.4.2.4. Device Initialization
>  -----------------------------
>  
> @@ -1872,6 +1880,22 @@ device except where noted.
>    I/O lengths for the driver to use. This also does not affect the units
>    in the protocol, only performance.
>  
> +2.4.2.4.1. Legacy Interface: Device Initialization
> +-----------------------------
> +
> +The reserved field used to be called writeback.  If the
> +VIRTIO_BLK_F_CONFIG_WCE feature is offered, the cache mode should be
> +read from the writeback field of the configuration if available; the
> +driver can also write to the field in order to toggle the cache
> +between writethrough (0) and writeback (1) mode.  If the feature is
> +not available, the driver can instead look at the result of
> +negotiating VIRTIO_BLK_F_WCE: the cache will be in writeback mode
> +after reset if and only if VIRTIO_BLK_F_WCE is negotiated.

s/VIRTIO_BLK_F_WCE/VIRTIO_BLK_F_FLUSH/ everywhere

> +Note that buggy legacy devices are common, which always operate in
> +write back mode even if VIRTIO_BLK_F_WCE isn't accepted, or writeback
> +is turned off.

I would remove this paragraph.  You found bugs, true, but it's not that
the affected versions of QEMU are common---they are neither in Debian
nor in RHEL/CentOS, and frankly using anything else in production would
be crazy (also if you do I hope you'll update fast, and those hosts will
not be "common" anymore by the time standardization is finished).

Paolo

>  2.4.2.5. Device Operation
>  ------------------------
>  
> @@ -1907,6 +1931,9 @@ error or VIRTIO_BLK_S_UNSUPP for a request unsupported by host:
>  	#define VIRTIO_BLK_S_IOERR     1
>  	#define VIRTIO_BLK_S_UNSUPP    2
>  
> +Any writes completed before the submission of the flush command should
> +be committed to permanent storage by the device.
> +
>  2.4.2.5.1 Legacy Interface: Device Operation
>  ------------------------
>  The 'reserved' field was previously called ioprio.  The ioprio field


> 
> 
> ---------------------------------------------------------------------
> To unsubscribe from this mail list, you must leave the OASIS TC that 
> generates this mail.  Follow this link to all your TCs in OASIS at:
> https://www.oasis-open.org/apps/org/workgroup/portal/my_workgroups.php 
>