From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:36296) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwH64-0000aD-5d for qemu-devel@nongnu.org; Tue, 31 Jul 2012 14:23:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SwH62-0004jq-U3 for qemu-devel@nongnu.org; Tue, 31 Jul 2012 14:23:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35995) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwH62-0004jh-ML for qemu-devel@nongnu.org; Tue, 31 Jul 2012 14:23:06 -0400 Message-ID: <5018173B.2060506@redhat.com> Date: Tue, 31 Jul 2012 11:34:51 -0600 From: Eric Blake MIME-Version: 1.0 References: In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enigC88BC2DF6D71F04FEC61AE7D" Subject: Re: [Qemu-devel] [RFC PATCH 1/4] block: add support functions for live commit, to find and delete images. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, supriyak@linux.vnet.ibm.com, stefanha@gmail.com This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigC88BC2DF6D71F04FEC61AE7D Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 07/30/2012 11:16 PM, Jeff Cody wrote: > Add bdrv_find_image(), bdrv_find_base(), and bdrv_delete_intermediate()= =2E >=20 > bdrv_find_image(): given a filename and a BDS, find the image in the c= hain > that matches the passed filename. >=20 > bdrv_find_base(): given a BDS, find the base image (parent-most image= ) >=20 > bdrv_delete_intermediate(): > Given 3 BDS (active, top, base), delete images abov= e > base up to and including top, and set base to be th= e > parent of top's child node. A diagram, as was in your cover letter, would help (either here, or better yet in the comments describing this function in place)... >=20 > Signed-off-by: Jeff Cody > --- > block.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++++++++++= +++++++- > block.h | 4 ++ > 2 files changed, 139 insertions(+), 1 deletion(-) >=20 > diff --git a/block.c b/block.c > index 522acd1..a3c8d33 100644 > --- a/block.c > +++ b/block.c > @@ -1408,7 +1408,7 @@ int bdrv_commit(BlockDriverState *bs) > =20 > if (!drv) > return -ENOMEDIUM; > - =20 > + > if (!bs->backing_hd) { > return -ENOTSUP; > } Spurious whitespace cleanup, since nothing else in this hunk belongs to you. Is that trailing whitespace present upstream, or was it introduced in one of the patches you based off of? > @@ -1661,6 +1661,110 @@ int bdrv_change_backing_file(BlockDriverState *= bs, > return ret; > } > =20 > +typedef struct BlkIntermediateStates { > + BlockDriverState *bs; > + QSIMPLEQ_ENTRY(BlkIntermediateStates) entry; > +} BlkIntermediateStates; > + > + > +/* deletes images above 'base' up to and including 'top', and sets the= image > + * above 'top' to have base as its backing file. > + */ > +int bdrv_delete_intermediate(BlockDriverState *active, BlockDriverStat= e *top, > + BlockDriverState *base) =2E..that is, I think this would aid the reader: Converts: bottom <- base <- intermediate <- top <- active to bottom <- base <- active where top=3D=3Dactive is permitted > @@ -3128,6 +3232,36 @@ BlockDriverState *bdrv_find_backing_image(BlockD= riverState *bs, > return NULL; > } > =20 > +BlockDriverState *bdrv_find_image(BlockDriverState *bs, > + const char *filename) > +{ > + if (!bs || !bs->drv) { > + return NULL; > + } > + > + if (strcmp(bs->filename, filename) =3D=3D 0) { > + return bs; > + } else { > + return bdrv_find_image(bs->backing_hd, filename); Tail-recursive; are we worried enough about ever hitting stack overflow due to a really deep change to convert this into a while loop recursion instead? [Probably not] > + } > +} > + > +BlockDriverState *bdrv_find_base(BlockDriverState *bs) > +{ > + BlockDriverState *curr_bs =3D NULL; > + > + if (!bs) { > + return NULL; > + } > + > + curr_bs =3D bs; > + > + while (curr_bs->backing_hd) { > + curr_bs =3D curr_bs->backing_hd; > + } Then again, here you did while-loop recursion, so using the same style in both functions might be worthwhile. --=20 Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enigC88BC2DF6D71F04FEC61AE7D 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.12 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBCAAGBQJQGBc8AAoJEKeha0olJ0Nq/g4IAIjLY+h0sf8o9Pn/JF6wYjRQ obMCq1jD2F73NbI9l8OQQrelpSfBl7V7bdDSpuPN+j9TI1Yhft4PpBBK8sWsVSUJ Gis9O2IQ8+FJRtg9pkn+FuSdYA/Vu7ahzIX6BW+RlSwJvLZhXFbdXHC8RvtFL7iW SFzG2Bvv/ja5GYBBjURWkfZkN2HOXrg6Q209JOxLRJeYOrLqR49cd7/uSwbYkPOk 1NCe9Hgwr8gVBaSyy4VyfiQPg7MmMfpm4f4QAN4Ef5yO7zqaTsf2a9hzax9tmxcu EK4iGDl/8j3X4ZTPcZdAEdXh7NvfkVq5s3AZalmxkOv1V6mlVwfLzqHhj1qXojY= =t16V -----END PGP SIGNATURE----- --------------enigC88BC2DF6D71F04FEC61AE7D--