The full nightly build was built successfully and testings look good as well. I have posted the new incremental webrev under
http://cr.opensolaris.org/~stevep/6351623-2 The previous one has been moved under 6351623-1. Please take a look at and if you can get back to me with any new comment by cob Wed then that will great. Thanks Steve Steve Peng wrote: > David, > > Again responses inline. I will be making some new changes and do the > build. I will post a new incremental webrev once > the build is completed and tested. > > Thanks > > Steve > > > > David Bustos wrote: > >> Quoth Steve Peng on Tue, Jan 22, 2008 at 11:38:24AM -0700: >> >> >>> Thanks for your comments. Replies inline. With regard to idempotency, >>> I think the protocol should be idempotent and handle accordingly. >>> >>> >> Does that mean you'll change client.c:repository_switch() to use >> changeid logic? >> >> > > Yep. Working on it. > > >> >> >>> David Bustos wrote: >>> >>> >>>> Quoth Steve Peng on Tue, Jan 15, 2008 at 08:16:16PM -0700: >>>> >>>> >>>>> David Bustos wrote: >>>>> >>>>> >> ... >> >> >>>>> > 1256: Why is it ok to fail if src doesn't match? What should the >>>>> > client do in that case? >>>>> >>>>> Good point. I probably should assert it. >>>>> >>>>> >>>> No, then a bug in svcadm could cause svc.configd to crash. What >>>> I wonder is if src is really necessary; shouldn't we always switch from >>>> the current repository? If so, why should we refuse if the client has >>>> the wrong path? >>>> >>>> >>> What bug can cause configd to crash? >>> >>> >> If you make svc.configd assert() that strcmp(be->be_path, src) == 0, >> then if svcadm had a bug where it gave the wrong path to >> _scf_repository_switch(), then the condition would be false and >> svc.configd would abort. >> >> >> >>> Shouldn't the path (be_path) >>> always match the existing be_db? Is there any >>> possibility that be_path can refer to a file path which is different >>> from what be_db points to? >>> >>> >> I'm talking about line 1254, where you compare be->be_path to src, the >> path the client sent. The only guarantee that they will match is that >> svcadm knows where svc.configd's repository is. This seems like an >> unnecessary restriction to me, and would make the system more fragile to >> bugs or future changes. >> >> > > Ah! Ok. I will go ahead remove the restriction. > > >> Stepping back, I don't understand how the system could change so that it >> would make sense to request svc.configd to move its repository from one >> it's not using to somewhere else. It seems to me that the client should >> at most tell svc.configd where to move to, and svc.configd should always >> switch from the repository it's currently using. The only choice in >> source that I see is when svc.configd is using multiple database files, >> which is the case today because it uses persistent and nonpersistent >> files. But even then I would think that which to switch should be >> selected abstractly (i.e., persistent vs. nonpersistent), rather than by >> sending a path string. >> >> >> >>>>> > 1273: If backend_switch_copy() failed, but sw_back is true, will we >>>>> > try to sqlite_open(dst) anyway? >>>>> >>>>> Are you refering to line 1278? Do you see any reason that we shouldn't >>>>> sqlite_open? >>>>> >>>>> >>>> No, I was just confused from the comment. I do wonder, though, if we >>>> really should switch to the file the client gave us, or if we should >>>> switch back to the file which we opened when whe first started. >>>> >>>> >>> In this case, I prefer switching to the original /etc/svc repository >>> instead of staying in the tmpfs location. >>> >>> >> Right, I agree. I'm just wondering if we should try to open whatever >> path the client gave us, or if we should ignore dst and always open >> REPOSITORY_DB. >> >> >> >>>>> > lib/libscf/inc/libscf_priv.h >>>>> > 297-301: Why do we need a structure? Can't _scf_repository_switch() >>>>> > accept sw_src, sw_dst, and sw_back as separate parameters? >>>>> >>>>> Yes, I was thinking that too but decided to put them in a structure so >>>>> in the future if we decide to add more parameters we can do so w/o chaning >>>>> the interfaces prototype. >>>>> >>>>> >>>> Do you know of other places in ON where we have done that? >>>> >>>> >>> I think it should be easy to find the code where the interface/function >>> can be expanded easily by adding new fields in the >>> structure passed to the function. Say in the near future, if we decide >>> to pass more than three parameters we can do so by >>> just adding new members to the structure w/o changing the interface >>> prototype. This can be useful if the interface is called >>> everywhere. >>> >>> >> Right, this would be a good idea if we add fields which we don't need to >> modify current consumers for. Since this is a private function, I don't >> think that there will be enough consumers to justify the extra >> structure. But I'll leave it up to you. >> >> >> David >> >> > > _______________________________________________ > smf-discuss mailing list > smf-discuss at opensolaris.org >
