From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42545) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SgROy-0006Tf-FU for qemu-devel@nongnu.org; Sun, 17 Jun 2012 22:09:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SgROw-0007Hy-Fq for qemu-devel@nongnu.org; Sun, 17 Jun 2012 22:09:12 -0400 Received: from mail-we0-f173.google.com ([74.125.82.173]:52778) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SgROw-0007Hi-4f for qemu-devel@nongnu.org; Sun, 17 Jun 2012 22:09:10 -0400 Received: by werf3 with SMTP id f3so3864530wer.4 for ; Sun, 17 Jun 2012 19:09:07 -0700 (PDT) MIME-Version: 1.0 Sender: donald.open@gmail.com In-Reply-To: <4FD9C136.3020309@redhat.com> References: <1339598189-17933-1-git-send-email-wdongxu@linux.vnet.ibm.com> <4FD8AD4C.6020708@redhat.com> <4FD9C136.3020309@redhat.com> From: Dong Xu Wang Date: Mon, 18 Jun 2012 10:08:26 +0800 Message-ID: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/6 v10] docs: spec for add-cow file format List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Eric Blake , qemu-devel@nongnu.org On Thu, Jun 14, 2012 at 6:47 PM, Kevin Wolf wrote: > Am 14.06.2012 05:06, schrieb Dong Xu Wang: >> On Wed, Jun 13, 2012 at 11:10 PM, Eric Blake wrote: >>> On 06/13/2012 08:36 AM, Dong Xu Wang wrote: >>>> Introduce a new file format:add-cow. The usage can be found at this pa= tch. >>>> >>>> Signed-off-by: Dong Xu Wang >>>> --- >>>> =A0docs/specs/add-cow.txt | =A0 87 +++++++++++++++++++++++++++++++++++= +++++++++++++ >>>> =A01 files changed, 87 insertions(+), 0 deletions(-) >>>> =A0create mode 100644 docs/specs/add-cow.txt > >>>> + >>>> +=3D=3D Header =3D=3D >>>> + >>>> +The Header is included in the first bytes: >>>> + >>>> + =A0 =A0Byte =A0 =A00 - =A07: =A0 =A0 magic >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0add-cow magic string = ("ADD_COW\xff") >>>> + >>>> + =A0 =A0 =A0 =A0 =A0 =A08 - =A011: =A0 =A0version >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Version number (only = valid value is 1 now) >>>> + >>>> + =A0 =A0 =A0 =A0 =A0 =A012 - 15: =A0 =A0backing_filename_offset >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Offset in the add-cow= file at which the backing file name >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0is stored (NB: The st= ring is not null terminated). 0 if the >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0image doesn't have a = backing file. >>> >>> Mention that if this is not 0, then it must be between 36 and 4094 (a >>> file name must be at least 1 byte). =A0What are the semantics if the >>> filename is relative? >> >> relative filename is ok, I tested it just now. > > I believe Eric wanted to know what a relative path means, i.e. that it's > relative to the image file rather than relative to the working directory. > Now it is relative to working directory. >>>> + >>>> + =A0 =A0 =A0 =A0 =A0 =A016 - 19: =A0 =A0backing_filename_size >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Length of the backing= file name in bytes. Undefined if the >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0image doesn't have a = backing file. >>> >>> Better to require 0 if backing_filename_offset is 0, than to leave this >>> field undefined; also if backing_filename_offset is non-zero, then this >>> must be non-zero. =A0Must be less than 4096-36 to fit in the reserved p= art >>> of the header. >>> >> >> Okay. > > Does an add-cow image without a backing file even make sense? > Okay, I think so, it will be v11. >>>> + =A0 =A0 =A0 =A0 =A0 =A028 - 35: =A0 =A0features >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Currently only 2 feat= ure bits are used: >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Feature bits: >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0The image use= s a backing file: >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* ADD_COW_F_B= ACKING_FILE =3D 0x01. >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0The backing f= ile's format is raw: >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* ADD_COW_F_B= ACKING_FORMAT_NO_PROBE =3D 0x02. >>> >>> Should this follow the qcow2v3 proposal of splitting into mandatory vs. >>> optional feature bits? >>> >>> I agree that ADD_COW_F_BACKING_FORMAT_NO_PROBE is sufficient to avoid >>> security implications, but do we want the extra flexibility of >>> specifying the backing format file format rather than just requiring >>> probes on all but raw? >> >> Kevin, or Stefan, can you give some comments for this? thanks. > > I tend to agree that a format name is better than relying on probing. > > Also, I think we need the same thing for image_file. add-cow is not only > useful for raw images, but also for other image format types for which > we don't support backing files. > Okay. >>>> +=3D=3D Reserved =3D=3D >>>> + >>>> + =A0 =A0Byte =A0 =A036 - 4095: =A0Reserved field: >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0It is used to make su= re COW bitmap field starts at the >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A04096th byte, backing_= file name and image_file name will >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0be stored here. >>> >>> Do we want to keep a fixed-size header, or should we be planning on the >>> possibility of future extensions requiring enough other header >>> extensions that a variable-sized header would be wiser? =A0That is, I'm >>> fine with requiring that the header be a multiple of 4k, but maybe it >>> would make sense to have a mandatory header field that states how many >>> header pages are present before the COW bitmap begins. =A0In the first >>> round of implementation, this header field can be required to be 1 (tha= t >>> is, for now, we require exactly 4k header), but having the field would >>> let us change in the future to a design with an 8k header to hold more >>> metadata as needed. > > I have the impression that this simple add-cow hack is starting to get > seriously overengineered... :-) > > Kevin >