Hi All, I have some review comments on the BE library for Snap Upgrade Design Specification, Revision 0.5. Apologies for the delay.
-Section 1.2, the BE definition states: The BE is currently defined by it's name which is the ZFS root pool it's in and the datasets that belong to it. For example a BE would have the following datasets... I wonder if we can be more precise here about what datasets are required to meet the BE requirements? And, what are optional or supported configurations. The reason I ask is that if a user has separate datasets, or multiple BE's in a root pool, upgrading the datasets via snap upgrade can mean different things depending on supported or required configurations. -Section 2.1, your diagram shows IPS but it doesn't show Transfer. I don't know if this service will be required or not at the time snap is available, but I am wondering if you had thought about this? You note the transfer module in your comments, but it isn't clear to me how you envision this all to work together. -Section 2.1, I would think that discovery of existing BE's, size data about BE's would be a Target Discovery service job? you list it as a requirement on libbe. Are you planning on refactoring the existing services in light of libbe requirements(fold libtd in to libbe, or something like that)? This is fine if that's your plan, I just wanted to point out the overlap in functionality. -Also, you show Target instantiation creating the root pool, but why would libbe create the datasets? What is the reason for the separation of the functionality? Seems to me datasets are simply another form of a 'target'. I am not saying that libbe isn't required, I just think we need to understand the functionality we want to provide with the existing services, what libbe will provide, and what will be brought together, etc... -Section 2.2, your diagram shows the libbe invoking the copy but the text says the 'installer' will do the copy. Just checking to see who does what. -Section 3.1.1, Why would we use a file to hold policy information. Maybe you don't mean 'file' exactly, but it seems like we could do a better job than just a flat file. Something like SMF extended profiles. -Section 3.1.1.1, what are the class definitions for the snapshots? I would think we would want pre-defined types, and perhaps the ability to allow user defined types. But there are issues with user defined types being understood by the software. Maybe the snapshot policy, instead of being a file or an attribute on a class we define, could be something ZFS could add itself as one of the snapshot dataset attributes? This would be cleaner in my opinion. -Who manages the retention policies? libbe? The text says that libbe will provide a way to delete, display, create snapshots, but if a policy indicates a retention of say 5 hours, who goes back and deletes this? In 3.1.1.3 you indicate that the persistence policy must be managed by caller. But, you also indicate a 'default' policy that somehow gets enforced by libbe? It might be good to outline exactly what libbe will do, say in the case of a default policy, and what it will or won't do in other cases. -Can an ABE be in a different root pool than the original BE? I would assume so, I can't think of a technical reason this wouldn't work, but I am just checking. -A comment, or perhaps just musings out loud... one of the key differences with ZFS snap upgrade is that a user isn't required to have an ABE created when creating an initial BE. With the current live upgrade mechanism this is required in a sense. When you run lucreate it basically copies the current BE data(which it will designate the currently booted systems BE as the current BE) to the ABE. With Snap it seems as if we will always create a BE when doing an initial install, is this correct? I am asking because: -I assume a user does not have to have enough space for an ABE when doing an initial install? Up to the user really, right? -If only one BE is required in a root pool, the user doesn't have to specify a synclist initially like we do today. But, if they want an ABE they will have to do this if they need to keep some filesystems not shared. -How will synclist datasets be represented in the namespace? Based on your design it is assumed that non BE datasets will be under the root pool which they belong, and not in the BE namespace. But, if a user wants to have datasets not share, how will this be managed? That's it for now. thanks, sarah
