Bug 241950

Created attachment 117226 [details]
The patch

Looks sane to me.

Created attachment 117244 [details]
Updated version of the patch

This is an update of the previous patch with the following changes:

- diffed to 0.8.2 release as shipped in distribution

- actually tested with qemu

- don't fail on missing product file because some devices don't make the kernel provide one

Uli, please test and submit new packages for STABLE and 10.2.

Update for 10.2 is approved: MaintenanceTracker-8241

Tested and working, submitted for STABLE and 10.2.

This may be a dumb question, but what's so bad about the linux/usbdevice_fs.h include file? We seem to ship it in 10.2.

There is no particular bad thing with this header but there are several reasons I moved the releant parts to the qemu implementation:

- It seems Greg is actively pushing all this usbdevfs stuff out of the kernel. Thus nobody actually nows how long this header will live there in its current state.

- The qemu implementation already defined one struct on its own to be "independent from kernel changes" --- whether this makes sense is one question but doing this in a consistent way for all structs seems reasonable to me.

- Building qemu is a horrible adventure (they even need an older gcc version because their code makes assumptions about the code layout that is no longer true with recent compilers unless you configure them to death). Working on this code outside the qemu build infrastructure does not make live easier if you also have to care about special macros used in kernel headers.

Now, after work is completed it might make sense to change this back (especially because I have seen an additional problem resulting from not using the header file). But for a long term solution the whole file should be reimplemented to use libusb anyway since the current implementation is really ugly.

> - It seems Greg is actively pushing all this usbdevfs stuff out of the kernel.
> Thus nobody actually nows how long this header will live there in its current
> state.

No, this isn't true.  I'm not going to be removing the header file at all.  And usbfs is still there, just the interaction is through the device nodes in /dev/usb/ instead of /proc/bus/usb/.

So the header file is still going to be there.

Well, maybe... --- How can one be sure?

Actually Novell stopped to communicate effectively about relevant changes regarding the kernel to their customers in recent times.

First there was the communication disaster of removing non-GPL modules without prior notice. I respect that change because I also think that the kernel license should be honoured. But the way this was done was communicated in an extremely unprofessional way.

Next with usbdevfs you changed a binary interface many user space applications relied on without notice and even without checking that the applications shipped with your product actually suffer from that.

Thus it seems quite logical to me to anticipate similar breakage related to the Novell shipped kernel in upcoming times.

Patchinfo created.

Created attachment 117828 [details]
(cleaner) version of the patch using the kernel headers

This is a update for the patch using the kernel headers again and thus supporting disconnect again. This was lost by accident when removing the kernel header without redefining the respective macro.

/tmp/gcc33/bin/gcc -O2 -fmessage-length=0 -Wall -D_FORTIFY_SOURCE=2  -fno-strict-aliasing -Wall -O2 -g -fno-strict-aliasing -I. -I.. -I/usr/src/packages/BUILD/qemu-0.9.0/target-i386 -I/usr/src/package
s/BUILD/qemu-0.9.0 -D_GNU_SOURCE -D_BSD_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -I/usr/src/packages/BUILD/qemu-0.9.0/fpu -DHAS_AUDIO -I/usr/src/packages/BUILD/qemu-0.9.0/slirp  -c -o usb-lin
ux.o /usr/src/packages/BUILD/qemu-0.9.0/usb-linux.c
In file included from /usr/src/packages/BUILD/qemu-0.9.0/usb-linux.c:29:
/usr/include/linux/usbdevice_fs.h:49: error: variable or field `__user' declared void
/usr/include/linux/usbdevice_fs.h:49: error: syntax error before '*' token
/usr/include/linux/usbdevice_fs.h:56: error: variable or field `__user' declared void
/usr/include/linux/usbdevice_fs.h:56: error: syntax error before '*' token
/usr/include/linux/usbdevice_fs.h:66: error: variable or field `__user' declared void
/usr/include/linux/usbdevice_fs.h:66: error: syntax error before '*' token
/usr/include/linux/usbdevice_fs.h:100: error: variable or field `__user' declared void
/usr/include/linux/usbdevice_fs.h:100: error: syntax error before '*' token
/usr/include/linux/usbdevice_fs.h:109: error: syntax error before '}' token
/usr/include/linux/usbdevice_fs.h:116: error: variable or field `__user' declared void
/usr/include/linux/usbdevice_fs.h:116: error: syntax error before '*' token
/usr/src/packages/BUILD/qemu-0.9.0/usb-linux.c: In function `usb_host_handle_control':
/usr/src/packages/BUILD/qemu-0.9.0/usb-linux.c:102: error: invalid application of `sizeof' to an incomplete type
/usr/src/packages/BUILD/qemu-0.9.0/usb-linux.c: In function `usb_host_handle_data':
/usr/src/packages/BUILD/qemu-0.9.0/usb-linux.c:119: error: storage size of `bt' isn't known
/usr/src/packages/BUILD/qemu-0.9.0/usb-linux.c:131: error: invalid application of `sizeof' to an incomplete type
/usr/src/packages/BUILD/qemu-0.9.0/usb-linux.c:119: warning: unused variable `bt'
/usr/src/packages/BUILD/qemu-0.9.0/usb-linux.c: In function `usb_host_device_open':
/usr/src/packages/BUILD/qemu-0.9.0/usb-linux.c:198: error: storage size of `ctrl' isn't known
/usr/src/packages/BUILD/qemu-0.9.0/usb-linux.c:201: error: invalid application of `sizeof' to an incomplete type
/usr/src/packages/BUILD/qemu-0.9.0/usb-linux.c:198: warning: unused variable `ctrl'
make[1]: *** [usb-linux.o] Error 1
make[1]: Leaving directory `/usr/src/packages/BUILD/qemu-0.9.0/x86_64-softmmu'
make: *** [subdir-x86_64-softmmu] Error 2


Please provide a patch that both works and compiles.

Ok, this is what I meant when I said that building qemu is a horrible adventure.  Actually I have seen this error as well while developing the solution but it works for me when integrating the patch to the current factory RPM. Thus I need more detailed information with what versions of involved packages this does actually happen and what special switches you used to build. Otherwise this might become difficult to reproduce.

Note that this error happens at a position where I have _not_ changed anything in my current versions of the patch! All my changes are later in the file.

I got this when updating to 0.9.0. I have not actually tried to build it _without_ your patch, though. Maybe it breaks then, too. I'll check.

Well, I am pretty sure that I already know what is the source of this problem. But I would need your current version of the sources to verify this and make a fix. If you send me that stuff (you can skip parts that are publicly available) I will send you a patch.

Created attachment 120548 [details]
build log with rpath patch

Sorry, wrong bug, please ignore.

0.9.0 was missing an #include <linux/compiler.h>.

New patch submitted for 10.2 (0.8.2) and STABLE (0.9.0).

Uli, this was rejected a while ago because you wanted to submit e newer fix. Is this done now? I have no new patchinfo. Please help :-)

Eek, the patchinfo somehow slipped through. Package was checked in 2007-02-22,
but never released to the maint web.

So, no longer my issue.

released