[PATCH] Talk_to_backend on xenbus_probe only if drv->probe was sucessful

[PATCH] Talk_to_backend on xenbus_probe only if drv->probe was sucessful

[Murillo Bernardes]

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

Hi,

This patch makes xenbus_probe only call talk_to_backend if drv->probe() was 
sucessful.

There is no sense in add a watch before drv->probe, because if it fails the 
otherend_changed callback will be called and receive a struct with invalid 
pointers and we get a segfault (ooops).

-- 
Murillo Fernandes Bernardes
IBM Linux Technology Center

[-- Attachment #2: talk_to_backend-after-drv_probe.patch --]
[-- Type: text/x-diff, Size: 1215 bytes --]

# HG changeset patch
# User root@localhost.localdomain
# Node ID 9ecd0d7fc6693e2aaceeb5e8c28af84109e9d3d8
# Parent  bdcb115c667a12a5514517456639142c1273b0f1

Put watch on device node only if probe was sucessful.

This fix segfault on netfront when creating more than three network devices on domU.

Signed-off-by: Murillo F. Bernardes <mfb@br.ibm.com>

diff -r bdcb115c667a -r 9ecd0d7fc669 linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c
--- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c	Sat Dec 10 23:20:08 2005
+++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c	Mon Dec 12 17:50:37 2005
@@ -338,6 +338,21 @@
 
 	DPRINTK("");
 
+	if (!drv->probe) {
+		err = -ENODEV;
+		goto fail;
+	}
+
+	id = match_device(drv->ids, dev);
+	if (!id) {
+		err = -ENODEV;
+		goto fail;
+	}
+
+	err = drv->probe(dev, id);
+	if (err)
+		goto fail;
+
 	err = talk_to_otherend(dev);
 	if (err) {
 		printk(KERN_WARNING
@@ -345,21 +360,6 @@
 		       dev->nodename);
 		return err;
 	}
-
-	if (!drv->probe) {
-		err = -ENODEV;
-		goto fail;
-	}
-
-	id = match_device(drv->ids, dev);
-	if (!id) {
-		err = -ENODEV;
-		goto fail;
-	}
-
-	err = drv->probe(dev, id);
-	if (err)
-		goto fail;
 
 	return 0;
 fail:

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

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

Re: [PATCH] Talk_to_backend on xenbus_probe only if drv->probe was sucessful

[Ewan Mellor]

On Mon, Dec 12, 2005 at 04:35:45PM -0200, Murillo Bernardes wrote:

> Hi,
> 
> This patch makes xenbus_probe only call talk_to_backend if drv->probe() was 
> sucessful.
> 
> There is no sense in add a watch before drv->probe, because if it fails the 
> otherend_changed callback will be called and receive a struct with invalid 
> pointers and we get a segfault (ooops).

Applied, thanks Murillo.

Ewan.

Re: [PATCH] Talk_to_backend on xenbus_probe only if drv->probe was sucessful

[Murillo Bernardes]

On Thursday 15 December 2005 11:38, Ewan Mellor wrote:
> On Mon, Dec 12, 2005 at 04:35:45PM -0200, Murillo Bernardes wrote:
> > Hi,
> >
> > This patch makes xenbus_probe only call talk_to_backend if drv->probe()
> > was sucessful.
> >
> > There is no sense in add a watch before drv->probe, because if it fails
> > the otherend_changed callback will be called and receive a struct with
> > invalid pointers and we get a segfault (ooops).
>
> Applied, thanks Murillo.
>
> Ewan.
I just found a problem on that. It breaks block devices! I don't know why yet.

Do you know why block devices rely on that watch before probe?

Thanks


-- 
Murillo Fernandes Bernardes
IBM Linux Technology Center

Re: [PATCH] Talk_to_backend on xenbus_probe only if drv->probe was sucessful

[Ewan Mellor]

On Thu, Dec 15, 2005 at 02:59:31PM -0200, Murillo Bernardes wrote:

> On Thursday 15 December 2005 11:38, Ewan Mellor wrote:
> > On Mon, Dec 12, 2005 at 04:35:45PM -0200, Murillo Bernardes wrote:
> > > Hi,
> > >
> > > This patch makes xenbus_probe only call talk_to_backend if drv->probe()
> > > was sucessful.
> > >
> > > There is no sense in add a watch before drv->probe, because if it fails
> > > the otherend_changed callback will be called and receive a struct with
> > > invalid pointers and we get a segfault (ooops).
> >
> > Applied, thanks Murillo.
> >
> > Ewan.
> I just found a problem on that. It breaks block devices! I don't know why yet.
> 
> Do you know why block devices rely on that watch before probe?

Yes, I found that one too!  blkback_probe calls alloc_blkif(dev->otherend_id),
so it doesn't need the watch, but it does need the otherend details to have
been read.  I'm applying another patch right now.

Ewan.