[{"id":1763628,"web_url":"http://patchwork.ozlabs.org/comment/1763628/","msgid":"<a0659c96-31c6-d193-19bb-438fe6aa2601@redhat.com>","list_archive_url":null,"date":"2017-09-05T20:36:15","subject":"Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the\n\tfly","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/people/6591/","name":"Eric Blake","email":"eblake@redhat.com"},"content":"On 09/05/2017 08:22 AM, Michal Privoznik wrote:\n> Currently, the only time that users can set watchdog action is at\n> the start as all we expose is this -watchdog-action command line\n> argument. This is suboptimal when users want to plug the device\n> later via monitor. Alternatively, they might want to change the\n> action for already existing device on the fly.\n> \n> At the same time, drop local redefinition of the actions eum in\n\ns/eum/enum/\n\n> watchdog.h in favour of the ones defined in schema.\n> \n> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169\n> \n> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>\n> ---\n\n> @@ -77,27 +80,17 @@ int select_watchdog(const char *p)\n>  \n>  int select_watchdog_action(const char *p)\n>  {\n\n> +    int action;\n>  \n> +    action = qapi_enum_parse(WatchdogExpirationAction_lookup, p,\n> +                             WATCHDOG_EXPIRATION_ACTION__MAX, -1, NULL);\n\nSemantic conflict; you need to rebase now that the qapi enum lookup code\nhas landed (see commit ebf677c8 and parents)\n\n> +\n> +    case WATCHDOG_EXPIRATION_ACTION__MAX:\n> +        /* keep gcc happy */\n> +        break;\n\nCould use g_assert_not_reached() here instead.\n\n> +++ b/qapi-schema.json\n> @@ -6539,3 +6539,12 @@\n>  # Since 2.9\n>  ##\n>  { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }\n> +\n> +##\n> +# @watchdog-set-action:\n> +#\n> +# Set watchdog action\n> +#\n> +# Since 2.11\n> +##\n> +{ 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogExpirationAction'} }\n\nCan a machine have more than one watchdog device, in which case you'd\nwant a device name to select which watchdog gets which action?  But the\ncommand-line version that you are replacing seems to be global, so I\nguess you're okay with this interface.","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eblake@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 3xmz672MjSz9t2W\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed,  6 Sep 2017 06:37:00 +1000 (AEST)","from localhost ([::1]:33126 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dpKaf-0005RV-VU\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 16:36:57 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:41804)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <eblake@redhat.com>) id 1dpKa8-0005QX-7a\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 16:36:30 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <eblake@redhat.com>) id 1dpKa3-0000hK-DK\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 16:36:24 -0400","from mx1.redhat.com ([209.132.183.28]:48954)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <eblake@redhat.com>) id 1dpKa3-0000gU-3h\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 16:36:19 -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 8C1596E797\n\tfor <qemu-devel@nongnu.org>; Tue,  5 Sep 2017 20:36:17 +0000 (UTC)","from [10.10.120.228] (ovpn-120-228.rdu2.redhat.com [10.10.120.228])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id B4A4861345;\n\tTue,  5 Sep 2017 20:36:16 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 8C1596E797","To":"Michal Privoznik <mprivozn@redhat.com>, qemu-devel@nongnu.org","References":"<f9d56290959d378dfa65b3906bf0d822ab938ac9.1504617551.git.mprivozn@redhat.com>","From":"Eric Blake <eblake@redhat.com>","Openpgp":"url=http://people.redhat.com/eblake/eblake.gpg","Organization":"Red Hat, Inc.","Message-ID":"<a0659c96-31c6-d193-19bb-438fe6aa2601@redhat.com>","Date":"Tue, 5 Sep 2017 15:36:15 -0500","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":"<f9d56290959d378dfa65b3906bf0d822ab938ac9.1504617551.git.mprivozn@redhat.com>","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"4J347B0f9BMThNgmWl7nPoIQa5PKde8Ti\"","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.25]);\n\tTue, 05 Sep 2017 20:36:17 +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","X-Content-Filtered-By":"Mailman/MimeDel 2.1.21","Subject":"Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the\n\tfly","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<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>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"armbru@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1763827,"web_url":"http://patchwork.ozlabs.org/comment/1763827/","msgid":"<87k21cfjua.fsf@dusky.pond.sub.org>","list_archive_url":null,"date":"2017-09-06T05:59:41","subject":"Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the\n\tfly","submitter":{"id":2645,"url":"http://patchwork.ozlabs.org/api/people/2645/","name":"Markus Armbruster","email":"armbru@redhat.com"},"content":"Eric Blake <eblake@redhat.com> writes:\n\n> On 09/05/2017 08:22 AM, Michal Privoznik wrote:\n>> Currently, the only time that users can set watchdog action is at\n>> the start as all we expose is this -watchdog-action command line\n>> argument. This is suboptimal when users want to plug the device\n>> later via monitor. Alternatively, they might want to change the\n>> action for already existing device on the fly.\n>> \n>> At the same time, drop local redefinition of the actions eum in\n>\n> s/eum/enum/\n>\n>> watchdog.h in favour of the ones defined in schema.\n>> \n>> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169\n>> \n>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>\n>> ---\n>\n>> @@ -77,27 +80,17 @@ int select_watchdog(const char *p)\n>>  \n>>  int select_watchdog_action(const char *p)\n>>  {\n>\n>> +    int action;\n>>  \n>> +    action = qapi_enum_parse(WatchdogExpirationAction_lookup, p,\n>> +                             WATCHDOG_EXPIRATION_ACTION__MAX, -1, NULL);\n>\n> Semantic conflict; you need to rebase now that the qapi enum lookup code\n> has landed (see commit ebf677c8 and parents)\n>\n>> +\n>> +    case WATCHDOG_EXPIRATION_ACTION__MAX:\n>> +        /* keep gcc happy */\n>> +        break;\n>\n> Could use g_assert_not_reached() here instead.\n\nCatches one out of approximately 2^64 invalid values.  To catches all,\nand in fewer words:\n\n        default:\n            assert(0);\n\n>> +++ b/qapi-schema.json\n>> @@ -6539,3 +6539,12 @@\n>>  # Since 2.9\n>>  ##\n>>  { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }\n>> +\n>> +##\n>> +# @watchdog-set-action:\n>> +#\n>> +# Set watchdog action\n>> +#\n>> +# Since 2.11\n>> +##\n>> +{ 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogExpirationAction'} }\n>\n> Can a machine have more than one watchdog device, in which case you'd\n> want a device name to select which watchdog gets which action?  But the\n> command-line version that you are replacing seems to be global, so I\n> guess you're okay with this interface.\n\nFor better or worse, all watchdogs share the action, see\nhw/watchdog/watchdog.c.\n\nWatchdogs predate qdev.  Commit 9dd986c lets you configure up to one,\nwith two command line options: --watchdog and --watchdog-action.  In a\nway, --watchdog configures the frontend, and --watchdog-action the\nbackend (having watchdog.c in hw/watchdog/ contradicts this view,\nthough).\n\nSince the frontends got qdevified (commit 09aaa16), you can configure\nany number of watchdog frontends, but still only one backend shared by\nall.  Unsharing the backend wasn't worth the trouble, because having\nmultiple watchdogs would be weird, and multiple watchdogs with different\nactions would be weirder.  Trouble would include maintaining command\nline backward compatibility, as always.\n\nOf course, if we *want* to unshare the backend, we should do it *before*\nwe add QMP backward compatibility to the trouble.","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","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=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 3xnCbt32FQz9sBd\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed,  6 Sep 2017 16:00:10 +1000 (AEST)","from localhost ([::1]:34237 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dpTNg-0000hf-Jx\n\tfor incoming@patchwork.ozlabs.org; Wed, 06 Sep 2017 02:00:08 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:55924)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <armbru@redhat.com>) id 1dpTNN-0000hR-JA\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 01:59:50 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <armbru@redhat.com>) id 1dpTNI-0004e7-Kq\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 01:59:49 -0400","from mx1.redhat.com ([209.132.183.28]:56060)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <armbru@redhat.com>) id 1dpTNI-0004dl-Bl\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 01:59:44 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\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 57B198553C\n\tfor <qemu-devel@nongnu.org>; Wed,  6 Sep 2017 05:59:43 +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 23D11709EE;\n\tWed,  6 Sep 2017 05:59:43 +0000 (UTC)","by blackfin.pond.sub.org (Postfix, from userid 1000)\n\tid 9FDD51138645; Wed,  6 Sep 2017 07:59:41 +0200 (CEST)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 57B198553C","From":"Markus Armbruster <armbru@redhat.com>","To":"Eric Blake <eblake@redhat.com>","References":"<f9d56290959d378dfa65b3906bf0d822ab938ac9.1504617551.git.mprivozn@redhat.com>\n\t<a0659c96-31c6-d193-19bb-438fe6aa2601@redhat.com>","Date":"Wed, 06 Sep 2017 07:59:41 +0200","In-Reply-To":"<a0659c96-31c6-d193-19bb-438fe6aa2601@redhat.com> (Eric Blake's\n\tmessage of \"Tue, 5 Sep 2017 15:36:15 -0500\")","Message-ID":"<87k21cfjua.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.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]);\n\tWed, 06 Sep 2017 05:59: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 v2] watchdog: Allow setting action on the\n\tfly","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<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>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"Michal Privoznik <mprivozn@redhat.com>, qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1763828,"web_url":"http://patchwork.ozlabs.org/comment/1763828/","msgid":"<8173dc23-3027-9ca2-9567-c70534728d75@redhat.com>","list_archive_url":null,"date":"2017-09-06T06:02:31","subject":"Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the\n\tfly","submitter":{"id":11888,"url":"http://patchwork.ozlabs.org/api/people/11888/","name":"Michal Prívozník","email":"mprivozn@redhat.com"},"content":"On 09/05/2017 10:36 PM, Eric Blake wrote:\n> On 09/05/2017 08:22 AM, Michal Privoznik wrote:\n>> Currently, the only time that users can set watchdog action is at\n>> the start as all we expose is this -watchdog-action command line\n>> argument. This is suboptimal when users want to plug the device\n>> later via monitor. Alternatively, they might want to change the\n>> action for already existing device on the fly.\n>>\n>> At the same time, drop local redefinition of the actions eum in\n> \n> s/eum/enum/\n> \n>> watchdog.h in favour of the ones defined in schema.\n>>\n>> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169\n>>\n>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>\n>> ---\n> \n>> @@ -77,27 +80,17 @@ int select_watchdog(const char *p)\n>>  \n>>  int select_watchdog_action(const char *p)\n>>  {\n> \n>> +    int action;\n>>  \n>> +    action = qapi_enum_parse(WatchdogExpirationAction_lookup, p,\n>> +                             WATCHDOG_EXPIRATION_ACTION__MAX, -1, NULL);\n> \n> Semantic conflict; you need to rebase now that the qapi enum lookup code\n> has landed (see commit ebf677c8 and parents)\n\n\nD'oh. Okay.\n\n> \n>> +\n>> +    case WATCHDOG_EXPIRATION_ACTION__MAX:\n>> +        /* keep gcc happy */\n>> +        break;\n> \n> Could use g_assert_not_reached() here instead.\n> \n>> +++ b/qapi-schema.json\n>> @@ -6539,3 +6539,12 @@\n>>  # Since 2.9\n>>  ##\n>>  { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }\n>> +\n>> +##\n>> +# @watchdog-set-action:\n>> +#\n>> +# Set watchdog action\n>> +#\n>> +# Since 2.11\n>> +##\n>> +{ 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogExpirationAction'} }\n> \n> Can a machine have more than one watchdog device, in which case you'd\n> want a device name to select which watchdog gets which action?  But the\n> command-line version that you are replacing seems to be global, so I\n> guess you're okay with this interface.\n> \n\nYeah, I don't think that a guest can have more than one watchdog:\n\n/qemu.git $ ./x86_64-softmmu/qemu-system-x86_64 \\\n-watchdog ib700 -watchdog i6300esb\nqemu-system-x86_64: -watchdog i6300esb: only one watchdog option may be\ngiven\n\nAlso, would it make sense? I mean, what would be the benefit of having\ntwo or more watchdogs?\n\nMichal","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","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=mprivozn@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 3xnCg92zkFz9sBd\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed,  6 Sep 2017 16:03:00 +1000 (AEST)","from localhost ([::1]:34245 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dpTQP-0001ZJ-WF\n\tfor incoming@patchwork.ozlabs.org; Wed, 06 Sep 2017 02:02:58 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:56510)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <mprivozn@redhat.com>) id 1dpTQ7-0001ZA-Bd\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 02:02:40 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <mprivozn@redhat.com>) id 1dpTQ2-00067Z-CJ\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 02:02:39 -0400","from mx1.redhat.com ([209.132.183.28]:58726)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <mprivozn@redhat.com>) id 1dpTQ2-00066a-6E\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 02:02:34 -0400","from smtp.corp.redhat.com\n\t(int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])\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 307227E426\n\tfor <qemu-devel@nongnu.org>; Wed,  6 Sep 2017 06:02:33 +0000 (UTC)","from [10.43.2.192] (unknown [10.43.2.192])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 5B07A7B9F1;\n\tWed,  6 Sep 2017 06:02:32 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 307227E426","To":"Eric Blake <eblake@redhat.com>, qemu-devel@nongnu.org","References":"<f9d56290959d378dfa65b3906bf0d822ab938ac9.1504617551.git.mprivozn@redhat.com>\n\t<a0659c96-31c6-d193-19bb-438fe6aa2601@redhat.com>","From":"Michal Privoznik <mprivozn@redhat.com>","Message-ID":"<8173dc23-3027-9ca2-9567-c70534728d75@redhat.com>","Date":"Wed, 6 Sep 2017 08:02:31 +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":"<a0659c96-31c6-d193-19bb-438fe6aa2601@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.14","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.27]);\n\tWed, 06 Sep 2017 06:02:33 +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 v2] watchdog: Allow setting action on the\n\tfly","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<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>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"armbru@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1763829,"web_url":"http://patchwork.ozlabs.org/comment/1763829/","msgid":"<87bmmofjci.fsf@dusky.pond.sub.org>","list_archive_url":null,"date":"2017-09-06T06:10:21","subject":"Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the\n\tfly","submitter":{"id":2645,"url":"http://patchwork.ozlabs.org/api/people/2645/","name":"Markus Armbruster","email":"armbru@redhat.com"},"content":"Michal Privoznik <mprivozn@redhat.com> writes:\n\n> Currently, the only time that users can set watchdog action is at\n> the start as all we expose is this -watchdog-action command line\n> argument. This is suboptimal when users want to plug the device\n> later via monitor. Alternatively, they might want to change the\n> action for already existing device on the fly.\n>\n> At the same time, drop local redefinition of the actions eum in\n> watchdog.h in favour of the ones defined in schema.\n>\n> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169\n>\n> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>\n> ---\n>\n> diff to v1:\n> - instead of allowing 'action' to be arbitrary string, use enum for it and thus\n>   drop local redefinition of the enum.\n>\n>  hw/watchdog/watchdog.c    | 52 ++++++++++++++++++++++++-----------------------\n>  hw/watchdog/wdt_diag288.c |  6 +++---\n>  include/sysemu/watchdog.h | 12 ++---------\n>  qapi-schema.json          |  9 ++++++++\n>  4 files changed, 41 insertions(+), 38 deletions(-)\n>\n> diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c\n> index 0c5c9cde1c..a3e9d7b59b 100644\n> --- a/hw/watchdog/watchdog.c\n> +++ b/hw/watchdog/watchdog.c\n> @@ -29,8 +29,11 @@\n>  #include \"qapi-event.h\"\n>  #include \"hw/nmi.h\"\n>  #include \"qemu/help_option.h\"\n> +#include \"qmp-commands.h\"\n> +#include \"qapi/qmp/qerror.h\"\n> +#include \"qapi/util.h\"\n>  \n> -static int watchdog_action = WDT_RESET;\n> +static WatchdogExpirationAction watchdog_action = WATCHDOG_EXPIRATION_ACTION_RESET;\n>  static QLIST_HEAD(watchdog_list, WatchdogTimerModel) watchdog_list;\n>  \n>  void watchdog_add_model(WatchdogTimerModel *model)\n> @@ -77,27 +80,17 @@ int select_watchdog(const char *p)\n>  \n>  int select_watchdog_action(const char *p)\n>  {\n> -    if (strcasecmp(p, \"reset\") == 0)\n> -        watchdog_action = WDT_RESET;\n> -    else if (strcasecmp(p, \"shutdown\") == 0)\n> -        watchdog_action = WDT_SHUTDOWN;\n> -    else if (strcasecmp(p, \"poweroff\") == 0)\n> -        watchdog_action = WDT_POWEROFF;\n> -    else if (strcasecmp(p, \"pause\") == 0)\n> -        watchdog_action = WDT_PAUSE;\n> -    else if (strcasecmp(p, \"debug\") == 0)\n> -        watchdog_action = WDT_DEBUG;\n> -    else if (strcasecmp(p, \"none\") == 0)\n> -        watchdog_action = WDT_NONE;\n> -    else if (strcasecmp(p, \"inject-nmi\") == 0)\n> -        watchdog_action = WDT_NMI;\n> -    else\n> -        return -1;\n> +    int action;\n>  \n> +    action = qapi_enum_parse(WatchdogExpirationAction_lookup, p,\n> +                             WATCHDOG_EXPIRATION_ACTION__MAX, -1, NULL);\n> +    if (action < 0)\n> +        return -1;\n> +    qmp_watchdog_set_action(action, NULL);\n\n&error_abort\n\n>      return 0;\n>  }\n>  \n> -int get_watchdog_action(void)\n> +WatchdogExpirationAction get_watchdog_action(void)\n>  {\n>      return watchdog_action;\n>  }\n> @@ -108,21 +101,21 @@ int get_watchdog_action(void)\n>  void watchdog_perform_action(void)\n>  {\n>      switch (watchdog_action) {\n> -    case WDT_RESET:             /* same as 'system_reset' in monitor */\n> +    case WATCHDOG_EXPIRATION_ACTION_RESET:      /* same as 'system_reset' in monitor */\n>          qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_RESET, &error_abort);\n>          qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);\n>          break;\n>  \n> -    case WDT_SHUTDOWN:          /* same as 'system_powerdown' in monitor */\n> +    case WATCHDOG_EXPIRATION_ACTION_SHUTDOWN:   /* same as 'system_powerdown' in monitor */\n>          qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_SHUTDOWN, &error_abort);\n>          qemu_system_powerdown_request();\n>          break;\n>  \n> -    case WDT_POWEROFF:          /* same as 'quit' command in monitor */\n> +    case WATCHDOG_EXPIRATION_ACTION_POWEROFF:   /* same as 'quit' command in monitor */\n>          qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_POWEROFF, &error_abort);\n>          exit(0);\n>  \n> -    case WDT_PAUSE:             /* same as 'stop' command in monitor */\n> +    case WATCHDOG_EXPIRATION_ACTION_PAUSE:      /* same as 'stop' command in monitor */\n>          /* In a timer callback, when vm_stop calls qemu_clock_enable\n>           * you would get a deadlock.  Bypass the problem.\n>           */\n> @@ -131,19 +124,28 @@ void watchdog_perform_action(void)\n>          qemu_system_vmstop_request(RUN_STATE_WATCHDOG);\n>          break;\n>  \n> -    case WDT_DEBUG:\n> +    case WATCHDOG_EXPIRATION_ACTION_DEBUG:\n>          qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_DEBUG, &error_abort);\n>          fprintf(stderr, \"watchdog: timer fired\\n\");\n>          break;\n>  \n> -    case WDT_NONE:\n> +    case WATCHDOG_EXPIRATION_ACTION_NONE:\n>          qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_NONE, &error_abort);\n>          break;\n>  \n> -    case WDT_NMI:\n> +    case WATCHDOG_EXPIRATION_ACTION_INJECT_NMI:\n>          qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_INJECT_NMI,\n>                                   &error_abort);\n>          nmi_monitor_handle(0, NULL);\n>          break;\n> +\n> +    case WATCHDOG_EXPIRATION_ACTION__MAX:\n> +        /* keep gcc happy */\n> +        break;\n\n       default:\n           assert(0);\n\n>      }\n>  }\n> +\n> +void qmp_watchdog_set_action(WatchdogExpirationAction action, Error **errp)\n> +{\n> +    watchdog_action = action;\n> +}\n> diff --git a/hw/watchdog/wdt_diag288.c b/hw/watchdog/wdt_diag288.c\n> index 47f289216a..451644eb89 100644\n> --- a/hw/watchdog/wdt_diag288.c\n> +++ b/hw/watchdog/wdt_diag288.c\n> @@ -57,9 +57,9 @@ static void diag288_timer_expired(void *dev)\n>       * the BQL; reset before triggering the action to avoid races with\n>       * diag288 instructions. */\n>      switch (get_watchdog_action()) {\n> -    case WDT_DEBUG:\n> -    case WDT_NONE:\n> -    case WDT_PAUSE:\n> +    case WATCHDOG_EXPIRATION_ACTION_DEBUG:\n> +    case WATCHDOG_EXPIRATION_ACTION_NONE:\n> +    case WATCHDOG_EXPIRATION_ACTION_PAUSE:\n>          break;\n>      default:\n>          wdt_diag288_reset(dev);\n> diff --git a/include/sysemu/watchdog.h b/include/sysemu/watchdog.h\n> index 72a4da07a6..1b830df152 100644\n> --- a/include/sysemu/watchdog.h\n> +++ b/include/sysemu/watchdog.h\n> @@ -23,15 +23,7 @@\n>  #define QEMU_WATCHDOG_H\n>  \n>  #include \"qemu/queue.h\"\n> -\n> -/* Possible values for action parameter. */\n> -#define WDT_RESET        1      /* Hard reset. */\n> -#define WDT_SHUTDOWN     2      /* Shutdown. */\n> -#define WDT_POWEROFF     3      /* Quit. */\n> -#define WDT_PAUSE        4      /* Pause. */\n> -#define WDT_DEBUG        5      /* Prints a message and continues running. */\n> -#define WDT_NONE         6      /* Do nothing. */\n> -#define WDT_NMI          7      /* Inject nmi into the guest. */\n> +#include \"qapi-types.h\"\n>  \n>  struct WatchdogTimerModel {\n>      QLIST_ENTRY(WatchdogTimerModel) entry;\n> @@ -46,7 +38,7 @@ typedef struct WatchdogTimerModel WatchdogTimerModel;\n>  /* in hw/watchdog.c */\n>  int select_watchdog(const char *p);\n>  int select_watchdog_action(const char *action);\n> -int get_watchdog_action(void);\n> +WatchdogExpirationAction get_watchdog_action(void);\n>  void watchdog_add_model(WatchdogTimerModel *model);\n>  void watchdog_perform_action(void);\n>  \n> diff --git a/qapi-schema.json b/qapi-schema.json\n> index 802ea53d00..40605be36f 100644\n> --- a/qapi-schema.json\n> +++ b/qapi-schema.json\n> @@ -6539,3 +6539,12 @@\n>  # Since 2.9\n>  ##\n>  { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }\n> +\n> +##\n> +# @watchdog-set-action:\n> +#\n> +# Set watchdog action\n> +#\n> +# Since 2.11\n> +##\n> +{ 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogExpirationAction'} }\n\nI was wondering whether you forgot to include the enum definition, until\nI discovered you smartly reused the existing one.\n\nIf it was a new one instead, I would suggest naming it WatchdogAction,\nbecause a watchdog's one and only action is the expiration action, and\ntherefore the shorter name serves just fine.  Hmm, it's used in just two\nplaces...  okay, I'm suggesting you rename it, in a separate preparatory\npatch.\n\nThis cleans up code duplication.  Worth mentioning in the commit\nmessage.\n\nLooks pretty good overall; a quick respin should get us there.","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","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 3xnCrH2lZ2z9sNV\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed,  6 Sep 2017 16:10:53 +1000 (AEST)","from localhost ([::1]:34333 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dpTY2-0002vW-Lq\n\tfor incoming@patchwork.ozlabs.org; Wed, 06 Sep 2017 02:10:50 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:59868)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <armbru@redhat.com>) id 1dpTXg-0002vH-LU\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 02:10:30 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <armbru@redhat.com>) id 1dpTXc-0001TT-IC\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 02:10:28 -0400","from mx1.redhat.com ([209.132.183.28]:42842)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <armbru@redhat.com>) id 1dpTXc-0001Sx-9S\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 02:10:24 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\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 277957E428\n\tfor <qemu-devel@nongnu.org>; Wed,  6 Sep 2017 06:10:23 +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 BD4B8709EF;\n\tWed,  6 Sep 2017 06:10:22 +0000 (UTC)","by blackfin.pond.sub.org (Postfix, from userid 1000)\n\tid 2E4081138645; Wed,  6 Sep 2017 08:10:21 +0200 (CEST)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 277957E428","From":"Markus Armbruster <armbru@redhat.com>","To":"Michal Privoznik <mprivozn@redhat.com>","References":"<f9d56290959d378dfa65b3906bf0d822ab938ac9.1504617551.git.mprivozn@redhat.com>","Date":"Wed, 06 Sep 2017 08:10:21 +0200","In-Reply-To":"<f9d56290959d378dfa65b3906bf0d822ab938ac9.1504617551.git.mprivozn@redhat.com>\n\t(Michal Privoznik's message of \"Tue, 5 Sep 2017 15:22:04 +0200\")","Message-ID":"<87bmmofjci.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.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.27]);\n\tWed, 06 Sep 2017 06:10:23 +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 v2] watchdog: Allow setting action on the\n\tfly","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<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>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1763839,"web_url":"http://patchwork.ozlabs.org/comment/1763839/","msgid":"<49513875-a104-5150-7687-42a5203356fa@redhat.com>","list_archive_url":null,"date":"2017-09-06T06:53:08","subject":"Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the\n\tfly","submitter":{"id":11888,"url":"http://patchwork.ozlabs.org/api/people/11888/","name":"Michal Prívozník","email":"mprivozn@redhat.com"},"content":"On 09/06/2017 07:59 AM, Markus Armbruster wrote:\n> Eric Blake <eblake@redhat.com> writes:\n> \n>> On 09/05/2017 08:22 AM, Michal Privoznik wrote:\n>>> Currently, the only time that users can set watchdog action is at\n>>> the start as all we expose is this -watchdog-action command line\n>>> argument. This is suboptimal when users want to plug the device\n>>> later via monitor. Alternatively, they might want to change the\n>>> action for already existing device on the fly.\n>>>\n>>> At the same time, drop local redefinition of the actions eum in\n>>\n>> s/eum/enum/\n>>\n>>> watchdog.h in favour of the ones defined in schema.\n>>>\n>>> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169\n>>>\n>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>\n>>> ---\n>>\n>>> @@ -77,27 +80,17 @@ int select_watchdog(const char *p)\n>>>  \n>>>  int select_watchdog_action(const char *p)\n>>>  {\n>>\n>>> +    int action;\n>>>  \n>>> +    action = qapi_enum_parse(WatchdogExpirationAction_lookup, p,\n>>> +                             WATCHDOG_EXPIRATION_ACTION__MAX, -1, NULL);\n>>\n>> Semantic conflict; you need to rebase now that the qapi enum lookup code\n>> has landed (see commit ebf677c8 and parents)\n>>\n>>> +\n>>> +    case WATCHDOG_EXPIRATION_ACTION__MAX:\n>>> +        /* keep gcc happy */\n>>> +        break;\n>>\n>> Could use g_assert_not_reached() here instead.\n> \n> Catches one out of approximately 2^64 invalid values.  To catches all,\n> and in fewer words:\n> \n>         default:\n>             assert(0);\n\nI'm not a fan of this, but I'm not a qemu developer so I don't know what\nyour coding style is (if any for this case). However, since this switch\nis over an enum, compiler will scream if a new value is introduced to\nthe enum and not handled in the switch. IMO it's useful because when I'm\nadding new value I can immediately see what other places I need to consider.\n\nAlso, yeah, I am going to rename the enum in v3.\n\nMichal","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=mprivozn@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 3xnDnc50ssz9sNd\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed,  6 Sep 2017 16:53:38 +1000 (AEST)","from localhost ([::1]:34411 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dpUDP-0001ej-Q8\n\tfor incoming@patchwork.ozlabs.org; Wed, 06 Sep 2017 02:53:35 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:42344)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <mprivozn@redhat.com>) id 1dpUD5-0001eN-5G\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 02:53:16 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <mprivozn@redhat.com>) id 1dpUD1-0002G7-QB\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 02:53:15 -0400","from mx1.redhat.com ([209.132.183.28]:54714)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <mprivozn@redhat.com>) id 1dpUD1-0002FU-KD\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 02:53:11 -0400","from smtp.corp.redhat.com\n\t(int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])\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 55972356C2\n\tfor <qemu-devel@nongnu.org>; Wed,  6 Sep 2017 06:53:10 +0000 (UTC)","from [10.43.2.192] (unknown [10.43.2.192])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id A67DA7B9DC;\n\tWed,  6 Sep 2017 06:53:09 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 55972356C2","To":"Markus Armbruster <armbru@redhat.com>, Eric Blake <eblake@redhat.com>","References":"<f9d56290959d378dfa65b3906bf0d822ab938ac9.1504617551.git.mprivozn@redhat.com>\n\t<a0659c96-31c6-d193-19bb-438fe6aa2601@redhat.com>\n\t<87k21cfjua.fsf@dusky.pond.sub.org>","From":"Michal Privoznik <mprivozn@redhat.com>","Message-ID":"<49513875-a104-5150-7687-42a5203356fa@redhat.com>","Date":"Wed, 6 Sep 2017 08:53:08 +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":"<87k21cfjua.fsf@dusky.pond.sub.org>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.14","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tWed, 06 Sep 2017 06:53:10 +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 v2] watchdog: Allow setting action on the\n\tfly","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<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>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1763857,"web_url":"http://patchwork.ozlabs.org/comment/1763857/","msgid":"<8760cwcmqu.fsf@dusky.pond.sub.org>","list_archive_url":null,"date":"2017-09-06T07:25:13","subject":"Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the\n\tfly","submitter":{"id":2645,"url":"http://patchwork.ozlabs.org/api/people/2645/","name":"Markus Armbruster","email":"armbru@redhat.com"},"content":"Michal Privoznik <mprivozn@redhat.com> writes:\n\n> On 09/06/2017 07:59 AM, Markus Armbruster wrote:\n>> Eric Blake <eblake@redhat.com> writes:\n>> \n>>> On 09/05/2017 08:22 AM, Michal Privoznik wrote:\n>>>> Currently, the only time that users can set watchdog action is at\n>>>> the start as all we expose is this -watchdog-action command line\n>>>> argument. This is suboptimal when users want to plug the device\n>>>> later via monitor. Alternatively, they might want to change the\n>>>> action for already existing device on the fly.\n>>>>\n>>>> At the same time, drop local redefinition of the actions eum in\n>>>\n>>> s/eum/enum/\n>>>\n>>>> watchdog.h in favour of the ones defined in schema.\n>>>>\n>>>> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169\n>>>>\n>>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>\n>>>> ---\n>>>\n>>>> @@ -77,27 +80,17 @@ int select_watchdog(const char *p)\n>>>>  \n>>>>  int select_watchdog_action(const char *p)\n>>>>  {\n>>>\n>>>> +    int action;\n>>>>  \n>>>> +    action = qapi_enum_parse(WatchdogExpirationAction_lookup, p,\n>>>> +                             WATCHDOG_EXPIRATION_ACTION__MAX, -1, NULL);\n>>>\n>>> Semantic conflict; you need to rebase now that the qapi enum lookup code\n>>> has landed (see commit ebf677c8 and parents)\n>>>\n>>>> +\n>>>> +    case WATCHDOG_EXPIRATION_ACTION__MAX:\n>>>> +        /* keep gcc happy */\n>>>> +        break;\n>>>\n>>> Could use g_assert_not_reached() here instead.\n>> \n>> Catches one out of approximately 2^64 invalid values.  To catches all,\n>> and in fewer words:\n>> \n>>         default:\n>>             assert(0);\n>\n> I'm not a fan of this, but I'm not a qemu developer so I don't know what\n> your coding style is (if any for this case). However, since this switch\n> is over an enum, compiler will scream if a new value is introduced to\n> the enum and not handled in the switch. IMO it's useful because when I'm\n> adding new value I can immediately see what other places I need to consider.\n\nTwo separate purposes: catch invalid state, catch forgotten code update.\nI can't see how to serve both.  Your patch picks the latter.\n\n> Also, yeah, I am going to rename the enum in v3.\n\nThanks!","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx02.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 3xnFVc5G7Cz9sBZ\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed,  6 Sep 2017 17:25:43 +1000 (AEST)","from localhost ([::1]:34569 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dpUiS-0000S7-7t\n\tfor incoming@patchwork.ozlabs.org; Wed, 06 Sep 2017 03:25:40 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:52745)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <armbru@redhat.com>) id 1dpUi9-0000Ru-4d\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 03:25:22 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <armbru@redhat.com>) id 1dpUi4-00029f-5J\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 03:25:21 -0400","from mx1.redhat.com ([209.132.183.28]:54727)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <armbru@redhat.com>) id 1dpUi3-00029R-SW\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 03:25:16 -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 E81C4883A0\n\tfor <qemu-devel@nongnu.org>; Wed,  6 Sep 2017 07:25:14 +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 B6A525C88F;\n\tWed,  6 Sep 2017 07:25:14 +0000 (UTC)","by blackfin.pond.sub.org (Postfix, from userid 1000)\n\tid 1ED471138645; Wed,  6 Sep 2017 09:25:13 +0200 (CEST)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com E81C4883A0","From":"Markus Armbruster <armbru@redhat.com>","To":"Michal Privoznik <mprivozn@redhat.com>","References":"<f9d56290959d378dfa65b3906bf0d822ab938ac9.1504617551.git.mprivozn@redhat.com>\n\t<a0659c96-31c6-d193-19bb-438fe6aa2601@redhat.com>\n\t<87k21cfjua.fsf@dusky.pond.sub.org>\n\t<49513875-a104-5150-7687-42a5203356fa@redhat.com>","Date":"Wed, 06 Sep 2017 09:25:13 +0200","In-Reply-To":"<49513875-a104-5150-7687-42a5203356fa@redhat.com> (Michal\n\tPrivoznik's message of \"Wed, 6 Sep 2017 08:53:08 +0200\")","Message-ID":"<8760cwcmqu.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.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.26]);\n\tWed, 06 Sep 2017 07:25:15 +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 v2] watchdog: Allow setting action on the\n\tfly","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<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>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1763861,"web_url":"http://patchwork.ozlabs.org/comment/1763861/","msgid":"<87zia8b7s5.fsf@dusky.pond.sub.org>","list_archive_url":null,"date":"2017-09-06T07:33:46","subject":"Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the\n\tfly","submitter":{"id":2645,"url":"http://patchwork.ozlabs.org/api/people/2645/","name":"Markus Armbruster","email":"armbru@redhat.com"},"content":"Michal Privoznik <mprivozn@redhat.com> writes:\n\n> On 09/05/2017 10:36 PM, Eric Blake wrote:\n>> On 09/05/2017 08:22 AM, Michal Privoznik wrote:\n>>> Currently, the only time that users can set watchdog action is at\n>>> the start as all we expose is this -watchdog-action command line\n>>> argument. This is suboptimal when users want to plug the device\n>>> later via monitor. Alternatively, they might want to change the\n>>> action for already existing device on the fly.\n>>>\n>>> At the same time, drop local redefinition of the actions eum in\n>> \n>> s/eum/enum/\n>> \n>>> watchdog.h in favour of the ones defined in schema.\n>>>\n>>> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169\n>>>\n>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>\n[...]\n>>> +++ b/qapi-schema.json\n>>> @@ -6539,3 +6539,12 @@\n>>>  # Since 2.9\n>>>  ##\n>>>  { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }\n>>> +\n>>> +##\n>>> +# @watchdog-set-action:\n>>> +#\n>>> +# Set watchdog action\n>>> +#\n>>> +# Since 2.11\n>>> +##\n>>> +{ 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogExpirationAction'} }\n>> \n>> Can a machine have more than one watchdog device, in which case you'd\n>> want a device name to select which watchdog gets which action?  But the\n>> command-line version that you are replacing seems to be global, so I\n>> guess you're okay with this interface.\n>> \n>\n> Yeah, I don't think that a guest can have more than one watchdog:\n>\n> /qemu.git $ ./x86_64-softmmu/qemu-system-x86_64 \\\n> -watchdog ib700 -watchdog i6300esb\n> qemu-system-x86_64: -watchdog i6300esb: only one watchdog option may be\n> given\n\n-watchdog is a thin wrapper around -device, and all it adds is this\nerror.  It's quite redundant.  I recommend using -device instead.\n\n\"-device i6300esb -device i6300esb\" works.  It's just a PCI device after\nall.\n\n> Also, would it make sense? I mean, what would be the benefit of having\n> two or more watchdogs?\n\nNone.  All it would accomplish is complicating things.","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","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=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 3xnFhh5wVHz9sBZ\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed,  6 Sep 2017 17:34:28 +1000 (AEST)","from localhost ([::1]:34595 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dpUqw-00023F-Uf\n\tfor incoming@patchwork.ozlabs.org; Wed, 06 Sep 2017 03:34:26 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:55554)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <armbru@redhat.com>) id 1dpUqP-00021b-JV\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 03:33:54 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <armbru@redhat.com>) id 1dpUqK-0007NI-VE\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 03:33:52 -0400","from mx1.redhat.com ([209.132.183.28]:49740)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <armbru@redhat.com>) id 1dpUqK-0007Mb-Pm\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 03:33:48 -0400","from smtp.corp.redhat.com\n\t(int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])\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 DABAB85543\n\tfor <qemu-devel@nongnu.org>; Wed,  6 Sep 2017 07:33:47 +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 9948F7BAF8;\n\tWed,  6 Sep 2017 07:33:47 +0000 (UTC)","by blackfin.pond.sub.org (Postfix, from userid 1000)\n\tid 23E621138645; Wed,  6 Sep 2017 09:33:46 +0200 (CEST)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com DABAB85543","From":"Markus Armbruster <armbru@redhat.com>","To":"Michal Privoznik <mprivozn@redhat.com>","References":"<f9d56290959d378dfa65b3906bf0d822ab938ac9.1504617551.git.mprivozn@redhat.com>\n\t<a0659c96-31c6-d193-19bb-438fe6aa2601@redhat.com>\n\t<8173dc23-3027-9ca2-9567-c70534728d75@redhat.com>","Date":"Wed, 06 Sep 2017 09:33:46 +0200","In-Reply-To":"<8173dc23-3027-9ca2-9567-c70534728d75@redhat.com> (Michal\n\tPrivoznik's message of \"Wed, 6 Sep 2017 08:02:31 +0200\")","Message-ID":"<87zia8b7s5.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.14","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]);\n\tWed, 06 Sep 2017 07:33:48 +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 v2] watchdog: Allow setting action on the\n\tfly","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<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>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}}]