| Message ID | 1454941819-24513-3-git-send-email-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/08/2016 09:30 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 by checking for a LUN already existing when existence is > allowed (scsiback_do_add_lun() called with try == 1). > > As the LUN existence check is needed now for a third time, factor it > out into a function. This in turn leads to a more or less complete > rewrite of scsiback_del_translation_entry() which will now return a > proper error code in case of failure. > > Signed-off-by: Juergen Gross <jgross@suse.com> > Cc: stable@vger.kernel.org Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
On 08/02/16 14:30, 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 by checking for a LUN already existing when existence is > allowed (scsiback_do_add_lun() called with try == 1). > > As the LUN existence check is needed now for a third time, factor it > out into a function. This in turn leads to a more or less complete > rewrite of scsiback_del_translation_entry() which will now return a > proper error code in case of failure. > > Signed-off-by: Juergen Gross <jgross@suse.com> > Cc: stable@vger.kernel.org Avoiding warnings doesn't seem like a viable candidate for stable. Is there more to this patch than the description suggests? David
On 08/02/16 17:46, David Vrabel wrote: > On 08/02/16 14:30, 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 by checking for a LUN already existing when existence is >> allowed (scsiback_do_add_lun() called with try == 1). >> >> As the LUN existence check is needed now for a third time, factor it >> out into a function. This in turn leads to a more or less complete >> rewrite of scsiback_del_translation_entry() which will now return a >> proper error code in case of failure. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> Cc: stable@vger.kernel.org > > Avoiding warnings doesn't seem like a viable candidate for stable. Is > there more to this patch than the description suggests? No. Should I resend without the Cc: stable? Juergen
On 08/02/16 16:50, Juergen Gross wrote: > On 08/02/16 17:46, David Vrabel wrote: >> On 08/02/16 14:30, 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 by checking for a LUN already existing when existence is >>> allowed (scsiback_do_add_lun() called with try == 1). >>> >>> As the LUN existence check is needed now for a third time, factor it >>> out into a function. This in turn leads to a more or less complete >>> rewrite of scsiback_del_translation_entry() which will now return a >>> proper error code in case of failure. >>> >>> Signed-off-by: Juergen Gross <jgross@suse.com> >>> Cc: stable@vger.kernel.org >> >> Avoiding warnings doesn't seem like a viable candidate for stable. Is >> there more to this patch than the description suggests? > > No. > > Should I resend without the Cc: stable? No, I've dropped it. I've applied these two to for-linus-4.5, thanks. David
diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c index 51387d7..c46ee18 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); @@ -956,39 +967,40 @@ static void __scsiback_del_translation_entry(struct v2p_entry *entry) } /* - Delete the translation entry specfied + Delete the translation entry specified */ 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; + int ret = 0; 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); + else + ret = -ENOENT; spin_unlock_irqrestore(&info->v2p_lock, flags); - return 0; + return ret; } 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)) {
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 by checking for a LUN already existing when existence is allowed (scsiback_do_add_lun() called with try == 1). As the LUN existence check is needed now for a third time, factor it out into a function. This in turn leads to a more or less complete rewrite of scsiback_del_translation_entry() which will now return a proper error code in case of failure. Signed-off-by: Juergen Gross <jgross@suse.com> Cc: stable@vger.kernel.org --- v2: scsiback_del_translation_entry() returns error code instead of 1 (Boris Ostrovsky) --- drivers/xen/xen-scsiback.c | 70 +++++++++++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 29 deletions(-)