From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42715) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XGsW2-0002q0-NY for qemu-devel@nongnu.org; Mon, 11 Aug 2014 12:32:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XGsVv-0000uJ-Ll for qemu-devel@nongnu.org; Mon, 11 Aug 2014 12:32:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47678) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XGsVv-0000uF-Dg for qemu-devel@nongnu.org; Mon, 11 Aug 2014 12:32:03 -0400 Message-ID: <53E8F000.2090409@redhat.com> Date: Mon, 11 Aug 2014 10:32:00 -0600 From: Eric Blake MIME-Version: 1.0 References: <1407565595-18861-1-git-send-email-sanidhya.iiith@gmail.com> <1407565595-18861-4-git-send-email-sanidhya.iiith@gmail.com> In-Reply-To: <1407565595-18861-4-git-send-email-sanidhya.iiith@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="HmpTjRSkOt4BdMBR1TWvmAvuxGDlESsbb" Subject: Re: [Qemu-devel] [RFC PATCH v3 3/6] VMstate test: basic VMState testing mechanism List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sanidhya Kashyap , qemu list Cc: "Dr. David Alan Gilbert" , Juan Quintela This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --HmpTjRSkOt4BdMBR1TWvmAvuxGDlESsbb Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 08/09/2014 12:26 AM, Sanidhya Kashyap wrote: > This patch implements the basic way of testing the VMStates' informatio= n > whether it is correct or not while saving and loading the states. The q= mp > interface - test-vmstates can take three parameters as an input to test= > the device states. Now, one can check for any of the devices that have = been > registered with the SaveVMHandlers aka the migration protocol. Similarl= y, > the hmp interface (test_vmstates) has only two input parameters - itera= tions and > period. >=20 This paragraph: vvvvvvv > I have removed the following from the patch on previous comments: > * replaed DPRINTF with trace_##name. > * removed the noqdev and completely removed the support for resetting o= f devices > based on qemu_system_reset() ^^^^^^^ should not be part of the commit message proper, but... >=20 > As Juan pointed out that there might be a memory leak as I did not clos= e the > QEMUFile pointer. But, it is not required as that pointer is directly r= eferenced > by the QEMUBuffer that has been implemented by David. So, IMHO the case= pointed > out by Juan does not exist. >=20 >=20 > Signed-off-by: Sanidhya Kashyap > --- =2E..listed here, as a changelog to guide reviewers. Remember, anything before the --- should stand alone as what you would read in qemu.git, without regards to how many revisions the patch went through; and anything after the --- is useful for reviewers but intentionally stripped by the maintainer when using 'git am' as fluff that doesn't help explain the commit in isolation. > hmp-commands.hx | 16 ++++ > hmp.c | 17 ++++ > hmp.h | 1 + > qapi-schema.json | 22 ++++++ > qmp-commands.hx | 33 ++++++++ > savevm.c | 233 +++++++++++++++++++++++++++++++++++++++++++++++= ++++++++ > trace-events | 2 + > 7 files changed, 324 insertions(+) >=20 > +++ b/qapi-schema.json > @@ -3506,3 +3506,25 @@ > ## > { 'command': 'query-devices', > 'returns': [ 'QemuDevice' ] } > + > +## > +# @test-vmstates > +# > +# tests the vmstates' value by dumping and loading in memory > +# > +# @iterations: (optional) The total iterations for vmstate testing. For consistency, and in case we ever start generating documentation from the .json file, s/(optional)/#optional/ > +# The min and max defind value is 10 and 100 respectively. s/defind value is/defined values are/ > +# > +# @period: (optional) sleep interval between iteration (in millisecond= s). s/(optional)/#optional/ > +# The default interval is 100 milliseconds with min and max being > +# 1 and 10000 respectively. > +# > +# @devices: (optional) helps in resetting particular device(s) that > +# have been registered with SaveStateEntry. > +# > +# Since 2.2 > +## > +{ 'command': 'test-vmstates', > + 'data': {'*iterations': 'int', > + '*period': 'int', > + '*devices': [ 'QemuDevice' ] } } > +Example: > + > +-> { "execute": "test-vmstates", > + "arguments": { > + "iterations": 10, > + "period": 100, } } That is not valid JSON. You cannot have a trailing , inside {}. Also, it might be worth an example of the proper use of the optional "devices" parameter. > + > +static inline bool test_vmstates_check_device_name(VMStateLogState *v,= The compiler is good enough about inlining static functions that you seldom need to use 'inline'. That, and 'static inline' has changed semantics over the years of gcc, so you really want to avoid it unless you know what you are doing. > +static void test_vmstates_cb(void *opaque) > +{ > + VMStateLogState *v =3D opaque; > + int saved_vm_running =3D runstate_is_running(); > + const QEMUSizedBuffer *qsb; > + > + qsb =3D qemu_buf_get(f); > + > + /* clearing the states of the guest */ > + test_vmstates_reset_devices(v); > + > + start_time =3D qemu_clock_get_ms(QEMU_CLOCK_REALTIME); > + f =3D qemu_bufopen("r", (QEMUSizedBuffer *)qsb); Ewww. Why are you casting away const? Make qsb the correct type to begin with. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --HmpTjRSkOt4BdMBR1TWvmAvuxGDlESsbb 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 iQEcBAEBCAAGBQJT6PAAAAoJEKeha0olJ0Nqe+UH/RPwyily6H8ozb6K2M2DecRz K9Lr5e6fHuJh7/aizcQdXRB3BlMqODTsbSR3NNWYw0cyeQAmfoZDYpi6msY74utU Kb1qzBzr7AAqPdu8/I22wGG+BhQPSAkNr3IpJi1V0Eaq/mKfgmFHwVOuEe68Kpig JY8HXGQFS2l8A+7fUtcQ6lsE49HtmMpkJ4lhxcqlH5EsLz/KKrkKTM+8NinVmFa8 TdGjdXy/EiIlrs6Ll/JQtmRCAQiRPojTY7KtPoe1oTTd9vKmB5d+Fp22WsIW4MZP 25Q5ZzIz/Rn4LaPfMe8nThFNp0A6VWNYGc2nov4jFhgMbQ+UYG5ydWZOrcLTv5g= =5Ck/ -----END PGP SIGNATURE----- --HmpTjRSkOt4BdMBR1TWvmAvuxGDlESsbb--