From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:46723)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1VozEb-0003kd-FK
	for qemu-devel@nongnu.org; Fri, 06 Dec 2013 12:30:43 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1VozEV-00044k-3j
	for qemu-devel@nongnu.org; Fri, 06 Dec 2013 12:30:37 -0500
Received: from mx1.redhat.com ([209.132.183.28]:9800)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1VozEU-00044W-Qh
	for qemu-devel@nongnu.org; Fri, 06 Dec 2013 12:30:31 -0500
Message-ID: <52A20996.8020308@redhat.com>
Date: Fri, 06 Dec 2013 18:29:58 +0100
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1386349395-5710-1-git-send-email-imammedo@redhat.com>
	<1386349395-5710-8-git-send-email-imammedo@redhat.com>
In-Reply-To: <1386349395-5710-8-git-send-email-imammedo@redhat.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use
	"hotplug-device" interface.
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: Igor Mammedov <imammedo@redhat.com>
Cc: peter.maydell@linaro.org, ehabkost@redhat.com, mst@redhat.com, marcel.a@redhat.com, qemu-devel@nongnu.org, blauwirbel@gmail.com, alex.williamson@redhat.com, anthony@codemonkey.ws, afaerber@suse.de

Il 06/12/2013 18:03, Igor Mammedov ha scritto:
> +    hotplug_dev = DEVICE(object_property_get_link(OBJECT(bus), "hotplug-device",
> +                                                  &local_err));
> +    if (error_is_set(&local_err)) {
> +        goto error_exit;
> +    }
> +    if (hotplug_dev) {
> +        HotplugDeviceClass *hdc = HOTPLUG_DEVICE_GET_CLASS(hotplug_dev);
> +
> +        /* handler can differentiate between hotplug and when device is
> +         * enabled during qemu machine creation by inspecting
> +         * dev->hotplugged field. */
> +        if (hdc->hotplug) {
> +            hdc->hotplug(hotplug_dev, qdev, &local_err);
> +            if (error_is_set(&local_err)) {
> +                int r = pci_unregister_device(&pci_dev->qdev);
> +                assert(!r);
> +                goto error_exit;
> +            }
>          }
>      }

> +
> +    hotplug_dev = DEVICE(object_property_get_link(OBJECT(bus), "hotplug-device",
> +                                                  &local_err));
> +    if (error_is_set(&local_err)) {
> +        goto error_exit;
> +    }
> +    if (hotplug_dev) {
> +        HotplugDeviceClass *hdc = HOTPLUG_DEVICE_GET_CLASS(hotplug_dev);
> +
> +        if (hdc->hot_unplug) {
> +            hdc->hot_unplug(hotplug_dev, qdev, &local_err);
> +            if (error_is_set(&local_err)) {
> +                goto error_exit;
> +            }
> +        }
> +    }

Please move the parts under the "if" to hotplug.c (something like
hotplug_handler_plug and hotplug_handler_unplug).  Also, should this be
moved up to generic code (e.g. bus_add_child/bus_remove_child)?

A NULL link can be a no-op for coldplugged devices, an error for
hotplugged devices, and an assertion failure for hot-unplug.

Paolo