From patchwork Tue Oct 19 07:15:31 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Vivier X-Patchwork-Id: 1543033 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=TlyWtkZ+; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HYQ9b04YFz9sPB for ; Tue, 19 Oct 2021 18:19:45 +1100 (AEDT) Received: from localhost ([::1]:59560 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mcjPR-0006Yy-OR for incoming@patchwork.ozlabs.org; Tue, 19 Oct 2021 03:19:41 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:52292) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mcjMG-0003EH-KA for qemu-devel@nongnu.org; Tue, 19 Oct 2021 03:16:28 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:29896) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mcjMF-0005J6-13 for qemu-devel@nongnu.org; Tue, 19 Oct 2021 03:16:24 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1634627782; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=KUFoCN50rdSSVjE7HaqIY2uzdyARcugkhlW7pMfuSvk=; b=TlyWtkZ+d8kTiqcp3MBJLHgkUOvfgZqsjHAmxW//wDhhzI9iqEPJrztqOxD9d0z7x/qfqJ I/XRM7A/ZPPzNjBCVj+/J9xkyV1rhGSNQkU59gz4WJcRLShkP7RatqSxO4Yyv/pJM5o+Uu YKKd54kVsAuUw8rOBAKol8q4Mz/KLfw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-375-c34XGLrEMrWRNdToy72czQ-1; Tue, 19 Oct 2021 03:16:20 -0400 X-MC-Unique: c34XGLrEMrWRNdToy72czQ-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 1383B18414A6 for ; Tue, 19 Oct 2021 07:16:20 +0000 (UTC) Received: from thinkpad.redhat.com (unknown [10.39.193.19]) by smtp.corp.redhat.com (Postfix) with ESMTP id 324D469203; Tue, 19 Oct 2021 07:16:00 +0000 (UTC) From: Laurent Vivier To: qemu-devel@nongnu.org Subject: [PATCH 1/2] failover: fix a regression introduced by JSON'ification of -device Date: Tue, 19 Oct 2021 09:15:31 +0200 Message-Id: <20211019071532.682717-2-lvivier@redhat.com> In-Reply-To: <20211019071532.682717-1-lvivier@redhat.com> References: <20211019071532.682717-1-lvivier@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lvivier@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=lvivier@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.049, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Laurent Vivier , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , Eduardo Habkost , "Michael S. Tsirkin" , Jason Wang , Paolo Bonzini Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" The hide_device helper can be called several times for the same devices as it shouldn't change any state and should only return an information. But not to rely anymore on QemuOpts we have introduced a new field to store the parameters of the device and don't allow to update it once it is done. And as the function is called several times, we ends with: warning: Cannot attach more than one primary device to 'virtio0' That is not only a warning as it prevents to hide the device and breaks failover. Fix that by checking the device id. Now, we fail only if the virtio-net device is really used by two different devices, for instance: -device virtio-net-pci,id=virtio0,failover=on,... \ -device vfio-pci,id=hostdev0,failover_pair_id=virtio0,... \ -device e1000e,id=e1000e0,failover_pair_id=virtio0,... \ will exit with: Cannot attach more than one primary device to 'virtio0': 'hostdev0' and 'e1000e0' Fixes: 259a10dbcb4f ("virtio-net: Store failover primary opts pointer locally") Cc: kwolf@redhat.com Signed-off-by: Laurent Vivier --- hw/net/virtio-net.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 09e173a55854..83642c85b2e5 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3304,15 +3304,27 @@ static bool failover_hide_primary_device(DeviceListener *listener, return false; } + /* + * The hide helper can be called several times for a given device. + * Check there is only one primary for a virtio-net device but + * don't duplicate the qdict several times if it's called for the same + * device. + */ if (n->primary_opts) { - error_setg(errp, "Cannot attach more than one primary device to '%s'", - n->netclient_name); - return false; + const char *old, *new; + /* devices with failover_pair_id always have an id */ + old = qdict_get_str(n->primary_opts, "id"); + new = qdict_get_str(device_opts, "id"); + if (strcmp(old, new) != 0) { + error_setg(errp, "Cannot attach more than one primary device to " + "'%s': '%s' and '%s'", n->netclient_name, old, new); + return false; + } + } else { + n->primary_opts = qdict_clone_shallow(device_opts); + n->primary_opts_from_json = from_json; } - n->primary_opts = qdict_clone_shallow(device_opts); - n->primary_opts_from_json = from_json; - /* failover_primary_hidden is set during feature negotiation */ return qatomic_read(&n->failover_primary_hidden); } From patchwork Tue Oct 19 07:15:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Vivier X-Patchwork-Id: 1543035 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=VuMYFftw; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HYQJX3QpSz9sPB for ; Tue, 19 Oct 2021 18:25:47 +1100 (AEDT) Received: from localhost ([::1]:46368 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mcjVH-0008J5-ML for incoming@patchwork.ozlabs.org; Tue, 19 Oct 2021 03:25:43 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:52374) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mcjMb-0003el-UH for qemu-devel@nongnu.org; Tue, 19 Oct 2021 03:16:45 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:38991) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mcjMZ-0005dO-Qh for qemu-devel@nongnu.org; Tue, 19 Oct 2021 03:16:45 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1634627802; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+UMo1LKDpTkbziyuawa9Gn/ybfj1Pu+ugGQkq/8n7Nw=; b=VuMYFftwydS7Miakh62hlM2nUXtkR46XsFNjCdjjnFQS9r5q9ps3/9t6CnJjkBavdES/Wt QAqczFgQrHND0LgCrbMbtRuHWF+QnoIAxzdCaYbmrMJQaFtM/3KBVAxU3aoiHKVGanpC1Z HkDOmAlR4K5MNXam9hKJ7OvCr/vCkMU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-20-V5vvo414Oia9vx_1vmRDwg-1; Tue, 19 Oct 2021 03:16:41 -0400 X-MC-Unique: V5vvo414Oia9vx_1vmRDwg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 33A92362FB for ; Tue, 19 Oct 2021 07:16:40 +0000 (UTC) Received: from thinkpad.redhat.com (unknown [10.39.193.19]) by smtp.corp.redhat.com (Postfix) with ESMTP id 675BE69203; Tue, 19 Oct 2021 07:16:20 +0000 (UTC) From: Laurent Vivier To: qemu-devel@nongnu.org Subject: [PATCH 2/2] qdev/qbus: remove failover specific code Date: Tue, 19 Oct 2021 09:15:32 +0200 Message-Id: <20211019071532.682717-3-lvivier@redhat.com> In-Reply-To: <20211019071532.682717-1-lvivier@redhat.com> References: <20211019071532.682717-1-lvivier@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lvivier@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=lvivier@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.049, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Laurent Vivier , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , Eduardo Habkost , "Michael S. Tsirkin" , Jason Wang , Paolo Bonzini , Jens Freimann Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Commit f3a850565693 ("qdev/qbus: add hidden device support") has introduced a generic way to hide a device but it has modified qdev_device_add() to check a specific option of the failover device, "failover_pair_id", before calling the generic mechanism. It's not needed (and not generic) to do that in qdev_device_add() because this is also checked by the failover_hide_primary_device() function that uses the generic mechanism to hide the device. Cc: Jens Freimann Signed-off-by: Laurent Vivier --- hw/net/virtio-net.c | 12 +++++++++++- softmmu/qdev-monitor.c | 18 ++++++------------ 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 83642c85b2e5..3dd2896ff95c 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3299,7 +3299,17 @@ static bool failover_hide_primary_device(DeviceListener *listener, if (!device_opts) { return false; } - standby_id = qdict_get_try_str(device_opts, "failover_pair_id"); + + if (!qdict_haskey(device_opts, "failover_pair_id")) { + return false; + } + + if (!qdict_haskey(device_opts, "id")) { + error_setg(errp, "Device with failover_pair_id needs to have id"); + return false; + } + + standby_id = qdict_get_str(device_opts, "failover_pair_id"); if (g_strcmp0(standby_id, n->netclient_name) != 0) { return false; } diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 89c473cb22a2..4851de51a5cb 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -639,19 +639,13 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts, } } - if (qdict_haskey(opts, "failover_pair_id")) { - if (!qdict_haskey(opts, "id")) { - error_setg(errp, "Device with failover_pair_id don't have id"); - return NULL; - } - if (qdev_should_hide_device(opts, from_json, errp)) { - if (bus && !qbus_is_hotpluggable(bus)) { - error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); - } - return NULL; - } else if (*errp) { - return NULL; + if (qdev_should_hide_device(opts, from_json, errp)) { + if (bus && !qbus_is_hotpluggable(bus)) { + error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); } + return NULL; + } else if (*errp) { + return NULL; } if (phase_check(PHASE_MACHINE_READY) && bus && !qbus_is_hotpluggable(bus)) {