Stefano Stabellini wrote:
> On Wed, 7 Jul 2010, Don Dutile wrote:
>> Stefano Stabellini wrote:
>>> On Thu, 1 Jul 2010, Don Dutile wrote:
>>>> The problem with this check/enable is that if you run
>>>> this on an older qemu-xen that doesn't have unplug support,
>>>> it fails pv-hvm configuration.
>>>>
>>>> But, all that means is that you can't use an xvd as the boot device,
>>>> and you have to use an emulated IDE device as boot device.
>>>> There are a couple ways to configure the vnif correctly (in guest
>>>> or in xen guest config file).
>>>>
>>>> So, on an older (say, rhel5) xen, I don't have this check;
>>>> the boot device is required to be spec'd as hda, not vda, and
>>>> xen-blkfront is not allowed to configure blk major nums
>>>> for IDE (& SCSI) (to avoid 2 drivers twiddling w/same phys backend... not
>>>> good!).
>>>>
>>> Who is requiring that the boot device is spec'd as hda and not xvda?
>>> I don't think there is such limitation in xend or libxl at the moment.
>>>
>> If you take a previous xen HVM guest spec & just run it on a guest
>> that has pv-hvm added to it, then one has hda spec'd as boot device (by
>> default,
>> by not editing the guest config spec/file).
>>
>> Ideally, both config specs should/would work.
>
> I wouldn't want to cause data corruptions by default to people that
> specified xvda in their HVM config files by mistake (for example because
> they copied and pasted from a PV guest config file).
> But I agree that we should be able to do the right thing in your case
> scenario too.
>
> I propose the appended patch (to be merge with "Unplug emulated disks
> and nics"): if the user specifies xen_emul_unplug=ignore and the unplug
> protocol is not supported (old xen installations like rhel5), we
> continue with the PV on HVM initialization and we make sure that
> blkfront doesn't hook any IDE or SCSI device.
>
> Don, Jeremy, what do you think about it?
>
>
I guess what I'm wondering is why not set xen_emul_unplug to ignore by
default (static int xen_emul_unplug=XEN_UNPLUG_IGNORE), which handles
the case I mentioned (just take existing guest config file as is, no edits,
pre-pv-hvm added to guest kernel), and if person edits config file to
change boot device to xvda, they would then edit the config to add
-x xen_emul_unplug=[all|ide-disks|...] as well.
overall, i like the direction of the change to the unplug patch;
there'd be minor changes if you adopted the above default, but effectively
what's below would work well on older (rhel5) dom0's/tools.
- Don
> ---
>
>
> diff --git a/arch/x86/xen/platform-pci-unplug.c
> b/arch/x86/xen/platform-pci-unplug.c
> index 72a3da6..2f7f3fb 100644
> --- a/arch/x86/xen/platform-pci-unplug.c
> +++ b/arch/x86/xen/platform-pci-unplug.c
> @@ -29,9 +29,9 @@
> #define XEN_PLATFORM_ERR_PROTOCOL -2
> #define XEN_PLATFORM_ERR_BLACKLIST -3
>
> -/* boolean to signal that the platform pci device can be used */
> -bool xen_platform_pci_enabled;
> -EXPORT_SYMBOL_GPL(xen_platform_pci_enabled);
> +/* store the value of xen_emul_unplug after the unplug is done */
> +int xen_platform_pci_unplug;
> +EXPORT_SYMBOL_GPL(xen_platform_pci_unplug);
> static int xen_emul_unplug;
>
> static int __init check_platform_magic(void)
> @@ -76,13 +76,13 @@ void __init xen_unplug_emulated_devices(void)
> /* If the version matches enable the Xen platform PCI driver.
> * Also enable the Xen platform PCI driver if the version is really old
> * and the user told us to ignore it. */
> - if (!r || (r == XEN_PLATFORM_ERR_MAGIC &&
> - (xen_emul_unplug & XEN_UNPLUG_IGNORE)))
> - xen_platform_pci_enabled = 1;
> + if (r && !(r == XEN_PLATFORM_ERR_MAGIC &&
> + (xen_emul_unplug & XEN_UNPLUG_IGNORE)))
> + return;
and it should return ??? ^^^^
> /* Set the default value of xen_emul_unplug depending on whether or
> * not the Xen PV frontends and the Xen platform PCI driver have
> * been compiled for this kernel (modules or built-in are both OK). */
> - if (xen_platform_pci_enabled && !xen_emul_unplug) {
> + if (!xen_emul_unplug) {
> if (xen_must_unplug_nics()) {
> printk(KERN_INFO "Netfront and the Xen platform PCI
> driver have "
> "been compiled for this kernel: unplug
> emulated NICs.\n");
> @@ -98,8 +98,9 @@ void __init xen_unplug_emulated_devices(void)
> }
> }
> /* Now unplug the emulated devices */
> - if (xen_platform_pci_enabled && !(xen_emul_unplug & XEN_UNPLUG_IGNORE))
> + if (!(xen_emul_unplug & XEN_UNPLUG_IGNORE))
> outw(xen_emul_unplug, XEN_IOPORT_UNPLUG);
> + xen_platform_pci_unplug = xen_emul_unplug;
> }
>
> static int __init parse_xen_emul_unplug(char *arg)
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 82ed403..6eb2989 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -48,6 +48,7 @@
> #include <xen/grant_table.h>
> #include <xen/events.h>
> #include <xen/page.h>
> +#include <xen/platform_pci.h>
>
> #include <xen/interface/grant_table.h>
> #include <xen/interface/io/blkif.h>
> @@ -737,6 +738,22 @@ static int blkfront_probe(struct xenbus_device *dev,
> }
> }
>
> + /* no unplug has been done: do not hook devices != xen vbds */
> + if (xen_hvm_domain() && (xen_platform_pci_unplug & XEN_UNPLUG_IGNORE)) {
> + int major;
> +
> + if (!VDEV_IS_EXTENDED(vdevice))
> + major = BLKIF_MAJOR(vdevice);
> + else
> + major = XENVBD_MAJOR;
> +
> + if (major != XENVBD_MAJOR) {
> + printk(KERN_INFO
> + "%s: HVM does not support vbd %d as xen
> block device\n",
> + __FUNCTION__, vdevice);
> + return -ENODEV;
> + }
> + }
Close to what I have for rhel5 (except I specifically scanned for ide & scsi
blk major);
I like the above better -- xen-blkfront only supports.... xvd's ! :)
> info = kzalloc(sizeof(*info), GFP_KERNEL);
> if (!info) {
> xenbus_dev_fatal(dev, -ENOMEM, "allocating info structure");
> diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c
> index be8b4f3..c01b5dd 100644
> --- a/drivers/xen/platform-pci.c
> +++ b/drivers/xen/platform-pci.c
> @@ -196,7 +196,9 @@ static struct pci_driver platform_driver = {
>
> static int __init platform_pci_module_init(void)
> {
> - if (!xen_platform_pci_enabled)
> + /* no unplug has been done, IGNORE hasn't been specified: just
> + * return now */
> + if (!xen_platform_pci_unplug)
> return -ENODEV;
>
> return pci_register_driver(&platform_driver);
> diff --git a/drivers/xen/xenbus/xenbus_probe.c
> b/drivers/xen/xenbus/xenbus_probe.c
> index 243279a..34287da 100644
> --- a/drivers/xen/xenbus/xenbus_probe.c
> +++ b/drivers/xen/xenbus/xenbus_probe.c
> @@ -978,7 +978,7 @@ static void wait_for_devices(struct xenbus_driver *xendrv)
> #ifndef MODULE
> static int __init boot_wait_for_devices(void)
> {
> - if (xen_hvm_domain() && !xen_platform_pci_enabled)
> + if (xen_hvm_domain() && !xen_platform_pci_unplug)
> return -ENODEV;
>
> ready_to_wait_for_devices = 1;
> diff --git a/include/xen/platform_pci.h b/include/xen/platform_pci.h
> index afa8855..ce9d671 100644
> --- a/include/xen/platform_pci.h
> +++ b/include/xen/platform_pci.h
> @@ -44,6 +44,6 @@ static inline int xen_must_unplug_disks(void) {
> #endif
> }
>
> -extern bool xen_platform_pci_enabled;
> +extern int xen_platform_pci_unplug;
>
> #endif /* _XEN_PLATFORM_PCI_H */
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home