linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] xen-netback: clean-up
@ 2019-12-17 13:32 Paul Durrant
  2019-12-17 13:32 ` [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end Paul Durrant
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Paul Durrant @ 2019-12-17 13:32 UTC (permalink / raw)
  To: xen-devel, netdev, linux-kernel; +Cc: Paul Durrant

Paul Durrant (3):
  xen-netback: move netback_probe() and netback_remove() to the end...
  xen-netback: switch state to InitWait at the end of netback_probe()...
  xen-netback: remove 'hotplug-status' once it has served its purpose

 drivers/net/xen-netback/xenbus.c | 350 +++++++++++++++----------------
 1 file changed, 171 insertions(+), 179 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end...
  2019-12-17 13:32 [PATCH net-next 0/3] xen-netback: clean-up Paul Durrant
@ 2019-12-17 13:32 ` Paul Durrant
  2019-12-17 15:23   ` Wei Liu
  2019-12-18  7:03   ` David Miller
  2019-12-17 13:32 ` [PATCH net-next 2/3] xen-netback: switch state to InitWait at the end of netback_probe() Paul Durrant
  2019-12-17 13:32 ` [PATCH net-next 3/3] xen-netback: remove 'hotplug-status' once it has served its purpose Paul Durrant
  2 siblings, 2 replies; 10+ messages in thread
From: Paul Durrant @ 2019-12-17 13:32 UTC (permalink / raw)
  To: xen-devel, netdev, linux-kernel
  Cc: Paul Durrant, Wei Liu, Paul Durrant, David S. Miller

...of xenbus.c

This is a cosmetic function re-ordering to reduce churn in a subsequent
patch. Some style fix-up was done to make checkpatch.pl happier.

No functional change.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Wei Liu <wei.liu@kernel.org>
Cc: Paul Durrant <paul@xen.org>
Cc: "David S. Miller" <davem@davemloft.net>
---
 drivers/net/xen-netback/xenbus.c | 353 +++++++++++++++----------------
 1 file changed, 174 insertions(+), 179 deletions(-)

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index f533b7372d59..bb61316d79de 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -195,185 +195,6 @@ static void xenvif_debugfs_delif(struct xenvif *vif)
 }
 #endif /* CONFIG_DEBUG_FS */
 
-static int netback_remove(struct xenbus_device *dev)
-{
-	struct backend_info *be = dev_get_drvdata(&dev->dev);
-
-	set_backend_state(be, XenbusStateClosed);
-
-	unregister_hotplug_status_watch(be);
-	if (be->vif) {
-		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
-		xen_unregister_watchers(be->vif);
-		xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
-		xenvif_free(be->vif);
-		be->vif = NULL;
-	}
-	kfree(be->hotplug_script);
-	kfree(be);
-	dev_set_drvdata(&dev->dev, NULL);
-	return 0;
-}
-
-
-/**
- * Entry point to this code when a new device is created.  Allocate the basic
- * structures and switch to InitWait.
- */
-static int netback_probe(struct xenbus_device *dev,
-			 const struct xenbus_device_id *id)
-{
-	const char *message;
-	struct xenbus_transaction xbt;
-	int err;
-	int sg;
-	const char *script;
-	struct backend_info *be = kzalloc(sizeof(struct backend_info),
-					  GFP_KERNEL);
-	if (!be) {
-		xenbus_dev_fatal(dev, -ENOMEM,
-				 "allocating backend structure");
-		return -ENOMEM;
-	}
-
-	be->dev = dev;
-	dev_set_drvdata(&dev->dev, be);
-
-	be->state = XenbusStateInitialising;
-	err = xenbus_switch_state(dev, XenbusStateInitialising);
-	if (err)
-		goto fail;
-
-	sg = 1;
-
-	do {
-		err = xenbus_transaction_start(&xbt);
-		if (err) {
-			xenbus_dev_fatal(dev, err, "starting transaction");
-			goto fail;
-		}
-
-		err = xenbus_printf(xbt, dev->nodename, "feature-sg", "%d", sg);
-		if (err) {
-			message = "writing feature-sg";
-			goto abort_transaction;
-		}
-
-		err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv4",
-				    "%d", sg);
-		if (err) {
-			message = "writing feature-gso-tcpv4";
-			goto abort_transaction;
-		}
-
-		err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv6",
-				    "%d", sg);
-		if (err) {
-			message = "writing feature-gso-tcpv6";
-			goto abort_transaction;
-		}
-
-		/* We support partial checksum setup for IPv6 packets */
-		err = xenbus_printf(xbt, dev->nodename,
-				    "feature-ipv6-csum-offload",
-				    "%d", 1);
-		if (err) {
-			message = "writing feature-ipv6-csum-offload";
-			goto abort_transaction;
-		}
-
-		/* We support rx-copy path. */
-		err = xenbus_printf(xbt, dev->nodename,
-				    "feature-rx-copy", "%d", 1);
-		if (err) {
-			message = "writing feature-rx-copy";
-			goto abort_transaction;
-		}
-
-		/*
-		 * We don't support rx-flip path (except old guests who don't
-		 * grok this feature flag).
-		 */
-		err = xenbus_printf(xbt, dev->nodename,
-				    "feature-rx-flip", "%d", 0);
-		if (err) {
-			message = "writing feature-rx-flip";
-			goto abort_transaction;
-		}
-
-		/* We support dynamic multicast-control. */
-		err = xenbus_printf(xbt, dev->nodename,
-				    "feature-multicast-control", "%d", 1);
-		if (err) {
-			message = "writing feature-multicast-control";
-			goto abort_transaction;
-		}
-
-		err = xenbus_printf(xbt, dev->nodename,
-				    "feature-dynamic-multicast-control",
-				    "%d", 1);
-		if (err) {
-			message = "writing feature-dynamic-multicast-control";
-			goto abort_transaction;
-		}
-
-		err = xenbus_transaction_end(xbt, 0);
-	} while (err == -EAGAIN);
-
-	if (err) {
-		xenbus_dev_fatal(dev, err, "completing transaction");
-		goto fail;
-	}
-
-	/*
-	 * Split event channels support, this is optional so it is not
-	 * put inside the above loop.
-	 */
-	err = xenbus_printf(XBT_NIL, dev->nodename,
-			    "feature-split-event-channels",
-			    "%u", separate_tx_rx_irq);
-	if (err)
-		pr_debug("Error writing feature-split-event-channels\n");
-
-	/* Multi-queue support: This is an optional feature. */
-	err = xenbus_printf(XBT_NIL, dev->nodename,
-			    "multi-queue-max-queues", "%u", xenvif_max_queues);
-	if (err)
-		pr_debug("Error writing multi-queue-max-queues\n");
-
-	err = xenbus_printf(XBT_NIL, dev->nodename,
-			    "feature-ctrl-ring",
-			    "%u", true);
-	if (err)
-		pr_debug("Error writing feature-ctrl-ring\n");
-
-	script = xenbus_read(XBT_NIL, dev->nodename, "script", NULL);
-	if (IS_ERR(script)) {
-		err = PTR_ERR(script);
-		xenbus_dev_fatal(dev, err, "reading script");
-		goto fail;
-	}
-
-	be->hotplug_script = script;
-
-
-	/* This kicks hotplug scripts, so do it immediately. */
-	err = backend_create_xenvif(be);
-	if (err)
-		goto fail;
-
-	return 0;
-
-abort_transaction:
-	xenbus_transaction_end(xbt, 1);
-	xenbus_dev_fatal(dev, err, "%s", message);
-fail:
-	pr_debug("failed\n");
-	netback_remove(dev);
-	return err;
-}
-
-
 /*
  * Handle the creation of the hotplug script environment.  We add the script
  * and vif variables to the environment, for the benefit of the vif-* hotplug
@@ -1128,6 +949,180 @@ static int read_xenbus_vif_flags(struct backend_info *be)
 	return 0;
 }
 
+static int netback_remove(struct xenbus_device *dev)
+{
+	struct backend_info *be = dev_get_drvdata(&dev->dev);
+
+	set_backend_state(be, XenbusStateClosed);
+
+	unregister_hotplug_status_watch(be);
+	if (be->vif) {
+		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
+		xen_unregister_watchers(be->vif);
+		xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
+		xenvif_free(be->vif);
+		be->vif = NULL;
+	}
+	kfree(be->hotplug_script);
+	kfree(be);
+	dev_set_drvdata(&dev->dev, NULL);
+	return 0;
+}
+
+/**
+ * Entry point to this code when a new device is created.  Allocate the basic
+ * structures and switch to InitWait.
+ */
+static int netback_probe(struct xenbus_device *dev,
+			 const struct xenbus_device_id *id)
+{
+	const char *message;
+	struct xenbus_transaction xbt;
+	int err;
+	int sg;
+	const char *script;
+	struct backend_info *be = kzalloc(sizeof(*be), GFP_KERNEL);
+
+	if (!be) {
+		xenbus_dev_fatal(dev, -ENOMEM,
+				 "allocating backend structure");
+		return -ENOMEM;
+	}
+
+	be->dev = dev;
+	dev_set_drvdata(&dev->dev, be);
+
+	be->state = XenbusStateInitialising;
+	err = xenbus_switch_state(dev, XenbusStateInitialising);
+	if (err)
+		goto fail;
+
+	sg = 1;
+
+	do {
+		err = xenbus_transaction_start(&xbt);
+		if (err) {
+			xenbus_dev_fatal(dev, err, "starting transaction");
+			goto fail;
+		}
+
+		err = xenbus_printf(xbt, dev->nodename, "feature-sg", "%d", sg);
+		if (err) {
+			message = "writing feature-sg";
+			goto abort_transaction;
+		}
+
+		err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv4",
+				    "%d", sg);
+		if (err) {
+			message = "writing feature-gso-tcpv4";
+			goto abort_transaction;
+		}
+
+		err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv6",
+				    "%d", sg);
+		if (err) {
+			message = "writing feature-gso-tcpv6";
+			goto abort_transaction;
+		}
+
+		/* We support partial checksum setup for IPv6 packets */
+		err = xenbus_printf(xbt, dev->nodename,
+				    "feature-ipv6-csum-offload",
+				    "%d", 1);
+		if (err) {
+			message = "writing feature-ipv6-csum-offload";
+			goto abort_transaction;
+		}
+
+		/* We support rx-copy path. */
+		err = xenbus_printf(xbt, dev->nodename,
+				    "feature-rx-copy", "%d", 1);
+		if (err) {
+			message = "writing feature-rx-copy";
+			goto abort_transaction;
+		}
+
+		/* We don't support rx-flip path (except old guests who
+		 * don't grok this feature flag).
+		 */
+		err = xenbus_printf(xbt, dev->nodename,
+				    "feature-rx-flip", "%d", 0);
+		if (err) {
+			message = "writing feature-rx-flip";
+			goto abort_transaction;
+		}
+
+		/* We support dynamic multicast-control. */
+		err = xenbus_printf(xbt, dev->nodename,
+				    "feature-multicast-control", "%d", 1);
+		if (err) {
+			message = "writing feature-multicast-control";
+			goto abort_transaction;
+		}
+
+		err = xenbus_printf(xbt, dev->nodename,
+				    "feature-dynamic-multicast-control",
+				    "%d", 1);
+		if (err) {
+			message = "writing feature-dynamic-multicast-control";
+			goto abort_transaction;
+		}
+
+		err = xenbus_transaction_end(xbt, 0);
+	} while (err == -EAGAIN);
+
+	if (err) {
+		xenbus_dev_fatal(dev, err, "completing transaction");
+		goto fail;
+	}
+
+	/* Split event channels support, this is optional so it is not
+	 * put inside the above loop.
+	 */
+	err = xenbus_printf(XBT_NIL, dev->nodename,
+			    "feature-split-event-channels",
+			    "%u", separate_tx_rx_irq);
+	if (err)
+		pr_debug("Error writing feature-split-event-channels\n");
+
+	/* Multi-queue support: This is an optional feature. */
+	err = xenbus_printf(XBT_NIL, dev->nodename,
+			    "multi-queue-max-queues", "%u", xenvif_max_queues);
+	if (err)
+		pr_debug("Error writing multi-queue-max-queues\n");
+
+	err = xenbus_printf(XBT_NIL, dev->nodename,
+			    "feature-ctrl-ring",
+			    "%u", true);
+	if (err)
+		pr_debug("Error writing feature-ctrl-ring\n");
+
+	script = xenbus_read(XBT_NIL, dev->nodename, "script", NULL);
+	if (IS_ERR(script)) {
+		err = PTR_ERR(script);
+		xenbus_dev_fatal(dev, err, "reading script");
+		goto fail;
+	}
+
+	be->hotplug_script = script;
+
+	/* This kicks hotplug scripts, so do it immediately. */
+	err = backend_create_xenvif(be);
+	if (err)
+		goto fail;
+
+	return 0;
+
+abort_transaction:
+	xenbus_transaction_end(xbt, 1);
+	xenbus_dev_fatal(dev, err, "%s", message);
+fail:
+	pr_debug("failed\n");
+	netback_remove(dev);
+	return err;
+}
+
 static const struct xenbus_device_id netback_ids[] = {
 	{ "vif" },
 	{ "" }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 2/3] xen-netback: switch state to InitWait at the end of netback_probe()...
  2019-12-17 13:32 [PATCH net-next 0/3] xen-netback: clean-up Paul Durrant
  2019-12-17 13:32 ` [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end Paul Durrant
@ 2019-12-17 13:32 ` Paul Durrant
  2019-12-17 15:23   ` Wei Liu
  2019-12-18  7:03   ` David Miller
  2019-12-17 13:32 ` [PATCH net-next 3/3] xen-netback: remove 'hotplug-status' once it has served its purpose Paul Durrant
  2 siblings, 2 replies; 10+ messages in thread
From: Paul Durrant @ 2019-12-17 13:32 UTC (permalink / raw)
  To: xen-devel, netdev, linux-kernel
  Cc: Paul Durrant, Wei Liu, Paul Durrant, David S. Miller

...as the comment above the function states.

The switch to Initialising at the start of the function is somewhat bogus
as the toolstack will have set that initial state anyway. To behave
correctly, a backend should switch to InitWait once it has set up all
xenstore values that may be required by a initialising frontend. This
patch calls backend_switch_state() to make the transition at the
appropriate point.

NOTE: backend_switch_state() ignores errors from xenbus_switch_state()
      and so this patch removes an error path from netback_probe(). This
      means a failure to change state at this stage (in the absence of
      other failures) will leave the device instantiated. This is highly
      unlikley to happen as a failure to change state would indicate a
      failure to write to xenstore, and that will trigger other error
      paths. Also, a 'stuck' device can still be cleaned up using 'unbind'
      in any case.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Wei Liu <wei.liu@kernel.org>
Cc: Paul Durrant <paul@xen.org>
Cc: "David S. Miller" <davem@davemloft.net>
---
 drivers/net/xen-netback/xenbus.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index bb61316d79de..682e5e20971b 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -992,11 +992,6 @@ static int netback_probe(struct xenbus_device *dev,
 	be->dev = dev;
 	dev_set_drvdata(&dev->dev, be);
 
-	be->state = XenbusStateInitialising;
-	err = xenbus_switch_state(dev, XenbusStateInitialising);
-	if (err)
-		goto fail;
-
 	sg = 1;
 
 	do {
@@ -1098,6 +1093,8 @@ static int netback_probe(struct xenbus_device *dev,
 	if (err)
 		pr_debug("Error writing feature-ctrl-ring\n");
 
+	backend_switch_state(be, XenbusStateInitWait);
+
 	script = xenbus_read(XBT_NIL, dev->nodename, "script", NULL);
 	if (IS_ERR(script)) {
 		err = PTR_ERR(script);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 3/3] xen-netback: remove 'hotplug-status' once it has served its purpose
  2019-12-17 13:32 [PATCH net-next 0/3] xen-netback: clean-up Paul Durrant
  2019-12-17 13:32 ` [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end Paul Durrant
  2019-12-17 13:32 ` [PATCH net-next 2/3] xen-netback: switch state to InitWait at the end of netback_probe() Paul Durrant
@ 2019-12-17 13:32 ` Paul Durrant
  2019-12-17 15:23   ` Wei Liu
  2019-12-18  7:04   ` David Miller
  2 siblings, 2 replies; 10+ messages in thread
From: Paul Durrant @ 2019-12-17 13:32 UTC (permalink / raw)
  To: xen-devel, netdev, linux-kernel
  Cc: Paul Durrant, Wei Liu, Paul Durrant, David S. Miller

Removing the 'hotplug-status' node in netback_remove() is wrong; the script
may not have completed. Only remove the node once the watch has fired and
has been unregistered.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Wei Liu <wei.liu@kernel.org>
Cc: Paul Durrant <paul@xen.org>
Cc: "David S. Miller" <davem@davemloft.net>
---
 drivers/net/xen-netback/xenbus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 682e5e20971b..17b4950ec051 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -648,6 +648,7 @@ static void hotplug_status_changed(struct xenbus_watch *watch,
 
 		/* Not interested in this watch anymore. */
 		unregister_hotplug_status_watch(be);
+		xenbus_rm(XBT_NIL, be->dev->nodename, "hotplug-status");
 	}
 	kfree(str);
 }
@@ -959,7 +960,6 @@ static int netback_remove(struct xenbus_device *dev)
 	if (be->vif) {
 		kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
 		xen_unregister_watchers(be->vif);
-		xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
 		xenvif_free(be->vif);
 		be->vif = NULL;
 	}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 3/3] xen-netback: remove 'hotplug-status' once it has served its purpose
  2019-12-17 13:32 ` [PATCH net-next 3/3] xen-netback: remove 'hotplug-status' once it has served its purpose Paul Durrant
@ 2019-12-17 15:23   ` Wei Liu
  2019-12-18  7:04   ` David Miller
  1 sibling, 0 replies; 10+ messages in thread
From: Wei Liu @ 2019-12-17 15:23 UTC (permalink / raw)
  To: Paul Durrant
  Cc: xen-devel, netdev, linux-kernel, Wei Liu, Paul Durrant, David S. Miller

On Tue, Dec 17, 2019 at 01:32:18PM +0000, Paul Durrant wrote:
> Removing the 'hotplug-status' node in netback_remove() is wrong; the script
> may not have completed. Only remove the node once the watch has fired and
> has been unregistered.
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

Acked-by: Wei Liu <wei.liu@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end...
  2019-12-17 13:32 ` [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end Paul Durrant
@ 2019-12-17 15:23   ` Wei Liu
  2019-12-18  7:03   ` David Miller
  1 sibling, 0 replies; 10+ messages in thread
From: Wei Liu @ 2019-12-17 15:23 UTC (permalink / raw)
  To: Paul Durrant
  Cc: xen-devel, netdev, linux-kernel, Wei Liu, Paul Durrant, David S. Miller

On Tue, Dec 17, 2019 at 01:32:16PM +0000, Paul Durrant wrote:
> ...of xenbus.c
> 
> This is a cosmetic function re-ordering to reduce churn in a subsequent
> patch. Some style fix-up was done to make checkpatch.pl happier.
> 
> No functional change.
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

Acked-by: Wei Liu <wei.liu@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 2/3] xen-netback: switch state to InitWait at the end of netback_probe()...
  2019-12-17 13:32 ` [PATCH net-next 2/3] xen-netback: switch state to InitWait at the end of netback_probe() Paul Durrant
@ 2019-12-17 15:23   ` Wei Liu
  2019-12-18  7:03   ` David Miller
  1 sibling, 0 replies; 10+ messages in thread
From: Wei Liu @ 2019-12-17 15:23 UTC (permalink / raw)
  To: Paul Durrant
  Cc: xen-devel, netdev, linux-kernel, Wei Liu, Paul Durrant, David S. Miller

On Tue, Dec 17, 2019 at 01:32:17PM +0000, Paul Durrant wrote:
> ...as the comment above the function states.
> 
> The switch to Initialising at the start of the function is somewhat bogus
> as the toolstack will have set that initial state anyway. To behave
> correctly, a backend should switch to InitWait once it has set up all
> xenstore values that may be required by a initialising frontend. This
> patch calls backend_switch_state() to make the transition at the
> appropriate point.
> 
> NOTE: backend_switch_state() ignores errors from xenbus_switch_state()
>       and so this patch removes an error path from netback_probe(). This
>       means a failure to change state at this stage (in the absence of
>       other failures) will leave the device instantiated. This is highly
>       unlikley to happen as a failure to change state would indicate a
>       failure to write to xenstore, and that will trigger other error
>       paths. Also, a 'stuck' device can still be cleaned up using 'unbind'
>       in any case.
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

Acked-by: Wei Liu <wei.liu@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end...
  2019-12-17 13:32 ` [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end Paul Durrant
  2019-12-17 15:23   ` Wei Liu
@ 2019-12-18  7:03   ` David Miller
  1 sibling, 0 replies; 10+ messages in thread
From: David Miller @ 2019-12-18  7:03 UTC (permalink / raw)
  To: pdurrant; +Cc: xen-devel, netdev, linux-kernel, wei.liu, paul

From: Paul Durrant <pdurrant@amazon.com>
Date: Tue, 17 Dec 2019 13:32:16 +0000

> ...of xenbus.c
> 
> This is a cosmetic function re-ordering to reduce churn in a subsequent
> patch. Some style fix-up was done to make checkpatch.pl happier.
> 
> No functional change.
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

Applied.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 2/3] xen-netback: switch state to InitWait at the end of netback_probe()...
  2019-12-17 13:32 ` [PATCH net-next 2/3] xen-netback: switch state to InitWait at the end of netback_probe() Paul Durrant
  2019-12-17 15:23   ` Wei Liu
@ 2019-12-18  7:03   ` David Miller
  1 sibling, 0 replies; 10+ messages in thread
From: David Miller @ 2019-12-18  7:03 UTC (permalink / raw)
  To: pdurrant; +Cc: xen-devel, netdev, linux-kernel, wei.liu, paul

From: Paul Durrant <pdurrant@amazon.com>
Date: Tue, 17 Dec 2019 13:32:17 +0000

> ...as the comment above the function states.
> 
> The switch to Initialising at the start of the function is somewhat bogus
> as the toolstack will have set that initial state anyway. To behave
> correctly, a backend should switch to InitWait once it has set up all
> xenstore values that may be required by a initialising frontend. This
> patch calls backend_switch_state() to make the transition at the
> appropriate point.
> 
> NOTE: backend_switch_state() ignores errors from xenbus_switch_state()
>       and so this patch removes an error path from netback_probe(). This
>       means a failure to change state at this stage (in the absence of
>       other failures) will leave the device instantiated. This is highly
>       unlikley to happen as a failure to change state would indicate a
>       failure to write to xenstore, and that will trigger other error
>       paths. Also, a 'stuck' device can still be cleaned up using 'unbind'
>       in any case.
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

Applied.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 3/3] xen-netback: remove 'hotplug-status' once it has served its purpose
  2019-12-17 13:32 ` [PATCH net-next 3/3] xen-netback: remove 'hotplug-status' once it has served its purpose Paul Durrant
  2019-12-17 15:23   ` Wei Liu
@ 2019-12-18  7:04   ` David Miller
  1 sibling, 0 replies; 10+ messages in thread
From: David Miller @ 2019-12-18  7:04 UTC (permalink / raw)
  To: pdurrant; +Cc: xen-devel, netdev, linux-kernel, wei.liu, paul

From: Paul Durrant <pdurrant@amazon.com>
Date: Tue, 17 Dec 2019 13:32:18 +0000

> Removing the 'hotplug-status' node in netback_remove() is wrong; the script
> may not have completed. Only remove the node once the watch has fired and
> has been unregistered.
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

Applied.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-12-18  7:04 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-17 13:32 [PATCH net-next 0/3] xen-netback: clean-up Paul Durrant
2019-12-17 13:32 ` [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end Paul Durrant
2019-12-17 15:23   ` Wei Liu
2019-12-18  7:03   ` David Miller
2019-12-17 13:32 ` [PATCH net-next 2/3] xen-netback: switch state to InitWait at the end of netback_probe() Paul Durrant
2019-12-17 15:23   ` Wei Liu
2019-12-18  7:03   ` David Miller
2019-12-17 13:32 ` [PATCH net-next 3/3] xen-netback: remove 'hotplug-status' once it has served its purpose Paul Durrant
2019-12-17 15:23   ` Wei Liu
2019-12-18  7:04   ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).