qemu-img convert runs with cache=unsafe for every block storage except zfspool

qemu-img convert runs with cache=unsafe for every block storage except zfspool

[Ing. Alfonso Kuen Arroyo]

Hello,

Reporting a data-loss path in disk move / migration that we hit in production on
PVE 9.2.4 (qemu-server 9.1.18). Happy to move this to Bugzilla if that is the
preferred channel -- I do not have an account there yet.

THE ISSUE

PVE/QemuServer/QemuImage.pm protects the cache mode only when the storage is a
ZFS pool:

  122:  $cachemode = 'none' if $src_scfg->{type} eq 'zfspool';       # source (-T)
  149:  push @$cmd, '-t', 'none' if $dst_scfg->{type} eq 'zfspool';  # target (-t)

For every other destination, qemu-img convert falls back to its own default for
-t, which is 'unsafe', i.e. BDRV_O_NO_FLUSH. If a write fails while the data is
still a dirty page, the page is dropped, the errseq is never consumed by an
fsync that would report it, and qemu-img exits 0. PVE then reports TASK OK for a
copy that silently lost data.

This affects any block destination: iSCSI, FC, LVM over SAN, NVMe-oF, and
third-party storage plugins. It is not specific to any one of them.

WHY THE EXISTING SAFETY ARGUMENT DOES NOT HOLD

The pve-devel thread that introduced this (Aug 2016) asked exactly the right
question -- "is this really safe?" -- and the answer was that bdrv_close() sends
a flush. But QEMU's bdrv_co_flush() contains:

  /* But don't actually force it to the disk with cache=unsafe */
  if (bs->open_flags & BDRV_O_NO_FLUSH) {
          goto flush_children;
  }

The flush is issued, and cache=unsafe is precisely the mode in which it does
nothing. The safety argument invalidates itself, and it has been in production
for ten years.

WHAT IT COST US

Three PVE 9 nodes against a TrueNAS SCALE 25.10.4 NVMe/TCP target. The target
was failing large commands under memory pressure -- a separate bug, ours to fix,
written up at:

  https://github.com/truenas/truenas-proxmox-plugin/issues/96

Those failures should have surfaced as a failed migration. Instead:

  - A 2 TiB image copy lost roughly 98 GiB, and qemu-img exited 0.
  - The guest then ran 41 hours on that copy, logging 2346 XFS metadata
    corruption events, before we noticed.
  - All 520 "lost async page write" events were on the destination.

With -t none the copy would have failed loudly and no data would have been lost.
The storage bug was ours; the silence was not.

SUGGESTED FIX

Pass -t none for any destination that is not a file-backed storage where the
performance tradeoff was deliberately accepted. More conservatively: invert the
condition so 'unsafe' is opt-in per storage type, rather than the default for
everything except one.

REPRODUCING WITHOUT A FAULTY ARRAY

dmsetup a 'flakey' or 'error' target under a test LV, qemu-img convert onto it,
and check the exit code. The point of the report is that the exit code is 0.

Regards,
Alfonso Kuen

Re: SPAM: qemu-img convert runs with cache=unsafe for every block storage except zfspool

[Fiona Ebner]

Hi Alfonso,

thank you for the report!

Am 03.09.26 um 1:13 PM schrieb Ing. Alfonso Kuen Arroyo:
> Hello,
> 
> Reporting a data-loss path in disk move / migration that we hit in production on
> PVE 9.2.4 (qemu-server 9.1.18). Happy to move this to Bugzilla if that is the
> preferred channel -- I do not have an account there yet.

Yes, usually the Bugzilla is the preferred place for filing issues.

> 
> THE ISSUE
> 
> PVE/QemuServer/QemuImage.pm protects the cache mode only when the storage is a
> ZFS pool:
> 
>   122:  $cachemode = 'none' if $src_scfg->{type} eq 'zfspool';       # source (-T)
>   149:  push @$cmd, '-t', 'none' if $dst_scfg->{type} eq 'zfspool';  # target (-t)
> 
> For every other destination, qemu-img convert falls back to its own default for
> -t, which is 'unsafe', i.e. BDRV_O_NO_FLUSH. If a write fails while the data is
> still a dirty page, the page is dropped, the errseq is never consumed by an
> fsync that would report it, and qemu-img exits 0. PVE then reports TASK OK for a
> copy that silently lost data.
> 
> This affects any block destination: iSCSI, FC, LVM over SAN, NVMe-oF, and
> third-party storage plugins. It is not specific to any one of them.

This is an unfortunate default.

> WHY THE EXISTING SAFETY ARGUMENT DOES NOT HOLD
> 
> The pve-devel thread that introduced this (Aug 2016) asked exactly the right
> question -- "is this really safe?" -- and the answer was that bdrv_close() sends
> a flush. But QEMU's bdrv_co_flush() contains:
> 
>   /* But don't actually force it to the disk with cache=unsafe */
>   if (bs->open_flags & BDRV_O_NO_FLUSH) {
>           goto flush_children;
>   }
> 
> The flush is issued, and cache=unsafe is precisely the mode in which it does
> nothing. The safety argument invalidates itself, and it has been in production
> for ten years.
> 
> WHAT IT COST US
> 
> Three PVE 9 nodes against a TrueNAS SCALE 25.10.4 NVMe/TCP target. The target
> was failing large commands under memory pressure -- a separate bug, ours to fix,
> written up at:
> 
>   https://github.com/truenas/truenas-proxmox-plugin/issues/96
> 
> Those failures should have surfaced as a failed migration. Instead:
> 
>   - A 2 TiB image copy lost roughly 98 GiB, and qemu-img exited 0.
>   - The guest then ran 41 hours on that copy, logging 2346 XFS metadata
>     corruption events, before we noticed.
>   - All 520 "lost async page write" events were on the destination.
> 
> With -t none the copy would have failed loudly and no data would have been lost.
> The storage bug was ours; the silence was not.
> 
> SUGGESTED FIX
> 
> Pass -t none for any destination that is not a file-backed storage where the
> performance tradeoff was deliberately accepted. More conservatively: invert the
> condition so 'unsafe' is opt-in per storage type, rather than the default for
> everything except one.

Yes, we might want to use none or writeback depending on the storage
type, similar to PBS restore.

> 
> REPRODUCING WITHOUT A FAULTY ARRAY
> 
> dmsetup a 'flakey' or 'error' target under a test LV, qemu-img convert onto it,
> and check the exit code. The point of the report is that the exit code is 0.

I'm not able to reproduce this directly, could you share more details?

[I] root@pve9a1 ~# dmsetup create test-error --table '
                   0 8388607 linear /dev/sdu 0
                   8388607 8388608 error
                   ' 8388608
[I] root@pve9a1 ~# dmsetup create test-flakey --table '
                   0 8388608 flakey /dev/sdu 0 1 1
                   ' 8388608
[I] root@pve9a1 ~# /usr/bin/qemu-img convert -p -n -f qcow2 -O raw
/mnt/pve/nfs/images/100/vm-100-disk-1.qcow2 /dev/mapper/test-error
qemu-img: error while writing at byte 4292870144: Input/output error

[I] root@pve9a1 ~ [1]# /usr/bin/qemu-img convert -p -n -f qcow2 -O raw
/mnt/pve/nfs/images/100/vm-100-disk-1.qcow2 /dev/mapper/test-flakey
qemu-img: error while writing at byte 1694498816: Input/output error

Best Regards,
Fiona

Re: SPAM: qemu-img convert runs with cache=unsafe for every block storage except zfspool

[Ing. Alfonso Kuen Arroyo]

Hi Fiona,

thanks for trying it, and sorry for the slow reply. I re-ran it today on a
PVE 9.2.4 node (pve-qemu-kvm 11.0.2-1, kernel 7.0.14-4-pve) and got the
silent exit 0, so here is the exact recipe, with what I think is the
difference from your runs.

Recipe (loop device on a file, dm-flakey with a 1 s "up" window so the
device fails while the copy is still in flight):

  cd /var/tmp
  head -c 2G /dev/urandom > src.raw          # no zero regions
  truncate -s 2G dst.img
  LOOP=$(losetup --find --show dst.img); SZ=$(blockdev --getsz $LOOP)

  # reset before each run
  dd if=/dev/zero of=$LOOP bs=1M count=2048 conv=fsync status=none
  sync; echo 3 > /proc/sys/vm/drop_caches; dmesg -C
  dmsetup create flk --table "0 $SZ flakey $LOOP 0 1 60"   # 1 s up, 60 s down

  /usr/bin/qemu-img convert -f raw -O raw src.raw /dev/mapper/flk; echo rc=$?
  sleep 70                                   # let the down window and writeback pass
  dmesg | grep -c 'lost async page write'
  dmsetup remove flk
  cmp -l src.raw $LOOP | wc -l

Default cache mode:
  rc=0 (copy took 1.4 s)
  dmesg: "Buffer I/O error on dev dm-0, logical block N, lost async page write"
         (10 lines, then "buffer_io_error: 493941 callbacks suppressed")
  cmp:   2015325567 bytes differ from the source (about 1.9 of 2 GiB never
         made it to the device)

Same run with -t none:
  "qemu-img: error while writing at byte 1763704832: Input/output error"
  rc=1

(qemu-img is the unmodified pve-qemu-kvm binary; strace of that run shows the
target opened O_RDWR without O_DIRECT and a single fdatasync, from the initial
BLKZEROOUT. Our nodes also carry a local wrapper that forces -t none - that is
not what was measured, and the binary name in the -t none error was edited.)

Why I think your runs failed loudly instead: in both of them the device was
already failing at the moment qemu-img wrote to it - dm-error from the start,
and flakey "0 1 1" is down every other second, while mine stays down for 60 s
so the failure lands while the pages are still dirty (a 4 s-up run with a
512 MiB source lost nothing: writeback finished inside the window). With a
qcow2 source I would also expect the holes to go out as write_zeroes, which on
a block device is a synchronous BLKZEROOUT ioctl and fails loudly - but I have
not verified that with a qcow2 source, so treat it as a guess. What I did
verify is that the writes must be accepted into the page cache first and fail
at writeback time, and that the same failure is loud with -t none.

I'll file this on bugzilla.proxmox.com with the recipe and both outputs,
unless you'd prefer to open it on your side.

Best regards,
Alfonso