Re: [Devel] [PATCH hci-8.1 v5 0/3] dm-qcow2: truncate unused tail of image on suspend

[Konstantin Khorenko <khorenko@virtuozzo.com>]

Whatever we decide about patch 3, the review turned up problems in
dm-qcow2 itself. They need fixing either way.

1. A failed L1/L2 write frees a cluster that L2 on disk still uses.

   If the L2 page is written but vfs_fsync() in
   complete_metadata_writeback() fails, complete_wbd() reverts the
   update: revert_l_entries_update() drops the refcount of the new
   cluster to zero. The page may already be on disk with the new
   entry. The allocator then hands the cluster out again, and two L2
   entries point to the same cluster.

   md_writeback_error is not set in this case (clear_writeback_status()
   flags R1/R2 pages only), so nothing asks for a check either.

   Fix: on a failed L1/L2 writeback, do not free the clusters. Leak
   them and set md_writeback_error, so preresume refuses to resume
   until "qemu-img check -r all" has run. A leak is safe, and the
   check repairs it. This is the "FIXME: We should reread md page on
   error" case.

2. Metadata writes give up on ENOSPC.

   Data writes that fail with ENOSPC are delayed and retried
   (qcow2_try_delay_enospc()). Metadata writeback is not:
   - R1/R2: md_writeback_error is set, and the device can't be resumed
     until it is reloaded;
   - L1/L2: the update is reverted (see 1), and the guest write fails.

   Fix: on ENOSPC, redirty the md page and retry it after
   ENOSPC_TIMEOUT_JI, the way data writes are retried. This is the
   "FIXME: call md_make_dirty() and try once again?" case.

3. place_r2() leaves a refblock above file_preallocated_area_start.

   place_r2() links a new refblock into R1, but moves neither the
   watermark nor file_size. Only allocate_cluster() raises the
   watermark, to the end of the cluster it returns after place_r2().
   The refblock stays above it if the file extend in
   allocate_cluster() fails, or if a parallel mark_cluster_unused()
   lowers free_cluster_search_pos.

   Today the postsuspend truncation cuts such a refblock only when
   file_size is above the watermark. With patch 3 it is cut whenever
   the clusters below it were discarded.

   Fix: place_r2() extends the file and raises the watermark to
   r2_pos + clu_size before it links the refblock into R1, and fails
   before R1 is dirtied.

If patch 3 is dropped, ploop needs one change too.
test_release_should_give_back_the_discarded_tail from ploop PR 486
expects an empty stderr on umount. Without the kernel truncation,
qemu-img check cuts the tail itself and prints "Repairing space leaked
at the end of the image", so the test has to expect that instead.

-- 
Konstantin Khorenko <khorenko@virtuozzo.com>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel