From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55223) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UE6CH-0001XN-0z for qemu-devel@nongnu.org; Fri, 08 Mar 2013 17:55:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UE6CA-0005Ob-Vd for qemu-devel@nongnu.org; Fri, 08 Mar 2013 17:55:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:8106) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UE6CA-0005OH-NM for qemu-devel@nongnu.org; Fri, 08 Mar 2013 17:55:22 -0500 Message-ID: <513A6C55.2080507@redhat.com> Date: Fri, 08 Mar 2013 15:55:17 -0700 From: Eric Blake MIME-Version: 1.0 References: <1362636445-7188-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1362636445-7188-9-git-send-email-xiawenc@linux.vnet.ibm.com> In-Reply-To: <1362636445-7188-9-git-send-email-xiawenc@linux.vnet.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2XJBPFLQOWVERWRMOPDGH" Subject: Re: [Qemu-devel] [PATCH V8 08/20] block: add filter for vm snapshot in bdrv_query_snapshot_info_list() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia Cc: kwolf@redhat.com, aliguori@us.ibm.com, stefanha@gmail.com, qemu-devel@nongnu.org, armbru@redhat.com, pbonzini@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2XJBPFLQOWVERWRMOPDGH Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 03/06/2013 11:07 PM, Wenchao Xia wrote: > This patch adds a parameter to tell whether return valid snapshots > for whole VM only. >=20 > Signed-off-by: Wenchao Xia > --- > block/qapi.c | 39 +++++++++++++++++++++++++++++++++++++-- > include/block/qapi.h | 1 + > qemu-img.c | 3 ++- > 3 files changed, 40 insertions(+), 3 deletions(-) >=20 > + * @sn: snapshot info need to be checked. s/need // > + * return 0 if valid, it means load_vmstate() for it could succeed. Reads awkwardly; how about: it means load_vmstate() could load that snapshot. > + */ > +static int snapshot_filter_vm(const QEMUSnapshotInfo *sn, BlockDriverS= tate *bs) > +{ > + BlockDriverState *bs1 =3D NULL; > + QEMUSnapshotInfo s, *sn_info =3D &s; > + int ret =3D 0; > + > + /* Check logic is connected with load_vmstate(): > + Only check the devices that can snapshot, other devices that ca= n't > + take snapshot, for example, readonly ones, will be ignored in > + load_vmstate(). */ > + while ((bs1 =3D bdrv_next(bs1))) { > + if (bdrv_can_snapshot(bs1) && bs1 !=3D bs) { > + ret =3D bdrv_snapshot_find(bs1, sn_info, sn->id_str, NULL)= ; > + if (ret < 0) { > + ret =3D -1; This function only returns 0 or -1... > +/* return 0 on success, and @p_list will be set only on success. If > + @vm_snapshot is true, only the snapshot valid for whole vm will be = got. */ grammar If @vm_snapshot is true, limit the results to snapshots valid for the whole vm. > - > + if (vm_snapshot && snapshot_filter_vm(&sn_tab[i], bs)) { =2E..yet you are only ever calling it in a boolean context. Would it be smarter to have the function return bool (true for valid vm snapshot, false if the image snapshot is not usable as a vm snapshot)? Also, it might be nicer to name it snapshot_valid_for_vm, as in: if (vm_snapshot && !snapshot_valid_for_vm(...)) { continue; } --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2XJBPFLQOWVERWRMOPDGH 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.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJROmxWAAoJEKeha0olJ0Nqb8cIAIgtSru0CfLuj6U4xxoRbBkA aXsrXRUhXKKBJZZf4BODYUOzbNj3OkGZKhus5wEe5kqtx+nWe+Mb5iMnzOeRHmid 5R1qQUvhB+o/pMPQV4rGbRGkP0b1J0YSXSk4USe/Pmkb1WxdQ4OoAl9b8lcsU/kl WRmFfApmBLmGB+4cAwGaThBJLJaRCtUcGB7Y81r8vXLACQ3AEIQ9tj2BIJ2UKbBp LaeqdvSiCqv88Uu4DYx+dNVrP4uDdTN96ldEKt8tmWP01z14BUQcBak/nYWRQT6K pr11/DqV4N7/cVPPAnfs5GSfrjJrHl2Bvi9AD24OJjXOhZqXeF/iCbgyWIpG3NI= =3SWT -----END PGP SIGNATURE----- ------enig2XJBPFLQOWVERWRMOPDGH--