On Thu, Nov 17, 2011 at 01:02:33PM +0000, David Holland wrote: > > > Neither is good enough if you're providing backwards compatibility; > > > > An error is still better than a crash. > > ...and neither is acceptable for the quotactl system call, either in > the kernel processing it or in userlevel processing the returned data. > The code has to be right; consequently, what happens if it's wrong is > less important or even entirely unimportant.
and I still believe it's easier to get it right with something like prolib (where you have to pass the data you got through problib functions), than using ad-hoc checks, that may or may not be used by all involved parties. > > > > it > > > has to *work*. This is the standard we're committed to, and I continue > > > to think there's no particular advantage for proplib in this regard, > > > particularly for this particular kind of data. > > > > And I still think a text-based format is better than a binary format for > > backward compatibility. > > And I still don't agree, or even really see where the substance of the > argument is. So we agree to disagree :) > > > > (I don't think any semistructured or "self-describing" data model, > > > including the perfect one I'd replace proplib with if I could wave a > > > wand to do so, provides any particular advantage for procedure call > > > compatibility. Sure, you can tag data bundles with version codes and > > > such, but we can and do already do that by tagging the call itself and > > > have lots of support architecture in place for doing it that way. The > > > advantages appear when you're dealing with irregularly structured > > > material, like when there are large numbers of optional fields or > > > optional parameters and so forth.) > > > > Or when you send different commands which takes different arguments, > > which is the case for the quota syscall and VNOPS. > > That's true but each command code is associated with a specific type, > in the same way that functions are associated with a specific type. > > Well, in a similar way. Is what you're objecting to here that the old > quotactl interface used void pointers and wasn't (even halfway) > typesafe? That seems like a stronger point but it has nothing > whatsoever to do with compatibility... the old quotactl interfaces has more issues that that, but yes, this is one thing I didn't like. If you have different quotactl commands, you'll likely have different arguments so you'll end up using a void pointer (or something equivalent, like a pointer to a union). > > > > I don't do Perl. I might be persuaded to do Python bindings, but it > > > would probably be more effective to enlist someone who already knows > > > how to do this and won't therefore make newbie mistakes with the > > > interpreter. Anyway, the hard part is making the library interface > > > available; wrapping Perl or Python around it should be entirely > > > trivial. > > > > So you propose to remplace something that we can work with using existing > > modules it various language, by something that needs a specific module (to > > be written) for each langage. Clearly that's not a win. > > That line of reasoning can be used to justify almost any bad > architectural decision (ranging from HTML email to using msdosfs as an > application save format...) so I'm not going to accept it. > > Writing language bindings for a simple and straightforward library is > a simple and straightforward undertaking. OK, so prove it by writing a perl binding format :) I've never written a language binding, so it's not going to be straightforward for me anyway. > [...] > > > I don't think that feature is worth preserving either, since it's > > > purely a side-effect of having visible quota files. And I don't see > > > the point; it's not like mounting the volume to run edquota will cause > > > a catastrophe. > > > > I dropped this functionnality for ufs-quota2; I used it for ufs-quota1 > > because otherwise things would go wrong (something to do with offsets: > > a uid would get the quotas for another uid - of course this is a bug, but > > I never managed to track it down). > > That is definitely a bug. And it's likely to still exist. Do you have > a way to reproduce it, or to try to reproduce it? Did it affect all > tools or only some/one? (And where's the PR number? :-p ) No, I dond't know how to reproduce it (or I would have debugged it), and it's not clear to me if the problem is in the kernel or userland. I noticed it on 2 different servers after a reboot, but I'm not sure how I ran into this situation. BTW, quotacheck always do quota update offline, because it's run before quotaon. On Thu, Nov 17, 2011 at 01:13:26PM +0000, David Holland wrote: > On Tue, Nov 15, 2011 at 07:56:09PM +0100, Manuel Bouyer wrote: > > > > Do we expect NetBSD developers to be checking in handlers for > > > functionally equivalent binary formats that are *more* likely to > > > cause a crash if invalid data are presented? That does not seem > > > good. > > > > If you pass e.g. a string to the legacy quotactl syscall instead of > > the required data structure, the kernel won't notice and will use > > the bogus data. > > That's no different from any other system call, though. s/any/some/ because for some of them, doing a sanity check is easy (it's part of copyin/copyout for example). > > > > Whether text or binary, the data have to be validated before being > > > used. Text or binary, code that doesn't do that is simply buggy. > > > > But it's easier to validate a text format than a binary one. > > But we don't have any schema handling or validation code for proplib > bundles. One can write checks by hand, but then they're likely to be > incomplete and/or wrong, and it ends up not being a net win. Also, the > more validation you do the less you can use the approach below. that's why having a schema validation is not a strong requirement for me. > > > > I think that if we aren't _actually_ going to provide backward > > > compatibility in some way _now_, it doesn't matter. Either way, to > > > actually get sane semantics, versioning is required. > > > > The actual quotactl interface has a version number embeeded, for > > this reason. But, for example, some fields can be added to the > > strucure without changing the version number. The consumer will > > just notice if the new field is present or not and real with it; > > this provides backward and forward compat (older consumers will > > just ignore the new fields). This is somethig you can't do with a > > binary format. > > Well, yes it is (see uvmexp_sysctl for example) but besides that, as > I've pointed out before, this only works if you can predict in advance > what kinds of changes you expect. Otherwise, when you get a proplib > bundle that contains unexpected data, it fails the validation checks > and you die with an error. You don't need to predict in advance, you just make sure to ignore what you don't need in the data. If a key/value pair you need is not there, it's an error. If you get an extra key/value pair that you don't need/don't know about (this is the same thing), you just ignore it. > > Also, without schema handling for proplib do you really expect this > kind of ad hoc approach to be workable over a span of years, involving > multiple releases and possibly 3rd-party code? If the code follow the rules of ignoring unknown/uneeded key/values, yes. This is how e.g. html has done it, and it's not that bad. > > (While I see the code you put in netatalk uses only struct > ufs_quota_entry and not the proplib bundles directly, you've been > arguing in this thread that struct ufs_quota_entry (after being > renamed) isn't good enough in the long term. Even though I don't > believe this, I do expect your arguments to be self-consistent...) the netatalk code is strongly tied to ufs-like quota semantics; if some new fields shows up it wouldn't know how to use it without a deep rework anyway. But if the changes don't remove key/value needed by ufs_quota_entry, then it'll work out of the box, even if the dictionary it got from the kenrel is larger. -- Manuel Bouyer <bou...@antioche.eu.org> NetBSD: 26 ans d'experience feront toujours la difference --