very slow disk creation, snapshotting

Bug #1847105 reported by Andreas Hasenack
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Native ZFS for Linux
New
Unknown
virt-manager
Fix Released
Low
libvirt (Ubuntu)
Low
Unassigned
Bionic
Undecided
Unassigned
Disco
Low
Unassigned
Focal
Undecided
Unassigned
Groovy
Low
Unassigned
virt-manager (Ubuntu)
Low
Unassigned
Bionic
Undecided
Unassigned
Disco
Low
Unassigned
Focal
Undecided
Unassigned
zfs-linux (Ubuntu)
Undecided
Unassigned
Bionic
Undecided
Unassigned
Disco
Undecided
Unassigned

Bug Description

[Impact]

 * The defaults of virt-manager for disk allocation always worked fine
   when qcow2 had nothing but sparse support. So sparse=True vs
   sparse=False made no difference. So it always set sparse=False.
   Then qcow2 grows non-sparse support, and virt-manager is suddenly
   defaulting to it, which is not the intention.

 * For upgraders from pre-Focal this was a regression in two ways:
    a) allocation of qcow changed to non-sparse (fallocate based)
       potentially consuming more space
    b) depending on the underlying FS this made the allocation much
       slower

 * Fix by applying the upstream change that Defaults to sparse when
   requested format isn't raw (for raw the assumption ill stay that
   users do that for runtime performance, so non-sparse on those stays
   as-is)

[Test Case]

 * open virt-manager and create a new guest which includes creating a
   new image for it
   * when clicking "finish" the image will be created (which e.g. on ZFS
     will take quite some time)
   * In the background grep ps output if the qemu-img call for qcow (the
     default) is using "preallocation=fallocate" (wrong) or
     "preallocation=metadata" (correct)

[Regression Potential]

 * For upgraders from pre-focal it actually ensures behavior stays as is
   (no change/regression there)
 * For users of ZFS where the allocation was slow it fixes this
   slowness.
 * But for people using Focal all the time and on non-ZFS there will be
   a behavior change in no more doing falloc pre-allocating the qemu
   image. To be cleear there are four modes:
   1. no allocation
   2. metadata allocation - is initially larger but can improve
      performance when the image needs to grow
   3. falloc - preallocates space for image by calling posix_fallocate
   4. full - writes data to the underlying storage
   The fix changes the default from 3->2 which it always was and should
   be. As the patch says: "If users want to override it, they can
   do it via custom created storage."
   (To be clear, upstream changed this as we fixed it here, so on
    further upgrades what we apply here will be the behavior anyway, it
    seems wrong to keep Focal the only one in between kept different)
 * [racb] The code path being added special cases the 'raw' format and in handling of the default sparse setting. Areas of potential regression are therefore in the handling of the default and override of the default in the cases of both 'raw' and not 'raw'.

[Other Info]

 * The slowness effect might be further mitigated by ZFS implementing
   more falloc options (thanks Richard for that hint) but that won't be
   in Focal.

----

This is a regression in eoan for me. I use virt-manager to create vms, and I noticed that creating one now takes more than a minute.

Looking at the process listing while the backing disk is being created, I see this qemu-img command line:
15658 ? Ssl 0:00 /usr/sbin/libvirtd
23726 ? Sl 0:04 \_ /usr/bin/qemu-img create -f qcow2 -o preallocation=falloc,compat=1.1,lazy_refcounts /var/lib/libvirt/images/live-server.qcow2 41943040K

If I run qemu-img with that preallocation parameter set, even on bionic, then it also takes a very long time.

On eoan, for comparison:
andreas@nsn7:~$ time qemu-img create -f qcow2 no-prealloc-image.qcow2 40G
Formatting 'no-prealloc-image.qcow2', fmt=qcow2 size=42949672960 cluster_size=65536 lazy_refcounts=off refcount_bits=16

real 0m0,016s
user 0m0,010s
sys 0m0,006s
andreas@nsn7:~$ qemu-img info no-prealloc-image.qcow2
image: no-prealloc-image.qcow2
file format: qcow2
virtual size: 40G (42949672960 bytes)
disk size: 17K
cluster_size: 65536
Format specific information:
    compat: 1.1
    lazy refcounts: false
    refcount bits: 16
    corrupt: false
andreas@nsn7:~$ du -hs no-prealloc-image.qcow2
17K no-prealloc-image.qcow2
andreas@nsn7:~$

and now with preallocation=falloc:
andreas@nsn7:~$ time qemu-img create -f qcow2 -o preallocation=falloc with-prealloc-image.qcow2 40G
Formatting 'with-prealloc-image.qcow2', fmt=qcow2 size=42949672960 cluster_size=65536 preallocation=falloc lazy_refcounts=off refcount_bits=16

real 1m43,196s
user 0m3,564s
sys 1m26,720s
andreas@nsn7:~$ qemu-img info with-prealloc-image.qcow2
image: with-prealloc-image.qcow2
file format: qcow2
virtual size: 40G (42949672960 bytes)
disk size: 2.7M
cluster_size: 65536
Format specific information:
    compat: 1.1
    lazy refcounts: false
    refcount bits: 16
    corrupt: false
andreas@nsn7:~$ du -hs with-prealloc-image.qcow2
2,8M with-prealloc-image.qcow2
andreas@nsn7:~$

ProblemType: Bug
DistroRelease: Ubuntu 19.10
Package: libvirt-daemon 5.4.0-0ubuntu5
ProcVersionSignature: Ubuntu 5.3.0-13.14-generic 5.3.0
Uname: Linux 5.3.0-13-generic x86_64
NonfreeKernelModules: zfs zunicode zavl icp zcommon znvpair
ApportVersion: 2.20.11-0ubuntu7
Architecture: amd64
Date: Mon Oct 7 11:36:03 2019
InstallationDate: Installed on 2019-10-07 (0 days ago)
InstallationMedia: Ubuntu 19.10 "Eoan Ermine" - Beta amd64 (20191006)
SourcePackage: libvirt
UpgradeStatus: No upgrade log present (probably fresh install)
modified.conffile..etc.libvirt.nwfilter.allow-arp.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/allow-arp.xml']
modified.conffile..etc.libvirt.nwfilter.allow-dhcp-server.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/allow-dhcp-server.xml']
modified.conffile..etc.libvirt.nwfilter.allow-dhcp.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/allow-dhcp.xml']
modified.conffile..etc.libvirt.nwfilter.allow-incoming-ipv4.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/allow-incoming-ipv4.xml']
modified.conffile..etc.libvirt.nwfilter.allow-ipv4.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/allow-ipv4.xml']
modified.conffile..etc.libvirt.nwfilter.clean-traffic-gateway.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/clean-traffic-gateway.xml']
modified.conffile..etc.libvirt.nwfilter.clean-traffic.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/clean-traffic.xml']
modified.conffile..etc.libvirt.nwfilter.no-arp-ip-spoofing.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/no-arp-ip-spoofing.xml']
modified.conffile..etc.libvirt.nwfilter.no-arp-mac-spoofing.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/no-arp-mac-spoofing.xml']
modified.conffile..etc.libvirt.nwfilter.no-arp-spoofing.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/no-arp-spoofing.xml']
modified.conffile..etc.libvirt.nwfilter.no-ip-multicast.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/no-ip-multicast.xml']
modified.conffile..etc.libvirt.nwfilter.no-ip-spoofing.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/no-ip-spoofing.xml']
modified.conffile..etc.libvirt.nwfilter.no-mac-broadcast.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/no-mac-broadcast.xml']
modified.conffile..etc.libvirt.nwfilter.no-mac-spoofing.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/no-mac-spoofing.xml']
modified.conffile..etc.libvirt.nwfilter.no-other-l2-traffic.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/no-other-l2-traffic.xml']
modified.conffile..etc.libvirt.nwfilter.no-other-rarp-traffic.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/no-other-rarp-traffic.xml']
modified.conffile..etc.libvirt.nwfilter.qemu-announce-self-rarp.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/qemu-announce-self-rarp.xml']
modified.conffile..etc.libvirt.nwfilter.qemu-announce-self.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/nwfilter/qemu-announce-self.xml']
modified.conffile..etc.libvirt.qemu.conf: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/qemu.conf']
modified.conffile..etc.libvirt.qemu.networks.default.xml: [inaccessible: [Errno 13] Permission denied: '/etc/libvirt/qemu/networks/default.xml']

Related branches

Revision history for this message
Andreas Hasenack (ahasenack) wrote :
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

A quick check confirmed your report, thanks Andreas!

"preallocation" does exactly what expected and that takes the time you are seeing:
Preallocation mode (allowed values: "off", "falloc", "full"). "falloc" mode preallocates space for image by calling posix_fallocate(). "full" mode preallocates space for image by writing zeros to underlying storage.

It already uses fallocate which is better than writing zeros and much faster depending on the FS setup. But obviously bother are slower (on creation) than lazy allocation later.

The change of this should be in virt-manager, not libvirt I'd assume.
I'll take a look.

P.S. already ahead of time I'd think that creating with pre-alloc - while slower - is certainly much safer against later odd errors due to disk space.

Changed in libvirt (Ubuntu):
status: New → Confirmed
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Interesting - at first the only related change seems to be in libvirt.
Function storageBackendCreateQemuImgOpts of src/storage/storage_util.c.
It is in there since [1] which on itself makes sense to me.
Also that particular change is in libvirt since v4.3 which means >=Cosmic and we haven't had major bugs in those releases nor in e.g. cloud archive backports of the same due to it.

Looking forther points in virt-manager to [2] but that is from 2013, but looking at recent related code shows [3] which is only in since v2.2.0 and that would match.

That is in /usr/share/virt-manager/virtinst/storage.py of binary "virtinst" of src:virt-manager.

From debug we see the config virt-manager defines for that disk:
virt-manager 8272] DEBUG (storage:643) Creating storage volume with xml:
<volume>
  <name>ubuntu19.04.qcow2</name>
  <capacity>16106127360</capacity>
  <allocation>16106127360</allocation>
  <target>
    <format type="qcow2"/>
    <features>
      <lazy_refcounts/>
    </features>
  </target>
</volume>

Which matches what libvirt receives:
debug : virStorageVolCreateXML:1499 : pool=0x2867ae0, xmlDesc=<volume>
  <name>ubuntu19.04.qcow2</name>
  <capacity>16106127360</capacity>
  <allocation>16106127360</allocation>
  <target>
    <format type="qcow2"/>
    <features>
      <lazy_refcounts/>
    </features>
  </target>
</volume>
, flags=0x1

[1]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=c9ec7088c7a3f4cd26bb471f1f243931fff6f4f9
[2]: https://github.com/virt-manager/virt-manager/commit/37350859ce6c46a1c7e6562db8e242a97bcf5bb3
[3]: https://github.com/virt-manager/virt-manager/commit/4f66c423f7833e270b61536d53a0772ce1242abc

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Checking that in Disco confirms the same behavior:

[Di, 08 Okt 2019 07:19:27 virt-manager 12667] DEBUG (storage:719) Creating storage volume 'ubuntu19.04.qcow2' with xml:
<volume>
  <name>ubuntu19.04.qcow2</name>
  <capacity>16106127360</capacity>
  <allocation>16106127360</allocation>
  <target>
    <format type="qcow2"/>
    <features>
      <lazy_refcounts/>
    </features>
  </target>
</volume>

Into:
/usr/bin/qemu-img create -f qcow2 -o preallocation=falloc,compat=1.1,lazy_refcounts /var/lib/libvirt/images/ubuntu19.04.qcow2 15728640K

So this might be - after all - the libvirt change in >=Cosmic?

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

In 18.04 it still passes the same XML for creation, but is fast (as expected/reported).
Exchanging just the libvirt e.g. from UCA Rocky triggers the new behavior.

Changed in virt-manager (Ubuntu):
status: New → Triaged
Changed in libvirt (Ubuntu):
status: Confirmed → Triaged
Changed in libvirt (Ubuntu Bionic):
status: New → Invalid
Changed in virt-manager (Ubuntu Bionic):
status: New → Invalid
Changed in virt-manager (Ubuntu Disco):
status: New → Triaged
Changed in libvirt (Ubuntu Disco):
status: New → Triaged
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Some updates for proper tracking.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

With the older (pre 4.3) libvirt in place this creates it with metadata preallocation.

/usr/bin/qemu-img create -f qcow2 -o preallocation=metadata,compat=1.1,lazy_refcounts /var/lib/libvirt/images/ubuntu18.04.qcow2 15728640K

In the related code this looks like:
 834 if (info.size_arg > info.allocation)
 835 virBufferAddLit(&buf, "preallocation=metadata,");
 836 else
 837 virBufferAddLit(&buf, "preallocation=falloc,");

And size_arg == capacity (from xml).
Per definition [1] we have three cases for "allocation":
a) allocation can be bigger than capacity for high-metadata overhead cases
b) If allocation is omitted when creating a volume, the volume will be fully allocated at time of
   creation.
c) If set to a value smaller than the capacity, the pool has the option of deciding to sparsely
   allocate a volume.

But as we see in the XMLs above virt-manager asks to fully allocate. We hit neither of (a)/(b).
Therefore the pool has -NOT- the option to do a sparse allocation as we also do not hit (c)

It isn't written/defined explicitly in those statements, but our case of capacity==allocation should (form the text) behave like (b) at least in my reading. It didn't do so in the past and the libvirt change [2] fixed that unclarity.

I don't think we'd want to change libvirt to go back to not allocate in this case.
But we should report and ask at virt-manager if it should/want to submit a smaller allocation 0<=allocation<=capacity to get back to the old behavior.

virt-manager upstream then has a chance to agree and fix it (which we would backport) or discuss pointing to a different reading of [1] that would end up in a libvirt discussion and fix then.

[1]: https://libvirt.org/formatstorage.html#StorageVol
[2]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=c9ec7088c7a3f4cd26bb471f1f243931fff6f4f9

Changed in virt-manager (Ubuntu):
importance: Undecided → Medium
Changed in virt-manager (Ubuntu Disco):
importance: Undecided → Medium
Changed in libvirt (Ubuntu Disco):
importance: Undecided → Low
Changed in libvirt (Ubuntu):
importance: Undecided → Low
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Since (for now) I assume it is a virt-manager bug I'll mark that medium and libvirt as low.
It isn't higher as no other component has triggered it for a full cycle of Disco and UCA-Rocky in the field as well as being "only" slower with the benefit of actually being faster later.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Reported at https://github.com/virt-manager/virt-manager/issues/56
Unfortunately LP can only conder one bug tracker for a project but virt-manager has bugzilla AND github-issues so I can't link it as-is.

Maybe I should report it in BZ instead ...

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

## Description of problem: ##
Hi,
in Ubuntu bug 1847105 [1] I think I have found a somewhat unexpected or unclear behavior mismatch between libvirt `>v4.3` and virt-manager.

For volume creation virt-manager since quite some time (I checked back until `v1.5`) will submit an XML like the following to libvirts volume creation:

<volume>
  <name>ubuntu19.04.qcow2</name>
  <capacity>16106127360</capacity>
  <allocation>16106127360</allocation>
  <target>
    <format type="qcow2"/>
    <features>
      <lazy_refcounts/>
    </features>
  </target>
</volume>

The important detail in regard to this issues is that capacity==allocation.
In the definition by libvirt [2] this is somewhat undefined. In my reading of it this case would also fully allocate the volume.

But - and here is the change in behavior - up until libvirt 'v4.3' it didn't.
It used to use `preallocation=metadata` until a change [3] made it issue `preallocation=falloc` nowadays.

eventual qemu-img call:
Old: `/usr/bin/qemu-img create -f qcow2 -o preallocation=metadata,compat=1.1,lazy_refcounts /var/lib/libvirt/images/ubuntu18.04.qcow2 15728640K`
New: `/usr/bin/qemu-img create -f qcow2 -o preallocation=falloc,compat=1.1,lazy_refcounts /var/lib/libvirt/images/live-server.qcow2 41943040K`

## Version-Release number of selected component (if applicable): ##
v1.5 - v2.2.1

## How reproducible: ##
100%

## Steps to Reproduce: ##
1. create a new guest (use PXE to not require any ISO)
2. when you hit "Finish" to start the installation it will issue a qemu-img command
3. with newer libvirt versions this will use preallocation=falloc

## Actual results: ##
qemu-img ... preallocation=falloc

## Expected results: ##
qemu-img ... preallocation=metadata (like in the past)
OR a statement that falloc is preferred by virt-manager as well and not just an accident due to a change in libvirt.

## Next steps: ##
I was now wondering about virt-managers position on this, do you:
 * want to modify virt-manager to get back to spare/lazy allocations
   (TL;DR submit an allocation value that is `0<allocation<capacity`?
 * consider this change a bug in libvirt, so that we should report/change
   it there? Maybe the comparison there should be `>=` and not just `>`
   in your opinion?
 * not changing anything as the `falloc` creation is preferred from the
   POV of virt-manager
 * some other way answer I didn't predict :-)

[1]: https://bugs.launchpad.net/ubuntu/+source/virt-manager/+bug/1847105
[2]: https://libvirt.org/formatstorage.html#StorageVol
[3]: https://libvirt.org/git/?p=libvirt.git;a=blobdiff;f=src/storage/storage_util.c;h=897dfdaaee4da967fb5dacc4327484f40b8aa717;hp=b4aed0f7009b8d7e81a64e4f4a5c920a422fbba4;hb=c9ec7088c7a3f4cd26bb471f1f243931fff6f4f9;hpb=c1bc9c662b411f8fa06071991921a180480a0f40

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

FYI: initially reported in https://github.com/virt-manager/virt-manager/issues/56 but then I realized that officially BZ is most likely the correct bug-tracker

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Revision history for this message
In , berrange (berrange-redhat-bugs) wrote :

This behaviour changed due to this commit:

commit c9ec7088c7a3f4cd26bb471f1f243931fff6f4f9
Author: Wim ten Have <email address hidden>
Date: Mon Apr 9 20:14:35 2018 +0200

    storage: extend preallocation flags support for qemu-img

    This patch adds support to qcow2 formatted filesystem object storage by
    instructing qemu-img to build them with preallocation=falloc whenever the
    XML described storage <allocation> matches its <capacity>. For all other
    cases the filesystem stored objects are built with preallocation=metadata.

    Signed-off-by: Wim ten Have <email address hidden>
    Signed-off-by: Michal Privoznik <email address hidden>

This is a semantic change, so should not have been allowed.

It has been 1 & 1/2 years already, but I'm wondering if we should none the less revert it and instead add some other way to declare which preallocation strategy to use.

Changed in virt-manager:
importance: Unknown → Undecided
status: Unknown → Confirmed
Revision history for this message
In , paelzer (paelzer-redhat-bugs) wrote :

Thanks Daniel,
yes that is the patch I was referring to.

If you really think reverting it in libvirt is the right path I'm fine following you on that and keep virt-manager untouched in that regard.
But as you say it is in the field for quite a while and other than this report I haven't seen any.
Therefore I wasn't going to suggest a revert, after that much time it almost is a semantic change "again".

Furthermore the reasoning to add it back in [1] was with virt-inst / virt -manager in mind and the exact definition in [2] IMHO is a bit weak for this particular case.
That was the reason I asked for guidance from virt-managers POV first.

If we end up reverting the change we might consider modifying the text in [2] to be more clear what is (expected) to happen if allocation==capacity.

[1]: https://www.redhat.com/archives/libvir-list/2018-April/msg00130.html
[2]: https://libvirt.org/formatstorage.html#StorageVol

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

I mentioned this only in the bug title, let me clarify a bit. Snapshotting that machine (via virt-manager again) also takes over a minute.

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

Looking at the launchpad bug I see the original complaint was the disk creation was slow. This is somewhat surprising to me. falloc should be fast on any mainstream, modern filesystem as it merely has to write metadata in the FS to reserve the new blocks. I don't see any report of what host filesystem was in use when the user saw > 1 minute disk allocation. It would be worth investigating that aspect more, because my expectation is that falloc should be negligible in time.

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

Hi Daniel,
yes falloc should be fast in many cases, but also isn't in others.

The reporter used this directly on a ZFS system.
I used to recreate the slowness on stacked containers which is EXT4 -> ZFS-image file -> LXD container -> libvirt.
So far it seems we can assume that ZFS is slow in this use case.

I agree that on ext4 and tmpfs this is <0.02 real time.
So to face the slowdown might have a ZFS component.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Andreas and I checked the effect of backing store to this as upstream correctly mentioned that fallocate is supposed to be rather fast.

SLOW: EXT4 -> ZFS-image file -> LXD container -> libvirt
SLOW: Host-on-ZFS -> libvirt
Fast: Host-on-ext4 -> libvirt
Fast: Host-on-tmpfs -> libvirt

It seems we can assume that ZFS is required to get the slowdown-effect in this use case.

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

Yes, it seems fallocate on ZFS is problematic:

  https://github.com/zfsonlinux/zfs/issues/326

With that in mind, the right answer for ZFS is probably for virt-manager/virt-install to NOT ask for any preallocation at all in the ZFS case.

The fun of course is how does virt-manager know whether the image it is about to create is on ZFS or not. This might point to an RFE for storage pools to report whether a given storage pool instance is capable of efficiently preallocating or not.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

ZFS has https://github.com/zfsonlinux/zfs/issues/326
Which as TL;DR: fallocate is only supported with certain flags, and not really matches the ZFS COW design. That might explain the slowness, as the fallback is to write zeros.

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

Yep Daniel, just found the same discussion a few minutes ago and updated the Ubuntu bug.

While correct, your suggestion of checking pool features for this seems very complex for the trivial issue that we see atm.

Maybe (only maybe) virt-manager would want to sparse-alloc always and could then change to not request allocation==capacity.
From most common virt-manager use cases I know of sparse would make more sense and be faster (at creation).
I'd expect non-sparse to be a special case that maybe gets a way to be specified if really needed.

And also on the libvirt side we still might consider reverting the commit that broke behavior which would render all the effort to flag those features at pools not needed (at least not for this).

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

Creating non-sparse images is a good thing as a default, because behaviour when sparse images fail on disk full is not transparent to users.

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

Ok, but maybe virt-manager should then get a switch to only create it as sparse if the user wants it to be fast.
Many people use virt-manager to quickly test things, so I'd assume often disks are created, only partially used for a very short time and then thrown away.

But I agree in general and the additional fact that the "pain" only comes in if this is also on ZFS makes it maybe too special to be of high importance.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

FYI - it seems upstream of libvirt/virt-manager settles on the new behavior actually being preferable. I tend to agree, and the pain only happens if you also run things on ZFS which would be resolved if [1] is ever fully resolved.

Adding a ZFS task for that.

[1]: https://github.com/zfsonlinux/zfs/issues/326

Changed in virt-manager (Ubuntu):
importance: Medium → Low
Changed in virt-manager (Ubuntu Disco):
importance: Medium → Low
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thanks Andreas for already commenting on the upstream ZFS issue!

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

I think the discussion above summarizes this for virt-manager well.
I'll lower the prio of the remaining task for virt-manager which is "add a UI switch to request creating images as sparse file"

@Daniel - for libvirt do you still consider reverting the changes around c9ec7088 as it was a semantic change?
And if so would you want an extra bug or ML post for it?
Or has your opinion changed due to the discussion we had here?

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

I'm honestly not sure about libvirt side. I think it is worth a mail on the list to discuss it more broadly.

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

I've commented upstream (with ZFS) that we should fake the pre-allocation (i.e. return success from fallocate() when mode == 0) because with ZFS it's worthless at best and counterproductive at worst:

https://github.com/zfsonlinux/zfs/issues/326#issuecomment-540162402

Replies (agreeing or disagreeing) are welcome there. If you only want to say "I agree", please use the emoji button (the + icon) on my comment to show your support rather than spamming the issue tracker with "me too" type comments.

Changed in virt-manager:
importance: Undecided → Low
Revision history for this message
In , dario.valenzano (dario.valenzano-redhat-bugs) wrote :

You may just help the user making an informed decision with all the pros and cons explained in the user interface. Such behavior, common to other virtualization solutions, was beautifully fulfilled before being removed with this commit: https://github.com/virt-manager/virt-manager/commit/15a6a7e2105440df528f75c4df4d2471df28bd1e#diff-03b5bf10a38d88f4a09a14e211c23e05. Every time I create a new virtual machine I need to use the command line to generate a dynamically allocated storage, which is not good.

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

*** Bug 1789853 has been marked as a duplicate of this bug. ***

Changed in zfs:
status: Unknown → New
Steve Langasek (vorlon)
Changed in libvirt (Ubuntu Disco):
status: Triaged → Won't Fix
Changed in virt-manager (Ubuntu Disco):
status: Triaged → Won't Fix
Changed in zfs-linux (Ubuntu Disco):
status: New → Won't Fix
Revision history for this message
In , crobinso (crobinso-redhat-bugs) wrote :

I think this was fixed in libvirt upstream, please reopen and correct me if I'm wrong

commit 81a3042a12c7c06adc8e95264b6143b2eeb4953f
Author: Pavel Hrdina <email address hidden>
Date: Tue Aug 25 15:09:53 2020 +0200

    storage_util: fix qemu-img sparse allocation

    Commit <c9ec7088c7a3f4cd26bb471f1f243931fff6f4f9> introduced a support
    to fully allocate qcow2 images when <allocation> matches <capacity> but
    it doesn't work as expected.

    The issue is that info.size_arg is in KB but the info.allocation
    introduced by the mentioned commit is in B. This results in using
    "preallocation=falloc," in cases where "preallocation=metadata," should
    be used.

    Signed-off-by: Pavel Hrdina <email address hidden>
    Reviewed-by: Michal Privoznik <email address hidden>

Changed in virt-manager:
status: Confirmed → Fix Released
Changed in libvirt (Ubuntu Focal):
status: New → Triaged
Revision history for this message
Colin Ian King (colin-king) wrote :

The underlying fallocate mode/0 compat was added to upstream ZFS with the following commit:

commit f734301d2267cbb33eaffbca195fc93f1dae7b74
Author: adilger <email address hidden>
Date: Thu Jun 18 12:22:11 2020 -0600

    linux: add basic fallocate(mode=0/2) compatibility

While the change isn't too large for a SRU, I'd rather wait for this to land in the next release of ZFS in Ubuntu in 21.04 and see how well this new functionality works before backporting it to Groovy and Focal.

Changed in zfs-linux (Ubuntu Bionic):
status: New → Won't Fix
Changed in zfs-linux (Ubuntu):
importance: Undecided → Low
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Tracing qemu-img calls when creating a new disk image from virt-manager:

Before change the qemu-img that libvirt emits due to the XML from virt-manager is:
$ /usr/bin/qemu-img create -f qcow2 -o preallocation=falloc,compat=1.1,lazy_refcounts /var/lib/libvirt/images/ubuntu20.04.qcow2 26214400K

And as we know from before, if you run that on ZFS that is slower than one would hope.

Trying the libvirt build from the PPA.
4 0 162200 142352 20 0 163100 5120 poll_s Sl ? 0:03 \_ /usr/bin/qemu-img create -f qcow2 -o preallocation=falloc,compat=1.1,lazy_refcounts /var/lib/libvirt/images/ubuntu20.04-b.qcow2 26214400K

That is still as slow since it is the same qemu-img call.
Despite [1] being applied.

Actually looking at it more critically

Let's check what happens in that function
break on virStorageBackendCreateQemuImgCmdFromVol in gdb with source and debug symbols from the PPA in place.
Thanks to the compiler we don't see a lot:
(gdb) p info
$1 = {format = <optimized out>, type = 0x7fb98e58bdbc "qcow2", inputType = <optimized out>, path = <optimized out>, size_arg = <optimized out>, allocation = <optimized out>,
  encryption = <optimized out>, preallocate = <optimized out>, compat = <optimized out>, features = <optimized out>, nocow = <optimized out>, backingPath = <optimized out>,
  backingFormat = <optimized out>, inputPath = <optimized out>, inputFormatStr = <optimized out>, inputFormat = <optimized out>, secretAlias = <optimized out>}

b virStorageBackendCreateQemuImgCmdFromVol
b storageBackendCreateQemuImgOpts
b storageBackendCreateQemuImgSetOptions
b ../../../src/storage/storage_util.c:712

712 if (info->preallocate) {
713 if (info->size_arg > info->allocation)
714 virBufferAddLit(&buf, "preallocation=metadata,");
715 else
716 virBufferAddLit(&buf, "preallocation=falloc,");
(gdb) p info->preallocate
$1 = <optimized out>
(gdb) p info->size_arg
$2 = <optimized out>
(gdb) p info->allocation
$3 = <optimized out>

Hrm, this needs an -O0 build or I'll go crazy

[1]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=81a3042a12c7c06

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

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

So we now have equal numbers, but not bigger.
Therefore falloc is chosen still.

I have to ping on the upstream bug if that is expected.

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

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Updated the upstream bug and testing a fix in https://gitlab.com/paelzer/libvirt/-/pipelines/184813619

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Got one Reviewed-by, lets 6.7.0 is released so it is not Frozen atm.
Giving it another day before pushing it.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

grml s/, lets/and/

Changed in virt-manager:
status: Fix Released → Confirmed
Revision history for this message
In , paelzer (paelzer-redhat-bugs) wrote :

Hi,
as outlined above the behavior is still wrong.
The patch suggested in [1] will fix the behavior of libvirt to what it was meant to be when preallocation=falloc was added.

But now virt-manager comes into play, the XML sent by virt-manager is exactly matching the definition of the use case to use falloc.
So virt-manager triggered storage request will continue to use falloc despite libvirt being "fixed to how it was intended".

The question now is should we consider changing the XML virt-manager submits so that we are back at the much faster preallocation=metadata mode?

[1]: https://www.redhat.com/archives/libvir-list/2020-September/msg00105.html

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Once the fix in libvirt landed we still have to change virt-manager as the current storage request exactly matches the definition of the "then use falloc" case.
I've requested that on the upstream bug [1], we will have to see how that turns out.

Until we have a change to land in virt-manager (unless we find a different important use case triggering this) it makes no sense to push these fixes for libvirt right now :-/

[1]: https://bugzilla.redhat.com/show_bug.cgi?id=1759454

Revision history for this message
Colin Ian King (colin-king) wrote :

Thanks Christian.

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

If you have followed the mailing list discussion, this might be:
- stay as-is in virt-manager
- improve libvirt to explicitly select the prealloc mode (optional)
- zfs got a fix to no more surface such an unexpected waiting time (but in to be released 2.0)

The discussion isn't over but I wanted to summarize here as well for everyone just reading this bug now or later.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

FYI:
Related potential improvements in libvirt got identified, but a fix of the "slowness" might indeed eventually land via ZFS (Details in the ML discussion linked above).

And thanks Richard btw!

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

Okay now that I actually look into this and wrap my head around it, virt-manager should change.
See this previously mentioned change: https://github.com/virt-manager/virt-manager/commit/15a6a7e2105440df528f75c4df4d2471df28bd1e

The idea behind virt-manager's default was if user selected 'raw' for disk images, assume the user wants
to maximize performance, so fully allocate the disk.

qcow2 didn't support anything except sparse, so the sparse=True vs sparse=False made no difference.
So we always set sparse=False

Then qcow2 grows non-sparse support, and virt-manager is suddenly defaulting to it. That's a pretty big semantic
change. Even if falloc is fast enough in most cases, it is still claiming all the storage up front. Which
is not the historically intended behavior.

So I pushed a change to virt-manager to default to only use non-sparse for raw format by default.
That should side step this issue

I'll be pushing a new release with the fix sometime this coming week

commit ba08f84b3408744e9aa9763d100e8aa217c1f5ff
Author: Cole Robinson <email address hidden>
Date: Sat Sep 19 18:06:45 2020 -0400

    addstorage: Return to using qcow2 sparse by default

Changed in virt-manager:
status: Confirmed → Fix Released
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Now that we got the change we sort of wanted from the beginning I have created a test build of virt-manager with the change applied.

I need to debug if that now really enters the right code paths for non fallocate.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

It works (as expected):

before:
4 0 17337 263 20 0 163108 4240 - Sl ? 0:47 \_ /usr/bin/qemu-img create -f qcow2 -o preallocation=falloc,compat=1.1,lazy_refcounts /var/lib/libvirt/images/ubuntu20.04.qcow2 26214400K

Now:
4 0 43901 263 20 0 163108 5436 - Sl ? 0:00 \_ /usr/bin/qemu-img create -f qcow2 -o preallocation=metadata,compat=1.1,lazy_refcounts /var/lib/libvirt/images/ubuntu20.04.qcow2 26214400K

Even without the change to libvirt it works (as that is a valid fix, but for a different use case).

And it is going much faster is on e.g. the ZFS based setup that was mentioned.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package virt-manager - 1:2.2.1-4ubuntu2

---------------
virt-manager (1:2.2.1-4ubuntu2) groovy; urgency=medium

  * d/p/lp-1847105-addstorage-Return-to-using-qcow2-sparse-by-default.patch:
    fix slow disk allocation when using the defaults to to falloc (LP: #1847105)

 -- Christian Ehrhardt <email address hidden> Mon, 21 Sep 2020 16:28:36 +0200

Changed in virt-manager (Ubuntu):
status: Triaged → Fix Released
description: updated
Robie Basak (racb)
tags: added: focal regression-release
Revision history for this message
Robie Basak (racb) wrote :

As you explained I acknowledge that we are changing behaviour in Focal here, but I agree with you that this is the least worst option.

description: updated
Revision history for this message
Robie Basak (racb) wrote : Please test proposed package

Hello Andreas, or anyone else affected,

Accepted virt-manager into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/virt-manager/1:2.2.1-3ubuntu2.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in virt-manager (Ubuntu Focal):
status: New → Fix Committed
tags: added: verification-needed verification-needed-focal
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

On a system affected by the issue I was upgrading to focal-proposed:

The time on ZFS got down from >20sec to ~<2sec now and in ps I can catch it is back on metadata as intended:

$ ps axlf | grep -e qemu-img | grep -v grep
0 0 48981 11928 20 0 164860 5020 ? Sl ? 0:00 \_ /usr/bin/qemu-img create -f qcow2 -o preallocation=metadata,compat=1.1,lazy_refcounts /var/lib/libvirt/images/ubuntu20.04.qcow2 15728640K

Setting verified

tags: added: verification-done verification-done-focal
removed: verification-needed verification-needed-focal
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package virt-manager - 1:2.2.1-3ubuntu2.1

---------------
virt-manager (1:2.2.1-3ubuntu2.1) focal; urgency=medium

  * d/p/lp-1847105-addstorage-Return-to-using-qcow2-sparse-by-default.patch:
    fix slow disk allocation when using the defaults to to falloc (LP: #1847105)

 -- Christian Ehrhardt <email address hidden> Wed, 23 Sep 2020 07:19:04 +0200

Changed in virt-manager (Ubuntu Focal):
status: Fix Committed → Fix Released
Revision history for this message
Brian Murray (brian-murray) wrote : Update Released

The verification of the Stable Release Update for virt-manager has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Changed in zfs-linux (Ubuntu):
importance: Low → Undecided
status: New → Won't Fix
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.