Evan Layton wrote: > Evan Layton wrote: >> Ethan Quach wrote: >>> Evan Layton wrote: >>>> Ethan Quach wrote: >>>>> Evan Layton wrote: >>>>>>> I don't think you can make this change without the other. >>>>>>> For the single BE case now, you don't ever process the root >>>>>>> dataset's snapshots. Maybe just do a simple if/else to call >>>>>>> either zfs_iter_children/filesystem for the corresponding >>>>>>> need? >>>>>>> >>>>>> Actually this does work because the zfs_iter_filesystem is only used >>>>>> for the initial root dataset. >>>>> Have you tested this? Based on code inspection, I see it only >>>>> iterating down the root dataset's children *filesystems*, I >>>>> don't see how the root dataset's snapshots ever get processed. >>>> Yes of course I've tested this. :-) >>>> >>>> It iterates the root dataset then inside be_add_children it iterates >>>> through the root dataset's snapshots and datasets using >>>> zfs_iter_children. It is this zfs_iter_children call at the end of >>>> be_add_children which will process the snapshots for the root dataset. >>> I don't see how that's possible. Remember, I'm talking about the >>> *single BE case*. >> You're referring to the "beadm list opensolaris" case where we specify >> the BE name for beadm list, correct? >> >>> For this case, the zhp handle is the root dataset of the BE. >>> Calling zfs_iter_filesystems(zhp, ...) means it iterates all >>> children filesystems of that handle, but does not iterate that >>> handle itself (i.e. the callback, be_add_children_callback() >>> is never called for the root dataset itself.) >> I understand what you're getting at and I had originally added a >> call to "zfs_iter_snapshots(zhp, be_add_children_callback, cb)" to >> grab the snapshots but it worked the same with or without that in >> there. >> >>> In your test, if you do a "beadm list -a <bename>" for a BE with >>> snapshots, do you see snapshots of the BE's root dataset listed? >> Yes that's correct. I ran that exact command and I do see the snapshots >> for that BE along with the rest of the info... >> >> I'm wondering if there has been a change to beadm such that it never >> passes the name of the BE to be_list() but instead parses out the data >> for the specified BE at that end and not in libbe? I don't remember a >> change like that being made but after a bit more digging that appears >> to be what's happening and why this is working... >> >> -evan > > It does indeed look like beadm is never calling be_list with a BE name > so we're not exercising that part of the code. I'll add a -a option to > tbeadm so that we can at least test it there. I'll make these changes > and update the webrev.
It looks like what I'll have to do is call zfs_iter_snapshots to get the snapshots for the root dataset before making the call to zfs_iter_filesystems. Once the changes for 3168, 3169 and 3170 have been made I'll be able to change this since all of the datasets including the root dataset will be in the list of datasets to be processed and the snapshots will be part of the dataset structure and not the node structure. This will eliminate the need for this extra call to zfs_iter_snapshots. I've made these changes to both be_list.c and tbeadm.c and updated the webrev. Thanks! -evan
