Re: [Bug 1612] Can't start VM when vif set and udev version is greater than 151

Re: [Bug 1612] Can't start VM when vif set and udev version is greater than 151

[Konrad Rzeszutek Wilk]

> ------- Comment #28 from silversens@gmail.com  2010-07-05 10:20 -------
> (In reply to comment #26)
> > (In reply to comment #25)
> > > Great, 
> > > 
> > > Will it be applyed in every xen kernel, or only from 2.6.32 ?
> > > 
> > The policy is to stick bug-fixes in xen/next which are then pulled in
> > xen/stable-2.6.32.x. But this being such a serious issue, and a small patch
> > that I think Jeremy would stick it in 2.6.31 once it has passed review.
> > 
> 
> Its been a few weeks now, do you know when this patch will be included in
> jeremy's repository ? If not can you paste a link here to this patch ?
> 

Soooo.. the last I remember it was that the patch for this was not
appropiate. But I don't remember the details - is there a forthcoming
patch that can fix the outstanding issue?

Or is the patch ready and we forgot about it?

Re: [Bug 1612] Can't start VM when vif set and udev version is greater than 151

[Jan Beulich]

>>> On 06.07.10 at 17:10, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>> ------- Comment #28 from silversens@gmail.com  2010-07-05 10:20 -------
>> (In reply to comment #26)
>> > (In reply to comment #25)
>> > > Great, 
>> > > 
>> > > Will it be applyed in every xen kernel, or only from 2.6.32 ?
>> > > 
>> > The policy is to stick bug-fixes in xen/next which are then pulled in
>> > xen/stable-2.6.32.x. But this being such a serious issue, and a small patch
>> > that I think Jeremy would stick it in 2.6.31 once it has passed review.
>> > 
>> 
>> Its been a few weeks now, do you know when this patch will be included in
>> jeremy's repository ? If not can you paste a link here to this patch ?
>> 
> 
> Soooo.. the last I remember it was that the patch for this was not
> appropiate. But I don't remember the details - is there a forthcoming
> patch that can fix the outstanding issue?
> 
> Or is the patch ready and we forgot about it?

For reference, below is the diff for what we're now using.

Jan

--- a/drivers/xen/netback/xenbus.c
+++ b/drivers/xen/netback/xenbus.c
@@ -19,6 +19,7 @@
 
 #include <stdarg.h>
 #include <linux/module.h>
+#include <linux/rwsem.h>
 #include <xen/xenbus.h>
 #include "common.h"
 
@@ -28,6 +29,7 @@
     printk("netback/xenbus (%s:%d) " fmt ".\n", __FUNCTION__, __LINE__, ##args)
 #endif
 
+static DECLARE_RWSEM(teardown_sem);
 
 static int connect_rings(struct backend_info *);
 static void connect(struct backend_info *);
@@ -39,13 +41,18 @@ static int netback_remove(struct xenbus_
 
 	netback_remove_accelerators(be, dev);
 
-	if (be->netif) {
+	if (be->netif)
 		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
+
+	down_write(&teardown_sem);
+	if (be->netif) {
 		netif_disconnect(be->netif);
 		be->netif = NULL;
 	}
-	kfree(be);
 	dev_set_drvdata(&dev->dev, NULL);
+	up_write(&teardown_sem);
+	kfree(be);
+
 	return 0;
 }
 
@@ -151,8 +158,7 @@ fail:
  */
 static int netback_uevent(struct xenbus_device *xdev, struct kobj_uevent_env *env)
 {
-	struct backend_info *be = dev_get_drvdata(&xdev->dev);
-	netif_t *netif = be->netif;
+	struct backend_info *be;
 	char *val;
 
 	DPRINTK("netback_uevent");
@@ -163,12 +169,15 @@ static int netback_uevent(struct xenbus_
 		xenbus_dev_fatal(xdev, err, "reading script");
 		return err;
 	}
-	else {
-		add_uevent_var(env, "script=%s", val);
-		kfree(val);
-	}
 
-	add_uevent_var(env, "vif=%s", netif->dev->name);
+	add_uevent_var(env, "script=%s", val);
+	kfree(val);
+
+	down_read(&teardown_sem);
+	be = dev_get_drvdata(&xdev->dev);
+	if (be && be->netif)
+		add_uevent_var(env, "vif=%s", be->netif->dev->name);
+	up_read(&teardown_sem);
 
 	return 0;
 }
@@ -179,6 +188,7 @@ static void backend_create_netif(struct 
 	int err;
 	long handle;
 	struct xenbus_device *dev = be->dev;
+	netif_t *netif;
 
 	if (be->netif != NULL)
 		return;
@@ -189,13 +199,13 @@ static void backend_create_netif(struct 
 		return;
 	}
 
-	be->netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
-	if (IS_ERR(be->netif)) {
-		err = PTR_ERR(be->netif);
-		be->netif = NULL;
+	netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
+	if (IS_ERR(netif)) {
+		err = PTR_ERR(netif);
 		xenbus_dev_fatal(dev, err, "creating interface");
 		return;
 	}
+	be->netif = netif;
 
 	kobject_uevent(&dev->dev.kobj, KOBJ_ONLINE);
 }

Re: Re: [Bug 1612] Can't start VM when vif set and udev version is greater than 151

[Konrad Rzeszutek Wilk]

> > Soooo.. the last I remember it was that the patch for this was not
> > appropiate. But I don't remember the details - is there a forthcoming
> > patch that can fix the outstanding issue?
> > 
> > Or is the patch ready and we forgot about it?
> 
> For reference, below is the diff for what we're now using.

Ah nice.. 

I get this:
patch -p1 --dry-run < ~/p.patch 
patching file drivers/xen/netback/xenbus.c
Hunk #1 succeeded at 19 with fuzz 1.
Hunk #2 FAILED at 29.
Hunk #3 FAILED at 40.
patch: **** malformed patch at line 120: nv *env)

If it would not be too much trouble, can you attach it as an attachment?

> 
> Jan
> 
> --- a/drivers/xen/netback/xenbus.c
> +++ b/drivers/xen/netback/xenbus.c
> @@ -19,6 +19,7 @@
>  
>  #include <stdarg.h>
>  #include <linux/module.h>
> +#include <linux/rwsem.h>
>  #include <xen/xenbus.h>
>  #include "common.h"
>  
> @@ -28,6 +29,7 @@
>      printk("netback/xenbus (%s:%d) " fmt ".\n", __FUNCTION__, __LINE__, ##args)
>  #endif
>  
> +static DECLARE_RWSEM(teardown_sem);
>  
>  static int connect_rings(struct backend_info *);
>  static void connect(struct backend_info *);
> @@ -39,13 +41,18 @@ static int netback_remove(struct xenbus_
>  
>  	netback_remove_accelerators(be, dev);
>  
> -	if (be->netif) {
> +	if (be->netif)
>  		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
> +
> +	down_write(&teardown_sem);
> +	if (be->netif) {
>  		netif_disconnect(be->netif);
>  		be->netif = NULL;
>  	}
> -	kfree(be);
>  	dev_set_drvdata(&dev->dev, NULL);
> +	up_write(&teardown_sem);
> +	kfree(be);
> +
>  	return 0;
>  }
>  
> @@ -151,8 +158,7 @@ fail:
>   */
>  static int netback_uevent(struct xenbus_device *xdev, struct kobj_uevent_env *env)
>  {
> -	struct backend_info *be = dev_get_drvdata(&xdev->dev);
> -	netif_t *netif = be->netif;
> +	struct backend_info *be;
>  	char *val;
>  
>  	DPRINTK("netback_uevent");
> @@ -163,12 +169,15 @@ static int netback_uevent(struct xenbus_
>  		xenbus_dev_fatal(xdev, err, "reading script");
>  		return err;
>  	}
> -	else {
> -		add_uevent_var(env, "script=%s", val);
> -		kfree(val);
> -	}
>  
> -	add_uevent_var(env, "vif=%s", netif->dev->name);
> +	add_uevent_var(env, "script=%s", val);
> +	kfree(val);
> +
> +	down_read(&teardown_sem);
> +	be = dev_get_drvdata(&xdev->dev);
> +	if (be && be->netif)
> +		add_uevent_var(env, "vif=%s", be->netif->dev->name);
> +	up_read(&teardown_sem);
>  
>  	return 0;
>  }
> @@ -179,6 +188,7 @@ static void backend_create_netif(struct 
>  	int err;
>  	long handle;
>  	struct xenbus_device *dev = be->dev;
> +	netif_t *netif;
>  
>  	if (be->netif != NULL)
>  		return;
> @@ -189,13 +199,13 @@ static void backend_create_netif(struct 
>  		return;
>  	}
>  
> -	be->netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
> -	if (IS_ERR(be->netif)) {
> -		err = PTR_ERR(be->netif);
> -		be->netif = NULL;
> +	netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
> +	if (IS_ERR(netif)) {
> +		err = PTR_ERR(netif);
>  		xenbus_dev_fatal(dev, err, "creating interface");
>  		return;
>  	}
> +	be->netif = netif;
>  
>  	kobject_uevent(&dev->dev.kobj, KOBJ_ONLINE);
>  }
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: Re: [Bug 1612] Can't start VM when vif set and udev version is greater than 151

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 517 bytes --]

>>> On 06.07.10 at 18:46, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>> For reference, below is the diff for what we're now using.
> 
> Ah nice.. 
> 
> I get this:
> patch -p1 --dry-run < ~/p.patch 
> patching file drivers/xen/netback/xenbus.c
> Hunk #1 succeeded at 19 with fuzz 1.
> Hunk #2 FAILED at 29.
> Hunk #3 FAILED at 40.
> patch: **** malformed patch at line 120: nv *env)
> 
> If it would not be too much trouble, can you attach it as an attachment?

Sure, here you go.

Jan


[-- Attachment #2: netback-xenbus.diff --]
[-- Type: text/plain, Size: 2504 bytes --]

--- cpp/kernel/head-2010-05-12/drivers/xen/netback/xenbus.c	2010-03-02 10:23:37.000000000 +0100
+++ cpp/kernel/head-2010-06-22/drivers/xen/netback/xenbus.c	2010-07-05 12:03:12.000000000 +0200
@@ -19,6 +19,7 @@
 
 #include <stdarg.h>
 #include <linux/module.h>
+#include <linux/rwsem.h>
 #include <xen/xenbus.h>
 #include "common.h"
 
@@ -28,6 +29,7 @@
     printk("netback/xenbus (%s:%d) " fmt ".\n", __FUNCTION__, __LINE__, ##args)
 #endif
 
+static DECLARE_RWSEM(teardown_sem);
 
 static int connect_rings(struct backend_info *);
 static void connect(struct backend_info *);
@@ -39,13 +41,18 @@ static int netback_remove(struct xenbus_
 
 	netback_remove_accelerators(be, dev);
 
-	if (be->netif) {
+	if (be->netif)
 		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
+
+	down_write(&teardown_sem);
+	if (be->netif) {
 		netif_disconnect(be->netif);
 		be->netif = NULL;
 	}
-	kfree(be);
 	dev_set_drvdata(&dev->dev, NULL);
+	up_write(&teardown_sem);
+	kfree(be);
+
 	return 0;
 }
 
@@ -151,8 +158,7 @@ fail:
  */
 static int netback_uevent(struct xenbus_device *xdev, struct kobj_uevent_env *env)
 {
-	struct backend_info *be = dev_get_drvdata(&xdev->dev);
-	netif_t *netif = be->netif;
+	struct backend_info *be;
 	char *val;
 
 	DPRINTK("netback_uevent");
@@ -163,12 +169,15 @@ static int netback_uevent(struct xenbus_
 		xenbus_dev_fatal(xdev, err, "reading script");
 		return err;
 	}
-	else {
-		add_uevent_var(env, "script=%s", val);
-		kfree(val);
-	}
 
-	add_uevent_var(env, "vif=%s", netif->dev->name);
+	add_uevent_var(env, "script=%s", val);
+	kfree(val);
+
+	down_read(&teardown_sem);
+	be = dev_get_drvdata(&xdev->dev);
+	if (be && be->netif)
+		add_uevent_var(env, "vif=%s", be->netif->dev->name);
+	up_read(&teardown_sem);
 
 	return 0;
 }
@@ -179,6 +188,7 @@ static void backend_create_netif(struct 
 	int err;
 	long handle;
 	struct xenbus_device *dev = be->dev;
+	netif_t *netif;
 
 	if (be->netif != NULL)
 		return;
@@ -189,13 +199,13 @@ static void backend_create_netif(struct 
 		return;
 	}
 
-	be->netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
-	if (IS_ERR(be->netif)) {
-		err = PTR_ERR(be->netif);
-		be->netif = NULL;
+	netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
+	if (IS_ERR(netif)) {
+		err = PTR_ERR(netif);
 		xenbus_dev_fatal(dev, err, "creating interface");
 		return;
 	}
+	be->netif = netif;
 
 	kobject_uevent(&dev->dev.kobj, KOBJ_ONLINE);
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: Re: [Bug 1612] Can't start VM when vif set and udev version is greater than 151

[Konrad Rzeszutek Wilk]

[-- Attachment #1: Type: text/plain, Size: 3167 bytes --]

On Wed, Jul 07, 2010 at 08:30:19AM +0100, Jan Beulich wrote:
> >>> On 06.07.10 at 18:46, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> >> For reference, below is the diff for what we're now using.
> > 
> > Ah nice.. 
> > 
> > I get this:
> > patch -p1 --dry-run < ~/p.patch 
> > patching file drivers/xen/netback/xenbus.c
> > Hunk #1 succeeded at 19 with fuzz 1.
> > Hunk #2 FAILED at 29.
> > Hunk #3 FAILED at 40.
> > patch: **** malformed patch at line 120: nv *env)
> > 
> > If it would not be too much trouble, can you attach it as an attachment?
> 
> Sure, here you go.

And attached is it redone against the pv-ops kernel. Let me poke the
folks on the BZ to see if they would be willing to test this.


diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
index ba7b1de..805ce48 100644
--- a/drivers/xen/netback/xenbus.c
+++ b/drivers/xen/netback/xenbus.c
@@ -19,6 +19,7 @@
 
 #include <stdarg.h>
 #include <linux/module.h>
+#include <linux/rwsem.h>
 #include <xen/xenbus.h>
 #include "common.h"
 
@@ -28,6 +29,7 @@
     printk("netback/xenbus (%s:%d) " fmt ".\n", __FUNCTION__, __LINE__, ##args)
 #endif
 
+static DECLARE_RWSEM(teardown_sem);
 
 static int connect_rings(struct backend_info *);
 static void connect(struct backend_info *);
@@ -41,14 +43,19 @@ static int netback_remove(struct xenbus_device *dev)
 	//netback_remove_accelerators(be, dev);
 
 	unregister_hotplug_status_watch(be);
-	if (be->netif) {
+	if (be->netif)
 		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
+
+	down_write(&teardown_sem);
+	if (be->netif) {
 		xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
 		netif_disconnect(be->netif);
 		be->netif = NULL;
 	}
-	kfree(be);
 	dev_set_drvdata(&dev->dev, NULL);
+	up_write(&teardown_sem);
+	kfree(be);
+
 	return 0;
 }
 
@@ -168,7 +175,7 @@ static int netback_uevent(struct xenbus_device *xdev, struct kobj_uevent_env *en
 
 	DPRINTK("netback_uevent");
 
-	be = dev_get_drvdata(&xdev->dev);
+	be = dev_get_drvdata(&xdev->dev);/
 	if (!be)
 		return 0;
 	netif = be->netif;
@@ -187,9 +194,14 @@ static int netback_uevent(struct xenbus_device *xdev, struct kobj_uevent_env *en
 		kfree(val);
 	}
 
-	if (add_uevent_var(env, "vif=%s", netif->dev->name))
-		return -ENOMEM;
-
+	down_read(&teardown_sem);
+	be = dev_get_drvdata(&xdev->dev);
+	if (be && be->netif)
+		if (add_uevent_var(env, "vif=%s", netif->dev->name)) {
+			up_read(&teardown_sem);
+			return -ENOMEM;
+		}
+	up_read(&teardown_sem);
 	return 0;
 }
 
@@ -199,6 +211,7 @@ static void backend_create_netif(struct backend_info *be)
 	int err;
 	long handle;
 	struct xenbus_device *dev = be->dev;
+	netif_t *netif;
 
 	if (be->netif != NULL)
 		return;
@@ -209,13 +222,13 @@ static void backend_create_netif(struct backend_info *be)
 		return;
 	}
 
-	be->netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
-	if (IS_ERR(be->netif)) {
-		err = PTR_ERR(be->netif);
-		be->netif = NULL;
+	netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
+	if (IS_ERR(netif)) {
+		err = PTR_ERR(netif);
 		xenbus_dev_fatal(dev, err, "creating interface");
 		return;
 	}
+	be->netif = netif;
 
 	kobject_uevent(&dev->dev.kobj, KOBJ_ONLINE);
 }

[-- Attachment #2: netback-xenbus-redone.diff --]
[-- Type: text/plain, Size: 2448 bytes --]

diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
index ba7b1de..805ce48 100644
--- a/drivers/xen/netback/xenbus.c
+++ b/drivers/xen/netback/xenbus.c
@@ -19,6 +19,7 @@
 
 #include <stdarg.h>
 #include <linux/module.h>
+#include <linux/rwsem.h>
 #include <xen/xenbus.h>
 #include "common.h"
 
@@ -28,6 +29,7 @@
     printk("netback/xenbus (%s:%d) " fmt ".\n", __FUNCTION__, __LINE__, ##args)
 #endif
 
+static DECLARE_RWSEM(teardown_sem);
 
 static int connect_rings(struct backend_info *);
 static void connect(struct backend_info *);
@@ -41,14 +43,19 @@ static int netback_remove(struct xenbus_device *dev)
 	//netback_remove_accelerators(be, dev);
 
 	unregister_hotplug_status_watch(be);
-	if (be->netif) {
+	if (be->netif)
 		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
+
+	down_write(&teardown_sem);
+	if (be->netif) {
 		xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
 		netif_disconnect(be->netif);
 		be->netif = NULL;
 	}
-	kfree(be);
 	dev_set_drvdata(&dev->dev, NULL);
+	up_write(&teardown_sem);
+	kfree(be);
+
 	return 0;
 }
 
@@ -168,7 +175,7 @@ static int netback_uevent(struct xenbus_device *xdev, struct kobj_uevent_env *en
 
 	DPRINTK("netback_uevent");
 
-	be = dev_get_drvdata(&xdev->dev);
+	be = dev_get_drvdata(&xdev->dev);/
 	if (!be)
 		return 0;
 	netif = be->netif;
@@ -187,9 +194,14 @@ static int netback_uevent(struct xenbus_device *xdev, struct kobj_uevent_env *en
 		kfree(val);
 	}
 
-	if (add_uevent_var(env, "vif=%s", netif->dev->name))
-		return -ENOMEM;
-
+	down_read(&teardown_sem);
+	be = dev_get_drvdata(&xdev->dev);
+	if (be && be->netif)
+		if (add_uevent_var(env, "vif=%s", netif->dev->name)) {
+			up_read(&teardown_sem);
+			return -ENOMEM;
+		}
+	up_read(&teardown_sem);
 	return 0;
 }
 
@@ -199,6 +211,7 @@ static void backend_create_netif(struct backend_info *be)
 	int err;
 	long handle;
 	struct xenbus_device *dev = be->dev;
+	netif_t *netif;
 
 	if (be->netif != NULL)
 		return;
@@ -209,13 +222,13 @@ static void backend_create_netif(struct backend_info *be)
 		return;
 	}
 
-	be->netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
-	if (IS_ERR(be->netif)) {
-		err = PTR_ERR(be->netif);
-		be->netif = NULL;
+	netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
+	if (IS_ERR(netif)) {
+		err = PTR_ERR(netif);
 		xenbus_dev_fatal(dev, err, "creating interface");
 		return;
 	}
+	be->netif = netif;
 
 	kobject_uevent(&dev->dev.kobj, KOBJ_ONLINE);
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: Re: [Bug 1612] Can't start VM when vif set and udev version is greater than 151

[Jeremy Fitzhardinge]

On 07/07/2010 07:48 AM, Konrad Rzeszutek Wilk wrote:
> On Wed, Jul 07, 2010 at 08:30:19AM +0100, Jan Beulich wrote:
>   
>>>>> On 06.07.10 at 18:46, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>>>>>           
>>>> For reference, below is the diff for what we're now using.
>>>>         
>>> Ah nice.. 
>>>
>>> I get this:
>>> patch -p1 --dry-run < ~/p.patch 
>>> patching file drivers/xen/netback/xenbus.c
>>> Hunk #1 succeeded at 19 with fuzz 1.
>>> Hunk #2 FAILED at 29.
>>> Hunk #3 FAILED at 40.
>>> patch: **** malformed patch at line 120: nv *env)
>>>
>>> If it would not be too much trouble, can you attach it as an attachment?
>>>       
>> Sure, here you go.
>>     
> And attached is it redone against the pv-ops kernel. Let me poke the
> folks on the BZ to see if they would be willing to test this.
>
>
> diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
> index ba7b1de..805ce48 100644
> --- a/drivers/xen/netback/xenbus.c
> +++ b/drivers/xen/netback/xenbus.c
> @@ -19,6 +19,7 @@
>  
>  #include <stdarg.h>
>  #include <linux/module.h>
> +#include <linux/rwsem.h>
>  #include <xen/xenbus.h>
>  #include "common.h"
>  
> @@ -28,6 +29,7 @@
>      printk("netback/xenbus (%s:%d) " fmt ".\n", __FUNCTION__, __LINE__, ##args)
>  #endif
>  
> +static DECLARE_RWSEM(teardown_sem);
>  
>  static int connect_rings(struct backend_info *);
>  static void connect(struct backend_info *);
> @@ -41,14 +43,19 @@ static int netback_remove(struct xenbus_device *dev)
>  	//netback_remove_accelerators(be, dev);
>  
>  	unregister_hotplug_status_watch(be);
> -	if (be->netif) {
> +	if (be->netif)
>  		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
> +
> +	down_write(&teardown_sem);
> +	if (be->netif) {
>  		xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
>  		netif_disconnect(be->netif);
>  		be->netif = NULL;
>  	}
> -	kfree(be);
>  	dev_set_drvdata(&dev->dev, NULL);
> +	up_write(&teardown_sem);
> +	kfree(be);
> +
>  	return 0;
>  }
>  
> @@ -168,7 +175,7 @@ static int netback_uevent(struct xenbus_device *xdev, struct kobj_uevent_env *en
>  
>  	DPRINTK("netback_uevent");
>  
> -	be = dev_get_drvdata(&xdev->dev);
> +	be = dev_get_drvdata(&xdev->dev);/
>   

Hm.  Post-compile typo?

>  	if (!be)
>  		return 0;
>  	netif = be->netif;
> @@ -187,9 +194,14 @@ static int netback_uevent(struct xenbus_device *xdev, struct kobj_uevent_env *en
>  		kfree(val);
>  	}
>  
> -	if (add_uevent_var(env, "vif=%s", netif->dev->name))
> -		return -ENOMEM;
> -
> +	down_read(&teardown_sem);
> +	be = dev_get_drvdata(&xdev->dev);
> +	if (be && be->netif)
> +		if (add_uevent_var(env, "vif=%s", netif->dev->name)) {
>   

I would tend to fold these into a single if(); the double if looks odd.

> +			up_read(&teardown_sem);
> +			return -ENOMEM;
> +		}
> +	up_read(&teardown_sem);
>  	return 0;
>  }
>  
> @@ -199,6 +211,7 @@ static void backend_create_netif(struct backend_info *be)
>  	int err;
>  	long handle;
>  	struct xenbus_device *dev = be->dev;
> +	netif_t *netif;
>  
>  	if (be->netif != NULL)
>  		return;
> @@ -209,13 +222,13 @@ static void backend_create_netif(struct backend_info *be)
>  		return;
>  	}
>  
> -	be->netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
> -	if (IS_ERR(be->netif)) {
> -		err = PTR_ERR(be->netif);
> -		be->netif = NULL;
> +	netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
> +	if (IS_ERR(netif)) {
> +		err = PTR_ERR(netif);
>  		xenbus_dev_fatal(dev, err, "creating interface");
>  		return;
>  	}
> +	be->netif = netif;
>  
>  	kobject_uevent(&dev->dev.kobj, KOBJ_ONLINE);
>  }
>   

    J

Re: Re: [Bug 1612] Can't start VM when vif set and udev version is greater than 151

[Konrad Rzeszutek Wilk]

[-- Attachment #1: Type: text/plain, Size: 3304 bytes --]

On Wed, Jul 07, 2010 at 08:26:20AM -0700, Jeremy Fitzhardinge wrote:
> On 07/07/2010 07:48 AM, Konrad Rzeszutek Wilk wrote:
> > On Wed, Jul 07, 2010 at 08:30:19AM +0100, Jan Beulich wrote:
> >   
> >>>>> On 06.07.10 at 18:46, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> >>>>>           
> >>>> For reference, below is the diff for what we're now using.
> >>>>         
> >>> Ah nice.. 
> >>>
> >>> I get this:
> >>> patch -p1 --dry-run < ~/p.patch 
> >>> patching file drivers/xen/netback/xenbus.c
> >>> Hunk #1 succeeded at 19 with fuzz 1.
> >>> Hunk #2 FAILED at 29.
> >>> Hunk #3 FAILED at 40.
> >>> patch: **** malformed patch at line 120: nv *env)
> >>>
> >>> If it would not be too much trouble, can you attach it as an attachment?
> >>>       
> >> Sure, here you go.
> >>     
> > And attached is it redone against the pv-ops kernel. Let me poke the
> > folks on the BZ to see if they would be willing to test this.

Here is a redux that is compile and regression tested. And I've
addressed your concerns Jeremy. Putting this on the BZ to solicit some
testing from folks.

diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
index ba7b1de..7a9ae2e 100644
--- a/drivers/xen/netback/xenbus.c
+++ b/drivers/xen/netback/xenbus.c
@@ -19,6 +19,7 @@
 
 #include <stdarg.h>
 #include <linux/module.h>
+#include <linux/rwsem.h>
 #include <xen/xenbus.h>
 #include "common.h"
 
@@ -28,6 +29,7 @@
     printk("netback/xenbus (%s:%d) " fmt ".\n", __FUNCTION__, __LINE__, ##args)
 #endif
 
+static DECLARE_RWSEM(teardown_sem);
 
 static int connect_rings(struct backend_info *);
 static void connect(struct backend_info *);
@@ -41,14 +43,19 @@ static int netback_remove(struct xenbus_device *dev)
 	//netback_remove_accelerators(be, dev);
 
 	unregister_hotplug_status_watch(be);
-	if (be->netif) {
+	if (be->netif)
 		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
+
+	down_write(&teardown_sem);
+	if (be->netif) {
 		xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
 		netif_disconnect(be->netif);
 		be->netif = NULL;
 	}
-	kfree(be);
 	dev_set_drvdata(&dev->dev, NULL);
+	up_write(&teardown_sem);
+	kfree(be);
+
 	return 0;
 }
 
@@ -187,9 +194,15 @@ static int netback_uevent(struct xenbus_device *xdev, struct kobj_uevent_env *en
 		kfree(val);
 	}
 
-	if (add_uevent_var(env, "vif=%s", netif->dev->name))
-		return -ENOMEM;
-
+	down_read(&teardown_sem);
+	be = dev_get_drvdata(&xdev->dev);
+	if (be && be->netif) {
+		if (add_uevent_var(env, "vif=%s", netif->dev->name)) {
+			up_read(&teardown_sem);
+			return -ENOMEM;
+		}
+	}
+	up_read(&teardown_sem);
 	return 0;
 }
 
@@ -199,6 +212,7 @@ static void backend_create_netif(struct backend_info *be)
 	int err;
 	long handle;
 	struct xenbus_device *dev = be->dev;
+	struct xen_netif *netif;
 
 	if (be->netif != NULL)
 		return;
@@ -209,13 +223,13 @@ static void backend_create_netif(struct backend_info *be)
 		return;
 	}
 
-	be->netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
-	if (IS_ERR(be->netif)) {
-		err = PTR_ERR(be->netif);
-		be->netif = NULL;
+	netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
+	if (IS_ERR(netif)) {
+		err = PTR_ERR(netif);
 		xenbus_dev_fatal(dev, err, "creating interface");
 		return;
 	}
+	be->netif = netif;
 
 	kobject_uevent(&dev->dev.kobj, KOBJ_ONLINE);
 }

[-- Attachment #2: netback-xenbus-redone.diff --]
[-- Type: text/plain, Size: 2211 bytes --]

diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
index ba7b1de..7a9ae2e 100644
--- a/drivers/xen/netback/xenbus.c
+++ b/drivers/xen/netback/xenbus.c
@@ -19,6 +19,7 @@
 
 #include <stdarg.h>
 #include <linux/module.h>
+#include <linux/rwsem.h>
 #include <xen/xenbus.h>
 #include "common.h"
 
@@ -28,6 +29,7 @@
     printk("netback/xenbus (%s:%d) " fmt ".\n", __FUNCTION__, __LINE__, ##args)
 #endif
 
+static DECLARE_RWSEM(teardown_sem);
 
 static int connect_rings(struct backend_info *);
 static void connect(struct backend_info *);
@@ -41,14 +43,19 @@ static int netback_remove(struct xenbus_device *dev)
 	//netback_remove_accelerators(be, dev);
 
 	unregister_hotplug_status_watch(be);
-	if (be->netif) {
+	if (be->netif)
 		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
+
+	down_write(&teardown_sem);
+	if (be->netif) {
 		xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
 		netif_disconnect(be->netif);
 		be->netif = NULL;
 	}
-	kfree(be);
 	dev_set_drvdata(&dev->dev, NULL);
+	up_write(&teardown_sem);
+	kfree(be);
+
 	return 0;
 }
 
@@ -187,9 +194,15 @@ static int netback_uevent(struct xenbus_device *xdev, struct kobj_uevent_env *en
 		kfree(val);
 	}
 
-	if (add_uevent_var(env, "vif=%s", netif->dev->name))
-		return -ENOMEM;
-
+	down_read(&teardown_sem);
+	be = dev_get_drvdata(&xdev->dev);
+	if (be && be->netif) {
+		if (add_uevent_var(env, "vif=%s", netif->dev->name)) {
+			up_read(&teardown_sem);
+			return -ENOMEM;
+		}
+	}
+	up_read(&teardown_sem);
 	return 0;
 }
 
@@ -199,6 +212,7 @@ static void backend_create_netif(struct backend_info *be)
 	int err;
 	long handle;
 	struct xenbus_device *dev = be->dev;
+	struct xen_netif *netif;
 
 	if (be->netif != NULL)
 		return;
@@ -209,13 +223,13 @@ static void backend_create_netif(struct backend_info *be)
 		return;
 	}
 
-	be->netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
-	if (IS_ERR(be->netif)) {
-		err = PTR_ERR(be->netif);
-		be->netif = NULL;
+	netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
+	if (IS_ERR(netif)) {
+		err = PTR_ERR(netif);
 		xenbus_dev_fatal(dev, err, "creating interface");
 		return;
 	}
+	be->netif = netif;
 
 	kobject_uevent(&dev->dev.kobj, KOBJ_ONLINE);
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: Re: [Bug 1612] Can't start VM when vif set and udev version is greater than 151

[Jeremy Fitzhardinge]

On 07/07/2010 10:18 AM, Konrad Rzeszutek Wilk wrote:
> Here is a redux that is compile and regression tested. And I've
> addressed your concerns Jeremy. Putting this on the BZ to solicit some
> testing from folks.
>
> diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
> index ba7b1de..7a9ae2e 100644
> --- a/drivers/xen/netback/xenbus.c
> +++ b/drivers/xen/netback/xenbus.c
> @@ -19,6 +19,7 @@
>  
>  #include <stdarg.h>
>  #include <linux/module.h>
> +#include <linux/rwsem.h>
>  #include <xen/xenbus.h>
>  #include "common.h"
>  
> @@ -28,6 +29,7 @@
>      printk("netback/xenbus (%s:%d) " fmt ".\n", __FUNCTION__, __LINE__, ##args)
>  #endif
>  
> +static DECLARE_RWSEM(teardown_sem);
>  
>  static int connect_rings(struct backend_info *);
>  static void connect(struct backend_info *);
> @@ -41,14 +43,19 @@ static int netback_remove(struct xenbus_device *dev)
>  	//netback_remove_accelerators(be, dev);
>  
>  	unregister_hotplug_status_watch(be);
> -	if (be->netif) {
> +	if (be->netif)
>  		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
> +
> +	down_write(&teardown_sem);
> +	if (be->netif) {
>  		xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
>  		netif_disconnect(be->netif);
>  		be->netif = NULL;
>  	}
> -	kfree(be);
>  	dev_set_drvdata(&dev->dev, NULL);
> +	up_write(&teardown_sem);
> +	kfree(be);
> +
>  	return 0;
>  }
>  
> @@ -187,9 +194,15 @@ static int netback_uevent(struct xenbus_device *xdev, struct kobj_uevent_env *en
>  		kfree(val);
>  	}
>  
> -	if (add_uevent_var(env, "vif=%s", netif->dev->name))
> -		return -ENOMEM;
> -
> +	down_read(&teardown_sem);
> +	be = dev_get_drvdata(&xdev->dev);
> +	if (be && be->netif) {
> +		if (add_uevent_var(env, "vif=%s", netif->dev->name)) {
>   

The extra {} helps, but I was thinking more along the lines of:

	if (be && be->netif && add_uevent_var(env, "vif=%s", netif->dev->name)) {
		...

(with line break after the second && if that's too long).

> +			up_read(&teardown_sem);
> +			return -ENOMEM;
> +		}
> +	}
> +	up_read(&teardown_sem);
>  	return 0;
>  }
>  
> @@ -199,6 +212,7 @@ static void backend_create_netif(struct backend_info *be)
>  	int err;
>  	long handle;
>  	struct xenbus_device *dev = be->dev;
> +	struct xen_netif *netif;
>  
>  	if (be->netif != NULL)
>  		return;
> @@ -209,13 +223,13 @@ static void backend_create_netif(struct backend_info *be)
>  		return;
>  	}
>  
> -	be->netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
> -	if (IS_ERR(be->netif)) {
> -		err = PTR_ERR(be->netif);
> -		be->netif = NULL;
> +	netif = netif_alloc(&dev->dev, dev->otherend_id, handle);
> +	if (IS_ERR(netif)) {
> +		err = PTR_ERR(netif);
>  		xenbus_dev_fatal(dev, err, "creating interface");
>  		return;
>  	}
> +	be->netif = netif;
>  
>  	kobject_uevent(&dev->dev.kobj, KOBJ_ONLINE);
>  }
>   

Set up bridging under Xen 4.1-unstable ( 2.6.31.15 pvops) on top of Ubuntu 10.04 Server

[Boris Derzhavets]

[-- Attachment #1.1: Type: text/plain, Size: 702 bytes --]

After services 
 1.xencommons
 2.xend
 3.xendomains 
startup  xen bridge declaration via /etc/xen/xend-config.sxp completely ignored.

Bridging setup manually via /etc/network/interfaces :-

 # The loopback network interface
auto lo
iface lo inet loopback

# The primary network interface
# auto eth0
# iface eth0 inet dhcp
 auto br0
 iface br0 inet static
         address 192.168.1.7
         netmask 255.255.255.0
         network 192.168.1.0
         broadcast 192.168.1.255
         gateway 192.168.1.1
         bridge_ports eth0
         bridge_stp on
         bridge_maxwait 0

# /etc/init.d/networking restart

Am i missing something here ?

Boris.




      

[-- Attachment #1.2: Type: text/html, Size: 1238 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: Set up bridging under Xen 4.1-unstable ( 2.6.31.15 pvops) on top of Ubuntu 10.04 Server

[Sander Eikelenboom]

Hello Boris,

Dont the default vif scripts connect to xen_br0 and not br0 ?
I specify a different bridge name in the domU config files vif line with bridge=

--
Sander


Wednesday, July 7, 2010, 7:57:13 PM, you wrote:

> After services 
>  1.xencommons
>  2.xend
>  3.xendomains 
> startup  xen bridge declaration via /etc/xen/xend-config.sxp completely ignored.

> Bridging setup manually via /etc/network/interfaces :-

>  # The loopback network interface
> auto lo
> iface lo inet loopback

> # The primary network interface
> # auto eth0
> # iface eth0 inet dhcp
>  auto br0
>  iface br0 inet static
>          address 192.168.1.7
>          netmask 255.255.255.0
>          network 192.168.1.0
>          broadcast 192.168.1.255
>          gateway 192.168.1.1
>          bridge_ports eth0
>          bridge_stp on
>          bridge_maxwait 0

> # /etc/init.d/networking restart

> Am i missing something here ?

> Boris.




>       



-- 
Best regards,
 Sander                            mailto:linux@eikelenboom.it

Re: Set up bridging under Xen 4.1-unstable ( 2.6.31.15 pvops) on top of Ubuntu 10.04 Server

[Boris Derzhavets]

[-- Attachment #1.1: Type: text/plain, Size: 2040 bytes --]

Sander,

  I believe,  my question is a bit different . I can work with br0 with no problems
  for DomUs networking.
  Up to 4.0.1 i could define bridge in /etc/xen/xend-config.sxp and get this bridge
  reported by "brctl show" , but now i cannot do that. Seems like manual intervention
  in Ubuntu /etc/network/interfaces is required.

Boris.

--- On Wed, 7/7/10, Sander Eikelenboom <linux@eikelenboom.it> wrote:

From: Sander Eikelenboom <linux@eikelenboom.it>
Subject: Re: [Xen-devel] Set up bridging under Xen 4.1-unstable ( 2.6.31.15 pvops) on top of Ubuntu 10.04 Server
To: "Boris Derzhavets" <bderzhavets@yahoo.com>
Cc: "Jeremy Fitzhardinge" <jeremy@goop.org>, xen-devel@lists.xensource.com, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>
Date: Wednesday, July 7, 2010, 2:21 PM

Hello Boris,

Dont the default vif scripts connect to xen_br0 and not br0 ?
I specify a different bridge name in the domU config files vif line with bridge=

--
Sander


Wednesday, July 7, 2010, 7:57:13 PM, you wrote:

> After services 
>  1.xencommons
>  2.xend
>  3.xendomains 
> startup  xen bridge declaration via /etc/xen/xend-config.sxp completely ignored.

> Bridging setup manually via /etc/network/interfaces :-

>  # The loopback network interface
> auto lo
> iface lo inet loopback

> # The primary network interface
> # auto eth0
> # iface eth0 inet dhcp
>  auto br0
>  iface br0 inet static
>          address 192.168.1.7
>          netmask 255.255.255.0
>          network 192.168.1.0
>          broadcast 192.168.1.255
>          gateway 192.168.1.1
>          bridge_ports eth0
>          bridge_stp on
>          bridge_maxwait 0

> # /etc/init.d/networking restart

> Am i missing something here ?

> Boris.




>       



-- 
Best regards,
 Sander                            mailto:linux@eikelenboom.it


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



      

[-- Attachment #1.2: Type: text/html, Size: 3330 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: Set up bridging under Xen 4.1-unstable ( 2.6.31.15 pvops) on top of Ubuntu 10.04 Server

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 664 bytes --]

On Wed, 7 Jul 2010, Boris Derzhavets wrote:
> Sander,
> 
>   I believe,  my question is a bit different . I can work with br0 with no problems
>   for DomUs networking.
>   Up to 4.0.1 i could define bridge in /etc/xen/xend-config.sxp and get this bridge
>   reported by "brctl show" , but now i cannot do that. Seems like manual intervention
>   in Ubuntu /etc/network/interfaces is required.

Yes, a little while ago there was a discussion on the list and the
outcome was that the proper way to setup bridges is to use the tools
provided by the distro.
However users can still call the old script manually, that is
/etc/xen/scripts/network-bridge.

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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