All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matt Wilson <msw@linux.com>
To: xen-devel@lists.xenproject.org
Cc: Ben Cressey <bcressey@amazon.com>, Matt Wilson <msw@amazon.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Subject: [PATCH v2 4/4] minios: fix xenbus_rm() calls in frontend drivers
Date: Fri,  6 Sep 2013 12:52:07 -0700	[thread overview]
Message-ID: <1378497127-809-5-git-send-email-msw@linux.com> (raw)
In-Reply-To: <1378497127-809-1-git-send-email-msw@linux.com>

From: Ben Cressey <bcressey@amazon.com>

The commit "minios: refactor xenbus state machine" caused "/state" to
be appended to the local value of nodename. Previously the nodename
variable pointed to dev->nodename.

The xenbus_rm() calls were not updated to reflect this change, and
refer to paths that do not exist.

For example, shutdown_blkfront() for vbd 2049 would issue these calls:
    xenbus_rm(XBT_NIL, "device/vbd/2049/state/ring-ref");
    xenbus_rm(XBT_NIL, "device/vbd/2049/state/event-channel");

This patch restores the previous behavior, issuing these calls
instead:
    xenbus_rm(XBT_NIL, "device/vbd/2049/ring-ref");
    xenbus_rm(XBT_NIL, "device/vbd/2049/event-channel");

This causes frontend drivers to not be properly reset when PV-GRUB
exists. Some PV Linux drivers fail to re-initialize frontend devices
if PV-GRUB leaves them in this state.

Signed-off-by: Ben Cressey <bcressey@amazon.com>
Reviewed-by: Matt Wilson <msw@amazon.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
[msw: adjusted commit message to include consequences, split out
 changes into separate patches]
Signed-off-by: Matt Wilson <msw@amazon.com>

---
Changes since v1:
- adjusted commit message to include consequences
- split out changes into separate patches
---
 extras/mini-os/blkfront.c |    8 ++++----
 extras/mini-os/fbfront.c  |   28 ++++++++++++++--------------
 extras/mini-os/netfront.c |   16 ++++++++--------
 extras/mini-os/pcifront.c |    8 ++++----
 4 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/extras/mini-os/blkfront.c b/extras/mini-os/blkfront.c
index dd5e95f..ddcf665 100644
--- a/extras/mini-os/blkfront.c
+++ b/extras/mini-os/blkfront.c
@@ -297,10 +297,10 @@ close:
     if (err) free(err);
     xenbus_unwatch_path_token(XBT_NIL, path, path);
 
-    snprintf(path, sizeof(path), "%s/ring-ref", nodename);
-    xenbus_rm(XBT_NIL, path);
-    snprintf(path, sizeof(path), "%s/event-channel", nodename);
-    xenbus_rm(XBT_NIL, path);
+    snprintf(nodename, sizeof(nodename), "%s/ring-ref", dev->nodename);
+    xenbus_rm(XBT_NIL, nodename);
+    snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename);
+    xenbus_rm(XBT_NIL, nodename);
 
     if (!err)
         free_blkfront(dev);
diff --git a/extras/mini-os/fbfront.c b/extras/mini-os/fbfront.c
index 8b4466a..3d0b8f5 100644
--- a/extras/mini-os/fbfront.c
+++ b/extras/mini-os/fbfront.c
@@ -280,12 +280,12 @@ close_kbdfront:
     if (err) free(err);
     xenbus_unwatch_path_token(XBT_NIL, path, path);
 
-    snprintf(path, sizeof(path), "%s/page-ref", nodename);
-    xenbus_rm(XBT_NIL, path);
-    snprintf(path, sizeof(path), "%s/event-channel", nodename);
-    xenbus_rm(XBT_NIL, path);
-    snprintf(path, sizeof(path), "%s/request-abs-pointer", nodename);
-    xenbus_rm(XBT_NIL, path);
+    snprintf(nodename, sizeof(nodename), "%s/page-ref", dev->nodename);
+    xenbus_rm(XBT_NIL, nodename);
+    snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename);
+    xenbus_rm(XBT_NIL, nodename);
+    snprintf(nodename, sizeof(nodename), "%s/request-abs-pointer", dev->nodename);
+    xenbus_rm(XBT_NIL, nodename);
 
     if (!err)
         free_kbdfront(dev);
@@ -671,14 +671,14 @@ close_fbfront:
     if (err) free(err);
     xenbus_unwatch_path_token(XBT_NIL, path, path);
 
-    snprintf(path, sizeof(path), "%s/page-ref", nodename);
-    xenbus_rm(XBT_NIL, path);
-    snprintf(path, sizeof(path), "%s/event-channel", nodename);
-    xenbus_rm(XBT_NIL, path);
-    snprintf(path, sizeof(path), "%s/protocol", nodename);
-    xenbus_rm(XBT_NIL, path);
-    snprintf(path, sizeof(path), "%s/feature-update", nodename);
-    xenbus_rm(XBT_NIL, path);
+    snprintf(nodename, sizeof(nodename), "%s/page-ref", dev->nodename);
+    xenbus_rm(XBT_NIL, nodename);
+    snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename);
+    xenbus_rm(XBT_NIL, nodename);
+    snprintf(nodename, sizeof(nodename), "%s/protocol", dev->nodename);
+    xenbus_rm(XBT_NIL, nodename);
+    snprintf(nodename, sizeof(nodename), "%s/feature-update", dev->nodename);
+    xenbus_rm(XBT_NIL, nodename);
 
     if (!err)
         free_fbfront(dev);
diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
index dc29f14..4e087a5 100644
--- a/extras/mini-os/netfront.c
+++ b/extras/mini-os/netfront.c
@@ -549,14 +549,14 @@ close:
     if (err) free(err);
     xenbus_unwatch_path_token(XBT_NIL, path, path);
 
-    snprintf(path, sizeof(path), "%s/tx-ring-ref", nodename);
-    xenbus_rm(XBT_NIL, path);
-    snprintf(path, sizeof(path), "%s/rx-ring-ref", nodename);
-    xenbus_rm(XBT_NIL, path);
-    snprintf(path, sizeof(path), "%s/event-channel", nodename);
-    xenbus_rm(XBT_NIL, path);
-    snprintf(path, sizeof(path), "%s/request-rx-copy", nodename);
-    xenbus_rm(XBT_NIL, path);
+    snprintf(nodename, sizeof(nodename), "%s/tx-ring-ref", dev->nodename);
+    xenbus_rm(XBT_NIL, nodename);
+    snprintf(nodename, sizeof(nodename), "%s/rx-ring-ref", dev->nodename);
+    xenbus_rm(XBT_NIL, nodename);
+    snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename);
+    xenbus_rm(XBT_NIL, nodename);
+    snprintf(nodename, sizeof(nodename), "%s/request-rx-copy", dev->nodename);
+    xenbus_rm(XBT_NIL, nodename);
 
     if (!err)
         free_netfront(dev);
diff --git a/extras/mini-os/pcifront.c b/extras/mini-os/pcifront.c
index df300b5..cdf9c9b 100644
--- a/extras/mini-os/pcifront.c
+++ b/extras/mini-os/pcifront.c
@@ -363,10 +363,10 @@ close_pcifront:
     if (err) free(err);
     xenbus_unwatch_path_token(XBT_NIL, path, path);
 
-    snprintf(path, sizeof(path), "%s/info-ref", nodename);
-    xenbus_rm(XBT_NIL, path);
-    snprintf(path, sizeof(path), "%s/event-channel", nodename);
-    xenbus_rm(XBT_NIL, path);
+    snprintf(nodename, sizeof(nodename), "%s/info-ref", dev->nodename);
+    xenbus_rm(XBT_NIL, nodename);
+    snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename);
+    xenbus_rm(XBT_NIL, nodename);
 
     if (!err)
         free_pcifront(dev);
-- 
1.7.9.5

  parent reply	other threads:[~2013-09-06 19:52 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-05  0:25 [PATCH] minios: Fix xenbus_rm() calls in frontend drivers Matt Wilson
2013-09-05  9:17 ` Ian Campbell
2013-09-05 18:06   ` Matt Wilson
2013-09-06  9:00     ` Ian Campbell
2013-09-06 16:24       ` Matt Wilson
2013-09-06 16:53         ` Ian Campbell
2013-09-06 19:52           ` [PATCH v2 0/4] minios: various cleanups and fixes Matt Wilson
2013-09-06 19:52             ` [PATCH v2 1/4] minios: correct char array allocation for xenbus paths Matt Wilson
2013-09-09 14:00               ` Ian Campbell
2013-09-09 18:53               ` Samuel Thibault
2013-09-06 19:52             ` [PATCH v2 2/4] minios: clean up allocation of char arrays used " Matt Wilson
2013-09-09 14:02               ` Ian Campbell
2013-09-06 19:52             ` [PATCH v2 3/4] minios: clean up unneeded "err = NULL" in frontend drivers Matt Wilson
2013-09-06 19:52             ` Matt Wilson [this message]
2013-09-09 14:04             ` [PATCH v2 0/4] minios: various cleanups and fixes Ian Campbell
2013-09-10 10:48             ` Ian Campbell
2013-09-05 18:17 ` [PATCH] minios: Fix xenbus_rm() calls in frontend drivers Samuel Thibault

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1378497127-809-5-git-send-email-msw@linux.com \
    --to=msw@linux.com \
    --cc=bcressey@amazon.com \
    --cc=msw@amazon.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.