Evan Layton wrote: > Ethan Quach wrote: >> Evan, >> >> >> be_activate.c >> ------------- >> 1073 - nit - 'The' is capitalized. > > Fixed > >> >> 1098 - nit - not part of your changes here but this Description >> is inaccurate. We use this function generically for the global BE's >> datasets as well. > > Sounds like something good to change so I've made this change as well. > > * Description: This function is used to promote the subordinate datasets > * for the BE being activated as well as the subordinate > * datasets for the zones BE being activated. >
Removed the term subordinate since all datasets are promoted... > >> >> >> be_create.c >> ----------- >> The changes in this file would actually fix 10990 as well >> since the zone snapshot names would now be unique. But as >> noted in (b) in 10990, if you do this here, then upon >> activate, we end up promoting a zone dataset above its origin >> from a 'zoneadm clone' as well. Don't know if we want to do >> this now. I thought we were going to leave this for 10990. > > Does this fix all of 10990? I thought as part of 10990 we would also > need to add checking for the zone path and I didn't make changes for > doing that. I did however mention that in the comments. > > As for the promoting the a zone dataset above its origin I found > that the zone clones dataset ends up owning it's own snapshot at > this point and when the parent zone dataset is promoted that > snapshot gets left with the zone clone. This didn't impact either > activations from that point on or the destroy. Testing with this > also proved to be fine with all the tests passing as expected. > > One of the reasons I did this here is that I need this for 5711 as > well. I wanted to get this in the code so I can finish the fix for > 5711. As discussed I added 10990 to the list of bugs being fixed here and filed a new bug (bug 11641). This new bug covers only the check for the zonepath when doing the promote of the zones' datasets. I also reference bug 11641 in bug 10990. The updated webrev is available at; http://cr.opensolaris.org/~evanl/11062v2/ > > > Thanks! > > -evan > >> >> >> >> thanks, >> -ethan >> >> >> Evan Layton wrote: >>> >>> I need to get a code review for bug 11062. >>> >>> This has been tested using both manual tests to verify that it solves >>> the issue as state in the bug and using the libbe automated test suite. >>> All testing has completed successfully. >>> >>> The bug: >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=11062 >>> >>> The webrev: >>> http://cr.opensolaris.org/~evanl/11062/ >>> >>> Some background: This bug was caused by not checking for a return code >>> from calls to zfs_promote. This was not checked originally based on >>> invalid information and from looking at the use of zfs_promote in other >>> areas of ZFS and other consumers of libzfs which don't check this >>> output. What this lack of checking caused was a failure to activate >>> a BE if there was a snapshot name that conflicted. This conflict could >>> be caused if a zone was cloned using zoneadm clone and then an older >>> BE with the original zone but without the new zone was activated. >>> >>> The fix adds checking for a failure from zfs_promote, the possible >>> snapshot name conflict and changes the auto snapshot naming for >>> zonesdataset to limit exposure to the >>> possible naming conflict. >>> >>> Thanks! >>> -evan >>> >>> _______________________________________________ >>> caiman-discuss mailing list >>> caiman-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> > > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
