On Wed, 24 Sep 2014 14:04:46 +0200
Paolo Bonzini <[email protected]> wrote:
> Il 24/09/2014 13:48, Igor Mammedov ha scritto:
> > @@ -239,10 +239,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
> > hotplug_handler_unplug(dev->parent_bus->hotplug_handler, dev,
> > errp);
> > }
> > } else {
> > - assert(dc->unplug != NULL);
> > - if (dc->unplug(dev) < 0) { /* legacy handler */
> > - error_set(errp, QERR_UNDEFINED_ERROR);
> > - }
> > + assert(0);
> > }
>
> This is not particularly nice, but it makes sense at this part of the
> series, since an
>
> assert(dev->parent_bus && dev->parent_bus->hotplug_handler);
>
> would be changed immediately in the next patch. Also, it would change
> indentation and make the patch bigger. Hence, please consider adding a
> 31st patch that changes
>
> hotplug_ctrl = qdev_get_hotplug_handler(dev);
> if (hotplug_ctrl) {
> ...
> } else {
> assert(0);
> }
>
> to
>
> hotplug_ctrl = qdev_get_hotplug_handler(dev);
> assert(hotplug_ctrl);
sure, I'll add extra patch
> ...
>
> Still, this patch is okay.
>
> Reviewed-by: Paolo Bonzini <[email protected]>
>
> Paolo
