From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45836) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ebCHf-0000Mn-NM for qemu-devel@nongnu.org; Mon, 15 Jan 2018 16:27:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ebCHe-0005bv-Ky for qemu-devel@nongnu.org; Mon, 15 Jan 2018 16:27:11 -0500 References: <76a29a1e-7fdb-37a8-9f79-df618d385b05@virtuozzo.com> <9e11cf33-cbe1-9fd5-f265-180e4e14dfa7@redhat.com> <6a2d73cc-5042-54c8-364d-18d273547f27@virtuozzo.com> From: John Snow Message-ID: Date: Mon, 15 Jan 2018 16:26:55 -0500 MIME-Version: 1.0 In-Reply-To: <6a2d73cc-5042-54c8-364d-18d273547f27@virtuozzo.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] qcow2 autoloading bitmaps List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladimir Sementsov-Ogievskiy , Eric Blake , qemu-devel , qemu block Cc: Kevin Wolf , Fam Zheng , Max Reitz , Nikolay Shirokovskiy , Stefan Hajnoczi , Paolo Bonzini , "Denis V. Lunev" On 01/11/2018 10:15 AM, Vladimir Sementsov-Ogievskiy wrote: > 11.01.2018 17:43, Eric Blake wrote: >> On 01/11/2018 08:26 AM, Vladimir Sementsov-Ogievskiy wrote: >> >>> # @autoload: the bitmap will be automatically loaded when the image i= t >>> is stored >>> #=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 i= n is opened. This flag may only be specified for persistent >>> #=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 b= itmaps. Default is false for block-dirty-bitmap-add. >>> (Since: 2.10) >>> >>> so, if we consider only enabled bitmaps it is ok to direct map autolo= ad >>> <=3D> auto. >>> >>> But now we've faced into necessity of load/store disabled bitmaps. >>> >>> Current behavior is definitely wrong: user sets autoload flag for >>> disabled bitmap, but on next >>> Qemu start the bitmap will be autoloaded and enabled. >> Can we make it an error to set autoload to true when requesting a >> disabled bitmap? Or can we not even request a disabled bitmap? >=20 > currently, autoload is set on bitmap creation, so it is always enabled > at this time. >=20 >> >>> >>> Proposed solution: >>> =C2=A0- deprecate @autoload flag for bitmap creation, ignore it >> Is there ever a case where you'd create an enabled persistent >> dirty-tracking bitmap, but not want it to be re-enabled the next time >> the image is created? >=20 > looks like we do not. For now we use only persistent+autoload. >=20 >> Your argument for ignoring/deleting the parameter >> is that you can decide whether to set the "auto" flag solely based on >> whether the bitmap is both persistent and enabled, without needing any >> additional user input? >=20 > The main point is that, when we want to load/store disabled bitmaps thi= s > parameter is confusing: In user's point of view it's ok to have autoloa= ding > disabled bitmap, which he wants to be autoloaded and disabled on next Q= emu > start. So, to maintain this flag (autoload) for disabled bitmaps, we > will need > to add some additional flag "disabled" to Qcow2 spec, to make it possib= le to > have "auto" but "disabled" bitmaps. And we will have to change qcow2 "a= uto" > specification, to consider "disabled" bit set in parallel. But all this > looks > superfluous for now. We already have type "dirty tracking bitmaps" in q= cow2 > spec for dirty bitmaps. So, for now, "auto" flag set on "dirty tracking > bitmaps" means that it is enabled. It is logical to assume that all oth= er > "dirty tracking bitmaps" are disabled. >=20 Oh, so you want to be able to "autoload" a disabled bitmap, is that right= ? Is it important to distinguish these two cases? (1) A bitmap is saved to the qcow2, but autoload is set to false (2) A bitmap is saved to the qcow2, autoload is true, but a (new) disabled bit is set. Is it okay, basically, to always autoload bitmaps in a disabled state? Is it imperative we add the new bit? (Genuinely asking. I'm not sure.) > On the other hand, "autoloading" is a bad property for disabled bitmap > at all. > Disabled bitmap is simpler than enabled (or "auto") bitmap, it doesn't > require > any care like "auto" bitmap. So, it is more natural to let Qemu decide > to load > or not any disabled bitmaps. >=20 If we don't autoload the disabled ones, I imagine you'll eventually want some way to manually load them into memory to manipulate them for various reasons. >=20 >=20 >> >>> =C2=A0- save persistent enabled bitmaps with "auto" flag >>> =C2=A0- save persistent disabled bitmaps without "auto" flag >>> =C2=A0- on Qemu start load all bitmaps, mapping "auto" flag state to >>> "enabled" state. >> So if I follow the argument, the state of the 'auto' flag stored into >> the file on BDS close (often at qemu exit) is based on whether the >> bitmap was currently enabled, and the user can then control whether to >> enable/disable the bitmap on the fly to control whether the 'auto' fla= g >> is stored; thus, having the flag at creation time is redundant. >=20 > Yes. >=20 >> >>> >>> Note: we may store a lot of disabled bitmaps in qcow2 image, but load= ing >>> them all into RAM may be >>> inefficient. Actually such bitmap will be needed only on demand (for >>> export through nbd or >>> making some kind of backup). So in future it may be optimized by "laz= y >>> load" of disabled bitmaps, >>> postponing their actual load up to first read or enabling. This >>> optimization doesn't need changing >>> of qapi or qcow2 format (at first sight). >>> >>> >>> Note2: now there is no way to disable/enable bitmaps, but there is a >>> =C2=A0 [PATCH for-2.12 0/4] qmp dirty bitmap API >>> with big conversation, but I hope, I'll post a new version with a sma= ll >>> fix soon and it will be merged. >> In other words, you can incorporate your QAPI tweaks proposed here int= o >> your respin of that series. >> >=20 > right idea, I'll do so, just wait a bit for others to comment here. >=20 Stay tuned... > --=20 > Best regards, > Vladimir >=20