Do not complete a device removal that hit IO errors

spa_vdev_remove_thread() checks vca_read_error_bytes and
vca_write_error_bytes at the end of each metaslab it copies and sets
svr_thread_exit, so that the removal is cancelled instead of completed.
Both counters are incremented from the copy zio callbacks in
spa_vdev_copy_segment_read_done() and
spa_vdev_copy_segment_write_done(), so the errors of the segments
copied last can arrive after that check has already run.  The loop then
ends with svr_thread_exit still B_FALSE and the thread calls
vdev_remove_complete(), dropping the vdev even though part of its data
was never written to the new location.

Write errors are the way to hit this.  A write error is only known once
the write completes, while a read error is recorded before the write it
feeds is even issued, so read errors are almost always seen in time.
With enough data to copy, the errors of one metaslab are noticed while
the next one is being copied, which is why this mostly goes unnoticed;
it is the errors of the metaslab copied last that are missed.

Check the counters once more after txg_wait_synced(), where every copy
has already reported -- the ASSERT0(vca_outstanding_bytes) next to it
relies on that -- and before the decision to complete the removal is
made.  No wait is added: that txg_wait_synced() call is already on this
path.

The write error half of ZTS removal_with_errors is what fails on this.
In a VM it failed 9 of 20 iterations before this change and 0 of 20
after.  Reducing the test's data to a single metaslab of the removed
vdev makes it fail 8 of 10 iterations, since then the only error check
is the one that races the writes it is meant to observe.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: George Melikov <[email protected]>
Closes #19041

module/zfs/vdev_removal.c

@@ -1802,6 +1802,20 @@ again:
 	txg_wait_synced(spa->spa_dsl_pool, 0);
 	ASSERT0(vca.vca_outstanding_bytes);
 
+	/*
+	 * Every copy has reported by now.  The errors of the last segments
+	 * copied may have arrived after the check at the end of the loop
+	 * above, so check again before concluding that the removal can be
+	 * completed.  Otherwise the removal of the last metaslab racing its
+	 * own write errors would drop the vdev with data left uncopied.
+	 */
+	if (zfs_removal_ignore_errors == 0 &&
+	    (vca.vca_read_error_bytes > 0 || vca.vca_write_error_bytes > 0)) {
+		mutex_enter(&svr->svr_lock);
+		svr->svr_thread_exit = B_TRUE;
+		mutex_exit(&svr->svr_lock);
+	}
+
 	mutex_destroy(&vca.vca_lock);
 	cv_destroy(&vca.vca_cv);