Older kernels, such as those found in Debian Squeeze:
* Have bugs in handling of AIO into foreign pages
* Have blktap modules, which will cause qemu not to use AIO, but
which are not loaded on boot.
Attempt to load blktap in xencommons, to make sure modern qemu''s which
use AIO will work properly on those kernels.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
diff -r 99244350516a -r db614e92faf7 tools/hotplug/Linux/init.d/xencommons
--- a/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:48:49 2012 +0100
+++ b/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:49:32 2012 +0100
@@ -59,6 +59,7 @@ do_start () {
modprobe evtchn 2>/dev/null
modprobe gntdev 2>/dev/null
modprobe xen-acpi-processor 2>/dev/null
+ modprobe blktap 2>/dev/null
mkdir -p /var/run/xen
if ! `xenstore-read -s / >/dev/null 2>&1`
>>> On 15.05.12 at 17:49, George Dunlap <george.dunlap@eu.citrix.com> wrote: > Older kernels, such as those found in Debian Squeeze: > * Have bugs in handling of AIO into foreign pages > * Have blktap modules, which will cause qemu not to use AIO, but > which are not loaded on boot. > > Attempt to load blktap in xencommons, to make sure modern qemu''s which > use AIO will work properly on those kernels. > > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com> > > diff -r 99244350516a -r db614e92faf7 tools/hotplug/Linux/init.d/xencommons > --- a/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:48:49 2012 +0100 > +++ b/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:49:32 2012 +0100 > @@ -59,6 +59,7 @@ do_start () { > modprobe evtchn 2>/dev/null > modprobe gntdev 2>/dev/null > modprobe xen-acpi-processor 2>/dev/null > + modprobe blktap 2>/dev/nullCan we stop manually loading all kinds of drivers here? I was glad this went away with the switch to xencommons, and now this is coming back. Drivers definitely needed in all cases are acceptable imo, but backend drivers should be loaded as backends get created by the tools (similarly frontend drivers for the local attach case, though they should get auto-loaded normally anyway). Jan> mkdir -p /var/run/xen > > if ! `xenstore-read -s / >/dev/null 2>&1` > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On 15/05/12 17:16, Jan Beulich wrote:>>>> On 15.05.12 at 17:49, George Dunlap<george.dunlap@eu.citrix.com> wrote: >> Older kernels, such as those found in Debian Squeeze: >> * Have bugs in handling of AIO into foreign pages >> * Have blktap modules, which will cause qemu not to use AIO, but >> which are not loaded on boot. >> >> Attempt to load blktap in xencommons, to make sure modern qemu''s which >> use AIO will work properly on those kernels. >> >> Signed-off-by: George Dunlap<george.dunlap@eu.citrix.com> >> >> diff -r 99244350516a -r db614e92faf7 tools/hotplug/Linux/init.d/xencommons >> --- a/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:48:49 2012 +0100 >> +++ b/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:49:32 2012 +0100 >> @@ -59,6 +59,7 @@ do_start () { >> modprobe evtchn 2>/dev/null >> modprobe gntdev 2>/dev/null >> modprobe xen-acpi-processor 2>/dev/null >> + modprobe blktap 2>/dev/null > Can we stop manually loading all kinds of drivers here? I was > glad this went away with the switch to xencommons, and > now this is coming back. Drivers definitely needed in all cases > are acceptable imo, but backend drivers should be loaded as > backends get created by the tools (similarly frontend drivers > for the local attach case, though they should get auto-loaded > normally anyway).I tend to agree with you; I did it this way because that''s what was suggested to me. But I don''t at the moment know enough about the backend creation stuff in xl / qemu to DTRT here. If you want to volunteer to do a patch that DTRT, I think it makes sense to hold off. But if not, I suggest we accept this patch, and I''ll come back and try to write a proper one before the 4.2 release. I think it''s really important we do something before 4.2, as it causes pretty serious problems on systems which are affected (almost always a host crash, possibly with some disk corruption). -George
>>> On 15.05.12 at 18:21, George Dunlap <george.dunlap@eu.citrix.com> wrote: > On 15/05/12 17:16, Jan Beulich wrote: >>>>> On 15.05.12 at 17:49, George Dunlap<george.dunlap@eu.citrix.com> wrote: >>> Older kernels, such as those found in Debian Squeeze: >>> * Have bugs in handling of AIO into foreign pages >>> * Have blktap modules, which will cause qemu not to use AIO, but >>> which are not loaded on boot. >>> >>> Attempt to load blktap in xencommons, to make sure modern qemu''s which >>> use AIO will work properly on those kernels. >>> >>> Signed-off-by: George Dunlap<george.dunlap@eu.citrix.com> >>> >>> diff -r 99244350516a -r db614e92faf7 tools/hotplug/Linux/init.d/xencommons >>> --- a/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:48:49 2012 +0100 >>> +++ b/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:49:32 2012 +0100 >>> @@ -59,6 +59,7 @@ do_start () { >>> modprobe evtchn 2>/dev/null >>> modprobe gntdev 2>/dev/null >>> modprobe xen-acpi-processor 2>/dev/null >>> + modprobe blktap 2>/dev/null >> Can we stop manually loading all kinds of drivers here? I was >> glad this went away with the switch to xencommons, and >> now this is coming back. Drivers definitely needed in all cases >> are acceptable imo, but backend drivers should be loaded as >> backends get created by the tools (similarly frontend drivers >> for the local attach case, though they should get auto-loaded >> normally anyway). > I tend to agree with you; I did it this way because that''s what was > suggested to me. But I don''t at the moment know enough about the > backend creation stuff in xl / qemu to DTRT here. > > If you want to volunteer to do a patch that DTRT, I think it makes sense > to hold off.No, I won''t.> But if not, I suggest we accept this patch, and I''ll come > back and try to write a proper one before the 4.2 release. I think it''s > really important we do something before 4.2, as it causes pretty serious > problems on systems which are affected (almost always a host crash, > possibly with some disk corruption).A host crash because of a driver not loaded? That would suggest bugs elsewhere... Jan
On 15/05/12 17:36, Jan Beulich wrote:>>>> On 15.05.12 at 18:21, George Dunlap<george.dunlap@eu.citrix.com> wrote: >> On 15/05/12 17:16, Jan Beulich wrote: >>>>>> On 15.05.12 at 17:49, George Dunlap<george.dunlap@eu.citrix.com> wrote: >>>> Older kernels, such as those found in Debian Squeeze: >>>> * Have bugs in handling of AIO into foreign pages >>>> * Have blktap modules, which will cause qemu not to use AIO, but >>>> which are not loaded on boot. >>>> >>>> Attempt to load blktap in xencommons, to make sure modern qemu''s which >>>> use AIO will work properly on those kernels. >>>> >>>> Signed-off-by: George Dunlap<george.dunlap@eu.citrix.com> >>>> >>>> diff -r 99244350516a -r db614e92faf7 tools/hotplug/Linux/init.d/xencommons >>>> --- a/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:48:49 2012 +0100 >>>> +++ b/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:49:32 2012 +0100 >>>> @@ -59,6 +59,7 @@ do_start () { >>>> modprobe evtchn 2>/dev/null >>>> modprobe gntdev 2>/dev/null >>>> modprobe xen-acpi-processor 2>/dev/null >>>> + modprobe blktap 2>/dev/null >>> Can we stop manually loading all kinds of drivers here? I was >>> glad this went away with the switch to xencommons, and >>> now this is coming back. Drivers definitely needed in all cases >>> are acceptable imo, but backend drivers should be loaded as >>> backends get created by the tools (similarly frontend drivers >>> for the local attach case, though they should get auto-loaded >>> normally anyway). >> I tend to agree with you; I did it this way because that''s what was >> suggested to me. But I don''t at the moment know enough about the >> backend creation stuff in xl / qemu to DTRT here. >> >> If you want to volunteer to do a patch that DTRT, I think it makes sense >> to hold off. > No, I won''t. > >> But if not, I suggest we accept this patch, and I''ll come >> back and try to write a proper one before the 4.2 release. I think it''s >> really important we do something before 4.2, as it causes pretty serious >> problems on systems which are affected (almost always a host crash, >> possibly with some disk corruption). > A host crash because of a driver not loaded? That would suggest > bugs elsewhere...Yes -- a bug in the AIO implementation for foreign pages, as the description states. -George
>>> On 15.05.12 at 18:40, George Dunlap <george.dunlap@eu.citrix.com> wrote: > On 15/05/12 17:36, Jan Beulich wrote: >>>>> On 15.05.12 at 18:21, George Dunlap<george.dunlap@eu.citrix.com> wrote: >>> But if not, I suggest we accept this patch, and I''ll come >>> back and try to write a proper one before the 4.2 release. I think it''s >>> really important we do something before 4.2, as it causes pretty serious >>> problems on systems which are affected (almost always a host crash, >>> possibly with some disk corruption). >> A host crash because of a driver not loaded? That would suggest >> bugs elsewhere... > Yes -- a bug in the AIO implementation for foreign pages, as the > description states.But if there''s no backend driver, there shouldn''t be any I/O at all, and hence no crash? Or are you saying that in the absence of the driver another (qdisk?) backend gets selected (which doesn''t match my own observations, at least not as far as local attach to Dom0 is concerned, and I would hope [expect] that backend selection doesn''t depend on which domain the frontend lives in)? Finally, loading "blktap" here is the right thing for pvops, but wrong for legacy/forward ported kernels - blktap2 would be the right module name there afaict. Perhaps, if this really is to go in (temporarily), loading an alias (devname: or xen-backend:, though the latter apears to be missing from the pvops driver) would be better here? Jan
On Tue, 2012-05-15 at 17:21 +0100, George Dunlap wrote:> On 15/05/12 17:16, Jan Beulich wrote: > >>>> On 15.05.12 at 17:49, George Dunlap<george.dunlap@eu.citrix.com> wrote: > >> Older kernels, such as those found in Debian Squeeze: > >> * Have bugs in handling of AIO into foreign pages > >> * Have blktap modules, which will cause qemu not to use AIO, but > >> which are not loaded on boot. > >> > >> Attempt to load blktap in xencommons, to make sure modern qemu''s which > >> use AIO will work properly on those kernels. > >> > >> Signed-off-by: George Dunlap<george.dunlap@eu.citrix.com> > >> > >> diff -r 99244350516a -r db614e92faf7 tools/hotplug/Linux/init.d/xencommons > >> --- a/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:48:49 2012 +0100 > >> +++ b/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:49:32 2012 +0100 > >> @@ -59,6 +59,7 @@ do_start () { > >> modprobe evtchn 2>/dev/null > >> modprobe gntdev 2>/dev/null > >> modprobe xen-acpi-processor 2>/dev/null > >> + modprobe blktap 2>/dev/null > > Can we stop manually loading all kinds of drivers here? I was > > glad this went away with the switch to xencommons, and > > now this is coming back. Drivers definitely needed in all cases > > are acceptable imo, but backend drivers should be loaded as > > backends get created by the tools (similarly frontend drivers > > for the local attach case, though they should get auto-loaded > > normally anyway). > I tend to agree with you; I did it this way because that''s what was > suggested to me. But I don''t at the moment know enough about the > backend creation stuff in xl / qemu to DTRT here.The issue preventing autoloading here is that blktap is effectively optional and libxl tries to do a best effort search for a usable disk backend. If blktap is loaded the libxl will choose it, otherwise it will fallback to qdisk. The problem is that if blktap is available but not loaded then that is something which libxl cannot detect, I''m not sure how we could go about fixing that. Ian.
On Wed, 2012-05-16 at 08:47 +0100, Jan Beulich wrote:> >>> On 15.05.12 at 18:40, George Dunlap <george.dunlap@eu.citrix.com> wrote: > > On 15/05/12 17:36, Jan Beulich wrote: > >>>>> On 15.05.12 at 18:21, George Dunlap<george.dunlap@eu.citrix.com> wrote: > >>> But if not, I suggest we accept this patch, and I''ll come > >>> back and try to write a proper one before the 4.2 release. I think it''s > >>> really important we do something before 4.2, as it causes pretty serious > >>> problems on systems which are affected (almost always a host crash, > >>> possibly with some disk corruption). > >> A host crash because of a driver not loaded? That would suggest > >> bugs elsewhere... > > Yes -- a bug in the AIO implementation for foreign pages, as the > > description states. > > But if there''s no backend driver, there shouldn''t be any I/O at all, > and hence no crash? Or are you saying that in the absence of the > driver another (qdisk?) backend gets selected (which doesn''t match > my own observations, at least not as far as local attach to Dom0 > is concerned, and I would hope [expect] that backend selection > doesn''t depend on which domain the frontend lives in)?backend selection depends on what drivers are available in the backend domain, not the frontend domain. libxl will try blktap(2) first and fallback to qdisk if blktap is not present. The issue George is seeing is that qdisk can sometimes exercise AIO bugs which cause host crashes. The fix is to try harder to use blktap when it is really available by ensuring it gets loaded.> Finally, loading "blktap" here is the right thing for pvops, but > wrong for legacy/forward ported kernels - blktap2 would be > the right module name there afaict. Perhaps, if this really is to > go in (temporarily), loading an alias (devname: or xen-backend:, > though the latter apears to be missing from the pvops driver) > would be better here?There is no blktap driver at all in mainline pvops, which is part of the problem and the reason for falling back to qdisk. Which pvops driver do you mean? To confuse matters there is a DKMS thing floating around which works against modern kernels too. Perhaps the right answer is (roughly) "modprobe blktap2 || modprobe blktap"? Ian.
>>> On 16.05.12 at 10:25, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Wed, 2012-05-16 at 08:47 +0100, Jan Beulich wrote: >> Finally, loading "blktap" here is the right thing for pvops, but >> wrong for legacy/forward ported kernels - blktap2 would be >> the right module name there afaict. Perhaps, if this really is to >> go in (temporarily), loading an alias (devname: or xen-backend:, >> though the latter apears to be missing from the pvops driver) >> would be better here? > > There is no blktap driver at all in mainline pvops, which is part of the > problem and the reason for falling back to qdisk. Which pvops driver do > you mean?The one in Jeremy''s tree (on branch xen/next-2.6.32), which is easily forward ported to current kernels.> Perhaps the right answer is (roughly) "modprobe blktap2 || modprobe > blktap"?modprobe devname:xen/blktap-2/control or modprobe xen-backend:tap2 (but I now realize the former is a 2.6.35 addition [or really a counterpart of devname: aliases added elsewhere in that release], and the latter one done in our patches for 3.1, where other xen-backend: aliases also got added). So yes, looks like the only half-way compatible way would be what you suggest (perhaps extended to redirect stderr to /dev/null). Jan
>>> On 16.05.12 at 10:21, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Tue, 2012-05-15 at 17:21 +0100, George Dunlap wrote: >> On 15/05/12 17:16, Jan Beulich wrote: >> >>>> On 15.05.12 at 17:49, George Dunlap<george.dunlap@eu.citrix.com> wrote: >> >> Older kernels, such as those found in Debian Squeeze: >> >> * Have bugs in handling of AIO into foreign pages >> >> * Have blktap modules, which will cause qemu not to use AIO, but >> >> which are not loaded on boot. >> >> >> >> Attempt to load blktap in xencommons, to make sure modern qemu''s which >> >> use AIO will work properly on those kernels. >> >> >> >> Signed-off-by: George Dunlap<george.dunlap@eu.citrix.com> >> >> >> >> diff -r 99244350516a -r db614e92faf7 tools/hotplug/Linux/init.d/xencommons >> >> --- a/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:48:49 2012 +0100 >> >> +++ b/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:49:32 2012 +0100 >> >> @@ -59,6 +59,7 @@ do_start () { >> >> modprobe evtchn 2>/dev/null >> >> modprobe gntdev 2>/dev/null >> >> modprobe xen-acpi-processor 2>/dev/null >> >> + modprobe blktap 2>/dev/null >> > Can we stop manually loading all kinds of drivers here? I was >> > glad this went away with the switch to xencommons, and >> > now this is coming back. Drivers definitely needed in all cases >> > are acceptable imo, but backend drivers should be loaded as >> > backends get created by the tools (similarly frontend drivers >> > for the local attach case, though they should get auto-loaded >> > normally anyway). >> I tend to agree with you; I did it this way because that''s what was >> suggested to me. But I don''t at the moment know enough about the >> backend creation stuff in xl / qemu to DTRT here. > > The issue preventing autoloading here is that blktap is effectively > optional and libxl tries to do a best effort search for a usable disk > backend. If blktap is loaded the libxl will choose it, otherwise it will > fallback to qdisk. The problem is that if blktap is available but not > loaded then that is something which libxl cannot detect, I''m not sure > how we could go about fixing that.Why not simply run a (series of) modprobe(s) from there? Or is that precluded by not being OS-neutral? The same would obviously apply to other backends (netback most notably). Jan
On Wed, 2012-05-16 at 09:58 +0100, Jan Beulich wrote:> >>> On 16.05.12 at 10:21, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Tue, 2012-05-15 at 17:21 +0100, George Dunlap wrote: > >> On 15/05/12 17:16, Jan Beulich wrote: > >> >>>> On 15.05.12 at 17:49, George Dunlap<george.dunlap@eu.citrix.com> wrote: > >> >> Older kernels, such as those found in Debian Squeeze: > >> >> * Have bugs in handling of AIO into foreign pages > >> >> * Have blktap modules, which will cause qemu not to use AIO, but > >> >> which are not loaded on boot. > >> >> > >> >> Attempt to load blktap in xencommons, to make sure modern qemu''s which > >> >> use AIO will work properly on those kernels. > >> >> > >> >> Signed-off-by: George Dunlap<george.dunlap@eu.citrix.com> > >> >> > >> >> diff -r 99244350516a -r db614e92faf7 tools/hotplug/Linux/init.d/xencommons > >> >> --- a/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:48:49 2012 +0100 > >> >> +++ b/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:49:32 2012 +0100 > >> >> @@ -59,6 +59,7 @@ do_start () { > >> >> modprobe evtchn 2>/dev/null > >> >> modprobe gntdev 2>/dev/null > >> >> modprobe xen-acpi-processor 2>/dev/null > >> >> + modprobe blktap 2>/dev/null > >> > Can we stop manually loading all kinds of drivers here? I was > >> > glad this went away with the switch to xencommons, and > >> > now this is coming back. Drivers definitely needed in all cases > >> > are acceptable imo, but backend drivers should be loaded as > >> > backends get created by the tools (similarly frontend drivers > >> > for the local attach case, though they should get auto-loaded > >> > normally anyway). > >> I tend to agree with you; I did it this way because that''s what was > >> suggested to me. But I don''t at the moment know enough about the > >> backend creation stuff in xl / qemu to DTRT here. > > > > The issue preventing autoloading here is that blktap is effectively > > optional and libxl tries to do a best effort search for a usable disk > > backend. If blktap is loaded the libxl will choose it, otherwise it will > > fallback to qdisk. The problem is that if blktap is available but not > > loaded then that is something which libxl cannot detect, I''m not sure > > how we could go about fixing that. > > Why not simply run a (series of) modprobe(s) from there? Or is > that precluded by not being OS-neutral?An interesting idea. The blktap detection code is necessarily OS-specific. I previously discounted it because of the possibility of a race between the completion of modprobe and the driver actually registering enough for detection to succeed. Maybe I was too pessimistic or someone has a bright idea?> The same would obviously apply to other backends (netback most > notably).We use netback unconditionally (there are no alternatives, at least not ones we want to use (does qnet exist? not sure)) so I think the normal driver autoloading ought to work there -- at least on kernels where it exists, it was a relatively recent addition IIRC. Ian.
>>> On 16.05.12 at 11:16, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Wed, 2012-05-16 at 09:58 +0100, Jan Beulich wrote: >> >>> On 16.05.12 at 10:21, Ian Campbell <Ian.Campbell@citrix.com> wrote: >> > On Tue, 2012-05-15 at 17:21 +0100, George Dunlap wrote: >> >> On 15/05/12 17:16, Jan Beulich wrote: >> >> >>>> On 15.05.12 at 17:49, George Dunlap<george.dunlap@eu.citrix.com> wrote: >> >> >> Older kernels, such as those found in Debian Squeeze: >> >> >> * Have bugs in handling of AIO into foreign pages >> >> >> * Have blktap modules, which will cause qemu not to use AIO, but >> >> >> which are not loaded on boot. >> >> >> >> >> >> Attempt to load blktap in xencommons, to make sure modern qemu''s which >> >> >> use AIO will work properly on those kernels. >> >> >> >> >> >> Signed-off-by: George Dunlap<george.dunlap@eu.citrix.com> >> >> >> >> >> >> diff -r 99244350516a -r db614e92faf7 tools/hotplug/Linux/init.d/xencommons >> >> >> --- a/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:48:49 2012 +0100 >> >> >> +++ b/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:49:32 2012 +0100 >> >> >> @@ -59,6 +59,7 @@ do_start () { >> >> >> modprobe evtchn 2>/dev/null >> >> >> modprobe gntdev 2>/dev/null >> >> >> modprobe xen-acpi-processor 2>/dev/null >> >> >> + modprobe blktap 2>/dev/null >> >> > Can we stop manually loading all kinds of drivers here? I was >> >> > glad this went away with the switch to xencommons, and >> >> > now this is coming back. Drivers definitely needed in all cases >> >> > are acceptable imo, but backend drivers should be loaded as >> >> > backends get created by the tools (similarly frontend drivers >> >> > for the local attach case, though they should get auto-loaded >> >> > normally anyway). >> >> I tend to agree with you; I did it this way because that''s what was >> >> suggested to me. But I don''t at the moment know enough about the >> >> backend creation stuff in xl / qemu to DTRT here. >> > >> > The issue preventing autoloading here is that blktap is effectively >> > optional and libxl tries to do a best effort search for a usable disk >> > backend. If blktap is loaded the libxl will choose it, otherwise it will >> > fallback to qdisk. The problem is that if blktap is available but not >> > loaded then that is something which libxl cannot detect, I''m not sure >> > how we could go about fixing that. >> >> Why not simply run a (series of) modprobe(s) from there? Or is >> that precluded by not being OS-neutral? > > An interesting idea. > > The blktap detection code is necessarily OS-specific. I previously > discounted it because of the possibility of a race between the > completion of modprobe and the driver actually registering enough for > detection to succeed. Maybe I was too pessimistic or someone has a > bright idea?modprobe only exits when the driver''s init function completed, at which point the driver can be expected to be in a usable state.>> The same would obviously apply to other backends (netback most >> notably). > > We use netback unconditionally (there are no alternatives, at least not > ones we want to use (does qnet exist? not sure)) so I think the normal > driver autoloading ought to work there -- at least on kernels where it > exists, it was a relatively recent addition IIRC.It was a recent addition, and I wonder whether calling this auto-loading really is the right term (unless I''m mis-understanding, you''re referring to the xen-backend: module aliases) - what would trigger their loading? My current understanding of this it that it only allows loading the drivers without referring to a hard-coded driver module names. Jan
On Wed, 2012-05-16 at 10:51 +0100, Jan Beulich wrote:> >>> On 16.05.12 at 11:16, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Wed, 2012-05-16 at 09:58 +0100, Jan Beulich wrote: > >> >>> On 16.05.12 at 10:21, Ian Campbell <Ian.Campbell@citrix.com> wrote: > >> > On Tue, 2012-05-15 at 17:21 +0100, George Dunlap wrote: > >> >> On 15/05/12 17:16, Jan Beulich wrote: > >> >> >>>> On 15.05.12 at 17:49, George Dunlap<george.dunlap@eu.citrix.com> wrote: > >> >> >> Older kernels, such as those found in Debian Squeeze: > >> >> >> * Have bugs in handling of AIO into foreign pages > >> >> >> * Have blktap modules, which will cause qemu not to use AIO, but > >> >> >> which are not loaded on boot. > >> >> >> > >> >> >> Attempt to load blktap in xencommons, to make sure modern qemu''s which > >> >> >> use AIO will work properly on those kernels. > >> >> >> > >> >> >> Signed-off-by: George Dunlap<george.dunlap@eu.citrix.com> > >> >> >> > >> >> >> diff -r 99244350516a -r db614e92faf7 tools/hotplug/Linux/init.d/xencommons > >> >> >> --- a/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:48:49 2012 +0100 > >> >> >> +++ b/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:49:32 2012 +0100 > >> >> >> @@ -59,6 +59,7 @@ do_start () { > >> >> >> modprobe evtchn 2>/dev/null > >> >> >> modprobe gntdev 2>/dev/null > >> >> >> modprobe xen-acpi-processor 2>/dev/null > >> >> >> + modprobe blktap 2>/dev/null > >> >> > Can we stop manually loading all kinds of drivers here? I was > >> >> > glad this went away with the switch to xencommons, and > >> >> > now this is coming back. Drivers definitely needed in all cases > >> >> > are acceptable imo, but backend drivers should be loaded as > >> >> > backends get created by the tools (similarly frontend drivers > >> >> > for the local attach case, though they should get auto-loaded > >> >> > normally anyway). > >> >> I tend to agree with you; I did it this way because that''s what was > >> >> suggested to me. But I don''t at the moment know enough about the > >> >> backend creation stuff in xl / qemu to DTRT here. > >> > > >> > The issue preventing autoloading here is that blktap is effectively > >> > optional and libxl tries to do a best effort search for a usable disk > >> > backend. If blktap is loaded the libxl will choose it, otherwise it will > >> > fallback to qdisk. The problem is that if blktap is available but not > >> > loaded then that is something which libxl cannot detect, I''m not sure > >> > how we could go about fixing that. > >> > >> Why not simply run a (series of) modprobe(s) from there? Or is > >> that precluded by not being OS-neutral? > > > > An interesting idea. > > > > The blktap detection code is necessarily OS-specific. I previously > > discounted it because of the possibility of a race between the > > completion of modprobe and the driver actually registering enough for > > detection to succeed. Maybe I was too pessimistic or someone has a > > bright idea? > > modprobe only exits when the driver''s init function completed, > at which point the driver can be expected to be in a usable state.OK, so maybe that works then.> >> The same would obviously apply to other backends (netback most > >> notably). > > > > We use netback unconditionally (there are no alternatives, at least not > > ones we want to use (does qnet exist? not sure)) so I think the normal > > driver autoloading ought to work there -- at least on kernels where it > > exists, it was a relatively recent addition IIRC. > > It was a recent addition, and I wonder whether calling this > auto-loading really is the right term (unless I''m mis-understanding, > you''re referring to the xen-backend: module aliases) - what would > trigger their loading? My current understanding of this it that it only > allows loading the drivers without referring to a hard-coded driver > module names.My understanding was that creating a xenstore backend path would cause the core generic xenstore backend code in the kernel to go "ah, I need a driver for foo" (where foo is the backend name) and if one is not already registered it will do the "modprobe foo" userspace event, which the modaliases then cause to pickup the right kernel module. Ian.
On 16/05/12 10:57, Ian Campbell wrote:>>> The blktap detection code is necessarily OS-specific. I previously >>> discounted it because of the possibility of a race between the >>> completion of modprobe and the driver actually registering enough for >>> detection to succeed. Maybe I was too pessimistic or someone has a >>> bright idea? >> modprobe only exits when the driver''s init function completed, >> at which point the driver can be expected to be in a usable state. > OK, so maybe that works then.So what''s the plan? Options seem to be: 1. Put this on the blocker list, so it definitely gets done before release 2. Accept the patch that started this thread (or a version of it), and put "do it right" on the "nice-to-have" list. I can do it if I get a chance. 3. Someone can volunteer who can prioritize it. -George
On Wed, 2012-05-16 at 11:49 +0100, George Dunlap wrote:> On 16/05/12 10:57, Ian Campbell wrote: > >>> The blktap detection code is necessarily OS-specific. I previously > >>> discounted it because of the possibility of a race between the > >>> completion of modprobe and the driver actually registering enough for > >>> detection to succeed. Maybe I was too pessimistic or someone has a > >>> bright idea? > >> modprobe only exits when the driver''s init function completed, > >> at which point the driver can be expected to be in a usable state. > > OK, so maybe that works then. > So what''s the plan? Options seem to be: > 1. Put this on the blocker list, so it definitely gets done before release > 2. Accept the patch that started this thread (or a version of it), and > put "do it right" on the "nice-to-have" list. I can do it if I get a > chance. > 3. Someone can volunteer who can prioritize it.My preference, in decreasing order would be 2, 3, 1. Ian.
Ian Campbell writes ("Re: [Xen-devel] [PATCH] xencommons: Attempt to load
blktap driver"):> On Wed, 2012-05-16 at 11:49 +0100, George Dunlap wrote:
> > On 16/05/12 10:57, Ian Campbell wrote:
> > >>> The blktap detection code is necessarily OS-specific. I
previously
> > >>> discounted it because of the possibility of a race
between the
> > >>> completion of modprobe and the driver actually
registering enough for
> > >>> detection to succeed. Maybe I was too pessimistic or
someone has a
> > >>> bright idea?
> > >> modprobe only exits when the driver''s init function
completed,
> > >> at which point the driver can be expected to be in a usable
state.
> > > OK, so maybe that works then.
> > So what''s the plan? Options seem to be:
> > 1. Put this on the blocker list, so it definitely gets done before
release
> > 2. Accept the patch that started this thread (or a version of it), and
> > put "do it right" on the "nice-to-have" list. I
can do it if I get a
> > chance.
> > 3. Someone can volunteer who can prioritize it.
>
> My preference, in decreasing order would be 2, 3, 1.
AFAICT this is still outstanding. Have we made a decision ?
I''m inclined to throw all of these best-effort modprobes into 4.2 and
try to sort out something saner (if possible) in 4.3.
Ian: can you make sure this is on the 4.2 TODO ?
Ian.
On Fri, 2012-06-08 at 16:35 +0100, Ian Jackson wrote:> Ian Campbell writes ("Re: [Xen-devel] [PATCH] xencommons: Attempt to load blktap driver"): > > On Wed, 2012-05-16 at 11:49 +0100, George Dunlap wrote: > > > On 16/05/12 10:57, Ian Campbell wrote: > > > >>> The blktap detection code is necessarily OS-specific. I previously > > > >>> discounted it because of the possibility of a race between the > > > >>> completion of modprobe and the driver actually registering enough for > > > >>> detection to succeed. Maybe I was too pessimistic or someone has a > > > >>> bright idea? > > > >> modprobe only exits when the driver''s init function completed, > > > >> at which point the driver can be expected to be in a usable state. > > > > OK, so maybe that works then. > > > So what''s the plan? Options seem to be: > > > 1. Put this on the blocker list, so it definitely gets done before release > > > 2. Accept the patch that started this thread (or a version of it), and > > > put "do it right" on the "nice-to-have" list. I can do it if I get a > > > chance. > > > 3. Someone can volunteer who can prioritize it. > > > > My preference, in decreasing order would be 2, 3, 1. > > AFAICT this is still outstanding. Have we made a decision ? > > I''m inclined to throw all of these best-effort modprobes into 4.2 and > try to sort out something saner (if possible) in 4.3. > > Ian: can you make sure this is on the 4.2 TODO ?I''ve added, to tools nice to have: * Load blktap driver from xencommons initscript if available, thread at: <db614e92faf743e20b3f.1337096977@kodo2>. To be fixed more properly in 4.3 I''ll post an update of the TODO list after I''ve processed my post-vacation email backlog. Ian.
On Tue, 2012-05-15 at 16:49 +0100, George Dunlap wrote:> Older kernels, such as those found in Debian Squeeze: > * Have bugs in handling of AIO into foreign pages > * Have blktap modules, which will cause qemu not to use AIO, but > which are not loaded on boot. > > Attempt to load blktap in xencommons, to make sure modern qemu''s which > use AIO will work properly on those kernels. > > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>Rereading the thread which this patch spawned I think the conclusion was that we should take this patch (or a version of it) for 4.2. I think the "or a version of it" refers to the selection of the precise set of names which we need to try to get the best chance of loading something useful. But I''m not sure what the conclusion was in that regard. Ian.> > diff -r 99244350516a -r db614e92faf7 tools/hotplug/Linux/init.d/xencommons > --- a/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:48:49 2012 +0100 > +++ b/tools/hotplug/Linux/init.d/xencommons Tue May 15 16:49:32 2012 +0100 > @@ -59,6 +59,7 @@ do_start () { > modprobe evtchn 2>/dev/null > modprobe gntdev 2>/dev/null > modprobe xen-acpi-processor 2>/dev/null > + modprobe blktap 2>/dev/null > mkdir -p /var/run/xen > > if ! `xenstore-read -s / >/dev/null 2>&1` > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
George Dunlap writes ("[Xen-devel] [PATCH] xencommons: Attempt to load
blktap driver"):> Older kernels, such as those found in Debian Squeeze:
> * Have bugs in handling of AIO into foreign pages
> * Have blktap modules, which will cause qemu not to use AIO, but
> which are not loaded on boot.
>
> Attempt to load blktap in xencommons, to make sure modern qemu''s
which
> use AIO will work properly on those kernels.
>
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
For 4.2.
Ian.
On Wed, 2012-05-16 at 08:47 +0100, Jan Beulich wrote:> Finally, loading "blktap" here is the right thing for pvops, but > wrong for legacy/forward ported kernels - blktap2 would be > the right module name there afaict. Perhaps, if this really is to > go in (temporarily), loading an alias (devname: or xen-backend:, > though the latter apears to be missing from the pvops driver) > would be better here?George''s patch doesn''t seem to apply any more (hardly surprising). From what you say I think we want to modprobe blktap if blktap2 didn''t exist. blktap2 isn''t actually a xenbus backend driver (since it uses blkback to do the guest facing bit) so I don''t think a xen-backend: alias is available. I can''t see any other aliases defined in the code in either the 2.6.18-xen tree, the SLES 2.6.32.12-0.7.1 kernel (which is the latest I happen to have to hand) or a mainline kernel. If there is something else we should be trying please let me know. So I intend to commit the following: 8<-------------------------------------------------- xencommons: Attempt to load blktap2 driver Older kernels, such as those found in Debian Squeeze: * Have bugs in handling of AIO into foreign pages * Have blktap modules, which will cause qemu not to use AIO, but which are not loaded on boot. Attempt to load blktap in xencommons, to make sure modern qemu''s which use AIO will work properly on those kernels. Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com> Prefer to load blktap2 if it exists. This is the name of the driver in classic-Xen ports, while in mainline kernels the driver is called just blktap. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> diff -r 1126b3079bef -r 451724678dd4 tools/hotplug/Linux/init.d/xencommons --- a/tools/hotplug/Linux/init.d/xencommons Fri Aug 24 12:38:18 2012 +0100 +++ b/tools/hotplug/Linux/init.d/xencommons Fri Aug 31 09:32:37 2012 +0100 @@ -68,6 +68,7 @@ do_start () { modprobe usbbk 2>/dev/null modprobe pciback 2>/dev/null modprobe xen-acpi-processor 2>/dev/null + modprobe blktap2 2>/dev/null || modprobe blktap 2>/dev/null mkdir -p /var/run/xen if ! `xenstore-read -s / >/dev/null 2>&1`
>>> On 31.08.12 at 10:40, Ian Campbell <Ian.Campbell@citrix.com> wrote: > From what you say I think we want to modprobe blktap if blktap2 didn''t > exist. > > blktap2 isn''t actually a xenbus backend driver (since it uses blkback to > do the guest facing bit) so I don''t think a xen-backend: alias is > available. I can''t see any other aliases defined in the code in either > the 2.6.18-xen tree, the SLES 2.6.32.12-0.7.1 kernel (which is the > latest I happen to have to hand) or a mainline kernel. If there is > something else we should be trying please let me know.There''s a "devname:xen/blktap-2/control" alias in our SLE11 SP2 and newer openSUSE ones (as of 2.6.35). Whether that''s fully appropriate to be there and/or to be used as a modprobe argument I''m not sure though. The bad thing about the "blktap" name is that that''s also the name of the blktap1 driver in the 2.6.18 tree and its forward ports, but I don''t think there''s anything we can reasonably do about that. So I''m fine with the change you suggest from that perspective (whether to use the module alias pointed out ). Jan
On Fri, 2012-08-31 at 10:03 +0100, Jan Beulich wrote:> >>> On 31.08.12 at 10:40, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > From what you say I think we want to modprobe blktap if blktap2 didn''t > > exist. > > > > blktap2 isn''t actually a xenbus backend driver (since it uses blkback to > > do the guest facing bit) so I don''t think a xen-backend: alias is > > available. I can''t see any other aliases defined in the code in either > > the 2.6.18-xen tree, the SLES 2.6.32.12-0.7.1 kernel (which is the > > latest I happen to have to hand) or a mainline kernel. If there is > > something else we should be trying please let me know. > > There''s a "devname:xen/blktap-2/control" alias in our SLE11 SP2 > and newer openSUSE ones (as of 2.6.35). Whether that''s fully > appropriate to be there and/or to be used as a modprobe > argument I''m not sure though. > > The bad thing about the "blktap" name is that that''s also the > name of the blktap1 driver in the 2.6.18 tree and its forward > ports, but I don''t think there''s anything we can reasonably do > about that.I thought about that. Most kernels which have blktap1 nowadays also have blktap2 so the number of systems where you would actually end up with only blktap1 loaded is pretty small. It''s also AFAIK reasonably harmless other than the memory usage etc. In retrospect renaming blktap2->blktap ni pvops was a stupid idea (I can say that since it was my idea...)> So I''m fine with the change you suggest from that > perspective (whether to use the module alias pointed out ).Can I take that as an Acked-by: Jan Beulich <JBeulich@suse.com> ? I think I''ll skip the alias for now. Ian.
>>> On 31.08.12 at 11:33, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Fri, 2012-08-31 at 10:03 +0100, Jan Beulich wrote: >> >>> On 31.08.12 at 10:40, Ian Campbell <Ian.Campbell@citrix.com> wrote: >> > From what you say I think we want to modprobe blktap if blktap2 didn''t >> > exist. >> > >> > blktap2 isn''t actually a xenbus backend driver (since it uses blkback to >> > do the guest facing bit) so I don''t think a xen-backend: alias is >> > available. I can''t see any other aliases defined in the code in either >> > the 2.6.18-xen tree, the SLES 2.6.32.12-0.7.1 kernel (which is the >> > latest I happen to have to hand) or a mainline kernel. If there is >> > something else we should be trying please let me know. >> >> There''s a "devname:xen/blktap-2/control" alias in our SLE11 SP2 >> and newer openSUSE ones (as of 2.6.35). Whether that''s fully >> appropriate to be there and/or to be used as a modprobe >> argument I''m not sure though. >> >> The bad thing about the "blktap" name is that that''s also the >> name of the blktap1 driver in the 2.6.18 tree and its forward >> ports, but I don''t think there''s anything we can reasonably do >> about that. > > I thought about that. Most kernels which have blktap1 nowadays also have > blktap2 so the number of systems where you would actually end up with > only blktap1 loaded is pretty small. It''s also AFAIK reasonably harmless > other than the memory usage etc. > > In retrospect renaming blktap2->blktap ni pvops was a stupid idea (I can > say that since it was my idea...) > >> So I''m fine with the change you suggest from that >> perspective (whether to use the module alias pointed out ). > > Can I take that as an > Acked-by: Jan Beulich <JBeulich@suse.com> > ?Yes, feel free to do so. Jan