Re: [PATCH 2/2] qemu-iotests: Test convert to qcow2 compressed to NBD

[Nir Soffer]

On Mon, Jul 27, 2020 at 5:14 PM Eric Blake <eblake@redhat.com> wrote:
>
> On 7/27/20 5:04 AM, Max Reitz wrote:
> > On 26.07.20 17:25, Nir Soffer wrote:
> >> Add test for "qemu-img convert -O qcow2 -c" to NBD target. The use case
> >> is writing compressed disk content to OVA archive.
> >>
> >> Signed-off-by: Nir Soffer <nsoffer@redhat.com>
> >> ---
>
> >
> >> +# The use case is writing qcow2 image directly into a tar file. Code to 
> >> create
> >> +# real tar file not included.
> >> +#
> >> +# offset    content
> >> +# -------------------------------
> >> +#      0    first memebr header
> >
> > *member

Sorry for the typos, I need to setup automated spelling check :-)

> >
> >> +#    512    first member data
> >> +#   1024    second memeber header
> >
> > *member
> >
> >> +#   1536    second member data
> >> +
> >> +tar_file = file_path("test.tar")
>
> I guess it's okay that you don't create a real tar file here, but
> listing the commands to create it (even as a comment) is better than
> just saying "trust me".  And it doesn't seem like that much more work -
> it looks like the key to your test is that you created a tar file
> containing two files, where the first file was less than 512 bytes and
> the second file is your target destination that you will be rewriting.

The real code is more complicated, something like:

    offset = tar.fileobj.tell() + BLOCK_SIZE

    with open(tar.name, "r+") as f:
        f.truncate(offset + measure["required"])

    convert_image(image, tar.name, offset)

    check = check_image(tar.name, offset)
    size = check["image-end-offset"]

    member = tarfile.TarInfo(name)
    member.size = size
    tar.addfile(member)

    tar_size = offset + round_up(size)

    tar.fileobj.seek(tar_size)
    with open(tar.name, "r+") as f:
        f.truncate(tar_size)

I'm not sure it helps qemu developers working on these tests.

> >> +out = qemu_img_pipe("measure", "-O", "qcow2", "--output", "json", 
> >> src_disk)
> >> +measure = json.loads(out)
> >> +qemu_img_create("-f", "raw", tar_file, str(measure["required"]))
> >
> > Should this be measure["required"] + 1536?
>
> The test works without it (because of compression), but yes, if you are
> going to test writing into an offset, you should oversize your file by
> that same offset.

Right, in the real code using this I indeed use offset + required.

> >> +
> >> +nbd_sock = file_path("nbd-sock", base_dir=iotests.sock_dir)
> >> +nbd_uri = "nbd+unix:///exp?socket=" + nbd_sock
> >> +
> >> +# Use raw format to allow creating qcow2 directy into tar file.
> >> +qemu_nbd(
> >> +    "--socket", nbd_sock,
> >> +    "--persistent",
> >> +    "--export-name", "exp",
> >> +    "--format", "raw",
> >> +    "--offset", "1536",
> >> +    tar_file)
> >> +
> >> +iotests.log("=== Target image info ===")
> >> +qemu_img_log("info", nbd_uri)
> >> +
> >> +# Write image into the tar file. In a real applicatio we would write a tar
> >
> > *application
> >
>
> >> +=== Converted image check ===
> >> +No errors were found on the image.
> >> +1/160 = 0.62% allocated, 100.00% fragmented, 100.00% compressed clusters
> >> +Image end offset: 393216
> >
> > I hope none of this is fs-dependant.  (I don’t think it is, but who
> > knows.  I suppose we’ll find out.)
>
> Indeed - time to see what CI thinks of this.
>
> At any rate, given the urgency of getting pull requests for -rc2 in
> before slamming Peter tomorrow, I'll probably try to touch up the issues
> Max pointed out and queue it today.

Thanks Max and Eric.

Should I post a fixed version later today?