[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V2 2/3] xen: scsiback: add LUN of restored domain



On 02/24/2015 01:06 AM, Juergen Gross wrote:
On 02/18/2015 06:02 PM, Boris Ostrovsky wrote:
On 02/17/2015 02:02 AM, Juergen Gross wrote:
When a xen domain is being restored the LUN state of a pvscsi device
is "Connected" and not "Initialising" as in case of attaching a new
pvscsi LUN.

This must be taken into account when adding a new pvscsi device for
a domain as otherwise the pvscsi LUN won't be connected to the
SCSI target associated with it.


scsiback_do_add_lun() is being called from scsiback_frontend_changed()
which eventually sets the state to Connected. Is the added test of
'try' an optimization or is it needed for correctness?

It's needed for correctness. In case of resume the translation entry
will be already existing, thus scsiback_add_translation_entry() will
return a value unequal to zero. We don't want to set the device state
to "Closed" in this case.


Right, I understand that. I thought that after you

        xenbus_printf(XBT_NIL, info->dev->nodename, state,
                    "%d", XenbusStateClosed);

in scsiback_do_add_lun() you'd eventually

        xenbus_switch_state(dev, XenbusStateConnected);

in scsiback_frontend_changed().

What I didn't realize was that this switch would only happen if dev->state is not XenbusStateConnected, which it still will be, even after xenbus_printf(XenbusStateClosed).

So

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>


Juergen


-boris



Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
---
  drivers/xen/xen-scsiback.c | 19 ++++++++++++++-----
  1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 9faca6a..9d60176 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -992,7 +992,7 @@ found:
  }

  static void scsiback_do_add_lun(struct vscsibk_info *info, const
char *state,
-                char *phy, struct ids_tuple *vir)
+                char *phy, struct ids_tuple *vir, int try)
  {
      if (!scsiback_add_translation_entry(info, phy, vir)) {
          if (xenbus_printf(XBT_NIL, info->dev->nodename, state,
@@ -1000,7 +1000,7 @@ static void scsiback_do_add_lun(struct
vscsibk_info *info, const char *state,
              pr_err("xen-pvscsi: xenbus_printf error %s\n", state);
              scsiback_del_translation_entry(info, vir);
          }
-    } else {
+    } else if (!try) {
          xenbus_printf(XBT_NIL, info->dev->nodename, state,
                    "%d", XenbusStateClosed);
      }
@@ -1060,10 +1060,19 @@ static void scsiback_do_1lun_hotplug(struct
vscsibk_info *info, int op,

      switch (op) {
      case VSCSIBACK_OP_ADD_OR_DEL_LUN:
-        if (device_state == XenbusStateInitialising)
-            scsiback_do_add_lun(info, state, phy, &vir);
-        if (device_state == XenbusStateClosing)
+        switch (device_state) {
+        case XenbusStateInitialising:
+            scsiback_do_add_lun(info, state, phy, &vir, 0);
+            break;
+        case XenbusStateConnected:
+            scsiback_do_add_lun(info, state, phy, &vir, 1);
+            break;
+        case XenbusStateClosing:
              scsiback_do_del_lun(info, state, &vir);
+            break;
+        default:
+            break;
+        }
          break;

      case VSCSIBACK_OP_UPDATEDEV_STATE:


--
To unsubscribe from this list: send the line "unsubscribe
linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.