On Wed, Jul 04, 2012 at 06:08:50PM +0200, Paolo Bonzini wrote: > Il 04/07/2012 18:02, Michael S. Tsirkin ha scritto: > > On Wed, Jul 04, 2012 at 05:54:16PM +0200, Paolo Bonzini wrote: > >> Il 04/07/2012 17:42, Michael S. Tsirkin ha scritto: > >>> On Tue, Jul 03, 2012 at 03:19:37PM +0200, Paolo Bonzini wrote: > >>>> This patch adds support for the new VIRTIO_BLK_F_CONFIG_WCE feature, > >>>> which exposes the cache mode in the configuration space and lets the > >>>> driver modify it. The cache mode is exposed via sysfs. > >>>> > >>>> Even if the host does not support the new feature, the cache mode is > >>>> visible (thanks to the existing VIRTIO_BLK_F_WCE), but not modifiable. > >>>> > >>>> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx> > >>> > >>> I note this has been applied but I think the userspace > >>> API is a bit painful to use. Let's fix it before > >>> it gets set in stone? > >> > >> I'm trying to mimic the existing userspace API for SCSI disks. FWIW I > >> would totally agree with you. > > > > Hmm. Want to try fixing scsi? Need to be compatible but it could > > maybe ignore spaces. > > Honestly I'm not sure it's really worthwhile... And you also have the > same problem when printing. You cannot remove the spaces, because > clients will look for the "old" string, with the spaces. Right. Oh well. > >>>> +static int virtblk_get_cache_mode(struct virtio_device *vdev) > >>> > >>> Why are you converting u8 to int here? > >> > >> The fact that it is a u8 is really an internal detail. Perhaps the bug > >> is using u8 in the callers. > > > > Make it bool then? > > > > You are using u8 in the config. So you could get any value > > besides 0 and 1, and you interpret that as 1. > > Is 1 always a safe value? If not maybe it's better to set > > to a safe value if it is not 0 or 1, that is we don't know how to interpret it. > > That would be a host bug; the spec only gives meaning to 0 and 1. Yes but if the other side does not validate values implementations *will* have bugs. Why not declare bits 1-7 reserved? Then we can reuse other bits later. > In > any case, "have a cache" means "needs to flush", so it's always safe. > If you interpret another value as 0, you risk omitting flushes. > > Paolo OK, that's good. -- MST _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
