From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:55005)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1SDFCf-0002f9-K0
	for qemu-devel@nongnu.org; Thu, 29 Mar 2012 09:15:55 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1SDFCZ-0007Mu-71
	for qemu-devel@nongnu.org; Thu, 29 Mar 2012 09:15:49 -0400
Received: from mx1.redhat.com ([209.132.183.28]:61314)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1SDFCY-0007Mh-Q3
	for qemu-devel@nongnu.org; Thu, 29 Mar 2012 09:15:43 -0400
Date: Thu, 29 Mar 2012 10:15:46 -0300
From: Luiz Capitulino <lcapitulino@redhat.com>
Message-ID: <20120329101546.71666aa7@doriath.home>
In-Reply-To: <20120329070015.GB13344@stefanha-thinkpad.localdomain>
References: <1332967854-29106-1-git-send-email-lcapitulino@redhat.com>
	<1332967854-29106-2-git-send-email-lcapitulino@redhat.com>
	<20120329070015.GB13344@stefanha-thinkpad.localdomain>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 1/2] qdev: qdev_unplug(): Use error_set()
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: Stefan Hajnoczi <stefanha@gmail.com>
Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org

On Thu, 29 Mar 2012 08:00:15 +0100
Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Wed, Mar 28, 2012 at 05:50:53PM -0300, Luiz Capitulino wrote:
> > @@ -268,7 +270,14 @@ static int pci_device_hot_remove(Monitor *mon, const char *pci_addr)
> >          monitor_printf(mon, "slot %d empty\n", slot);
> >          return -1;
> >      }
> > -    return qdev_unplug(&d->qdev);
> > +
> > +    ret = qdev_unplug(&d->qdev, &errp);
> > +    if (error_is_set(&errp)) {
> > +        monitor_printf(mon, "%s\n", error_get_pretty(errp));
> > +        error_free(errp);
> > +    }
> 
> Minor thing if you respin: this if statement could be replaced with hmp_handle_error(mon, &errp).

I'm not sure I'd like to see hmp_handle_error() spread over the tree. It uses
the monitor object and I've added it just because having the same code
duplicated among HMP functions bothered me... I think it's better to
restrict it to hmp.c.