lkml.org 
[lkml]   [2012]   [Apr]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] xen/xenbus: Add quirk to deal with misconfigured backends.
    On Wed, 18 Apr 2012, Konrad Rzeszutek Wilk wrote:
    > From: konrad <konrad@localhost.localdomain>

    wrong email address


    > A rather annoying and common case is when booting a PVonHVM guest
    > and exposing the PV KBD and PV VFB - as both of those do not
    > make any sense.

    I would rather write: "as broken toolstacks don't always initialize the
    backends correctly."


    > The HVM guest is using the VGA driver and the emulated
    > keyboard for this. So we provide a very basic quirk framework
    > (can be expanded in the future) to not wait for 6 minutes for those devices
    > to initialize - they never wont.
    >
    > To trigger this, put this in your guest config:
    >
    > vfb = [ 'vnc=1, vnclisten=0.0.0.0 ,vncunused=1']
    >
    > instead of this:
    > vnc=1
    > vnclisten="0.0.0.0"
    >
    > [v3: Split delay in non-essential (30 seconds) and essential
    > devices per Ian and Stefano suggestion]
    > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>quirk
    > ---
    > drivers/xen/xenbus/xenbus_probe_frontend.c | 69 +++++++++++++++++++++------
    > 1 files changed, 53 insertions(+), 16 deletions(-)
    >
    > diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c
    > index f20c5f1..7422913 100644
    > --- a/drivers/xen/xenbus/xenbus_probe_frontend.c
    > +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
    > @@ -135,7 +135,7 @@ static int read_backend_details(struct xenbus_device *xendev)
    > return xenbus_read_otherend_details(xendev, "backend-id", "backend");
    > }
    >
    > -static int is_device_connecting(struct device *dev, void *data)
    > +static int is_device_connecting(struct device *dev, void *data, bool ignore_vkbd)
    > {
    > struct xenbus_device *xendev = to_xenbus_device(dev);
    > struct device_driver *drv = data;
    > @@ -152,16 +152,41 @@ static int is_device_connecting(struct device *dev, void *data)
    > if (drv && (dev->driver != drv))
    > return 0;
    >
    > + if (ignore_vkbd) {

    I would name the parameter ignore_nonessential, just to make it clearer
    that is not just about vkbd


    > + /* With older QEMU, for PVonHVM guests the guest config files
    > + * could contain: vfb = [ 'vnc=1, vnclisten=0.0.0.0']
    > + * which is nonsensical as there is no PV FB (there can be
    > + * a PVKB) running as HVM guest. */
    > +
    > + if ((strncmp(xendev->nodename, "device/vkbd", 11) == 0))
    > + return 0;
    > +
    > + if ((strncmp(xendev->nodename, "device/vfb", 10) == 0))
    > + return 0;
    > + }
    > xendrv = to_xenbus_driver(dev->driver);
    > return (xendev->state < XenbusStateConnected ||
    > (xendev->state == XenbusStateConnected &&
    > xendrv->is_ready && !xendrv->is_ready(xendev)));
    > }
    > +static int essential_device_connecting(struct device *dev, void *data)
    > +{
    > + return is_device_connecting(dev, data, true /* ignore PVKB+PVDB */);
    > +}

    the comment is wrong


    > +static int non_essential_device_connecting(struct device *dev, void *data)
    > +{
    > + return is_device_connecting(dev, data, false);
    > +}
    >
    > -static int exists_connecting_device(struct device_driver *drv)
    > +static int exists_essential_connecting_device(struct device_driver *drv)
    > {
    > return bus_for_each_dev(&xenbus_frontend.bus, NULL, drv,
    > - is_device_connecting);
    > + essential_device_connecting);
    > +}
    > +static int exists_non_essential_connecting_device(struct device_driver *drv)
    > +{
    > + return bus_for_each_dev(&xenbus_frontend.bus, NULL, drv,
    > + non_essential_device_connecting);
    > }
    >
    > static int print_device_status(struct device *dev, void *data)
    > @@ -192,6 +217,23 @@ static int print_device_status(struct device *dev, void *data)
    > /* We only wait for device setup after most initcalls have run. */
    > static int ready_to_wait_for_devices;
    >
    > +static bool wait_loop(unsigned long start, unsigned int max_delay,
    > + unsigned int *seconds_waited)
    > +{
    > + if (time_after(jiffies, start + (*seconds_waited+5)*HZ)) {
    > + if (!*seconds_waited)
    > + printk(KERN_WARNING "XENBUS: Waiting for "
    > + "devices to initialise: ");
    > + *seconds_waited += 5;
    > + printk("%us...", max_delay - *seconds_waited);
    > + if (*seconds_waited == max_delay)
    > + return true;
    > + }
    > +
    > + schedule_timeout_interruptible(HZ/10);
    > +
    > + return false;
    > +}
    > /*
    > * On a 5-minute timeout, wait for all devices currently configured. We need
    > * to do this to guarantee that the filesystems and / or network devices
    > @@ -215,19 +257,14 @@ static void wait_for_devices(struct xenbus_driver *xendrv)
    > if (!ready_to_wait_for_devices || !xen_domain())
    > return;
    >
    > - while (exists_connecting_device(drv)) {
    > - if (time_after(jiffies, start + (seconds_waited+5)*HZ)) {
    > - if (!seconds_waited)
    > - printk(KERN_WARNING "XENBUS: Waiting for "
    > - "devices to initialise: ");
    > - seconds_waited += 5;
    > - printk("%us...", 300 - seconds_waited);
    > - if (seconds_waited == 300)
    > - break;
    > - }
    > -
    > - schedule_timeout_interruptible(HZ/10);
    > - }
    > + while (exists_non_essential_connecting_device(drv))
    > + if (wait_loop(start, 30, &seconds_waited))
    > + break;
    > +
    > + /* Skips PVKB and PVFB check.*/
    > + while (exists_essential_connecting_device(drv))
    > + if (wait_loop(start, 270, &seconds_waited))
    > + break;
    >
    > if (seconds_waited)
    > printk("\n");
    > --
    > 1.7.7.5
    >

    Other than the comments I wrote above, I think the patch is good



    \
     
     \ /
      Last update: 2012-04-19 13:35    [W:0.035 / U:0.592 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site