On Mon, Oct 23, 2017 at 04:26:36AM +0000, Liu, Changpeng wrote: > > > > -----Original Message----- > > From: Stefan Hajnoczi [mailto:stefa...@gmail.com] > > Sent: Friday, October 20, 2017 5:55 PM > > To: Liu, Changpeng <changpeng....@intel.com> > > Cc: qemu-devel@nongnu.org; pbonz...@redhat.com; m...@redhat.com; > > marcandre.lur...@redhat.com; fel...@nutanix.com; Harris, James R > > <james.r.har...@intel.com> > > Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk > > host > > device > > > > On Fri, Oct 20, 2017 at 01:47:58AM +0000, Liu, Changpeng wrote: > > > > > +static Property vhost_user_blk_properties[] = { > > > > > + DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev), > > > > > + DEFINE_PROP_UINT16("num_queues", VHostUserBlk, num_queues, 1), > > > > > + DEFINE_PROP_UINT32("queue_size", VHostUserBlk, queue_size, 128), > > > > > + DEFINE_PROP_BIT64("f_size_max", VHostUserBlk, host_features, > > > > > + VIRTIO_BLK_F_SIZE_MAX, true), > > > > > + DEFINE_PROP_BIT64("f_sizemax", VHostUserBlk, host_features, > > > > > + VIRTIO_BLK_F_SIZE_MAX, true), > > > > > + DEFINE_PROP_BIT64("f_segmax", VHostUserBlk, host_features, > > > > > + VIRTIO_BLK_F_SEG_MAX, true), > > > > > + DEFINE_PROP_BIT64("f_geometry", VHostUserBlk, host_features, > > > > > + VIRTIO_BLK_F_GEOMETRY, true), > > > > > + DEFINE_PROP_BIT64("f_readonly", VHostUserBlk, host_features, > > > > > + VIRTIO_BLK_F_RO, false), > > > > > + DEFINE_PROP_BIT64("f_blocksize", VHostUserBlk, host_features, > > > > > + VIRTIO_BLK_F_BLK_SIZE, true), > > > > > + DEFINE_PROP_BIT64("f_topology", VHostUserBlk, host_features, > > > > > + VIRTIO_BLK_F_TOPOLOGY, true), > > > > > + DEFINE_PROP_BIT64("f_multiqueue", VHostUserBlk, host_features, > > > > > + VIRTIO_BLK_F_MQ, true), > > > > > + DEFINE_PROP_BIT64("f_flush", VHostUserBlk, host_features, > > > > > + VIRTIO_BLK_F_FLUSH, true), > > > > > + DEFINE_PROP_BIT64("f_barrier", VHostUserBlk, host_features, > > > > > + VIRTIO_BLK_F_BARRIER, false), > > > > > + DEFINE_PROP_BIT64("f_scsi", VHostUserBlk, host_features, > > > > > + VIRTIO_BLK_F_SCSI, false), > > > > > + DEFINE_PROP_BIT64("f_wce", VHostUserBlk, host_features, > > > > > + VIRTIO_BLK_F_WCE, false), > > > > > > > > Please explain how feature negotation works. The vhost-user slave > > > > advertises support features in the return value from > > > > VHOST_USER_GET_FEATURES. How does this additional feature mask work > > and > > > > why is it useful? > > > According to Paolo's previous comments, VIRTIO_BLK_F_WCE/ > > VIRTIO_BLK_F_SCSI/ VIRTIO_BLK_F_BARRIER > > > should be removed. Here I added all the feature flags just want to avoid > > > the > > case that vhost-user slave target > > > can support but Qemu vhost block driver cannot support it. > > > > Please explain a bit more how these options can be used. > > > > When I looked at the vhost code it seemed like the vhost slave can > > report any feature bits it wishes (even things QEMU doesn't know about). > > What is the purpose of override some of the feature bits on the QEMU > > command-line? > Hi Stefan, > Here I added a switch which can override vhost-slave's feature bits, for > example, vhost-slave reported `VIRTIO_BLK_F_RO`, > but Qemu vhost-master can disable it through command line when started the > Qemu. Users don't need to change any > vhost-slave's code to disable this feature, and this is also aligned with > vhost-scsi and vhost-net's implementation.
Yes but I don't see these properties in virtio_blk_properties. Please make the names consistent at least when virtio-blk has them. I am pretty sure you don't want to expose e.g. _F_SCSI. > > Stefan
