[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] vhost-user-blk: introduce a new vhost-user-
From: |
Liu, Changpeng |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] vhost-user-blk: introduce a new vhost-user-blk host device |
Date: |
Sat, 29 Jul 2017 03:21:16 +0000 |
> -----Original Message-----
> From: Stefan Hajnoczi [mailto:address@hidden
> Sent: Friday, July 28, 2017 6:36 PM
> To: Liu, Changpeng <address@hidden>
> Cc: address@hidden; address@hidden; address@hidden;
> address@hidden
> 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:address@hidden
> > > Sent: Thursday, July 27, 2017 5:49 PM
> > > To: Liu, Changpeng <address@hidden>
> > > Cc: address@hidden; address@hidden; address@hidden;
> > > address@hidden
> > > 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
- [Qemu-devel] [PATCH 0/2] Introduce a new vhost-user-blk device and sample application, Changpeng Liu, 2017/07/25
- [Qemu-devel] [PATCH 2/2] vhost-user-blk: introduce a vhost-user-blk sample application, Changpeng Liu, 2017/07/25
- [Qemu-devel] [PATCH 1/2] vhost-user-blk: introduce a new vhost-user-blk host device, Changpeng Liu, 2017/07/25
- Re: [Qemu-devel] [PATCH 1/2] vhost-user-blk: introduce a new vhost-user-blk host device, Stefan Hajnoczi, 2017/07/26
- Re: [Qemu-devel] [PATCH 1/2] vhost-user-blk: introduce a new vhost-user-blk host device, Liu, Changpeng, 2017/07/26
- Re: [Qemu-devel] [PATCH 1/2] vhost-user-blk: introduce a new vhost-user-blk host device, Stefan Hajnoczi, 2017/07/27
- Re: [Qemu-devel] [PATCH 1/2] vhost-user-blk: introduce a new vhost-user-blk host device, Liu, Changpeng, 2017/07/27
- Re: [Qemu-devel] [PATCH 1/2] vhost-user-blk: introduce a new vhost-user-blk host device, Stefan Hajnoczi, 2017/07/28
- Re: [Qemu-devel] [PATCH 1/2] vhost-user-blk: introduce a new vhost-user-blk host device,
Liu, Changpeng <=
- Re: [Qemu-devel] [PATCH 1/2] vhost-user-blk: introduce a new vhost-user-blk host device, Stefan Hajnoczi, 2017/07/31
- Re: [Qemu-devel] [PATCH 1/2] vhost-user-blk: introduce a new vhost-user-blk host device, Paolo Bonzini, 2017/07/31
- Re: [Qemu-devel] [PATCH 1/2] vhost-user-blk: introduce a new vhost-user-blk host device, Liu, Changpeng, 2017/07/31