[ewg] Re: [PATCH 2/2] IB/iser: add backport & kernel addons foropen-iscsiover iSER support for RHAS4 up3 and up4

[Michael S. Tsirkin]

Quoting Erez Zilber <erezz at voltaire.com>:
Subject: Re: [PATCH 2/2] IB/iser: add backport & kernel addons foropen-iscsiover iSER support for RHAS4 up3 and up4

>> >
>> > However, you are copying a ton of files from upstream kernel.
>> > Sticking extra files in include might interfere with newer
>> > kernels, so I don't have better ideas for this for 1.2
>> > (for 1.3 I am hoping we'll use the submodule support in git,
>> > so we'll be able to re-use headers as well).
>> >
>> > But, for files *not* in "include/", I suggest that, instead of
> sticking our
>> > own version in addons, we should check out the files from upstream
> and tweak
>> > makefiles to pick them up: maintaining these in OFED tree long-term will
>> > be a
>> > problem.
>>
>> Do you suggest to add a new mechanism to OFED that will do that?
> 
> No, this is the same mechanism that we use for the rest of the files:
> check them out of the kernel tree.
> Look at file ofed_scripts/ofed_checkout.sh
> 
> But I stress that we can not do this for files under
> include/ *unless* they only include packet structure definitions.
> Otherwise we'll get weird data corruption on newer kernels.

See below.

> > 
> >> >
> >> >> >> +
> >> >> >> + struct iscsi_internal {
> >> >> >> +  int daemon_pid;
> >> >> >> +@@ -65,6 +69,8 @@ static DEFINE_SPINLOCK(iscsi_transport_l
> >> >> >> + #define cdev_to_iscsi_internal(_cdev) \
> >> >> >> +  container_of(_cdev, struct iscsi_internal, cdev)
> >> >> >> +
> >> >> >> ++extern int attribute_container_init(void);
> >> >> >> ++
> >> >> >
> >> >> > This does not look scsi-related. Why does this belong here?
> >> >>
> >> >> This is a hack. In 2.6.20, attribute_container_init is called from
> >> > drivers/base/init.c. Since I cannot do that, I'm calling it from the
> >> > init function in scsi_transport_iscsi (because scsi_transport_iscsi uses
> >> > the attribute container). Do you have a better suggestion?
> >> >
> >> > Aha. No better ideas for the header, let it be for now.
> >> > But the code in drivers/base/init.c can be checked out rather than
> >> > copied over.
> >>
> >> I'm using only a very small part of init.c. I'm not sure that we
> > should copy it.
> > 
> > OK then.
> > What about the stuff like scsi.c?
> 
> I have the following files in backport/2.6.9_UX/include/src/:
> 
> attribute_container.c - almost identical to the file on 2.6.20. I had to change one line in it.

could be a patch ...
which line?

> init.c - only a small part of the original file in 2.6.20
> 
> klist.c - almost identical to the file on 2.6.20. I had to change one line in it.

which line?

> kref_new.c - based on kref.c

Sounds scary ... how different is it?

> scsi.c - only a small part of the original file in 2.6.20
> 
> scsi_lib.c - only a small part of the original file in 2.6.20
> 
> scsi_scan.c - only a small part of the original file in 2.6.20
> 
> transport_class.c - identical to 2.6.20
> 
> So, the only file identical to 2.6.20 is transport_class.c. We can copy it from 2.6.20, but since it's only a single file, I'm not sure if it will make a real difference.

transport_class.c, attribute_container.c and klist.c are quite big together:
more than 1000 lines. So by all means, let's check them out from kernel tree.

-- 
MST