From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33964) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VF1i3-0001qQ-0X for qemu-devel@nongnu.org; Thu, 29 Aug 2013 08:52:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VF1hx-0001dg-18 for qemu-devel@nongnu.org; Thu, 29 Aug 2013 08:52:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:62302) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VF1hw-0001db-Pf for qemu-devel@nongnu.org; Thu, 29 Aug 2013 08:52:16 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r7TCqFnM007768 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 29 Aug 2013 08:52:15 -0400 Message-ID: <521F43FC.1060604@redhat.com> Date: Thu, 29 Aug 2013 14:52:12 +0200 From: Max Reitz MIME-Version: 1.0 References: <1377775241-26278-1-git-send-email-mreitz@redhat.com> <1377775241-26278-3-git-send-email-mreitz@redhat.com> <521F425D.5030400@redhat.com> In-Reply-To: <521F425D.5030400@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed 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: Eric Blake Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi 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_f= mt >> and lazy_refcounts. >> >> Downgrading images from compat=3D1.1 to compat=3D0.10 is achieved thro= ugh >> handling all incompatible flags accordingly, clearing all compatible a= nd >> 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 ar= e > 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 by > 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 alternate > method as it results in a smaller file and potentially faster conversio= n. This seems non-trivial to optimize to me (at least doing that check=20 fast), at least too non-trivial for implementing it solely for an image=20 version downgrade (which nobody who is concerned about image size should=20 do anyway, imho). For non-backed images however, we could certainly just unallocate the=20 blocks, I guess, since the spec explicitly states for that case that "if=20 a cluster is unallocated, read requests [=E2=80=A6] shall read zeros for = all=20 parts that are not covered by the backing file" (also applies if there=20 is no backing file at all). >> + >> + /* the refcount order might be different in newer images - howeve= r, qemu >> + * doesn't support anything different than 4 anyway, so nothing t= o fix >> + * there */ > This sounds risky. Wouldn't it be safer to error out if the image > didn't have a refcount order of 4, than to just ignore it; on the > grounds that if qemu DOES add support for non-4 refcount order, an erro= r > will at least alert someone to the fact that they need to add some > (potentially complicated) code here? > Oh, yes, of course. I'll fix it. Max