From: Don Dutile <ddutile@redhat.com>
Date: Sat, 13 Dec 2008 16:26:39 -0500
Subject: [xen] re-enable using xenpv in boot path for FV guests
Message-id: 4944288F.5040005@redhat.com
O-Subject: Re: [RHEL5.3 PATCH] Re-enable using xenpv in boot path for FV guests [V2]
Bugzilla: 473899
RH-Acked-by: Rik van Riel <riel@redhat.com>
RH-Acked-by: Chris Lalancette <clalance@redhat.com>

Don Dutile wrote:
> Don Dutile wrote:
>> BZ 473899 -- it says its against ia64, but it occurs on any x86 guest as well.
>>
>> When fixing the bad modules.dep problem when a 5.3 upgrade
>> is done on a 5.2 system with kmod-xenpv on it, by making xen-platform-pci.ko
>> a module versus a built-in to the kernel,  it removed the
>> ability to use the xen-vbd as the driver for the (hda) boot device.
>> By making xen-platform-pci.ko a loadable the code that
>> parses the kernel args line for ide settings is never invoked;
>> the kernel args parsing code (a hack) is needed to disable a
>> sys-device-removal workaround for anaconda on installs;
>> without the disable, xen-vbd cannot be used in the boot path.
>> Thus, the fix for a proper module dependency broke a capability
>> (read: regression) that rhel5.2 + kmod-xenpv + post-mkinitrd allowed
>> (some critical customers).
>>
>> Unfortunately, the kernel cmdline is not exported to loadable
>> modules.  the only way to scan the kernel boot line is to do so
>> during early init.
>>
>> So, this patch moves the kernel parsing code used by the xen-pv-on-hvm
>> modules into a separate, built-in file (for i686, i686-PAE, x86_64, ia64
>> bare metal, aka FV guest kernels) to the kernel, allowing the parsing
>> of the kernel args, and exports the resulting parse so the xen pv-on-hvm code
>> can do the right magic in the boot path as a loadable (that's been re-built
>> into the initrd).
>>
>> I had run into this problem at nearly the same time as the bz doing rhel4-based
>> testing of the xen-platform-pci.ko-as-a-module backport, so I also implemented
>> the suggestion made by Mark McLoughlin to use a common routine (boot_wait_for_devices())
>> to do several common steps.  So this patch includes that suggestion,
>> as I was testing that on RHEL4 as well.
>>
>> brew build:
>> http://brewweb.devel.redhat.com/brew/taskinfo?taskID=1610366
>>
>> Testing
>> 1. rebuild initrd with --with=xen-vbd, set ide=disable on kernel command line,
>>    and connect virt-disk via tap:aio: instead of file in guest config file.
>>    Successfully boot using xen-vbd
>> 2. use initial initrd & boot using emulated IDE & rtl-net
>> 3. perform xm block-attach, xm block-detach, repeatedly.
>> 3. do all of the above on i686 & x86_64 FV guests
>> 4. post rpm for reporter to do above tests on ia64 systems
>>
>> Note: Waiting for a full brew to do i686 testing & get ia64 testing done.
>>       Will report those results in follow-up email.
>>
>> Please review & ACK (asap, so it can get into RHEL5.3).
>>
>> - Don
>>
>>
>>
>
> TESTING RESULTS:
> (a) kernel-xen tests successful
>     (i) boot, pv-disk, pv-net/vnif
>     (ii) block attach/detach
>     (iii) kudzu sees vbd (needed for install)
> (b) kernel (FV guests)
>     i) can boot hda using vbd driver
>    ii) can block attach/detach
>    iii) can boot hda via emulated IDE disk (driver)
>    iii) kudzu does not show vbd when booting from IDE disk
>          (what original & existing hack is suppose to do)
>    iv) FAILURE: cannot spec a second, third, etc. xvd device
>        & have it attach to guest via xen pv-on-hvm --
>        which could be done with kmod-xenpv pkg on 5.2,
>        thus a different regression.
>      -> so, solved booting problem; secondary v-disk connection lost
>         if spec'd in xen guest config file; can block-attach/-detach
>         to get them.
>
> Unfortunately, due to lost power (and will be so for next day or so),
> I have to run home & crank on the generator for a while to heat house,
> cool refrig, etc.
> I have a lead on possible fix, but have run out of daylight, and more
> specifically, power.
> Will work on this probable fix tomorrow a.m. & post an update if
> get all of the above working as well as (b)(iv) above.
>
> - Don
>
>

Nothing like no power at home and a welcomed shower at work to get these
last minute bugs fixed.

Attached is an updated patch that works for all test scenarios.

The problem with the first patch was that it removed unconnected
xenbus devices (xendev's) too soon in the driver load-and-device-configure
path, i.e., at the end of xenbus configuration.  It needed to be delayed
until after the blkfront driver had completed its device configuration, i.e.,
a call at the end of blkfront's module_init().

Additionally, a finer granularity for identifying xen-vbd configure devices
was ascertained when debugging this last effort.

The updated brew build with this patch is:
 http://brewweb.devel.redhat.com/brew/taskinfo?taskID=1612542

The following tests all passed; all done on x86_64 guests:
(1) kernel-xen
    (a) single xvd in guest config file
    (b) multiple xvd in guest config file
    (c) block-attach, -detach, repeated 3 times under (a) & (b)
(2) kernel (FV test)
    a) boot w/emulated IDE & no additional xvd devices
       i) block-attach, -detach 3X
      ii) ran kudzu -p -c HD to see that only IDE seen (not redundant xvd);
          -- if xvd seen, this will fail anaconda during installation of FV guest
    b) boot w/emulated IDE & additional xvd in xen-guest config file
       i) block-attach, -detach 3X
      ii) kudzu -p -c HD sees IDE & single xvd device (extra xvd for hda hookup
          not seen, similar to (a)(ii) kudzu test
    c) rebuilt initrd w/patched xen-platform & xen-vbd;
       booted w/ide=disable on kernel command line
       i) block-attach, -detach 3X
      ii) kudzu only sees valid xvd's
    d) emulated rtl8139 used as network device w/fully emulated IDE
    e) xvnif configured as network device w/fully emulated IDE &
    f) xvnif configured as network device w/hda connected via xen-vbd\

Did I miss anything ????

ok, getting dark once again, & it's colder today.
Off to re-start the generator & re-heat the house, re-chill the refrig.

Please review & ACK.

- Don

ps -- this is a diff to -126

diff --git a/drivers/xen/blkfront/blkfront.c b/drivers/xen/blkfront/blkfront.c
index 6b4b0c2..febf734 100644
--- a/drivers/xen/blkfront/blkfront.c
+++ b/drivers/xen/blkfront/blkfront.c
@@ -860,10 +860,21 @@ static struct xenbus_driver blkfront = {
 
 static int __init xlblk_init(void)
 {
+	int rtn_val;
+#ifdef CONFIG_XEN_PV_ON_HVM
+	extern void xvd_dev_shutdown(void);
+#endif
+
 	if (!is_running_on_xen())
 		return -ENODEV;
 
-	return xenbus_register_frontend(&blkfront);
+	rtn_val = xenbus_register_frontend(&blkfront);
+
+#ifdef CONFIG_XEN_PV_ON_HVM
+	/* remove any xvd's in xvd_rem_list after all dev's configured */
+	xvd_dev_shutdown();
+#endif
+	return rtn_val;
 }
 module_init(xlblk_init);
 
diff --git a/drivers/xen/blkfront/vbd.c b/drivers/xen/blkfront/vbd.c
index 36d8d0f..ba9fa45 100644
--- a/drivers/xen/blkfront/vbd.c
+++ b/drivers/xen/blkfront/vbd.c
@@ -334,6 +334,7 @@ xlvbd_add(blkif_sector_t capacity, int vdevice, u16 vdisk_info,
 	struct block_device *bd;
 	int err = 0;
 	int major, minor;
+	extern void xendev_rem_add(struct xenbus_device *dev);
 
 	if ((vdevice>>EXT_SHIFT) > 1) {
 		/* this is above the extended range; something is wrong */
@@ -358,6 +359,12 @@ xlvbd_add(blkif_sector_t capacity, int vdevice, u16 vdisk_info,
 	err = xlvbd_alloc_gendisk(major, minor, capacity, vdevice, vdisk_info,
 				  sector_size, info);
 
+#ifdef CONFIG_XEN_PV_ON_HVM
+	/* tag these devices for removal; unconnected hd's */
+	if (err) {
+		xendev_rem_add(info->xbdev);
+	}
+#endif
 	bdput(bd);
 	return err;
 }
diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index 40cf81f..04f1fd3 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -67,7 +67,11 @@ static unsigned long xen_store_mfn;
 
 extern struct mutex xenwatch_mutex;
 
+#ifdef CONFIG_XEN_PV_ON_HVM
+extern int xenpv_notify_ide_disable;
+#else
 static int xenpv_notify_ide_disable = 0;
+#endif
 
 /* struct & lock to remove unconnected vbd's at boostrap */
 struct xendev_rem {
@@ -80,6 +84,7 @@ static spinlock_t xendev_rem_lock = SPIN_LOCK_UNLOCKED;
 static BLOCKING_NOTIFIER_HEAD(xenstore_notifier_list);
 
 static void wait_for_devices(struct xenbus_driver *xendrv);
+int boot_wait_for_devices(void);
 
 static int xenbus_probe_frontend(const char *type, const char *name);
 #ifdef CONFIG_XEN
@@ -91,7 +96,6 @@ static int xenbus_dev_probe(struct device *_dev);
 static int xenbus_dev_remove(struct device *_dev);
 static void xenbus_dev_shutdown(struct device *_dev);
 static int print_device_status(struct device *dev, void *data);
-static void xvd_dev_shutdown(void);
 
 /* If something in array of ids matches this device, return it. */
 static const struct xenbus_device_id *
@@ -1039,11 +1043,6 @@ EXPORT_SYMBOL_GPL(unregister_xenstore_notifier);
 
 void xenbus_probe(void *unused)
 {
-#ifdef CONFIG_XEN_PV_ON_HVM
-#ifdef MODULE
-	struct device_driver *drv = NULL;
-#endif
-#endif
 	BUG_ON((xenstored_ready <= 0));
 
 	/* Enumerate devices in xenstore. */
@@ -1057,16 +1056,6 @@ void xenbus_probe(void *unused)
 	register_xenbus_watch(&be_watch);
 #endif
 
-#ifdef CONFIG_XEN_PV_ON_HVM
-#ifdef MODULE /* to mark unused xen devices after auto-load */
-	bus_for_each_dev(&xenbus_frontend.bus, NULL, drv,
-			 print_device_status);
-#endif
-	/* now see about removing unused xvd's */
-	xvd_dev_shutdown();
-#endif
-
-
 	/* Notify others that xenstore is up */
 	blocking_notifier_call_chain(&xenstore_notifier_list, 0, NULL);
 }
@@ -1259,11 +1248,12 @@ static int exists_disconnected_device(struct device_driver *drv)
 {
         if (xenbus_frontend.error)
                 return xenbus_frontend.error;
+
 	return bus_for_each_dev(&xenbus_frontend.bus, NULL, drv,
 				is_disconnected_device);
 }
 
-static void xendev_rem_add(struct xenbus_device *dev)
+void xendev_rem_add(struct xenbus_device *dev)
 {
 	struct xendev_rem *new;
 	struct xendev_rem *ptr;
@@ -1311,6 +1301,8 @@ static void xendev_rem_add(struct xenbus_device *dev)
 
 	return;
 }
+EXPORT_SYMBOL_GPL(xendev_rem_add);
+
 static int print_device_status(struct device *dev, void *data)
 {
 	struct xenbus_device *xendev = to_xenbus_device(dev);
@@ -1324,7 +1316,6 @@ static int print_device_status(struct device *dev, void *data)
 		/* Information only: is this too noisy? */
 		printk(KERN_INFO "XENBUS: Device with no driver: %s\n",
 		       xendev->nodename);
-		xendev_rem_add(xendev);
 	} else if (xendev->state != XenbusStateConnected) {
 		printk(KERN_WARNING "XENBUS: Timeout connecting "
 		       "to device: %s (state %d)\n",
@@ -1340,12 +1331,14 @@ static int print_device_status(struct device *dev, void *data)
  * Remove unused xvd's (vbd devices) at bootstrap so anaconda
  * doesn't have a nutty seeing xvd's that are not connected
  */
-static void 
-xvd_dev_shutdown(void)
+void xvd_dev_shutdown(void)
 {
 	struct xendev_rem *ptr;
 	struct xendev_rem *tmp;
 	
+	if (xendev_rem_hd.list.next == NULL)
+		return;
+
 	spin_lock(&xendev_rem_lock);
 	list_for_each_entry_safe(ptr, tmp, &xendev_rem_hd.list, list) {
 		struct device *dev = &ptr->xendev->dev;
@@ -1360,6 +1353,7 @@ xvd_dev_shutdown(void)
 	}
 	spin_unlock(&xendev_rem_lock);
 }
+EXPORT_SYMBOL_GPL(xvd_dev_shutdown);
 #endif
 
 /* We only wait for device setup after most initcalls have run. */
@@ -1384,6 +1378,11 @@ static void wait_for_devices(struct xenbus_driver *xendrv)
 	unsigned long timeout = jiffies + 10*HZ;
 	struct device_driver *drv = xendrv ? &xendrv->driver : NULL;
 
+#ifdef CONFIG_XEN_PV_ON_HVM
+	/* force wait_for_devices check */
+	ready_to_wait_for_devices = 1;
+#endif
+
 	if (!ready_to_wait_for_devices || !is_running_on_xen())
 		return;
 
@@ -1392,19 +1391,12 @@ static void wait_for_devices(struct xenbus_driver *xendrv)
 			break;
 		schedule_timeout_interruptible(HZ/10);
 	}
-
 	bus_for_each_dev(&xenbus_frontend.bus, NULL, drv,
 			 print_device_status);
-
-#ifdef CONFIG_XEN_PV_ON_HVM
-	/* now see about removing unused xvd's */
-	xvd_dev_shutdown();
-#endif
-
 }
 
 #ifndef MODULE
-static int __init boot_wait_for_devices(void)
+int boot_wait_for_devices(void)
 {
 	INIT_LIST_HEAD(&xendev_rem_hd.list);
 	if (!xenbus_frontend.error) {
@@ -1416,46 +1408,3 @@ static int __init boot_wait_for_devices(void)
 
 late_initcall(boot_wait_for_devices);
 #endif
-
-
-/*
- * xen_ide_cmdline_check_setup() gets called VERY EARLY during initialization,
- * to scan kernel "command line" strings beginning with "ide0=noprobe" 
- * or "ide=disable".
- *
- * Note: always return 0, so as not to indicate consumption of cmdline,
- *       enabling ide subsystem to receive & parse it.
- */
-int __init xen_ide_cmdline_check_setup(char *s)
-{
-
-	/*
-	 * only look at cmdline args starting with 'ide'
-	 */
-	if (strncmp(s, "ide", 3))
-		return 0;
-
-/*	printk(KERN_DEBUG "XENBUS: xen_ide_cmdline_check_setup: %s \n", s);  */
-
-	/* assume disable */
-	xenpv_notify_ide_disable=1;
-	if (strncmp(s, "ide=disable", 11) == 0) { 
-		printk(KERN_INFO "drivers/ide subsystem to be disabled ");
-		printk("-- skipping xvd_dev_shutdown()\n");
-		return 0;
-	}
-
-	if (strncmp(s, "ide0=noprobe", 12) == 0) { 
-		printk(KERN_INFO "ide0 not going to be probed ");
-		printk("-- skipping xvd_dev_shutdown()\n");
-		return 0;
-	}
-	/* re-enable xvd_dev_shutdown if ide isn't disabled */
-	xenpv_notify_ide_disable=0;
-/*	printk(KERN_DEBUG " -- xvd_dev_shutdown to be exec'd \n"); */
-
-	return 0;
-}
-
-/* scan entire kernel cmdline, as ide subys does */
-__setup("", xen_ide_cmdline_check_setup);
diff --git a/drivers/xenpv_hvm/platform-pci/Makefile b/drivers/xenpv_hvm/platform-pci/Makefile
index 1907af9..1b4851b 100644
--- a/drivers/xenpv_hvm/platform-pci/Makefile
+++ b/drivers/xenpv_hvm/platform-pci/Makefile
@@ -2,6 +2,8 @@ include $(src)/../overrides.mk
 
 EXTRA_CFLAGS += -I$(src)/../platform-pci
 
+obj-y += xenpvhvm-parse-ide.o
+
 obj-m += xen-platform-pci.o
 
 xen-platform-pci-objs := evtchn.o platform-compat.o platform-pci.o xen_support.o
diff --git a/drivers/xenpv_hvm/platform-pci/xenpvhvm-parse-ide.c b/drivers/xenpv_hvm/platform-pci/xenpvhvm-parse-ide.c
new file mode 100644
index 0000000..9f2d53b
--- /dev/null
+++ b/drivers/xenpv_hvm/platform-pci/xenpvhvm-parse-ide.c
@@ -0,0 +1,59 @@
+/*
+ * boot-time parse of kernel args so
+ * hacky xen config of had-boot device
+ * attached to xen-vbd can be done
+ *
+ * Need to do this at kernel boot time,
+ * or can't scan ide kernel args
+ */
+ /*
+ * xen_ide_cmdline_check_setup() gets called VERY EARLY during initialization,
+ * to scan kernel "command line" strings beginning with "ide0=noprobe" 
+ * or "ide=disable".
+ *
+ * Note: always return 0, so as not to indicate consumption of cmdline,
+ *       enabling ide subsystem to receive & parse it.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/string.h>
+
+int xenpv_notify_ide_disable = 0;
+
+int __init xen_ide_cmdline_check_setup(char *s)
+{
+
+        /*
+         * only look at cmdline args starting with 'ide'
+         */
+        if (strncmp(s, "ide", 3))
+                return 0;
+
+/*      printk(KERN_DEBUG "XENBUS: xen_ide_cmdline_check_setup: %s \n", s);  */
+
+        /* assume disable */
+        xenpv_notify_ide_disable=1;
+        if (strncmp(s, "ide=disable", 11) == 0) {
+                printk(KERN_INFO "drivers/ide subsystem to be disabled ");
+                printk("-- skipping xvd_dev_shutdown()\n");
+                return 0;
+        }
+
+        if (strncmp(s, "ide0=noprobe", 12) == 0) {
+                printk(KERN_INFO "ide0 not going to be probed ");
+                printk("-- skipping xvd_dev_shutdown()\n");
+                return 0;
+        }
+        /* re-enable xvd_dev_shutdown if ide isn't disabled */
+        xenpv_notify_ide_disable=0;
+/*      printk(KERN_DEBUG " -- xvd_dev_shutdown to be exec'd \n"); */
+
+        return 0;
+}
+
+/* scan entire kernel cmdline, as ide subys does */
+__setup("", xen_ide_cmdline_check_setup);
+
+EXPORT_SYMBOL_GPL(xenpv_notify_ide_disable);