On Sat, Dec 31, 2011 at 9:17 AM, Dong Xu Wang <wdon...@linux.vnet.ibm.com> wrote: > On Fri, Dec 30, 2011 at 22:09, Stefan Hajnoczi <stefa...@gmail.com> wrote: >> On Thu, Dec 29, 2011 at 05:36:59PM +0800, Dong Xu Wang wrote: >> > + ret = bdrv_file_open(&backing_bs, bs->backing_file, 0); >> > + if (ret < 0) { >> > + return ret; >> > + } >> > + bdrv_delete(backing_bs); >> >> What does this do? (It leaks s->bitmap when it fails.) > > > I wanna make sure backing_file exists while opening.
block.c:bdrv_open() opens the backing file after .bdrv_open() has returned. It fails if the backing file does not exist. There's no need to duplicate this. >> > + s->image_hd = bdrv_new(""); >> > + >> > + if (path_has_protocol(s->image_file)) { >> > + pstrcpy(image_filename, sizeof(image_filename), >> > + s->image_file); >> > + } else { >> > + path_combine(image_filename, sizeof(image_filename), >> > + bs->filename, s->image_file); >> > + } >> > + >> > + image_drv = bdrv_find_format("raw"); >> > + ret = bdrv_open(s->image_hd, image_filename, flags, image_drv); >> > + if (ret < 0) { >> > + bdrv_delete(s->image_hd); >> > + s->image_hd = NULL; >> > + goto fail; >> > + } >> >> TODO If you were wondering why I put "TODO" here it's because I had some thoughts when reviewing but didn't fully investigate it yet. When I send the rest of my feedback I'll include my comment here. (I should have removed this before replying :)) >> > + 1036 - 2059: image_file >> > + image_file is a raw file, While using >> > image_file, must >> > + together with image_file. All unused bytes are >> > padded >> >> "While using image_file, must together with image_file" >> >> What does this mean? > > > I mean while using add-cow, must together with image_file and backing_file. > Both of them can not be missing. > Errors with sentences like that, I will correct them in v7. That sounds like qemu-img create behavior which should not be part of the file format specification. The only impact on the file format speficiation is that backing_file can be an empty string but image_file must always be a valid filename.