Am 14.11.2011 um 03:35 schrieb Ian Kent <ra...@themaw.net>: > On Mon, 2011-11-14 at 03:08 +0100, Kay Sievers wrote: >> On Mon, Nov 14, 2011 at 02:38, Ian Kent <ra...@themaw.net> wrote: >>> On Fri, 2011-11-11 at 14:33 +0100, Kay Sievers wrote: >>>> On Tue, Sep 20, 2011 at 14:26, Kay Sievers <kay.siev...@vrfy.org> wrote: >>>>> On Tue, Sep 20, 2011 at 12:58, Lennart Poettering >>>>> <lenn...@poettering.net> wrote: >>>>>> On Tue, 20.09.11 11:36, Ian Kent (ra...@themaw.net) wrote: >>>>>> >>>>>>>> This is a bug in the kernel, and it should be fixed in the kernel >>>>>>>> (which >>>>>>>> would mean the kernel folks have to define a new version of this >>>>>>>> struct/ioctl which fixes this). If that's defined we can then add >>>>>>>> support for this into systemd. Could you file a bug about this please >>>>>>>> on >>>>>>>> the kernel bugzilla? (please cc me, or post the url here!) >>>>>>> >>>>>>> Yes, it is a mistake that I made but it is a bad idea to change >>>>>>> something that will cause widespread failure of existing binaries and >>>>>>> that's why the discrepancy persists and will continue to do so. >>>>>> >>>>>> I am not expecting this to be changed in the kernel. Instead I expect >>>>>> that the kernel API is extended with a correct version of the same data >>>>>> structure. If the kernel broke it the kernel should fix it. >>>>>> >>>>>>> I chose to compensate for it in user space and to advise anyone that >>>>>>> encountered the problem on how to handle it and I have had no reports of >>>>>>> problems in autofs since I added the size calculation (which was a long >>>>>>> time ago now). >>>>>> >>>>>> Well, it's a workaround in userspace. Fix the kernel. Add a second union >>>>>> or something which can be used by newer clients. >>>>>> >>>>>>>> I am sorry but I am pretty sure I want to keep the compat kludges for >>>>>>>> broken things in systemd at a minimum, adding such a work around looks >>>>>>>> like the wrong way to me. We generally try to fix problems where they >>>>>>>> are these days, not where we notice them. >>>>>>> >>>>>>> Sure, it is unfortunate but, as far as the autofs kernel module is >>>>>>> concerned the decision about this was made a long time ago and, yes it >>>>>>> isn't what we want but the breakage it would have caused then and the >>>>>>> breakage it would cause now is too great. >>>>>> >>>>>> Adding a corrected version will not break anything. This is not about >>>>>> replacing the current borked interface, but simply by adding a fixed >>>>>> version that we can use from userspace without ugly compat glue. >>>>>> >>>>>> Fix the problems where they are, don't work around them where they >>>>>> aren't. >>>>> >>>>> I can only second that. >>>>> >>>>> Please add a new definition, leave the old around, and do not require >>>>> userspace to do wild things, like compare static lists of >>>>> architectures to work around things that can be expected to just work. >>>> >>>> Ian, is this fixed in the meantime? If not, mind fixing the kernel bug >>>> and introduce an non-broken version of the kernel interface. >>> >>> I spent some time having a look at how I might do it but I didn't end up >>> with anything suitable. >>> >>> How exactly did you think I would be able to do it without introducing >>> breakage? >> >> We thought by introducing a non-broken version of the ioctl. Like >> Thomas Meyer posted earlier in this thread. I did not look at the >> patch details, but what's the problem with that approach? > > I saw that and have now commented on it. > > We can do that but there are a couple of changes I'd like. I need to do > some testing and I also need to see if I can work out what I thought the > problem was with adding a packed struct to the union, but I may be > thinking about something different that I tried. >
Another solution would be to explicitly add 4 filler bytes to the structure to have an 8 byte alignment, like the compiler does it now implicitly on 64 bit. Another question: is there an ioctl available that returns the min and max supported protocol version of the driver? I think user space currently needs to mount with min/maxproto=6 and see it fail, then try to mount again with version=5. >> >>>> We like to have this fixed in systemd, but are still reluctant to >>>> compile lists of static 32 vs. 64 architecture bit matches in init. >>> >>> Why would you need to do that, it's easy to work out the expected packet >>> size at program start with the function I posted. >> >> This is about 'init', which is far more exposed than an autofs daemon. >> We really don't want to compile static lists of architectures into >> 'init', to work around such kernel bugs. Linux has a new architecture >> every month, and some of them switch from 32 to 64 bit. That fix >> belongs into the kernel, not in paper-over fixes in all userspace >> tools which use autofs. >> >>> Even if I had a way to >>> do it you would still need to use the function since you won't know what >>> kernel version your client users will be using. >> >> We have no problem to require a recent kernel and a minimal protocol >> version of autofs to fix these issues. Is that what you mean? > > Yes, that's all I meant. > That will be needed if you don't need to provide any backward > compatibility to earlier kernel versions. > > Ian > > _______________________________________________ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
