Re: [Xen-devel] libxl: problem with devices in PV

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Thu, 2011-07-21 at 10:19 +0100, Roger Pau Monnà wrote:
> 2011/7/21 Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>:
> > On Thu, 2011-07-21 at 09:39 +0100, Roger Pau Monnà wrote:
> >
> > I wonder if perhaps the netbsd pciback doesn't like seeing a backend
> > directory with no actual devices in it.
> >
> > Could you try experimentally commenting out the call to
> > libxl__create_pci_backend in do_domain_create, or gating it on "if
> > (d_config->num_pcidevs)".
> 
> Disabling or setting it with a if condition makes the guest boot, I
> through that setting the appropriate values in the status of xenstore
> should also work, but it's useless to have references to PCI devices
> in xenstore if no devices are actually configured.

There was a tricky interaction between the support for hotplugging PCI
devices and adding them one-by-one to a non-running guest during domain
build (the latter confuses the Linux pciback because it waits for the
frontend to respond to the reconfiguration requests for the second and
subsequent devices added and the guest ain't running). At the time we
made the creation of the pcibackend xenstore directory unconditional
because it simplified things

However I think enough fixes have accumulated since then that we can
switch to only creating the backend if the guest has any pci devices
configured and hotplug will still work once the guest is unpaused. (IOW
I think adding the if (...) is correct and safe but we should be mindful
of breaking hotplug)

> >
> > That call was added in 23565:72eafe80ebc1 but I don't think adding the
> > if would invalidate it.
> >
> >> /local/domain/1/device/pci = ""   (n0,r1)
> >> /local/domain/1/device/pci/0 = ""   (n1,r0)
> >> /local/domain/1/device/pci/0/backend =
> >> "/local/domain/0/backend/pci/1/0"   (n1,r0)
> >> /local/domain/1/device/pci/0/backend-id = "0"   (n1,r0)
> >> /local/domain/1/device/pci/0/state = "3"   (n1,r0)
> >> /local/domain/1/device/pci/0/pci-op-ref = "8"   (n1,r0)
> >> /local/domain/1/device/pci/0/event-channel = "8"   (n1,r0)
> >> /local/domain/1/device/pci/0/magic = "7"   (n1,r0)
> >>
> >> The patch attached breaks linux support, it is only attached for
> >> informative purposes.
> >>
> >> Regards, Roger.
> 
> Thanks again for all the help, I would prepare a new proper patch to
> be added to mainline Xen.

That would be great, thanks.

WRT the proper fix I think rather than sprinkling ifdef __NetBSD__ and
__Linux__ about it would preferable to collect that logic together in
one place (maybe in libxl_osdeps.h?) and define meaningful tags
(HAVE_DISK_PHY_FILE_SUPPORT and HAVE_DISK_BLKTAP_SUPPORT or something
better) to use in the actual code.

Ian


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel