[developer] Review: zfs filesystem and snapshot limits

Jerry Jelinek

2013-08-02 12:14:29 UTC

Matt,

Thanks a lot. I will pull these changes into my src and finish up. I'll
also address your other comments, along with Keith's comments, and do some
testing. Once I am done with that, I will post an updated webrev for review.

Thanks again!
Jerry

Post by Matthew Ahrens
Jerry, I applied the changes I've made at Delphix to add the
extensible_dataset feature to a branch of illumos, and then applied your
changes and then the last commit is an example of how I'm thinking you can
use this feature, rather than adding fields to the dsl_dir_phys_t. You
will need to convert all the remaining uses of dd_*_count. Let me know if
this makes sense to you.
https://github.com/ahrens/dpp/commits/fslimit
dsl_dir.c: there is no zfs_zone.h
dmu_objset.c:819 extra )
We will work on getting the extensible_dataset feature into illumos. In
the mean time, feel free to go off of that github branch.
--matt

Matt,
Thanks, if you can provide me with some sample code and a little guidance
on what you would like to see here to avoid modifying the dsl_dir_phys_t,
I would appreciate it.
Thanks again,
Jerry

1. I'd like to avoid adding fields to dsl_dir_phys_t,
dsl_dataset_phys_t, etc. if at all possible. The issue is that if another
feature is developed independently and uses the same field (same offset in
the struct) to mean something else, then illumos can't simultaneously
support both features.
We have features that would naturally use new fields in these structs,
but realized that this would impact anyone developing another such feature,
such as this one. Therefore I have implemented functionality to use the
dsl_dir and dsl_dataset objects as zap objects, so that we can add new ZAP
entries rather than adding fields to the structs. I can make that code
available later this week for you to take a look at. I can also help you
switch your code to use it, if you'd be willing to go that route. See
below for another alternative.
2. It would be nice to expose the filesystem and snapshot counts to the
user. This will help answer the question, "how close am I to my limit"?
We could solve both these issues at once by storing the counts as
properties (in the dd_props_zapobj).
--matt
On Mon, Jul 22, 2013 at 10:49 AM, Jerry Jelinek <

3897 zfs filesystem and snapshot limits
There have already been a few code reviews for this last fall with the
last one being in January. I was in the process of finalizing the responses
from that feedback in February when the big zfs re-write was pushed and I
didn't have any time to deal with that then.
I have finally gotten a chance to update the code to work with the new
zfs design and to also finish addressing the previous comments. Since this
code is somewhat different from what I had previously, and since so much
time has passed, it is probably best to start over and treat this as a
fresh review.
http://us-east.manta.joyent.com/jjelinek/public/webrevs/illumos3897/index.html
Thanks,
Jerry
*illumos-developer* | Archives<https://www.listbox.com/member/archive/182179/=now>
<https://www.listbox.com/member/archive/rss/182179/21175174-cd73734d>|
Modify<https://www.listbox.com/member/?&>Your Subscription
<http://www.listbox.com>

-------------------------------------------
illumos-zfs
Archives: https://www.listbox.com/member/archive/182191/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182191/23047029-187a0c8d
Modify Your Subscription: https://www.listbox.com/member/?member_id=23047029&id_secret=23047029-2e85923f
Powered by Listbox: http://www.listbox.com