From patchwork Thu Oct 18 20:31:47 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 986308 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="uUY0P5zJ"; dkim-atps=neutral Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 42bhbn5yljz9s5c for ; Fri, 19 Oct 2018 08:13:25 +1100 (AEDT) Received: from localhost ([::1]:44727 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gDFbf-0008KG-B2 for incoming@patchwork.ozlabs.org; Thu, 18 Oct 2018 17:13:23 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48172) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gDEyL-0007Lk-Aa for qemu-devel@nongnu.org; Thu, 18 Oct 2018 16:32:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gDEyJ-0006Ml-EK for qemu-devel@nongnu.org; Thu, 18 Oct 2018 16:32:45 -0400 Received: from mail-wm1-x341.google.com ([2a00:1450:4864:20::341]:55448) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gDEyH-0006KT-Vq for qemu-devel@nongnu.org; Thu, 18 Oct 2018 16:32:43 -0400 Received: by mail-wm1-x341.google.com with SMTP id 206-v6so1557163wmb.5 for ; Thu, 18 Oct 2018 13:32:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=lnWEJ0N4a/WoZ9aaazHe3KgZeg1wSJg067PW2c6O/sM=; b=uUY0P5zJ/2Jl8O24L1P0l473rwhiJgbi4c7XRkkmtWOqSjILGyJ0ukzsbU9jr+3veQ ZgPjebEEYndXJorXbEBJzHr9AJEIuJAJykmZ2ZLd3f27OvI7427qnr7A9mp3VXi3a72M oNXl5/B1/uvoLOjv39I+Wr3+RmnOn627ymidIp3Wl73d6+OR8wb6CmoNQMHHsbwEF6mk YZ8C4m8JF2qilkf8fcnW2UrSwjYkcLrooWDLkcgM+pMITFmCOXFdNsU/wO0TtS/bwTPn Wd632eV4zcTslC9GYUJ/va140w5AV8T2EtdCJ4OT/IxoWrg6SCjkB4k9HwANPiTh7ZVB IJVQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=lnWEJ0N4a/WoZ9aaazHe3KgZeg1wSJg067PW2c6O/sM=; b=MRmfMpIoUn2lYT2ieSKerZf60YZr0BKPRu0TKlp3i2TanX5wZ20n0/TYHdL9bHW75e oSTARSybeBMDe9Bu6RDuw8SsOZz2geJzFdL78RuXeo8VKk4BfE+8p7pKguSon330xPZF m5KWtMQhUUE7WBuMG8km0xiA2/4OEMfBGCp/oRfpizQu5GOhQXb6muO+pHftn0dnmAYR can3Xbpa5tAsTHqZyymhZD1KKTTQlAgbeWBjrmkUXQ0Nq2nkVw+HL/Mh5eszUOpmDbxd ltLHsgy7yYkeuGMObzVWgRl+BveXzZomaEQwCIOZHH2sJrr1yfMKB7nuiy2e0OVprSbH SsUw== X-Gm-Message-State: ABuFfogi8oPJ+zl1SLHyBqA5blKT5NArztZZGy/+l63Ppj9S/Y9XSANQ sG0qj85DZu0cyJgqSOHYJtaCzBEn X-Google-Smtp-Source: ACcGV62GwMR17OmDfczdwqhGzZBoIdWZA9X594dD40qHAAjdSCQ+0Cn90ayU368xOYx5R1ZGtQa+fQ== X-Received: by 2002:a1c:5fc2:: with SMTP id t185-v6mr1900874wmb.12.1539894759639; Thu, 18 Oct 2018 13:32:39 -0700 (PDT) Received: from 640k.lan (dynamic-adsl-78-12-231-174.clienti.tiscali.it. [78.12.231.174]) by smtp.gmail.com with ESMTPSA id a12-v6sm14270952wrr.71.2018.10.18.13.32.38 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Oct 2018 13:32:39 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Thu, 18 Oct 2018 22:31:47 +0200 Message-Id: <1539894735-14232-21-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1539894735-14232-1-git-send-email-pbonzini@redhat.com> References: <1539894735-14232-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:4864:20::341 Subject: [Qemu-devel] [PULL 20/48] call HotplugHandler->plug() as the last step in device realization X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Igor Mammedov Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" From: Igor Mammedov When [2] was fixed it was agreed that adding and calling post_plug() callback after device_reset() was low risk approach to hotfix issue right before release. So it was merged instead of moving already existing plug() callback after device_reset() is called which would be more risky and require all plug() callbacks audit. Looking at the current plug() callbacks, it doesn't seem that moving plug() callback after device_reset() is breaking anything, so here goes agreed upon [3] proper fix which essentially reverts [1][2] and moves plug() callback after device_reset(). This way devices always comes to plug() stage, after it's been fully initialized (including being reset), which fixes race condition [2] without need for an extra post_plug() callback. 1. (25e897881 "qdev: add HotplugHandler->post_plug() callback") 2. (8449bcf94 "virtio-scsi: fix hotplug ->reset() vs event race") 3. https://www.mail-archive.com/qemu-devel@nongnu.org/msg549915.html Signed-off-by: Igor Mammedov Message-Id: <1539696820-273275-1-git-send-email-imammedo@redhat.com> Reviewed-by: Stefan Hajnoczi Tested-by: Pierre Morel Acked-by: Pierre Morel Signed-off-by: Paolo Bonzini --- hw/core/hotplug.c | 10 ---------- hw/core/qdev.c | 16 ++++++---------- hw/scsi/virtio-scsi.c | 11 +---------- include/hw/hotplug.h | 11 ----------- 4 files changed, 7 insertions(+), 41 deletions(-) diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c index 2253072..17ac986 100644 --- a/hw/core/hotplug.c +++ b/hw/core/hotplug.c @@ -35,16 +35,6 @@ void hotplug_handler_plug(HotplugHandler *plug_handler, } } -void hotplug_handler_post_plug(HotplugHandler *plug_handler, - DeviceState *plugged_dev) -{ - HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler); - - if (hdc->post_plug) { - hdc->post_plug(plug_handler, plugged_dev); - } -} - void hotplug_handler_unplug_request(HotplugHandler *plug_handler, DeviceState *plugged_dev, Error **errp) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 046d8f1..6b3cc55 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -832,14 +832,6 @@ static void device_set_realized(Object *obj, bool value, Error **errp) DEVICE_LISTENER_CALL(realize, Forward, dev); - if (hotplug_ctrl) { - hotplug_handler_plug(hotplug_ctrl, dev, &local_err); - } - - if (local_err != NULL) { - goto post_realize_fail; - } - /* * always free/re-initialize here since the value cannot be cleaned up * in device_unrealize due to its usage later on in the unplug path @@ -869,8 +861,12 @@ static void device_set_realized(Object *obj, bool value, Error **errp) dev->pending_deleted_event = false; if (hotplug_ctrl) { - hotplug_handler_post_plug(hotplug_ctrl, dev); - } + hotplug_handler_plug(hotplug_ctrl, dev, &local_err); + if (local_err != NULL) { + goto child_realize_fail; + } + } + } else if (!value && dev->realized) { Error **local_errp = NULL; QLIST_FOREACH(bus, &dev->child_bus, sibling) { diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 5a3057d..3aa9971 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -797,16 +797,8 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, virtio_scsi_acquire(s); blk_set_aio_context(sd->conf.blk, s->ctx); virtio_scsi_release(s); - } -} -/* Announce the new device after it has been plugged */ -static void virtio_scsi_post_hotplug(HotplugHandler *hotplug_dev, - DeviceState *dev) -{ - VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev); - VirtIOSCSI *s = VIRTIO_SCSI(vdev); - SCSIDevice *sd = SCSI_DEVICE(dev); + } if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) { virtio_scsi_acquire(s); @@ -976,7 +968,6 @@ static void virtio_scsi_class_init(ObjectClass *klass, void *data) vdc->start_ioeventfd = virtio_scsi_dataplane_start; vdc->stop_ioeventfd = virtio_scsi_dataplane_stop; hc->plug = virtio_scsi_hotplug; - hc->post_plug = virtio_scsi_post_hotplug; hc->unplug = virtio_scsi_hotunplug; } diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h index 51541d6..1a0516a 100644 --- a/include/hw/hotplug.h +++ b/include/hw/hotplug.h @@ -47,8 +47,6 @@ typedef void (*hotplug_fn)(HotplugHandler *plug_handler, * @parent: Opaque parent interface. * @pre_plug: pre plug callback called at start of device.realize(true) * @plug: plug callback called at end of device.realize(true). - * @post_plug: post plug callback called after device.realize(true) and device - * reset * @unplug_request: unplug request callback. * Used as a means to initiate device unplug for devices that * require asynchronous unplug handling. @@ -63,7 +61,6 @@ typedef struct HotplugHandlerClass { /* */ hotplug_fn pre_plug; hotplug_fn plug; - void (*post_plug)(HotplugHandler *plug_handler, DeviceState *plugged_dev); hotplug_fn unplug_request; hotplug_fn unplug; } HotplugHandlerClass; @@ -87,14 +84,6 @@ void hotplug_handler_pre_plug(HotplugHandler *plug_handler, Error **errp); /** - * hotplug_handler_post_plug: - * - * Call #HotplugHandlerClass.post_plug callback of @plug_handler. - */ -void hotplug_handler_post_plug(HotplugHandler *plug_handler, - DeviceState *plugged_dev); - -/** * hotplug_handler_unplug_request: * * Calls #HotplugHandlerClass.unplug_request callback of @plug_handler.