Re: [Qemu-devel] [PATCH v3 0/8] virtio-blk: use alias properties in transport devices

[Stefan Hajnoczi <stefanha@gmail.com>]

On Tue, Jun 03, 2014 at 05:11:23PM +0200, Cornelia Huck wrote:
> On Fri, 30 May 2014 13:17:41 +0200
> Stefan Hajnoczi <stefanha@redhat.com> wrote:
> 
> > v3:
> >  * Split qdev_alias_all_properties() into its own patch [Peter Crosthwaite]
> >  * Do not dereference DEVICE_CLASS(class) inline [Peter Crosthwaite]
> > 
> > v2:
> >  * Add qdev_alias_all_properties() instead of virtio-blk-specific function [Paolo]
> >  * Explain refcount handling in doc comment [Paolo]
> >  * Fix "property" duplicate typo [Peter Crosthwaite]
> >  * Add "the same object or" to clarify commit description [Igor]
> > 
> > Thanks for the feedback on the RFC.  This time around the alias property is
> > implemented at the QOM property level instead of at the qdev property level.
> > 
> > Note that this series only addresses virtio-blk.  In later series we can
> > convert virtio net, scsi, rng, and serial.
> > 
> > The virtio transport/device split is broken as follows:
> > 
> > 1. The virtio-blk device is never finalized because the transport devices
> >    (virtio-blk-pci and friends) leak the refcount.
> > 
> > 2. If we fix the refcount leak then we double-free the 'serial' string property
> >    upon hot unplug since its char* is copied into the virtio-blk device which
> >    has an identical 'serial' qdev property.
> > 
> > This series solves both of these problems as follows:
> > 
> > 1. Introduce a QOM alias property that lets the transport device forward
> >    property accesses into the virtio device (the child).
> > 
> > 2. Use alias properties in transport devices, instead of keeping a duplicate
> >    copy of the VirtIOBlkConf struct.
> > 
> > 3. Fix the virtio-blk device refcount leak.  It's now safe to do this since the
> >    double-free has been resolved.
> > 
> > Tested that hotplug/hotunplug of virtio-blk-pci still works.
> 
> FWIW:
> 
> I gave your qom-alias-property branch a quick test on s390.
> 
> virtio-ccw: seems to work fine, hotunplug of virtio-blk-ccw is still
> fine, and the virtio-blk memory leaks due to missing finalization that
> valgind complained about are gone.
> 
> s390-virtio: still boots, but with x-data-plane=on we get the
> predictable segfault in virtio_blk_data_plane_start() since s390-virtio
> doesn't do notifiers. Maybe the dataplane code should do a quick check
> for existence of the notifier callback when it allocates the dataplane
> structure?

Okay, good idea.  Thanks!

Stefan