On Sat, Jul 29, 2017 at 03:21:16AM +0000, Liu, Changpeng wrote: > > > > -----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.
A vhost-user process -> vhost-user master message is not necessary. An
eventfd can be used to signal config changes instead.
I have CCed Marc-André because I don't know much about the vhost-user
protocol design.
Here is what I suggest for virtio config space:
typedef enum VhostUserRequest {
...
/* Submitted by the vhost-user master when the guest writes to
* virtio config space and also after live migration on the
* destination host.
*/
VHOST_USER_SET_CONFIG,
/* Submitted by the vhost-user master to fetch the contents of the
* virtio config space. The vhost-user master may cache the
* contents to avoid repeated VHOST_USER_GET_CONFIG calls.
*/
VHOST_USER_GET_CONFIG,
...
};
struct VuDev {
...
int config_change_fd;
...
};
/**
* vu_config_change_notify:
* @dev: a VuDev context
*
* Notify the vhost-user master that the device has updated virtio
* config space. The master must raise a config change interrupt in the
* guest and invalidate any cached virtio config space contents so that
* the next guest read results in a VHOST_USER_GET_CONFIG request.
*/
void vu_config_change_notify(VuDev *dev);
signature.asc
Description: PGP signature
