[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] qemu-iotests: Test convert to qcow2 compressed to NBD
From: |
Nir Soffer |
Subject: |
Re: [PATCH 2/2] qemu-iotests: Test convert to qcow2 compressed to NBD |
Date: |
Mon, 27 Jul 2020 17:35:28 +0300 |
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?