> -----Original Message----- > From: Stefan Hajnoczi [mailto:stefa...@redhat.com] > Sent: Friday, July 28, 2017 6:36 PM > To: Liu, Changpeng <changpeng....@intel.com> > Cc: qemu-devel@nongnu.org; pbonz...@redhat.com; fel...@nutanix.com; > m...@redhat.com > Subject: Re: [PATCH 1/2] vhost-user-blk: introduce a new vhost-user-blk host > device > > On Thu, Jul 27, 2017 at 10:08:49AM +0000, Liu, Changpeng wrote: > > > -----Original Message----- > > > From: Stefan Hajnoczi [mailto:stefa...@redhat.com] > > > Sent: Thursday, July 27, 2017 5:49 PM > > > To: Liu, Changpeng <changpeng....@intel.com> > > > Cc: qemu-devel@nongnu.org; pbonz...@redhat.com; fel...@nutanix.com; > > > m...@redhat.com > > > Subject: Re: [PATCH 1/2] vhost-user-blk: introduce a new vhost-user-blk > > > host > > > device > > > > > > On Thu, Jul 27, 2017 at 12:29:45AM +0000, Liu, Changpeng wrote: > > > > > > + .fields = (VMStateField[]) { > > > > > > + VMSTATE_VIRTIO_DEVICE, > > > > > > + VMSTATE_END_OF_LIST() > > > > > > + }, > > > > > > +}; > > > > > > + > > > > > > +static Property vhost_user_blk_properties[] = { > > > > > > + DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev), > > > > > > + DEFINE_BLOCK_PROPERTIES(VHostUserBlk, blkcfg), > > > > > > > > > > DEFINE_BLOCK_PROPERTIES cannot be used directly because it includes > > > > > the > > > > > 'drive' (blkcfg.blk) parameter. The command-line should not allow a > > > > > drive= parameter. > > > > > > > > > > Also, parameters like the discard granularity, optimal I/O size, > > > > > logical > > > > > block size, etc can be initialized to sensible defaults by QEMU's > > > > > block > > > > > layer when drive= is used. Since you are bypassing QEMU's block layer > > > > > there is no way for QEMU to set good defaults. > > > > > > > > > > Are you relying on the managment tools populating these parameters > > > > > with > > > > > the correct values from the vhost-user-blk process? Or did you have > > > > > some other mechanism in mind? > > > > Actually for this part and your comments about block resize, I think > > > > maybe > add > > > several > > > > additional vhost user messages such like: > > > VHOST_USER_GET_BLK_CFG/VHOST_USER_SET_BLK_CFG > > > > makes the code more clear, I'm okay to add such messages. > > > > > > New messages for virtio config space read/write might be useful for > > > other devices besides virtio-blk. > > I mean all the block device information like: block_size/capacity are > > stored in > another process, > > so add a new vhost user message to Qemu vhost-user-blk can get those > information when Qemu get > > started, once those messages stored to virtio_pci config space, we will not > change it. > > No, I think updates are necessary: > > The virtio block device can do online disk resize, so it will be > necessary to change the capacity field from the vhost-user-blk process > at runtime and raise a config change notification interrupt. > > The virtio block device also has a config space field ("wce") that is > writable by the guest. Supporting this feature also requires virtio > config space support in vhost-user. > > If you focus on adding generic virtio config space messages to > vhost-user then these virtio block features can be supported by > vhost-user-blk. > > Regarding the two approaches of adding "block device information" as you > have suggested versus adding generic virtio config space support as I'm > suggesting, from a protocol design perspective it's nicer to have > generic messages that are usable by all device types. I'm not aware of > a reason why high-level "block device information" is needed since QEMU > will just put that into the config space but otherwise does not > interpret the information. Agreed, adding a vhost message to get/set generic vitio_pci device config space is clear to me now, it's better than only for vhost-user-blk messages.
Here I still have an concern about *resize* feature for vhost-user-blk, currently Qemu vhost-user-blk is the client for vhost user messages, does this means the I/O processing process must send a new vhost message back to Qemu vhost-user-blk driver that the capacity of the block device is changed? Or you have better idea to do this? Thanks. > > > > It's worth checking how virtio config space is live migrated and how to > > > do that consistently if the vhost-user process is involved. > > Virtio will config spaces for virtio_blk, and even the config space > > migrated to > another machine, it should be > > same with the another I/O process, did I get your comments ? Thanks. > > The guest-writable "wce" config space field is an example that shows the > vhost-user-blk process on the destination host does not have all the > state necessary. QEMU needs to migrate the config space and send it to > the vhost-user-blk process on the destination. > > Stefan