David Bustos wrote: > Quoth Alan Maguire on Wed, Jun 11, 2008 at 10:55:53AM +0100: > >> Nice summary of the options. Defaulting to ordered >> storage/retrieval would be my preference here, with >> possible future changes based on explicit requirements >> for unordered value support. I'm not quite sure what >> constraints the network repository requirements would >> impose - what do you think Stephen? >> > > Let's just resolve to add an unordered-property creation interface in > the future. > > sounds good.
>>> Is supporting downgrade a hard requirement? >>> >> If we explicitly bumped the version number - and >> thus needed to do an explicit downgrade - we could >> add logic to bfu etc to remove the value_order column >> (this could be done using /lib/svc/bin/sqlite). In testing, >> it does seem like earlier configd versions can handle a repository >> with the additional value_order column, but it seems >> a bit hacky not to register the fact that the repository is >> upgraded just to make downgrade easier (adding a minor >> version to the schema_version table might be enough though). >> > > A minor version seems hygenic, but I don't see a lot of tangible > benefit. I suppose the only benefit would be that we'd have a slightly cleaner test (check minor version number - if any - versus check if value_order column exists) to determine if upgrade is needed. Probably not worth it really. > I think I agree that allowing downgrade is easy enough to just > do. > > By allowing downgrade, do you mean not bumping the version number and letting older svc.configd's use the upgraded repository? Or that we do bump the version number when we upgrade, and then have to support downgrade for backwards BFU/package removal? I was thinking about this a bit, and my assumption had been that to support downgrade in the case where we bump the version number, we'd need to modify bfu to catch backwards bfu cases and do the right thing. That's doable I think, but I'm less sure about the package-based downgrade scenario. My initial thought had been that we'd simply downgrade the repository in SUNWcsr's preremove script, but thinking about it that then breaks upgrade to a Nevada build that also supports value ordering, as the downgrade action would remove the value_order column which maintains all our ordering information. Looking forward, I'm also unsure how to handle a change like this in the IPS (no scripting) environment. For these reasons, I'm leaning towards the approach of avoiding bumping the schema version (and thus letting older svc.configds use the repository with the extra column. Since the value_order column makes values default to 0, this works fine). Perhaps there are there ways round these problems though - I haven't had to deal with these sorts of upgrade/downgrade issues much, so I'm not sure. >>> True. We should be able to assume that the nonpersistent repository is >>> in the new format, though, shouldn't we? Unless I suppose we want to >>> support changing the svc.configd binary without rebooting. >>> >> Hmm, perhaps the latter scenario may be required for "pkg upgrade" >> (though I believe by default it upgrades the inactive BE). >> > > Let's just assume that svc.configd is sufficiently critical that it > won't be changed without rebooting. If that changes in the future, it > should be trivial to fix. > > Sounds good - I'll modify to only upgrade the persistent repository. Nicolas Williams wrote: > On Wed, Jun 11, 2008 at 04:37:20PM -0500, Nicolas Williams wrote: > >> Comments: >> >> - Why not update BACKEND_SCHEMA_VERSION? >> >> One possible answer: because minor schema changes don't warrant the >> complexity of updating schema_version. It's easy enough to detect >> the actual schema version by testing whether some statement compiles >> or by looking at sqlite_master (idmapd does the latter, your changes >> do the former). >> > > And I see the real answer now too: to support downgrades. idmapd > doesn't, but that's OK there, whereas for SMF not supporting downgrades > would be... bad (no more backwards BFUs!). > > So yeah, just add a comment. > Thanks for taking a look at the code Nico! I'll certainly add some more comments to make things clearer, particularly upgrade/downgrade handling. I'll respin the webrev once we've converged on the right upgrade/downgrade approach. Alan
