From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60790) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1byitQ-0000Qd-22 for qemu-devel@nongnu.org; Mon, 24 Oct 2016 13:18:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1byitO-0004Qq-RS for qemu-devel@nongnu.org; Mon, 24 Oct 2016 13:18:36 -0400 References: <1475232808-4852-1-git-send-email-vsementsov@virtuozzo.com> <1475232808-4852-13-git-send-email-vsementsov@virtuozzo.com> <0cb068bb-e625-ecb3-0078-b20828920619@redhat.com> <5ce110e0-f204-affb-897f-7c6bada6739f@virtuozzo.com> <8ed2ee67-cf67-a80f-902f-c40a1409a2cd@redhat.com> From: Max Reitz Message-ID: <36fc9101-439b-0c4b-14d3-25045800a785@redhat.com> Date: Mon, 24 Oct 2016 19:18:23 +0200 MIME-Version: 1.0 In-Reply-To: <8ed2ee67-cf67-a80f-902f-c40a1409a2cd@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="JGlrwoFJGwdVXojdxcU0XA5OJJge9OSVh" Subject: Re: [Qemu-devel] [PATCH 12/22] qcow2-bitmap: add IN_USE flag List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladimir Sementsov-Ogievskiy , qemu-block@nongnu.org, qemu-devel@nongnu.org Cc: kwolf@redhat.com, armbru@redhat.com, eblake@redhat.com, jsnow@redhat.com, famz@redhat.com, den@openvz.org, stefanha@redhat.com, pbonzini@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --JGlrwoFJGwdVXojdxcU0XA5OJJge9OSVh From: Max Reitz To: Vladimir Sementsov-Ogievskiy , qemu-block@nongnu.org, qemu-devel@nongnu.org Cc: kwolf@redhat.com, armbru@redhat.com, eblake@redhat.com, jsnow@redhat.com, famz@redhat.com, den@openvz.org, stefanha@redhat.com, pbonzini@redhat.com Message-ID: <36fc9101-439b-0c4b-14d3-25045800a785@redhat.com> Subject: Re: [PATCH 12/22] qcow2-bitmap: add IN_USE flag References: <1475232808-4852-1-git-send-email-vsementsov@virtuozzo.com> <1475232808-4852-13-git-send-email-vsementsov@virtuozzo.com> <0cb068bb-e625-ecb3-0078-b20828920619@redhat.com> <5ce110e0-f204-affb-897f-7c6bada6739f@virtuozzo.com> <8ed2ee67-cf67-a80f-902f-c40a1409a2cd@redhat.com> In-Reply-To: <8ed2ee67-cf67-a80f-902f-c40a1409a2cd@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 24.10.2016 19:08, Max Reitz wrote: > On 24.10.2016 13:35, Vladimir Sementsov-Ogievskiy wrote: >> 24.10.2016 13:32, Vladimir Sementsov-Ogievskiy =D0=BF=D0=B8=D1=88=D0=B5= =D1=82: >>> 21.10.2016 22:58, Max Reitz =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >>>> On 21.10.2016 17:34, Vladimir Sementsov-Ogievskiy wrote: >>>>> 07.10.2016 22:44, Max Reitz =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >>>>>> On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote: >>>>>>> This flag means that the bitmap is now in use by the software or >>>>>>> was not >>>>>>> successfully saved. In any way, with this flag set the bitmap dat= a >>>>>>> must >>>>>>> be considered inconsistent and should not be loaded. >>>>>>> >>>>>>> With current implementation this flag is never set, as we just re= move >>>>>>> bitmaps from the image after loading. But it defined in qcow2 spe= c >>>>>>> and >>>>>>> must be handled. Also, it can be used in future, if async schemes= of >>>>>>> bitmap loading/saving are implemented. >>>>>>> >>>>>>> We also remove in-use bitmaps from the image on open. >>>>>>> >>>>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy >>>>>>> >>>>>>> --- >>>>>>> block/qcow2-bitmap.c | 17 ++++++++++++++++- >>>>>>> 1 file changed, 16 insertions(+), 1 deletion(-) >>>>>> Don't you want to make use of this flag? It would appear useful to= >>>>>> me if >>>>>> you just marked autoload bitmaps as in_use instead of deleting the= m >>>>>> from >>>>>> the image when it's opened and then overwrite them when the image = is >>>>>> closed. >>>>> And what is the use of it? >>>> You don't need to free any bitmaps when opening the file, and you do= n't >>>> need to allocate any new bitmap space when closing it. >>> >>> As bitmaps are sparce in file, I need to allocate new space when >>> closing. Or free it... >> >> >> Is it a real problem to reallocate space in qcow2? If so, to minimaze >> allocations, I will have to make a list of clusters of in_use bitmaps = on >> close, and then save current bitmaps to these clusters (and allocated >> some additional clusters, or free extra clusters). >=20 > It's not a real problem, but it does take time, and I maintain that it'= s > time it doesn't need to take because you can just use the in_use flag. >=20 > I wouldn't worry about reusing clusters of other bitmaps. Of course we > can do that later on in some optimization but not now. >=20 > I just mean the basic case of some auto-loaded bitmap that is not being= > deleted while the VM is running and is just saved back to the image fil= e > once it is closed. I don't expect that users will always consume all of= > the auto-loaded bitmaps while the VM is active... >=20 >> Also, if I don't free them on open, I'll have to free them on remove o= f >> bdrv dirty bitmap.. >=20 > Yes, so? That takes time, but that is something the user will probably > expect. I wouldn't expect opening of the file to take that time. >=20 > Overall, it doesn't matter time-wise whether you free the bitmap data > when opening the image file or when the dirty bitmap is actually delete= d > by the user or when the image file is closed. Just setting the single > in_use flag for all of the auto-loaded bitmaps will basically not take > any time. >=20 > On the other hand, as soon as just a single auto-loaded bitmap survives= > a VM (or qemu-img) invocation, you will very, very likely safe at least= > some time because writing the bitmap to the disk can reuse at least som= e > of the existing clusters. By the way, dealing with removal of bitmaps when they are deleted by the user is also positive when it comes to migration or read-only disks that are later reopened R/W: Currently, as far as I can see, you just keep the auto-load bitmaps in the image if the image is part of an incoming migration or opened read-only, but then you continue under the assumption that they are removed from the image. That's not very good. If you delete the bitmaps only when the user asks you to, then you can just return an error that the bitmap cannot be removed at this time because the image file is read-only or used in migration (and I don't think the latter can even happen when it's the user who has requested removal of the bitmap). Max --JGlrwoFJGwdVXojdxcU0XA5OJJge9OSVh Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQEvBAEBCAAZBQJYDkJfEhxtcmVpdHpAcmVkaGF0LmNvbQAKCRD0B9sAYdXPQBr5 B/9axUzSd4lfKZpSxcax8a+thyVY5yaMFbKbU5LNARrTF5h5+Anpzox9OeijM65s KtI60VJt3ZO7Iif3k1VqGcN6ttfQqL47hjBibfHsQxx2S67JZh1msHmrb5zn76+f wVshHqPy4J38r/QFiDZrr2Ke8src/zwp/9L5tezicnY72blUHAGgqIIt+MO2f82W ORYMt3qMrtRuuBtb3zaS4dx5vorF/6idXvIrXcr1dLi7e8qMj1JegWuxf+2zsQuY 3HuU7e6GdsTRz3jj42xvWOfWFRBzv732b4Ieoiv9zMihJ1sbtE0l6wVexEVlw+P8 3WEf4n0AAoCO4NTfP6BWHU2H =TqRk -----END PGP SIGNATURE----- --JGlrwoFJGwdVXojdxcU0XA5OJJge9OSVh--