Re: [Xen-devel] [PATCH v2] xen-netback: fix error handling on netback_probe()

[David Miller]

From: Filipe Manco 
Date: Thu, 15 Sep 2016 17:10:46 +0200

> In case of error during netback_probe() (e.g. an entry missing on the
> xenstore) netback_remove() is called on the new device, which will set
> the device backend state to XenbusStateClosed by calling
> set_backend_state(). However, the backend state wasn't initialized by
> netback_probe() at this point, which will cause and invalid transaction
> and set_backend_state() to BUG().
> 
> Initialize the backend state at the beginning of netback_probe() to
> XenbusStateInitialising, and create two new valid state transitions on
> set_backend_state(), from XenbusStateInitialising to XenbusStateClosed,
> and from XenbusStateInitialising to XenbusStateInitWait.
> 
> Signed-off-by: Filipe Manco 

Applied, thanks.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2] xen-netback: fix error handling on netback_probe()

[Wei Liu]

On Thu, Sep 15, 2016 at 05:10:46PM +0200, Filipe Manco wrote:
> In case of error during netback_probe() (e.g. an entry missing on the
> xenstore) netback_remove() is called on the new device, which will set
> the device backend state to XenbusStateClosed by calling
> set_backend_state(). However, the backend state wasn't initialized by
> netback_probe() at this point, which will cause and invalid transaction
> and set_backend_state() to BUG().
> 
> Initialize the backend state at the beginning of netback_probe() to
> XenbusStateInitialising, and create two new valid state transitions on
> set_backend_state(), from XenbusStateInitialising to XenbusStateClosed,
> and from XenbusStateInitialising to XenbusStateInitWait.
> 
> Signed-off-by: Filipe Manco 

Acked-by: Wei Liu 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2] xen-netback: fix error handling on netback_probe()

[Filipe Manco]

In case of error during netback_probe() (e.g. an entry missing on the
xenstore) netback_remove() is called on the new device, which will set
the device backend state to XenbusStateClosed by calling
set_backend_state(). However, the backend state wasn't initialized by
netback_probe() at this point, which will cause and invalid transaction
and set_backend_state() to BUG().

Initialize the backend state at the beginning of netback_probe() to
XenbusStateInitialising, and create two new valid state transitions on
set_backend_state(), from XenbusStateInitialising to XenbusStateClosed,
and from XenbusStateInitialising to XenbusStateInitWait.

Signed-off-by: Filipe Manco 
---
 drivers/net/xen-netback/xenbus.c | 46 ++--
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 6a31f2610c23..daf4c7867102 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -271,6 +271,11 @@ static int netback_probe(struct xenbus_device *dev,
be->dev = dev;
dev_set_drvdata(>dev, be);
 
+   be->state = XenbusStateInitialising;
+   err = xenbus_switch_state(dev, XenbusStateInitialising);
+   if (err)
+   goto fail;
+
sg = 1;
 
do {
@@ -383,11 +388,6 @@ static int netback_probe(struct xenbus_device *dev,
 
be->hotplug_script = script;
 
-   err = xenbus_switch_state(dev, XenbusStateInitWait);
-   if (err)
-   goto fail;
-
-   be->state = XenbusStateInitWait;
 
/* This kicks hotplug scripts, so do it immediately. */
err = backend_create_xenvif(be);
@@ -492,20 +492,20 @@ static inline void backend_switch_state(struct 
backend_info *be,
 
 /* Handle backend state transitions:
  *
- * The backend state starts in InitWait and the following transitions are
+ * The backend state starts in Initialising and the following transitions are
  * allowed.
  *
- * InitWait -> Connected
- *
- *^\ |
- *| \|
- *|  \   |
- *|   \  |
- *|\ |
- *| \|
- *|  V   V
+ * Initialising -> InitWait -> Connected
+ *  \
+ *   \^\ |
+ *\   | \|
+ * \  |  \   |
+ *  \ |   \  |
+ *   \|\ |
+ *\   | \|
+ * V  |  V   V
  *
- *  Closed  <-> Closing
+ *  Closed  <-> Closing
  *
  * The state argument specifies the eventual state of the backend and the
  * function transitions to that state via the shortest path.
@@ -515,6 +515,20 @@ static void set_backend_state(struct backend_info *be,
 {
while (be->state != state) {
switch (be->state) {
+   case XenbusStateInitialising:
+   switch (state) {
+   case XenbusStateInitWait:
+   case XenbusStateConnected:
+   case XenbusStateClosing:
+   backend_switch_state(be, XenbusStateInitWait);
+   break;
+   case XenbusStateClosed:
+   backend_switch_state(be, XenbusStateClosed);
+   break;
+   default:
+   BUG();
+   }
+   break;
case XenbusStateClosed:
switch (state) {
case XenbusStateInitWait:
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel