On Thu, Nov 29, 2012 at 03:45:55PM +0100, Stefan Hajnoczi wrote: > On Thu, Nov 29, 2012 at 03:12:35PM +0200, Michael S. Tsirkin wrote: > > On Thu, Nov 22, 2012 at 04:16:52PM +0100, Stefan Hajnoczi wrote: > > > The virtio-blk-data-plane feature is easy to integrate into > > > hw/virtio-blk.c. The data plane can be started and stopped similar to > > > vhost-net. > > > > > > Users can take advantage of the virtio-blk-data-plane feature using the > > > new -device virtio-blk-pci,x-data-plane=on property. > > > > > > The x-data-plane name was chosen because at this stage the feature is > > > experimental and likely to see changes in the future. > > > > > > If the VM configuration does not support virtio-blk-data-plane an error > > > message is printed. Although we could fall back to regular virtio-blk, > > > I prefer the explicit approach since it prompts the user to fix their > > > configuration if they want the performance benefit of > > > virtio-blk-data-plane. > > > > Not only that, this affects features exposed to guest so it really can't be > > trasparent. > > > > Which reminds me - shouldn't some features be turned off? > > For example, VIRTIO_BLK_F_SCSI? > > Yes, virtio-blk-data-plane only starts when you give -device > virtio-blk-pci,scsi=off,x-data-plane=on. If you use scsi=on an error > message is printed. > > > > Limitations: > > > * Only format=raw is supported > > > * Live migration is not supported > > > > This is probably fixable long term? > > Absolutely. There are two parts: > > 1. Marking written memory dirty so live RAM migration can work. Missing > today, easy cheat is to switch off virtio-blk-data-plane and silently > switch to regular virtio-blk emulation while memory dirty logging is > enabled. The more long-term solution is to actually communicate the > dirty information back to the memory API. > > 2. Synchronizing virtio-blk-data-plane vring state with virtio-blk so > save/load works. This should be relatively straightforward. > > I don't want to gate this patch series on live migration support but it > is on my TODO list for virtio-blk-data-plane after this initial series > has been merged. > > > > * Block jobs, hot unplug, and other operations fail with -EBUSY > > > > Hmm I don't see code to disable PCU unplug in this patch. > > I expected no_hotplug to be set. > > Where is it? > > It uses the bdrv_in_use() mechanism.
Hmm but PCI device can still go away if guest ejects it. Does this work fine? > > > * I/O throttling limits are ignored > > > > And this? > > Meanwhile can we have attempts to set them fail? > > This limitation exists because virtio-blk-data-plane today bypasses the > QEMU block layer. The next step is to get the block layer working > inside the data plane thread. At that point I/O limits work again. > > Adding an error would be a layering violation because I/O throttling > happens in the QEMU block layer and is unaware of the emulated storage > controller (virtio-blk, IDE, SCSI, etc). > > I think it's better to document the limitation and continue working on > AioContext so that we can soon support I/O throttling with > virtio-blk-data-plane. It would be quite ugly to add checks. > > > > @@ -33,6 +35,8 @@ typedef struct VirtIOBlock > > > VirtIOBlkConf *blk; > > > unsigned short sector_mask; > > > DeviceState *qdev; > > > + VirtIOBlockDataPlane *dataplane; > > > + Error *migration_blocker; > > > > Would be nice to move the migration disabling > > checking supported formats > > and all the rest of it out to dataplane code. > > The reason to do it in virtio-blk.c is that we already have access to > the device configuration. If we move it to hw/dataplane/virtio-blk.c > then that code needs to reach inside and check data that it doesn't > otherwise access. Not really, just pass it all necessary data. > IMO it's nice to keep data plane "dumb" and perform these checks where > we already have to deal with the relationship between VirtIOBlkConf and > friends. > > Stefan Yes but then it's not contained.
