From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:45699)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <imammedo@redhat.com>) id 1XWodk-0007In-Nl
	for qemu-devel@nongnu.org; Wed, 24 Sep 2014 11:38:06 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <imammedo@redhat.com>) id 1XWode-0001W6-CU
	for qemu-devel@nongnu.org; Wed, 24 Sep 2014 11:38:00 -0400
Received: from mx1.redhat.com ([209.132.183.28]:10946)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <imammedo@redhat.com>) id 1XWode-0001UP-4r
	for qemu-devel@nongnu.org; Wed, 24 Sep 2014 11:37:54 -0400
Date: Wed, 24 Sep 2014 17:37:40 +0200
From: Igor Mammedov <imammedo@redhat.com>
Message-ID: <20140924173740.2f3e37ec@nial.usersys.redhat.com>
In-Reply-To: <5422B35E.2020006@redhat.com>
References: <1411559299-19042-1-git-send-email-imammedo@redhat.com>
	<1411559299-19042-30-git-send-email-imammedo@redhat.com>
	<5422B35E.2020006@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 29/30] qdev: drop legacy hotplug
	fields/methods
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: dmitry@daynix.com, mst@redhat.com, agraf@suse.de, qemu-devel@nongnu.org, cornelia.huck@de.ibm.com, kraxel@redhat.com, amit.shah@redhat.com, borntraeger@de.ibm.com, rth@twiddle.net

On Wed, 24 Sep 2014 14:04:46 +0200
Paolo Bonzini <pbonzini@redhat.com> 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 <pbonzini@redhat.com>
> 
> Paolo