[developer] Re: [openzfs/openzfs] 8423 Implement large_dnode pool feature (#409)

[George Wilson]

grwilson approved this pull request.





-- 
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/409#pullrequestreview-58659280
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Tfefe6318017dd4c7-M880a8d1f8a373d1489e6a948
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8567 Inconsistent return value in zpool_read_label (#442)

[Prakash Surya]

@asomers I'll look at this later today. Can you comment on the motivation for 
this change? is it simply a "code cleanup" change, or does this actually "fix" 
something?

-- 
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/442#issuecomment-324945887
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T166e42f6e2808c99-Mcb059b55fd36637fa9febb7f
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8567 Inconsistent return value in zpool_read_label (#442)

[Alan Somers]

@prakashsurya it's really just cleanup.  My motivation was a bug in a 
FreeBSD-specific fault manager that could online an entire disk instead of a 
partition.  Fixing that bug required changing libzfs.  And while I was in there 
I noticed this inconsistency.
https://svnweb.freebsd.org/base/head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_import.c?r1=322854&r2=322853&pathrev=322854

-- 
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/442#issuecomment-324949347
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T166e42f6e2808c99-M81695d1947ba7ac054098bfe
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8081 Compiler warnings in zdb (#354)

[Alan Somers]

How can I build just zdb instead of rebuilding the entire OS?

-- 
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/354#issuecomment-324949719
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T95949071eb220b14-M71663c3e4bc00c279f5a6163
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8081 Compiler warnings in zdb (#354)

[Robert Mustacchi]

If the OS was previously built, you can use the bldenv utility and then cd into 
the zdb directory and run `dmake install`. More information on using the bldenv 
utility is at 
https://www.illumos.org/books/dev/workflow.html#your-best-friend-while-building-bldenv.

-- 
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/354#issuecomment-324953601
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T95949071eb220b14-Mcf35a5e0f20849e4d73e16c1
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8081 Compiler warnings in zdb (#354)

[Igor K]

you can use my example:
ksh93 bldenv.sh -d illumos.sh -c "cd usr/src/cmd/zdb && dmake clobber"
ksh93 bldenv.sh -d illumos.sh -c "cd usr/src/cmd/zdb && dmake -m serial install"

1 - will cleanup zdb
2 - will build it in one thread and failed with first error

if you don’t wont to bulid in one thread - you can remove '-m serial'

-Igor

> On Aug 25, 2017, at 6:25 PM, Robert Mustacchi  
> wrote:
> 
> If the OS was previously built, you can use the bldenv utility and then cd 
> into the zdb directory and run dmake install. More information on using the 
> bldenv utility is at 
> https://www.illumos.org/books/dev/workflow.html#your-best-friend-while-building-bldenv
>  
> .
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub 
> , or mute 
> the thread 
> .
> 



-- 
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/354#issuecomment-324956386
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T95949071eb220b14-M91956643021728bd61736e74
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8473 scrub does not detect errors on active spares (#422)

[George Wilson]

grwilson commented on this pull request.



>   c = vd->vdev_children;
 
mm = kmem_zalloc(offsetof(mirror_map_t, mm_child[c]), KM_SLEEP);
mm->mm_children = c;
-   mm->mm_replacing = (vd->vdev_ops == &vdev_replacing_ops ||
+   /*
+* If we are resilvering, then we should handle scrub reads
+* differently; we shouldn't issue them to the resilvering
+* device because it might not have those blocks.
+*
+* We are resilvering iff:
+* 1) We are a replacing vdev (ie our name is "replacing-1" or
+*"spare-1" or something like that), and
+* 2) The pool is currently being resilvered.
+*
+* We cannot simply check vd->vdev_resilvering, because that

I don't think `vdev_resilvering` exists anymore. You may want to fix this 
comment.

-- 
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/422#pullrequestreview-58688985
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Te100c9e22da2b1d8-M69918941048521e9fe731ef8
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8567 Inconsistent return value in zpool_read_label (#442)

[Prakash Surya]

prakashsurya approved this pull request.





-- 
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/442#pullrequestreview-58709837
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T166e42f6e2808c99-Ma801426a08fd32534e60953c
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8567 Inconsistent return value in zpool_read_label (#442)

[Prakash Surya]

@asomers thanks for the explanation, this looks reasonable to me. is this 
already in FreeBSD?

-- 
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/442#issuecomment-324981031
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T166e42f6e2808c99-Mb5dcdb35a8c5050ce4c869a2
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8567 Inconsistent return value in zpool_read_label (#442)

[Alan Somers]

@prakashsurya the change to zpool_read_label isn't already in FreeBSD, just our 
company's derivative OS.  I generally try to merge to openzfs first.

-- 
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/442#issuecomment-324982979
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T166e42f6e2808c99-Mc028e68a23cc1bab212ddbcf
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8423 Implement large_dnode pool feature (#409)

[Paul Dagnelie]

pcd1193182 approved this pull request.

Send receive code looks fine to me, just one small question.

>   int err;
 
-   if (dmu_object_info(rwa->os, obj, NULL) != 0)
+   err = dmu_object_info(rwa->os, obj, &doi);

Why change this to add the argument?

-- 
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/409#pullrequestreview-58746142
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Tfefe6318017dd4c7-Md98462035c105b3b87a597ca
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8423 Implement large_dnode pool feature (#409)

[Matthew Ahrens]

@ahrens pushed 1 commit.

5955b31  typo


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/409/files/5df44fb5d17d849a3104caf9fcf2bcff83c3c736..5955b3114619f6d79986993d9dedc9d213f63ed0

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Tfefe6318017dd4c7-Md514c641c328ee793118d6b9
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8081 Compiler warnings in zdb (#354)

[Alan Somers]

@asomers pushed 1 commit.

f283834  Enable some warnings when building zdb, as suggested by ahrens


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/354/files/56b3ec1d1f39cfe032b2087e78031a3cb964fec4..f283834d04a3e8a06548904cf2b9b8508362ecfe

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T95949071eb220b14-M9648c0cdf3a0ea1bbf4886b5
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8473 scrub does not detect errors on active spares (#422)

[Alan Somers]

asomers commented on this pull request.



>   c = vd->vdev_children;
 
mm = kmem_zalloc(offsetof(mirror_map_t, mm_child[c]), KM_SLEEP);
mm->mm_children = c;
-   mm->mm_replacing = (vd->vdev_ops == &vdev_replacing_ops ||
+   /*
+* If we are resilvering, then we should handle scrub reads
+* differently; we shouldn't issue them to the resilvering
+* device because it might not have those blocks.
+*
+* We are resilvering iff:
+* 1) We are a replacing vdev (ie our name is "replacing-1" or
+*"spare-1" or something like that), and
+* 2) The pool is currently being resilvered.
+*
+* We cannot simply check vd->vdev_resilvering, because that

I guess a lot changes in 3.5 years.  I just did a little experiment, and the 
replacement `vdev_resilver_txg` is also not set in this case.  I'll update the 
comment.

-- 
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/422#discussion_r135370900
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Te100c9e22da2b1d8-M4c8688f2c95371d92fa84a83
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8473 scrub does not detect errors on active spares (#422)

[Alan Somers]

@asomers pushed 1 commit.

3c5ae23  Fix an out-of-date comment as suggested by grwilson


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/422/files/09a25d80346e8685feccde4a312d02114ca7ad12..3c5ae23d880799bd393b09b75eff8fd2c18c17f0

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Te100c9e22da2b1d8-Me0324f91ea27659cbe190aa3
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8473 scrub does not detect errors on active spares (#422)

[Alan Somers]

Dear reviewers: my most recent commit is just a comment change, so no retest is 
necessary.

-- 
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/422#issuecomment-325055352
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Te100c9e22da2b1d8-Mb8cc23db062a4e606f8ede6c
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] 8473 scrub does not detect errors on active spares (#422)

[George Wilson]

grwilson approved this pull request.





-- 
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/422#pullrequestreview-58790150
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Te100c9e22da2b1d8-M7220f80d4eda69a462372957
Powered by Topicbox: https://topicbox.com