From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45518) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d10cx-0008OZ-PK for qemu-devel@nongnu.org; Wed, 19 Apr 2017 21:11:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d10cw-0000fk-RC for qemu-devel@nongnu.org; Wed, 19 Apr 2017 21:11:19 -0400 References: <20170419222258.13415-1-eblake@redhat.com> From: Eric Blake Message-ID: <3a0cce45-581b-2fe5-ffe7-ca76bc5b2448@redhat.com> Date: Wed, 19 Apr 2017 20:11:02 -0500 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="4NifvjXIOOpIXKlpJDpAR7OwMWL0mSU30" Subject: Re: [Qemu-devel] [PATCH v2] event: Add source information to SHUTDOWN List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alistair Francis Cc: "qemu-devel@nongnu.org Developers" , Peter Maydell , "open list:Overall" , "Michael S. Tsirkin" , Mark Cave-Ayland , Alexander Graf , Scott Wood , Gerd Hoffmann , "Edgar E. Iglesias" , Rob Herring , Stefano Stabellini , Magnus Damm , Markus Armbruster , Christian Borntraeger , Anthony Perard , "open list:X86" , Richard Henderson , Artyom Tarasenko , Eduardo Habkost , Stefan Weil , "open list:Calxeda Highbank" , Jan Kiszka , Igor Mammedov , Cornelia Huck , David Gibson , Paul Burton , Max Filippov , Marcelo Tosatti , Michael Walle , "open list:New World" , Yongbok Kim , Paolo Bonzini , Aurelien Jarno This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --4NifvjXIOOpIXKlpJDpAR7OwMWL0mSU30 From: Eric Blake To: Alistair Francis Cc: "qemu-devel@nongnu.org Developers" , Peter Maydell , "open list:Overall" , "Michael S. Tsirkin" , Mark Cave-Ayland , Alexander Graf , Scott Wood , Gerd Hoffmann , "Edgar E. Iglesias" , Rob Herring , Stefano Stabellini , Magnus Damm , Markus Armbruster , Christian Borntraeger , Anthony Perard , "open list:X86" , Richard Henderson , Artyom Tarasenko , Eduardo Habkost , Stefan Weil , "open list:Calxeda Highbank" , Jan Kiszka , Igor Mammedov , Cornelia Huck , David Gibson , Paul Burton , Max Filippov , Marcelo Tosatti , Michael Walle , "open list:New World" , Yongbok Kim , Paolo Bonzini , Aurelien Jarno Message-ID: <3a0cce45-581b-2fe5-ffe7-ca76bc5b2448@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2] event: Add source information to SHUTDOWN References: <20170419222258.13415-1-eblake@redhat.com> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 04/19/2017 05:36 PM, Alistair Francis wrote: > On Wed, Apr 19, 2017 at 3:22 PM, Eric Blake wrote: >> Libvirt would like to be able to distinguish between a SHUTDOWN >> event triggered solely by guest request and one triggered by a >> SIGTERM or other action on the host. qemu_kill_report() is >> already able to tell whether a shutdown was triggered by a host >> signal (but NOT by a host UI event, such as clicking the X on >> the window), but that information was then lost after being >> printed to stderr. >> >> Enhance the shutdown request path to take a parameter of which >> way it is being triggered, and update ALL callers. It would have >> been less churn to keep the common case with no arguments as >> meaning guest-triggered, and only modified the host-triggered >> code paths, via a wrapper function, but then we'd still have to >> audit that I didn't miss any host-triggered spots; changing the >> signature forces us to double-check that I correctly categorized >> all callers. > With all this churn is it not worth chaning the bool from_guest to an > int that we can expand in future? >=20 > I'm imagining an enum with multiple values for different shutdown > events. At the moment it will just be one for guest and one for host, > but at some point in the future we might want more. Yes, that makes sense. Probably easiest is creating a QMP enum now ( as in { 'enum': 'ShutdownCause', 'data': ['guest', 'host'] } - although such an enum defaults to starting at 0, and the shutdown_requested variable in vl.c would need a tweak to pick a different value when no shutdown is requested. > Not only does this future proof us, but I think it makes it more clear > what you are passing the function instead of just true/false. Under your proposal, it would be qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST), which does look a bit nicer. I'll wait for other review comments, but you have given me a good reason to do a v3. --=20 Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org --4NifvjXIOOpIXKlpJDpAR7OwMWL0mSU30 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJY+AqmAAoJEKeha0olJ0NqzRcH/jUkeHFlEhIk0/eYjZyTRfmO vdNzTL04R+v0uEhRPWqPgYqLdMBciuyEcUtIWGaJ1KSSss1eupV+LQNyxD81DnzT DIZweoW84xseJDFrQU5HwwC566FVchC3n815J1Iyy8AGpQKtXZ//hpRKsCkPeBhl XwRB6Q045+zMGqcuUZ+I4OYt9/va7QUfJCjCdaBK4jNerBAr4PxYNgwRtc5ix3p5 ia5rBGrmHmf23Wfb+bDcrZX3T2EYv72HaTkLcE1Lk7Re3/SsBhAwTWOkjUnQ10CO hcXLMYA6kGWeyl88IBTFolCpXHFcKKl4X6X7aPH6cykkKaXceAJ+5RK4QYhRt/E= =7PUk -----END PGP SIGNATURE----- --4NifvjXIOOpIXKlpJDpAR7OwMWL0mSU30--