Comment 40 for bug 1847105

Revision history for this message
In , paelzer (paelzer-redhat-bugs) wrote :

Hi Cole,
thanks for the hint on that fix.
I have applied and tried it but it still uses falloc for the allocation calls from virt-manager.

I have traced a -O0 build in libvirt and see this:

Thread 6 "rpc-worker" hit Breakpoint 4, storageBackendCreateQemuImgOpts (encinfo=0x0, opts=0x7fd1791102a0, info=0x7fd179110380) at ../../../src/storage/storage_util.c:712

(gdb) p *info
$3 = {format = 14, type = 0x7fd17ef920f0 "qcow2", inputType = 0x0, path = 0x7fd164018440 "/var/lib/libvirt/images/ubuntu20.04-l.qcow2", size_arg = 2097152, allocation = 2097152,
  encryption = false, preallocate = true, compat = 0x7fd16400deb0 "1.1", features = 0x7fd16400e170, nocow = false, backingPath = 0x0, backingFormat = 0, inputPath = 0x0,
  inputFormatStr = 0x0, inputFormat = 0, secretAlias = 0x0}

And in particular:
(gdb) p info->preallocate
$4 = true
(gdb) p info->size_arg
$5 = 2097152
(gdb) p info->allocation
$6 = 2097152

Due to that the following
712 if (info->preallocate) {
713 if (info->size_arg > info->allocation)
714 virBufferAddLit(&buf, "preallocation=metadata,");
715 else
716 virBufferAddLit(&buf, "preallocation=falloc,");

Still decides for falloc as the values are now (due to the fix) equal but not greater than.

Per the description of the initial patch: "build them with preallocation=falloc whenever the XML described storage <allocation> matches its <capacity>" I wonder if we should change the libvirt code to something like:

 if (info.preallocate) {
     if (info.size_arg == info.allocation)
         virBufferAddLit(&buf, "preallocation=falloc,");
     else
         virBufferAddLit(&buf, "preallocation=metadata,");
 }

That code would then work and read exactly as the description did.

P.S. reopening the case