From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46790) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1byzMZ-0007QM-Mr for qemu-devel@nongnu.org; Tue, 25 Oct 2016 06:53:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1byzMY-0002rw-L4 for qemu-devel@nongnu.org; Tue, 25 Oct 2016 06:53:47 -0400 Received: from mailhub.sw.ru ([195.214.232.25]:1276 helo=relay.sw.ru) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1byzMY-0002pF-8V for qemu-devel@nongnu.org; Tue, 25 Oct 2016 06:53:46 -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> From: Vladimir Sementsov-Ogievskiy Message-ID: <7a302497-b114-e60f-c5b2-5609e7e55b50@virtuozzo.com> Date: Tue, 25 Oct 2016 13:53:28 +0300 MIME-Version: 1.0 In-Reply-To: <36fc9101-439b-0c4b-14d3-25045800a785@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable 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: Max Reitz , 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 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 da= ta >>>>>>>> must >>>>>>>> be considered inconsistent and should not be loaded. >>>>>>>> >>>>>>>> With current implementation this flag is never set, as we just r= emove >>>>>>>> bitmaps from the image after loading. But it defined in qcow2 sp= ec >>>>>>>> and >>>>>>>> must be handled. Also, it can be used in future, if async scheme= s 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 t= o >>>>>>> me if >>>>>>> you just marked autoload bitmaps as in_use instead of deleting th= em >>>>>>> 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 d= on'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). >> 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. >> >> 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 bein= g >> deleted while the VM is running and is just saved back to the image fi= le >> once it is closed. I don't expect that users will always consume all o= f >> 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 probably >> 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 data >> when opening the image file or when the dirty bitmap is actually delet= ed >> 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. >> >> On the other hand, as soon as just a single auto-loaded bitmap survive= s >> a VM (or qemu-img) invocation, you will very, very likely safe at leas= t >> some time because writing the bitmap to the disk can reuse at least so= me >> of the existing clusters. > By the way, dealing with removal of bitmaps when they are deleted by th= e > user is also positive when it comes to migration or read-only disks tha= t > 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. You are right, I need to handle reopening more carefully. In my way, I=20 need to delete bitmaps when reopening R/W and in yours - set in_use bit. > > 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 > Ok, I'll rewrite this way, using 'in_use' bit and trying to avoid=20 allocate/free overhead. --=20 Best regards, Vladimir