WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 12/12] Unplug emulated disks and nics

To: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 12/12] Unplug emulated disks and nics
From: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Date: Fri, 18 Jun 2010 00:35:46 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, "ddutile@xxxxxxxxxx" <ddutile@xxxxxxxxxx>, "linux-kernel@xxxxxxxxxxxxxxx" <linux-kernel@xxxxxxxxxxxxxxx>, "sheng@xxxxxxxxxxxxxxx" <sheng@xxxxxxxxxxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Delivery-date: Thu, 17 Jun 2010 16:36:50 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <alpine.DEB.2.00.1006171541490.3401@kaball-desktop>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1275570645-27189-1-git-send-email-stefano.stabellini@xxxxxxxxxxxxx> <1275570645-27189-12-git-send-email-stefano.stabellini@xxxxxxxxxxxxx> <20100614212017.GA8600@xxxxxxxxxxxxxxxxxxx> <alpine.DEB.2.00.1006171541490.3401@kaball-desktop>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100430 Fedora/3.0.4-2.fc12 Lightning/1.0b2pre Thunderbird/3.0.4
On 06/17/2010 04:42 PM, Stefano Stabellini wrote:
> On Mon, 14 Jun 2010, Konrad Rzeszutek Wilk wrote:
>   
>>> +#if (defined(CONFIG_XEN_NETDEV_FRONTEND) || \
>>> +           defined(CONFIG_XEN_NETDEV_FRONTEND_MODULE)) && \
>>> +           (defined(CONFIG_XEN_PLATFORM_PCI) || \
>>> +            defined(CONFIG_XEN_PLATFORM_PCI_MODULE))
>>> +           printk(KERN_INFO "Netfront and the Xen platform PCI driver have 
>>> "
>>> +                           "been compiled for this kernel: unplug emulated 
>>> NICs.\n");
>>> +           xen_emul_unplug |= XEN_UNPLUG_ALL_NICS;
>>> +#endif
>>> +#if (defined(CONFIG_XEN_BLKDEV_FRONTEND) || \
>>> +           defined(CONFIG_XEN_BLKDEV_FRONTEND_MODULE)) && \
>>> +           (defined(CONFIG_XEN_PLATFORM_PCI) || \
>>> +            defined(CONFIG_XEN_PLATFORM_PCI_MODULE))
>>> +           printk(KERN_INFO "Blkfront and the Xen platform PCI driver have 
>>> "
>>> +                           "been compiled for this kernel: unplug emulated 
>>> disks.\n"
>>> +                           "You might have to change the root device\n"
>>> +                           "from /dev/hd[a-d] to /dev/xvd[a-d]\n"
>>> +                           "in your root= kernel command line option\n");
>>> +           xen_emul_unplug |= XEN_UNPLUG_ALL_IDE_DISKS;
>>> +#endif
>>>       
>> Wow. Can you move those checks to the header file and make it deal with
>> the #ifdef and setting of xen_emul_unplug?
>>
>>     
> I tried, but it didn't improve the elegance of the code, mainly because I
> want to keep the printk in place, so the code would look very much like
> this, but instead of being in platform-pci-unplug.c would be in
> platform_pci.h.
>   

What about using Kconfig to define an appropriate symbol and just #ifdef
on that?

>>> +   }
>>> +   /* Now unplug the emulated devices */
>>> +   if (xen_platform_pci_enabled && !(xen_emul_unplug & XEN_UNPLUG_IGNORE))
>>> +           outw(xen_emul_unplug, XEN_IOPORT_UNPLUG);
>>> +}
>>> +
>>> +static int __init parse_xen_emul_unplug(char *arg)
>>> +{
>>> +   char *p, *q;
>>> +
>>> +   for (p = arg; p; p = q) {
>>> +           q = strchr(arg, ',');
>>> +           if (q)
>>> +                   *q++ = '\0';
>>> +           if (!strcmp(p, "all"))
>>> +                   xen_emul_unplug |= XEN_UNPLUG_ALL;
>>>       
>> strncmp..
>>
>>     
> is it really needed considering that we know that both strings are NULL
> terminated and one of them is a constant?
>   

strncmp would avoid having to modify the string in place.

    J

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