On Mon, Jul 27, 2020 at 5:14 PM Eric Blake <ebl...@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 <nsof...@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?