From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35641) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VF1pW-00068s-G5 for qemu-devel@nongnu.org; Thu, 29 Aug 2013 09:00:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VF1pQ-00045C-Aw for qemu-devel@nongnu.org; Thu, 29 Aug 2013 09:00:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:17180) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VF1pQ-00044s-1D for qemu-devel@nongnu.org; Thu, 29 Aug 2013 09:00:00 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r7TCxxwv015215 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 29 Aug 2013 08:59:59 -0400 Date: Thu, 29 Aug 2013 15:00:12 +0200 From: Kevin Wolf Message-ID: <20130829130012.GK2961@dhcp-200-207.str.redhat.com> References: <1377775241-26278-1-git-send-email-mreitz@redhat.com> <1377775241-26278-3-git-send-email-mreitz@redhat.com> <521F425D.5030400@redhat.com> <521F43FC.1060604@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <521F43FC.1060604@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 2/3] qcow2: Implement bdrv_amend_options List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-devel@nongnu.org, Stefan Hajnoczi Am 29.08.2013 um 14:52 hat Max Reitz geschrieben: > Am 29.08.2013 14:45, schrieb Eric Blake: > >On 08/29/2013 05:20 AM, Max Reitz wrote: > >>Implement bdrv_amend_options for compat, size, backing_file, backing_= fmt > >>and lazy_refcounts. > >> > >>Downgrading images from compat=3D1.1 to compat=3D0.10 is achieved thr= ough > >>handling all incompatible flags accordingly, clearing all compatible = and > >>autoclear flags and expanding all zero clusters. > >> > >>Signed-off-by: Max Reitz > >>--- > >>+/* > >>+ * Expands all zero clusters on the image; important for downgrading= to a qcow2 > >>+ * version which doesn't yet support metadata zero clusters. > >Do we have to fully write 0 blocks into the image no matter what, or a= re > >there cases where, when the file has a backing image and we know the > >backing image has 0 bytes at the same offset, where we could flatten b= y > >removing the cluster and letting the reference defer to the backing > >file? It's always safer to write 0 blocks into this image, but it may > >be worth considering whether we need the (ability) to try the alternat= e > >method as it results in a smaller file and potentially faster conversi= on. > This seems non-trivial to optimize to me (at least doing that check > fast), at least too non-trivial for implementing it solely for an > image version downgrade (which nobody who is concerned about image > size should do anyway, imho). >=20 > For non-backed images however, we could certainly just unallocate > the blocks, I guess, since the spec explicitly states for that case > that "if a cluster is unallocated, read requests [=E2=80=A6] shall read > zeros for all parts that are not covered by the backing file" (also > applies if there is no backing file at all). Yup, checking for !bs->backing_hd is easy, so simple deallocating in this case sounds like a good idea to do. Reading from the backing file and checking if the buffer is zero isn't _that_ complicated either, but at least the conversion speed won't be improved by doing this. If we already had Paolo'sbdrv_get_block_status, we could try that, but as it is today I don't think it's worth doing anything else here. Downgrading an image is an unusual operation anyway. Kevin