From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50144) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bzQ8A-0004sa-Na for qemu-devel@nongnu.org; Wed, 26 Oct 2016 11:28:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bzQ89-00012F-Do for qemu-devel@nongnu.org; Wed, 26 Oct 2016 11:28:42 -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> <36fc9101-439b-0c4b-14d3-25045800a785@redhat.com> <7a302497-b114-e60f-c5b2-5609e7e55b50@virtuozzo.com> <03bec033-313e-978d-af1f-7d4203e9dcb8@virtuozzo.com> From: Max Reitz Message-ID: <5f557246-d2b4-dc0d-594d-7d951be5f4ad@redhat.com> Date: Wed, 26 Oct 2016 17:28:31 +0200 MIME-Version: 1.0 In-Reply-To: <03bec033-313e-978d-af1f-7d4203e9dcb8@virtuozzo.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="o79Jbv9PAsio1Jv5rBf8mv1MDerKTS7NI" 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) --o79Jbv9PAsio1Jv5rBf8mv1MDerKTS7NI 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: <5f557246-d2b4-dc0d-594d-7d951be5f4ad@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> <36fc9101-439b-0c4b-14d3-25045800a785@redhat.com> <7a302497-b114-e60f-c5b2-5609e7e55b50@virtuozzo.com> <03bec033-313e-978d-af1f-7d4203e9dcb8@virtuozzo.com> In-Reply-To: <03bec033-313e-978d-af1f-7d4203e9dcb8@virtuozzo.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 26.10.2016 11:04, Vladimir Sementsov-Ogievskiy wrote: > 25.10.2016 13:53, Vladimir Sementsov-Ogievskiy wrote: >> 24.10.2016 20:18, Max Reitz wrote: >>> 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 >>>>>>>>>> data >>>>>>>>>> must >>>>>>>>>> be considered inconsistent and should not be loaded. >>>>>>>>>> >>>>>>>>>> With current implementation this flag is never set, as we just= >>>>>>>>>> remove >>>>>>>>>> bitmaps from the image after loading. But it defined in qcow2 >>>>>>>>>> spec >>>>>>>>>> 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 >>>>>>>>> them >>>>>>>>> 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= >>>>>>> don'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 minima= ze >>>>> 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 allocat= ed >>>>> some additional clusters, or free extra clusters). >>>> 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 fla= g. >>>> >>>> I wouldn't worry about reusing clusters of other bitmaps. Of course = we >>>> can do that later on in some optimization but not now. >>>> >>>> I just mean the basic case of some auto-loaded bitmap that is not be= ing >>>> deleted while the VM is running and is just saved back to the image >>>> file >>>> once it is closed. I don't expect that users will always consume all= of >>>> the auto-loaded bitmaps while the VM is active... >>>> >>>>> Also, if I don't free them on open, I'll have to free them on >>>>> remove of >>>>> bdrv dirty bitmap.. >>>> Yes, so? That takes time, but that is something the user will probab= ly >>>> expect. I wouldn't expect opening of the file to take that time. >>>> >>>> Overall, it doesn't matter time-wise whether you free the bitmap dat= a >>>> when opening the image file or when the dirty bitmap is actually >>>> deleted >>>> by the user or when the image file is closed. Just setting the singl= e >>>> in_use flag for all of the auto-loaded bitmaps will basically not ta= ke >>>> any time. >>>> >>>> On the other hand, as soon as just a single auto-loaded bitmap survi= ves >>>> a VM (or qemu-img) invocation, you will very, very likely safe at le= ast >>>> some time because writing the bitmap to the disk can reuse at least >>>> some >>>> 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 t= hat >>> 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 incomin= g >>> migration or opened read-only, but then you continue under the >>> assumption that they are removed from the image. That's not very good= =2E >> >> You are right, I need to handle reopening more carefully. In my way, I= >> need to delete bitmaps when reopening R/W and in yours - set in_use bi= t. >> >>> >>> If you delete the bitmaps only when the user asks you to, then you ca= n >>> 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 >>> >> >> Ok, I'll rewrite this way, using 'in_use' bit and trying to avoid >> allocate/free overhead. >> >> >=20 > Trying to reuse clusters of in_use bitmaps (including contiguous > allocations for bitmap tables) will complicate things a lot.. We can us= e > extra clusters of one in_use bitmap to save another one, the same is fo= r > bitmap tables. Extra clusters of old bitmap table (in case of resize) > can be used for saving other bitmap data, etc.. I feel like you're trying to optimize too much. When rewriting the bitmap data, reuse only the clusters you have to write to anyway. Say the bitmap table looks like this when you open the image: [sparse 0, cluster 42, cluster 45, sparse 0, cluster 46] ("sparse 0" means no cluster is allocated and the bitmap table entry is 0, thus creating a sparse bitmap where that part is supposed to be read as 0.) Now during runtime, you set some bits, maybe you even clear the whole bitmap at some point because you're doing a backup, and then you set some other bits, so when you close the image, the bitmap table would have to look like this: [sparse 0, sparse 0, data, data, data] Now, when you want to save that bitmap data, you just walk through the existing bitmap table. The first entry is a sparse 0, and the bitmap is sparse there, too, so that can stay as it is. The second entry points to data but the bitmap is now sparse. You can then free the cluster (cluster 42) and write "sparse 0" into the bitmap table entry. The third entry points to data and the bitmap contains data there, so you just write the data to cluster 45. The fourth entry is sparse but your bitmap contains data. So now you need to allocate a cluster, maybe that will cluster 67, and write your data there, and make the bitmap table entry point there. The fifth entry finally is handled just like the third entry. So afterwards, your bitmap table would look like this: [sparse 0, sparse 0, cluster 45, cluster 67, cluster 46] It sounds as if you're trying to reuse all the clusters, i.e. you'd ideally get: [sparse 0, sparse 0, cluster 45, cluster 42, cluster 46] May be possible, but I really wouldn't worry about that. I'd consider it much too difficult. Just reuse the existing bitmap table. Max --o79Jbv9PAsio1Jv5rBf8mv1MDerKTS7NI Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQEvBAEBCAAZBQJYEMufEhxtcmVpdHpAcmVkaGF0LmNvbQAKCRD0B9sAYdXPQBkl B/9yf/t8bdzuDBzENj+gVrjOzy6RW3AeVOQDLSil7YD7JTVHTCHnhXDIHVXX6DaL P/pmqFjoUyy9Wb7VgLpV6sauk5BGGuhx3g3UCyUUGEERS6JaFAQ2oHGjq89CknR5 eZePKlQCm5gA6qlXl1xfn2CowKp6DliHQvKB3FYyTU6XSqAEBtWaOEeHph9hMV47 hy6PJAX4MEvR04N9e4E21xuzWZHfrZOioovUT/90Z4HUm35geFAPo1nMbbAhCQ6W DDq+gD0NzcZx+aw3ASf+Rz4/g3XZ1L8qIeWc4Us3AqAuw6vdbnvV+mXqPPFJaRlu Q8fgQRRsGXTcNigZxZegaJ+p =quGa -----END PGP SIGNATURE----- --o79Jbv9PAsio1Jv5rBf8mv1MDerKTS7NI--