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

Re: [Xen-devel] [PATCH] xen/xen-scsiback: Need go to fail after xenbus_dev_error()

On 9/30/14 15:50, Chen Gang wrote:
> On 9/30/14 14:59, Juergen Gross wrote:
>> On 09/30/2014 08:32 AM, Chen Gang wrote:
>>> On 9/29/14 21:57, David Vrabel wrote:
>>>> On 29/09/14 10:59, Chen Gang wrote:
>>>>> If no any additional reply within 2 days, I shall send patch v2 for it:
>>>>>    "use dev_warn() instead of xenbus_dev_error() and remove 'fail' code 
>>>>> block"
>>>> I think this driver is fine as-is and does not need any changes.
>>> OK, at least, at present, it is not a bug (will cause any issue).
>>> But for me, xenbus_dev_error() seems for printing generic errors,
>>> dev_warn() is more suitable than it.
>> I'm unbiased regarding this one.
> After check all related code for xenbus_printf() and xenbus_dev_error(),
> for me: if xenbus_printf() is for optional error, it will print warning;
> all xenbus_dev_error() are not for optional error, except 2 area:
>   drivers/pci/xen-pcifront.c:866:             xenbus_dev_error(pdev->xdev, 
> err,
>   drivers/pci/xen-pcifront.c:947:             xenbus_dev_error(pdev->xdev, 
> err,

And for this 2 xenbus_dev_error(), they have no much negative effect
(not check return value, and according to the code below, readers can
easily understand, they are for optional failure).

But for our case, I recommend to use dev_warn() instead of, or readers
is really easy to misunderstand (xenbus_dev_error, and 'grant'), then
may send spam again (like me).

> In fact, for me, not only they need be improved, but also skip 'err' for
> pcifront_scan_root() and pcifront_rescan_root(), are they bugs? (I guess
> they are). If they are really bugs, I shall send related patch for it.

If no any additional reply for them within 2 days, I shall assume they
are bugs, and send related patch for them, in next month (2014-10-??).

>>> And 'fail' code block is useless now, need be removed, too (which will
>>> let compiler report warning).
>> This should be part of the patch making the 'fail' block useless.

The original related patch is canceled, so we need not remove 'fail'
block (it still seems useful, although it is not).

> Yeah, originally, it really should be, but if this patch can continue,
> for me, can remove it in this patch, too (for the original patch, I
> intended to remain it for discussing and analysing in this patch).
> But all together, if you stick to remove 'fail' code block in original
> patch, for me, it is OK.

Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Xen-devel mailing list



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