On Thu, 2011-01-06 at 18:55 +0000, Pasi Kärkkäinen wrote:
> On Wed, Jan 05, 2011 at 09:07:49PM +0800, Nathanael Rensen wrote:
> > On Tue, Jan 04, 2011 at 09:33:39AM +0000, Ian Campbell wrote:
> > >
> > > I think the correct path for this functionality is to first get it
> > > accepted into the upstream kernel by working with the USB subsystem
> > > maintainer+list, fixing the issue arising from their review etc.
> >
> > I'm happy to have a shot at that. I don't imagine it will be a quick
> > process so in the meantime I think there is value in supporting pvusb
> > in stable/2.6.32.x to encourage people to test and identify issues,
> > and also to make it easier to track the maintenance.
> >
>
> Yeah, at least earlier Jeremy said he's happy to take most patches for
> xen/stable-2.6.32.x :)
I have fewer objections if Nathanael is stepping up to maintain the
functionality and to take on the upstreaming work etc (thanks
Nathanael!). The existing maintainers mostly have their plates full.
(I'm assuming the original author/maintainer of this code is no longer
active in the area, otherwise we really ought to be hearing from them
about this).
However what we need to avoid is having this conversation again WRT
xen/stable-2.6.{38,39,40,41,...}.x and so on for every stable (or long
term stable) branch.
I didn't review the code in details but checkpatch.pl says:
total: 7 errors, 104 warnings, 4226 lines checked
The majority look to be trivial to resolve whitespace or new typedef
issues. There's a bunch of 80-character line warnings but personally I'd
encourage using your own judgement on a case by case basis for those
rather than blindly following checkpatch.pl (applies in general too but
specifically wrt line length).
Also the patch needs a suitable commit message and a signed-off-by per
the D.C.O in Documentation/SubmittingPatches.
> > > The stuff necessary to get the frontend upstream has been upstream for
> > > ages. For the backend basic dom0 boot support is in 2.6.37 and the
> > > generic scaffolding for backends is currently in linux-next (via
> > > Konrad's tree) and is intended to be in the next merge window.
> > >
> > > I think the backend just looks like a regular USB driver to the host
> > > system so it probably belongs in drivers/usb/<something>/xen-usbback/
> > > and not drivers/xen. (I'm not sure what the <something> should be,
> > > perhaps "misc").
> >
> > I agree that the frontend driver looks like a regular USB host
> > controller driver and belongs in drivers/usb/host. From the dom0
> > perspective the backend driver is a consumer of USB services rather
> > than a provider so my inclination is that the proper place for the
> > backend is drivers/xen along with blkback, netback and pciback. Much
> > the same way as I would expect to find a USB TV tuner driver in
> > drivers/media rather than drivers/usb. I would prefer to remain
> > consistent with the model established by the block, net and pci
> > drivers,
A fair point.
> but I am happy to adopt whatever convention is acceptable to
> > upstream.
That's probably the best approach to take.
> Something I noticed about the pvusb backend..
> Should the name of the driver be xen-usbbk instead of just usbbk ?
>
> The other backend driver modules seem to have xen- prefix.
xen-usbback please but, yes, this is a good change to make which
happened to the other drivers in the pvops tree sometime ago e.g.:
a84aa84cd0db4dd6a1a911f2263846c9b6f30a49 xen: rename netbk module xen-netback.
e762bd5145e59c043e47228fe1a6c1eebef07c0c xen: rename blkbk module xen-blkback.
Skimming "git log --pretty=oneline --no-merges xen/xen/stable-2.6.32.x
-- drivers/xen/{blk,net}back drivers/net/xen-netfront.c
drivers/block/xen-blkfront.c" for other generic changes which would be
worth propagating gives:
71133087313f15db44ffb6ea802e5bdb2479a600 xen: use less generic names in
blkfront driver.
9c9c87f53f87d0368ef04207cce4c92884f4ae3d xen: use less generic names in
netfront driver.
0c989045948320d583a190b75a12ba0ec556b804 Fix compile warnings: ignoring return
value of 'xenbus_register_backend' ..
d2748d40e3e8a14706d0f3a378160def877aa222 xen/netback: don't include xen/evtchn.h
d3d3c63ce04b34ad6b45ed6da12d2be012861622 xen/blkback: don't include xen/evtchn.h
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home