[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 0/8] virtio-blk: use alias properties in tran
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v3 0/8] virtio-blk: use alias properties in transport devices |
Date: |
Wed, 4 Jun 2014 10:08:41 +0200 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Tue, Jun 03, 2014 at 05:11:23PM +0200, Cornelia Huck wrote:
> On Fri, 30 May 2014 13:17:41 +0200
> Stefan Hajnoczi <address@hidden> 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