From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52832) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3WZV-0000uG-Nm for qemu-devel@nongnu.org; Mon, 30 Nov 2015 17:05:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a3WZR-0003DX-KF for qemu-devel@nongnu.org; Mon, 30 Nov 2015 17:05:21 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49569) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3WZR-0003DG-Cj for qemu-devel@nongnu.org; Mon, 30 Nov 2015 17:05:17 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 4225FC07567F for ; Mon, 30 Nov 2015 22:05:16 +0000 (UTC) References: <1448883140-20249-1-git-send-email-peterx@redhat.com> <1448883140-20249-3-git-send-email-peterx@redhat.com> From: Eric Blake Message-ID: <565CC816.8020507@redhat.com> Date: Mon, 30 Nov 2015 15:05:10 -0700 MIME-Version: 1.0 In-Reply-To: <1448883140-20249-3-git-send-email-peterx@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="c5drSIQA3aNm2rv1KdfGXAb6cLCabsFsv" Subject: Re: [Qemu-devel] [PATCH v3 02/12] dump-guest-memory: add "detach" flag for QMP/HMP interfaces. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu , qemu-devel@nongnu.org Cc: drjones@redhat.com, famz@redhat.com, armbru@redhat.com, pbonzini@redhat.com, lcapitulino@redhat.com, lersek@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --c5drSIQA3aNm2rv1KdfGXAb6cLCabsFsv Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/30/2015 04:32 AM, Peter Xu wrote: > This patch only adds the interfaces, but not implements them. > "detach" parameter is made optional, to make sure that all the old > dump-guest-memory requests will still be able to work. >=20 > Signed-off-by: Peter Xu > --- > dump.c | 5 +++-- > hmp-commands.hx | 5 +++-- > hmp.c | 9 +++++++-- > qapi-schema.json | 8 ++++++-- > qmp-commands.hx | 6 ++++-- > 5 files changed, 23 insertions(+), 10 deletions(-) > +++ b/hmp.c > @@ -1586,8 +1586,10 @@ void hmp_dump_guest_memory(Monitor *mon, const Q= Dict *qdict) > const char *file =3D qdict_get_str(qdict, "filename"); > bool has_begin =3D qdict_haskey(qdict, "begin"); > bool has_length =3D qdict_haskey(qdict, "length"); > + bool has_detach =3D qdict_haskey(qdict, "detach"); Here, you probe whether 'detach' is present... > int64_t begin =3D 0; > int64_t length =3D 0; > + bool detach =3D false; =2E..here, you default 'detach' to false,.. > enum DumpGuestMemoryFormat dump_format =3D DUMP_GUEST_MEMORY_FORMA= T_ELF; > char *prot; > =20 > @@ -1615,11 +1617,14 @@ void hmp_dump_guest_memory(Monitor *mon, const = QDict *qdict) > if (has_length) { > length =3D qdict_get_int(qdict, "length"); > } > + if (has_detach) { > + detach =3D qdict_get_try_bool(qdict, "detach", false); =2E..therefore, this line is only reachable if 'detach' is present, which= means the default will never be needed. > + } > =20 > prot =3D g_strconcat("file:", file, NULL); > =20 > - qmp_dump_guest_memory(paging, prot, has_begin, begin, has_length, = length, > - true, dump_format, &err); > + qmp_dump_guest_memory(paging, prot, has_detach, detach, has_begin,= begin, > + has_length, length, true, dump_format, &err)= ; There are two competing ways to simplify this; I don't care which one you choose, although you can't do both: 1. Note that since the second assignment to detach only happens if has_detach is true, you can use the simpler: if (has_detach) { detach =3D qdict_get_bool(qdict, "detach"); } If you go with this approach, you could even get away without initializing 'detach' outside of the 'if (has_detach)' conditional (although it's still probably wiser to avoid uninitialized memory than to rely on qmp_dump_guest_memory() correctly ignoring 'detach' when 'has_detach' is false). 2. Note that since qdict_get_try_bool() lets you specify a default, you can eliminate the has_detach variable and instead just do: bool detach =3D qdict_get_try_bool(qdict, "detach", false); =2E.. qmp_dump_guest_memory(paging, prot, true, detach, ...); > +++ b/qapi-schema.json > @@ -2115,6 +2115,9 @@ > # 2. fd: the protocol starts with "fd:", and the following = string > # is the fd's name. > # > +# @detach: #optional if true, QMP will return immediately rather than > +# waiting dump to be finished (since 2.6). s/dump/for the dump/ s/be finished/finish/ > @@ -857,6 +857,8 @@ Arguments: > - "paging": do paging to get guest's memory mapping (json-bool) > - "protocol": destination file(started with "file:") or destination fi= le > descriptor (started with "fd:") (json-string) > +- "detach": if specificed, command will return immediately, without wa= iting s/specificed/specified/ > + for dump to be finished (json-bool) s/dump/the dump/ s/be finished/finish/ > - "begin": the starting physical address. It's optional, and should be= specified > with length together (json-int) > - "length": the memory size, in bytes. It's optional, and should be sp= ecified >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --c5drSIQA3aNm2rv1KdfGXAb6cLCabsFsv 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/ iQEcBAEBCAAGBQJWXMgWAAoJEKeha0olJ0NqkZkIAIggwfa0rx6ctzIYD2hBdnKQ G5haBveI12s1B0G0jVO4W4biGabm7RvPHu+9/h85278h9+bh8K3IpFGlXmx2kT/r QcZJLz2oHmjV0eg4ay6RD9rEgsY9fHEZJ1iJMVf7QPu0MKzgms4hrLd4Vd7fPVN6 eTPMCPE8p+YQe2kpDMR5uBQRjXKNvTijDFyGwcPpqg7K6DVxg3sO28bKNDY1VxGc GHwWVY9uGZHg8yym+MhwmkaMNYXgmwSqbtzED1VHzWLia70cbyLLSyp3595RTR/F xxua1gJrV+UQkg4KhU0+4LKzFAsXG64Il6rdvERKfgKhp00t0OK3hfXx2KcG0Mc= =g9qG -----END PGP SIGNATURE----- --c5drSIQA3aNm2rv1KdfGXAb6cLCabsFsv--