[{"id":1761613,"web_url":"http://patchwork.ozlabs.org/comment/1761613/","msgid":"","list_archive_url":null,"date":"2017-09-01T11:37:34","subject":"Re: [Qemu-devel] [PATCH] isa-fdc: assert replaced by proper error\n\texit","submitter":{"id":66152,"url":"http://patchwork.ozlabs.org/api/people/66152/","name":"Thomas Huth","email":"thuth@redhat.com"},"content":"On 01.09.2017 13:07, Eduardo Otubo wrote:\n> When not available, isa-fdc falls into assert instead of proper error\n> exit. This patch fixes this behavior.\n> \n> Signed-off-by: Eduardo Otubo \n> ---\n> hw/block/fdc.c | 6 +++++-\n> 1 file changed, 5 insertions(+), 1 deletion(-)\n> \n> diff --git a/hw/block/fdc.c b/hw/block/fdc.c\n> index 401129073b..0b6def4e1d 100644\n> --- a/hw/block/fdc.c\n> +++ b/hw/block/fdc.c\n> @@ -2699,11 +2699,15 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)\n> fdctrl->dma_chann = isa->dma;\n> if (fdctrl->dma_chann != -1) {\n> fdctrl->dma = isa_get_dma(isa_bus_from_device(isadev), isa->dma);\n> - assert(fdctrl->dma);\n> + if (!fdctrl->dma) {\n> + error_setg(errp, \"isa-fdc not supported\");\n> + goto error;\n> + }\n> }\n> \n> qdev_set_legacy_instance_id(dev, isa->iobase, 2);\n> fdctrl_realize_common(dev, fdctrl, &err);\n> +error:\n> if (err != NULL) {\n> error_propagate(errp, err);\n> return;\n\nMaybe add the reproducer to the commit message:\n\n qemu-system-ppc64 -S -machine powernv -device isa-fdc\n\nReviewed-by: Thomas Huth ","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=thuth@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xkHW70WlXz9sRV\n\tfor ;\n\tFri, 1 Sep 2017 21:45:55 +1000 (AEST)","from localhost ([::1]:36615 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnkOX-0005Iy-3W\n\tfor incoming@patchwork.ozlabs.org; Fri, 01 Sep 2017 07:45:53 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:33233)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnkGn-0006ix-1Q\n\tfor qemu-devel@nongnu.org; Fri, 01 Sep 2017 07:37:57 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnkGi-0000Xo-CU\n\tfor qemu-devel@nongnu.org; Fri, 01 Sep 2017 07:37:53 -0400","from mx1.redhat.com ([209.132.183.28]:47632)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dnkGd-0000VR-Rl; Fri, 01 Sep 2017 07:37:43 -0400","from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id D25DF61485;\n\tFri, 1 Sep 2017 11:37:42 +0000 (UTC)","from [10.36.116.102] (ovpn-116-102.ams2.redhat.com [10.36.116.102])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 8A86E9341C;\n\tFri, 1 Sep 2017 11:37:36 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com D25DF61485","To":"Eduardo Otubo , qemu-devel@nongnu.org","References":"<20170901110706.429-1-otubo@redhat.com>","From":"Thomas Huth ","Message-ID":"","Date":"Fri, 1 Sep 2017 13:37:34 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170901110706.429-1-otubo@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.11","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]);\n\tFri, 01 Sep 2017 11:37:43 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH] isa-fdc: assert replaced by proper error\n\texit","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"qemu-trivial@nongnu.org, jsnow@redhat.com, qemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1761701,"web_url":"http://patchwork.ozlabs.org/comment/1761701/","msgid":"<87y3py372n.fsf@dusky.pond.sub.org>","list_archive_url":null,"date":"2017-09-01T13:02:40","subject":"Re: [Qemu-devel] [PATCH] isa-fdc: assert replaced by proper error\n\texit","submitter":{"id":2645,"url":"http://patchwork.ozlabs.org/api/people/2645/","name":"Markus Armbruster","email":"armbru@redhat.com"},"content":"Eduardo Otubo writes:\n\n> When not available, isa-fdc falls into assert instead of proper error\n> exit. This patch fixes this behavior.\n\nWhen what exactly isn't available?\n\n>\n> Signed-off-by: Eduardo Otubo \n> ---\n> hw/block/fdc.c | 6 +++++-\n> 1 file changed, 5 insertions(+), 1 deletion(-)\n>\n> diff --git a/hw/block/fdc.c b/hw/block/fdc.c\n> index 401129073b..0b6def4e1d 100644\n> --- a/hw/block/fdc.c\n> +++ b/hw/block/fdc.c\n> @@ -2699,11 +2699,15 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)\n> fdctrl->dma_chann = isa->dma;\n> if (fdctrl->dma_chann != -1) {\n> fdctrl->dma = isa_get_dma(isa_bus_from_device(isadev), isa->dma);\n> - assert(fdctrl->dma);\n> + if (!fdctrl->dma) {\n> + error_setg(errp, \"isa-fdc not supported\");\n> + goto error;\n> + }\n> }\n> \n> qdev_set_legacy_instance_id(dev, isa->iobase, 2);\n> fdctrl_realize_common(dev, fdctrl, &err);\n> +error:\n> if (err != NULL) {\n> error_propagate(errp, err);\n> return;\n\nThe actual patch makes me suspect it's \"when ISA DMA isn't available\".","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=armbru@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xkL871yQ4z9t2d\n\tfor ;\n\tFri, 1 Sep 2017 23:44:39 +1000 (AEST)","from localhost ([::1]:41075 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnmFR-0002fs-A6\n\tfor incoming@patchwork.ozlabs.org; Fri, 01 Sep 2017 09:44:37 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:56543)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnlbU-0006rQ-U9\n\tfor qemu-devel@nongnu.org; Fri, 01 Sep 2017 09:03:25 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnlbQ-0001oG-FH\n\tfor qemu-devel@nongnu.org; Fri, 01 Sep 2017 09:03:20 -0400","from mx1.redhat.com ([209.132.183.28]:34302)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dnlb8-0001hz-HR; Fri, 01 Sep 2017 09:02:58 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 840647EBD3;\n\tFri, 1 Sep 2017 13:02:57 +0000 (UTC)","from blackfin.pond.sub.org (ovpn-116-75.ams2.redhat.com\n\t[10.36.116.75])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 8205878342;\n\tFri, 1 Sep 2017 13:02:41 +0000 (UTC)","by blackfin.pond.sub.org (Postfix, from userid 1000)\n\tid 0905F1138662; Fri, 1 Sep 2017 15:02:40 +0200 (CEST)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 840647EBD3","From":"Markus Armbruster ","To":"Eduardo Otubo ","References":"<20170901110706.429-1-otubo@redhat.com>","Date":"Fri, 01 Sep 2017 15:02:40 +0200","In-Reply-To":"<20170901110706.429-1-otubo@redhat.com> (Eduardo Otubo's message\n\tof \"Fri, 1 Sep 2017 13:07:06 +0200\")","Message-ID":"<87y3py372n.fsf@dusky.pond.sub.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux)","MIME-Version":"1.0","Content-Type":"text/plain","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.27]);\n\tFri, 01 Sep 2017 13:02:57 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH] isa-fdc: assert replaced by proper error\n\texit","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"qemu-trivial@nongnu.org, jsnow@redhat.com, qemu-devel@nongnu.org,\n\tqemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1761943,"web_url":"http://patchwork.ozlabs.org/comment/1761943/","msgid":"","list_archive_url":null,"date":"2017-09-01T19:29:27","subject":"Re: [Qemu-devel] [PATCH] isa-fdc: assert replaced by proper error\n\texit","submitter":{"id":64343,"url":"http://patchwork.ozlabs.org/api/people/64343/","name":"John Snow","email":"jsnow@redhat.com"},"content":"On 09/01/2017 07:07 AM, Eduardo Otubo wrote:\n> When not available, isa-fdc falls into assert instead of proper error\n> exit. This patch fixes this behavior.\n> \n> Signed-off-by: Eduardo Otubo \n> ---\n> hw/block/fdc.c | 6 +++++-\n> 1 file changed, 5 insertions(+), 1 deletion(-)\n> \n> diff --git a/hw/block/fdc.c b/hw/block/fdc.c\n> index 401129073b..0b6def4e1d 100644\n> --- a/hw/block/fdc.c\n> +++ b/hw/block/fdc.c\n> @@ -2699,11 +2699,15 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)\n> fdctrl->dma_chann = isa->dma;\n> if (fdctrl->dma_chann != -1) {\n> fdctrl->dma = isa_get_dma(isa_bus_from_device(isadev), isa->dma);\n> - assert(fdctrl->dma);\n> + if (!fdctrl->dma) {\n> + error_setg(errp, \"isa-fdc not supported\");\n> + goto error;\n> + }\n> }\n> \n> qdev_set_legacy_instance_id(dev, isa->iobase, 2);\n> fdctrl_realize_common(dev, fdctrl, &err);\n> +error:\n> if (err != NULL) {\n> error_propagate(errp, err);\n> return;\n> \n\nI was going to ask if we needn't take care of undoing the portio or IRQ\nregistration, but it turns out that there is not one single caller of\nportio_list_destroy in all of QEMU...\n\nPaolo admitted as much in e3fb0ade83420a86464ee50c71f2daf5641cab10 when\nhe updated the destroy function :)\n\nIt looks like the way we assign IRQs to ISA devices is kind of simple\nand not trivial to undo correctly, so instead of barking up that tree,\nwhy don't we hoist the isa_get_dma call up and fail before we try to\ninitialize portio or IRQs?","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=jsnow@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xkTpl2p0zz9sPt\n\tfor ;\n\tSat, 2 Sep 2017 05:30:07 +1000 (AEST)","from localhost ([::1]:56721 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnrdl-0000nB-HB\n\tfor incoming@patchwork.ozlabs.org; Fri, 01 Sep 2017 15:30:05 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:40811)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnrdM-0000mN-UH\n\tfor qemu-devel@nongnu.org; Fri, 01 Sep 2017 15:29:45 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnrdI-0007PB-9C\n\tfor qemu-devel@nongnu.org; Fri, 01 Sep 2017 15:29:40 -0400","from mx1.redhat.com ([209.132.183.28]:4506)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dnrdD-0007LR-L0; Fri, 01 Sep 2017 15:29:31 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id AC7DA7EA9D;\n\tFri, 1 Sep 2017 19:29:30 +0000 (UTC)","from [10.18.17.130] (dhcp-17-130.bos.redhat.com [10.18.17.130])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 5DDFF7BB6D;\n\tFri, 1 Sep 2017 19:29:28 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com AC7DA7EA9D","To":"Eduardo Otubo , qemu-devel@nongnu.org","References":"<20170901110706.429-1-otubo@redhat.com>","From":"John Snow ","Message-ID":"","Date":"Fri, 1 Sep 2017 15:29:27 -0400","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170901110706.429-1-otubo@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]);\n\tFri, 01 Sep 2017 19:29:30 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH] isa-fdc: assert replaced by proper error\n\texit","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"qemu-trivial@nongnu.org, qemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]