From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48016) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SEe0i-0001yX-42 for qemu-devel@nongnu.org; Mon, 02 Apr 2012 05:57:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SEe0a-0007gi-TS for qemu-devel@nongnu.org; Mon, 02 Apr 2012 05:57:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50194) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SEe0a-0007gY-Ku for qemu-devel@nongnu.org; Mon, 02 Apr 2012 05:57:08 -0400 Message-ID: <4F7978CB.9040005@redhat.com> Date: Mon, 02 Apr 2012 12:00:43 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1332860615-3047-1-git-send-email-kwolf@redhat.com> <1332860615-3047-2-git-send-email-kwolf@redhat.com> <4F71E9EA.1070306@redhat.com> In-Reply-To: <4F71E9EA.1070306@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 01/16] Specification for qcow2 version 3 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: stefanha@gmail.com, qemu-devel@nongnu.org Am 27.03.2012 18:25, schrieb Eric Blake: > On 03/27/2012 09:03 AM, Kevin Wolf wrote: >> This is the second draft for what I think could be added when we increase qcow2's >> version number to 3. This includes points that have been made by several people >> over the past few months. We're probably not going to implement this next week, >> but I think it's important to get discussions started early, so here it is. >> > >> +If the version is 3 or higher, the header has the following additional fields. >> +For version 2, the values are assumed to be zero, unless specified otherwise >> +in the description of a field. >> + >> + 72 - 79: incompatible_features >> + Bitmask of incompatible features. An implementation must >> + fail to open an image if an unknown bit is set. >> + >> + Bit 0: The reference counts in the image file may be >> + inaccurate. Implementations must check/rebuild >> + them if they rely on them. >> + >> + Bit 1: Enable subclusters. This affects the L2 table >> + format. >> + >> + Bits 2-31: Reserved (set to 0) > > Offsets 72-79 forms 8 bytes, so this should be bits 2-63 are reserved. Thanks, good catch! This was a 32 bit field initially and when I updated it, I forgot this. >> + >> + 96 - 99: refcount_bits >> + Size of a reference count block entry in bits. For version 2 >> + images, the size is always assumed to be 16 bits. The size >> + must be a power of two. >> + [ TODO: Define order in sub-byte sizes ] >> + >> + 100 - 103: header_length >> + Length of the header structure in bytes. For version 2 >> + images, the length is always assumed to be 72 bytes. > > Might be a good idea to require this to be a multiple of 8, since both > 72 and 104 qualify, and since header extensions are also required to be > padded out to multiples of 8. Do you see any arguments for padding to multiples of 8 besides consistency? If I did the format from scratch, without having to pay attention to compatibility, I would drop the requirement even for header extensions as I don't see what it buys us. Consistency is important and certainly good enough to make me unsure about this, but I don't like artificial restrictions either. If we had another good reason, it would be easier for me to decide. >> +== Feature name table == >> + >> +A feature name table is an optional header extension that contains the name for >> +features used by the image. It can be used by applications that don't know >> +the respective feature (e.g. because the feature was introduced only later) to >> +display a useful error message. >> + >> +The number of entries in the feature name table is determined by the length of >> +the header extension data. Its entries look like this: >> + >> + Byte 0: Type of feature (select feature bitmap) >> + 0: Incompatible feature >> + 1: Compatible feature >> + 2: Autoclear feature >> + >> + 1: Bit number within the selected feature bitmap >> + >> + 2 - 47: Feature name (padded with zeros, but not necessarily null >> + terminated if it has full length) > > Semantic nit: The NUL character is all zeros; it is one byte in all > unibyte and multi-byte encodings, and the NUL wide character is the > all-zero wchar_t value; while 'null' refers to a pointer to nowhere. > Saying a string is null terminated is wrong, because you don't have a 4- > or 8-byte NULL pointer at the end of the string, just a one-byte NUL > character. Therefore, strings are nul-terminated, not null-terminated. "null-terminated" is much more common. Google and Wikipedia are the proof. ;-) > Is this extension capped at 48 bytes, or it is a repeating table of as > many 48-byte multiples as necessary to represent each feature name? The latter. All feature names are in a single table in a single header extensions. Any suggestion how to clarify this? Would something like "There shall be at most one feature name table header extension in an image" be clear enough? Kevin