From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37932) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WXvtL-00032b-Lk for qemu-devel@nongnu.org; Wed, 09 Apr 2014 13:02:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WXvtG-0005Es-Nv for qemu-devel@nongnu.org; Wed, 09 Apr 2014 13:02:27 -0400 Message-ID: <53457D1A.7000602@redhat.com> Date: Wed, 09 Apr 2014 11:02:18 -0600 From: Eric Blake MIME-Version: 1.0 References: <1397062459-4141-1-git-send-email-wangbj@gmail.com> In-Reply-To: <1397062459-4141-1-git-send-email-wangbj@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="3KiPGrUIQtRhSaM8Ih56Ns5kkMAJHK6jk" Subject: Re: [Qemu-devel] [PATCH V4 1/1] qmp: add pmemload command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Baojun Wang , qemu-devel@nongnu.org Cc: qemu-trivial@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --3KiPGrUIQtRhSaM8Ih56Ns5kkMAJHK6jk Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 04/09/2014 10:54 AM, Baojun Wang wrote: > Signed-off-by: Baojun Wang You lost this part of your commit message, which gives more details about the 'why' (the subject line covers the 'what', but it is often the 'why' that is most needed when reviewing a commit later). Your earlier mail had: I found this could be useful to have qemu-softmmu as a cross debugger (launch with -s -S command line option), then if we can have a command to load gu= est physical memory, we can use cross gdb to do some target debug which gdb cannot do directly. > =20 > +void qmp_pmemload(int64_t addr, int64_t size, const char *filename, > + Error **errp) > +{ > + FILE *f; > + uint32_t l; > + uint8_t buf[1024]; > + > + f =3D fopen(filename, "rb"); I still think that fopen()/fread() is wrong, and that you should be using qemu_open()/read() - that way, libvirt could pass in a file descriptor and use qemu's already-existing /dev/fdset/nnn support rather than forcing qemu to open something from the filesystem. > +++ b/hmp-commands.hx > @@ -809,6 +809,19 @@ save to disk physical memory dump starting at @var= {addr} of size @var{size}. > ETEXI > =20 > { > + .name =3D "pmemload", > + .args_type =3D "val:l,size:i,filename:s", > + .params =3D "addr size file", > + .help =3D "load from disk physical memory dump starting = at 'addr' of size 'size'", > + .mhandler.cmd =3D hmp_pmemload, And I still think that you should list filename first, with val and size optional at the end. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --3KiPGrUIQtRhSaM8Ih56Ns5kkMAJHK6jk Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTRX0aAAoJEKeha0olJ0Nq3FQH/jja/VQ9JKszXZsVzNxyTeKW TaJQcQRPjfCF7HdCguXNg7g0gRNpY7JrQTyF8xT8xvXMpBXIHa9H99W0yYRZlqnM ICM0PBw8KbnkT7nBy7v/xSQ5pnfnwE5r6Xp9jK1uj5Gwaof/8PwXUCoOn8kiAyox M2v7uUnecWJQ1SUNsaWiWgya0dHA9ykh7aDd+1hDHTAMfZJkQIHf/4UJdvn6zlCa EJhMrMxnBHt0f0MGDbJU/Z1KRGh9VBzV9jI6YVQy6M00Hekb+4wL1JCsSuFDc2aP YpkskKZ7kRa9tc3CAvYBqEFkMsmxiR9CiVhoZOSnyXzZ0BwgqkwjMTArjS9SbSg= =IEUP -----END PGP SIGNATURE----- --3KiPGrUIQtRhSaM8Ih56Ns5kkMAJHK6jk--