From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35328) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a48fT-0001Rw-5b for qemu-devel@nongnu.org; Wed, 02 Dec 2015 09:46:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a48fO-0006xN-Sy for qemu-devel@nongnu.org; Wed, 02 Dec 2015 09:46:03 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33493) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a48fO-0006xF-L9 for qemu-devel@nongnu.org; Wed, 02 Dec 2015 09:45:58 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 20F92341AF8 for ; Wed, 2 Dec 2015 14:45:58 +0000 (UTC) References: <1448976530-15984-1-git-send-email-peterx@redhat.com> <1448976530-15984-9-git-send-email-peterx@redhat.com> <20151202011131.GE9399@ad.usersys.redhat.com> From: Eric Blake Message-ID: <565F0420.10409@redhat.com> Date: Wed, 2 Dec 2015 07:45:52 -0700 MIME-Version: 1.0 In-Reply-To: <20151202011131.GE9399@ad.usersys.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="h77BhPx5fQrIdrc7N7wVeBiItrMpgAoNB" Subject: Re: [Qemu-devel] [PATCH v4 08/11] dump-guest-memory: add qmp event DUMP_COMPLETED List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , Peter Xu Cc: drjones@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, pbonzini@redhat.com, lcapitulino@redhat.com, lersek@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --h77BhPx5fQrIdrc7N7wVeBiItrMpgAoNB Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 12/01/2015 06:11 PM, Fam Zheng wrote: > On Tue, 12/01 21:28, Peter Xu wrote: >> One new QMP event DUMP_COMPLETED is added. When a dump finishes, one >> DUMP_COMPLETED event will occur to notify the user. >> >> Signed-off-by: Peter Xu >> --- >> +++ b/qapi/event.json >> @@ -356,3 +356,16 @@ >> ## >> { 'event': 'MEM_UNPLUG_ERROR', >> 'data': { 'device': 'str', 'msg': 'str' } } >> + >> +## >> +# @DUMP_COMPLETED >> +# >> +# Emitted when background dump has completed >> +# >> +# @error: #optional human-readable error string that provides >> +# hint on why dump failed. >=20 > Please explicitly mention that successful dump emits DUMP_COMPLETED wit= hout > error, and failed dump emits DUMP_COMPLETED that has an error str. In fact, I wonder if it would also be worth having a 'status':'DumpStatus' field, which records the final status of the dump (either 'completed' or 'failed'), and which is always present. >> +++ b/util/error.c >> @@ -197,7 +197,11 @@ ErrorClass error_get_class(const Error *err) >> =20 >> const char *error_get_pretty(Error *err) >> { >> - return err->msg; >> + if (err) { >> + return err->msg; >> + } else { >> + return NULL; >> + } >=20 > This change belongs to a separate patch, if any. Indeed. When I was musing about the idea, I was not expecting you to actually implement it, so much as questioning whether it is a worthwhile idea. But as it impacts more than just your series, it definitely needs to be a separate patch, if at all. > But personally I don't like > it, because it doesn't work very well when error_get_pretty is used in > printf-like function parameters: >=20 > Error *err =3D NULL; > error_report("error: %s", error_get_pretty(err)); >=20 > will print "error: (null)" which is ugly, Or even segfault. glibc is nice for printing "(null)", but the behavior is undefined by POSIX and other libc aren't as nice as glibc. And that was not a consequence I thought about when first raising the question of whether it was even worth changing the contract of error_get_pretty(). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --h77BhPx5fQrIdrc7N7wVeBiItrMpgAoNB 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/ iQEcBAEBCAAGBQJWXwQgAAoJEKeha0olJ0NqoF8H/jGyTq0ojh+MGRhazu/IjVwW QxKgij1UB/6d1UckGs+TJvmuy25G0DgcUWw6pSfzRarWE9+xnzyGpg9IW+pIlKw4 53x52CdIkCgb/VqeZxUtRjTFTtyFjdRzK2WLTEJpKqt7SrJE7+LJgXnZSrudnsv1 WLK4gRWkdSMrwFbFfjLpRmwWUoN+sM0vXlU9MTsMEHKEvzZ+otupoyjcskmYNGik 9Srio1d9SYbsVrGUeWLVoadeBLq4frSXIwFdnsvvfkBkamxMqxCu+rPM/kXGTcwr zH/pLQkf4h1lL3bu4yoxc+/N6bWXdKGnuS0d1hHbK0EqeTWi8yq8cAUjYSp9BSs= =JmE0 -----END PGP SIGNATURE----- --h77BhPx5fQrIdrc7N7wVeBiItrMpgAoNB--