Re: [PATCH 2/2] xen/scsiback: avoid warnings when adding multiple LUNs to a domain

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

 





On 02/05/2016 08:21 AM, Juergen Gross wrote:
When adding more than one LUN to a frontend a warning for a failed
assignment is issued in dom0 for each already existing LUN. Avoid this
warning.

Aren't you just factoring out the check? The warning is still printed for each scsiback_add_translation_entry() invocation, no?



Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
---
  drivers/xen/xen-scsiback.c | 65 ++++++++++++++++++++++++++--------------------
  1 file changed, 37 insertions(+), 28 deletions(-)

diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 51387d7..69de879 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -849,15 +849,31 @@ static int scsiback_map(struct vscsibk_info *info)
  }
/*
+  Check for a translation entry being present
+*/
+static struct v2p_entry *scsiback_chk_translation_entry(
+	struct vscsibk_info *info, struct ids_tuple *v)
+{
+	struct list_head *head = &(info->v2p_entry_lists);
+	struct v2p_entry *entry;
+
+	list_for_each_entry(entry, head, l)
+		if ((entry->v.chn == v->chn) &&
+		    (entry->v.tgt == v->tgt) &&
+		    (entry->v.lun == v->lun))
+			return entry;
+
+	return NULL;
+}
+
+/*
    Add a new translation entry
  */
  static int scsiback_add_translation_entry(struct vscsibk_info *info,
  					  char *phy, struct ids_tuple *v)
  {
  	int err = 0;
-	struct v2p_entry *entry;
  	struct v2p_entry *new;
-	struct list_head *head = &(info->v2p_entry_lists);
  	unsigned long flags;
  	char *lunp;
  	unsigned long long unpacked_lun;
@@ -917,15 +933,10 @@ static int scsiback_add_translation_entry(struct vscsibk_info *info,
  	spin_lock_irqsave(&info->v2p_lock, flags);
/* Check double assignment to identical virtual ID */
-	list_for_each_entry(entry, head, l) {
-		if ((entry->v.chn == v->chn) &&
-		    (entry->v.tgt == v->tgt) &&
-		    (entry->v.lun == v->lun)) {
-			pr_warn("Virtual ID is already used. Assignment was not performed.\n");
-			err = -EEXIST;
-			goto out;
-		}
-
+	if (scsiback_chk_translation_entry(info, v)) {
+		pr_warn("Virtual ID is already used. Assignment was not performed.\n");
+		err = -EEXIST;
+		goto out;
  	}
/* Create a new translation entry and add to the list */
@@ -933,7 +944,7 @@ static int scsiback_add_translation_entry(struct vscsibk_info *info,
  	new->v = *v;
  	new->tpg = tpg;
  	new->lun = unpacked_lun;
-	list_add_tail(&new->l, head);
+	list_add_tail(&new->l, &info->v2p_entry_lists);
out:
  	spin_unlock_irqrestore(&info->v2p_lock, flags);
@@ -962,33 +973,31 @@ static int scsiback_del_translation_entry(struct vscsibk_info *info,
  					  struct ids_tuple *v)
  {
  	struct v2p_entry *entry;
-	struct list_head *head = &(info->v2p_entry_lists);
  	unsigned long flags;
spin_lock_irqsave(&info->v2p_lock, flags);
  	/* Find out the translation entry specified */
-	list_for_each_entry(entry, head, l) {
-		if ((entry->v.chn == v->chn) &&
-		    (entry->v.tgt == v->tgt) &&
-		    (entry->v.lun == v->lun)) {
-			goto found;
-		}
-	}
-
-	spin_unlock_irqrestore(&info->v2p_lock, flags);
-	return 1;
-
-found:
-	/* Delete the translation entry specfied */
-	__scsiback_del_translation_entry(entry);
+	entry = scsiback_chk_translation_entry(info, v);
+	if (entry)
+		__scsiback_del_translation_entry(entry);
spin_unlock_irqrestore(&info->v2p_lock, flags);
-	return 0;
+	return entry == NULL;

Might be better to return -ENOENT instead of 1 above and -EEXISTS if entry!=NULL, given that this returns an int.

-boris

  }
static void scsiback_do_add_lun(struct vscsibk_info *info, const char *state,
  				char *phy, struct ids_tuple *vir, int try)
  {
+	struct v2p_entry *entry;
+	unsigned long flags;
+
+	if (try) {
+		spin_lock_irqsave(&info->v2p_lock, flags);
+		entry = scsiback_chk_translation_entry(info, vir);
+		spin_unlock_irqrestore(&info->v2p_lock, flags);
+		if (entry)
+			return;
+	}
  	if (!scsiback_add_translation_entry(info, phy, vir)) {
  		if (xenbus_printf(XBT_NIL, info->dev->nodename, state,
  				  "%d", XenbusStateInitialised)) {

--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]