Tim,
1868 - "be_utils" -> "be_is_active_on_boot"
1901 - nit - can we give this function a better name?
maybe, be_activate_current_be()
1916 - Instead of iterating the entire BE list to find
the current BE, we can refactor some code to make
this simpler.
In many places throughout libbe, we find the current
BE by doing this:
if ((zret = zpool_iter(g_zfs,
be_zpool_find_current_be_callback, &bt)) == 0) {
be_print_err(gettext("be_copy: failed to "
"find current BE name\n"));
return (1);
} else if (zret < 0) {
be_print_err(gettext("be_copy: "
"zpool_iter failed: %s\n"),
libzfs_error_description(g_zfs));
return (1);
}
This chunk of code could be moved into a function called
be_find_current_be(), which can be called here to get the
name of the current BE, instead of doing this for loop.
All places where that chunk exists can also just be
replaced by this new function call.
1932 - Instead of having to generate an nv_list just to call
be_activate(), be_activate() can be split into a public
and internal version. (like be_mount() and _be_mount()).
This also prevents us from having to make sure we've
closed the libzfs handle before calling the activate
internally.
(I don't mean to push refactoring stuff on you, but since
you're adding new code that uses this functionality, we
might as well do it.)
thanks,
-ethan
Tim S. Knitter wrote:
> Can someone please review the following?
>
> 1868 beadm destroy creates an empty grub menu
>
> http://defect.opensolaris.org/bz/show_bug.cgi?id=1868
> http://cr.opensolaris.org/~tsk/1868_slim/
>
> Thanks
> Tim
> --
> This message posted from opensolaris.org
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
