dweeezil approved this pull request.
LGTM. Tested under ZoL as part of the pool checkpoint work.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/651#pullrequestreview-128454673
dweeezil approved this pull request.
This patch stack LGTM.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/627#pullrequestreview-122667203
--
dweeezil requested changes on this pull request.
WIP port to ZoL.
> + } else if (rs->rs_end - range_tree_min(segs) >
+ *max_alloc) {
+ /*
+* This additional segment would extend past
+
dweeezil commented on this pull request.
Note regarding WIP port to ZoL.
- range_tree_remove(svr->svr_allocd_segs, offset, length);
+ /*
+* Determine how big of a chunk to copy. We can allocate up
+* to max_alloc bytes, and we can span up to vdev_removal_max_span
+
dweeezil commented on this pull request.
So far, looks good (in the port to ZoL) except for the repair writes happening
for read-only pools.
> + if (is->is_child[is->is_good_child].ic_data == NULL) {
+ ret = EIO;
+ g
@ahrens I'll look it over and try to get this into the ZoL device evacuation
port this weekend.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/591#issuecomment-373924421
-
dweeezil commented on this pull request.
Renamed tunable possibly unrelated to device evacutation?
> int zfs_resilver_min_time_ms = 3000; /* min millisecs to resilver per txg */
boolean_t zfs_no_scrub_io = B_FALSE; /* set to disable scrub i/o */
boolean_t zfs_no_scrub_prefetch = B_FALSE; /* se
dweeezil commented on this pull request.
Changed event when removing a log device.
> + */
+ vd->vdev_removing = B_TRUE;
+
+ vdev_dirty_leaves(vd, VDD_DTL, *txg);
+ vdev_config_dirty(vd);
+
+ spa_history_log_internal(spa, "vdev remove", NULL,
+ "%s vdev %llu
I've also added the patch to ```vdev-clear()``` mentioned in
https://github.com/openzfs/openzfs/pull/482#issuecomment-347576750 to ZoL.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/
dweeezil requested changes on this pull request.
I'm still working through this on ZoL.
> + spa->spa_l2cache.sav_sync = B_TRUE;
+ } else if (vd != NULL && vd->vdev_islog) {
+ ASSERT(!locked);
+ error = spa_vdev_remove_log(vd, &txg);
+ } else if
This is looking pretty good. I'm going to submit it to the ZoL buildbot now
for additional verification.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/482#issuecomment-355137252
---
@ahrens I was out of town for a week and am getting caught up now. I'm working
on reconciling the changes to my ZoL PR and should be able to give this a final
review today.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
h
@ahrens Yes, I'm on way way home from a vacation now and will give it a final
review within the next day once I catch up the ZoL PR.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/48
dweeezil commented on this pull request.
> + * vdevs, we can not add redundant toplevels. This ensures that
+* we do not rely on resilver, which does not properly handle
+* indirect vdevs.
+*/
+ if (spa->spa_vdev_removal != NULL ||
+ spa->spa_removin
dweeezil commented on this pull request.
> + *
+ * 3. The last case to consider is if the offset actually falls
+ *within the mapping entry's range. If this is the case, the
+ *offset is considered to be "equal to" the mapping entry and
+ *0 will be returned.
+ *
+ *
dweeezil commented on this pull request.
> + *within the mapping entry's range. If this is the case, the
+ *offset is considered to be "equal to" the mapping entry and
+ *0 will be returned.
+ *
+ *NOTE: If the offset is equal to the entry's source offset,
+ *
dweeezil commented on this pull request.
> + * vdevs, we can not add redundant toplevels. This ensures that
+* we do not rely on resilver, which does not properly handle
+* indirect vdevs.
+*/
+ if (spa->spa_vdev_removal != NULL ||
+ spa->spa_removin
dweeezil commented on this pull request.
> + *within the mapping entry's range. If this is the case, the
+ *offset is considered to be "equal to" the mapping entry and
+ *0 will be returned.
+ *
+ *NOTE: If the offset is equal to the entry's source offset,
+ *
dweeezil commented on this pull request.
> + *
+ * 3. The last case to consider is if the offset actually falls
+ *within the mapping entry's range. If this is the case, the
+ *offset is considered to be "equal to" the mapping entry and
+ *0 will be returned.
+ *
+ *
@ahrens, I figured it out: In illumos, ```kmem_alloc(0, ...)``` returns NULL
but in Linux, we get a magic pointer. I've done this little patch to
```vdev_compact_children()``` because apparently with the vdev removal code, it
now depends on ```vdev_child``` being nulled out during compaction.
@ahrens I looked into the ```vdev_free()``` situation a bit closer and, for
some reason, I'm seeing a value of 0x10 in ```vdev_child``` for an "indirect"
vdev. I need to investigate further as to how this is happening.
--
You are receiving this because you are subscribed to this thread.
Reply
@ahrens Like this?
```
diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c
index 5185355..bfe7020 100644
--- a/module/zfs/vdev.c
+++ b/module/zfs/vdev.c
@@ -2921,6 +2921,12 @@ vdev_clear(spa_t *spa, vdev_t *vd)
vdev_clear(spa, vd->vdev_child[c]);
/*
+* It makes no s
dweeezil requested changes on this pull request.
> + *
+ * 3. The last case to consider is if the offset actually falls
+ *within the mapping entry's range. If this is the case, the
+ *offset is considered to be "equal to" the mapping entry and
+ *0 will be returned.
I've gotten the port to ZoL to the point that it works but am now going to have
to plow through a bunch of testing issues which may have applicability in
OpenZFS/illumos as well.
@ahrens The first issue I've run across is the addition of the addition of the
```vdev_is_concrete()``` check to ```
FYI, I'm working on porting this to ZoL. The basic porting is complete but
there's quite a bit of interference due to the removal of range tree functions
not managing caller's locks and also ZoL's extra range tree functions. Another
point of conflict is zfsonlinux/zfs@0ea05c6 and a few gratuit
25 matches
Mail list logo
