Re: [PATCHv2] xen: remove DEFINE_XENBUS_DRIVER() macro

[David Vrabel <david.vrabel@citrix.com>]

On 10/09/14 17:44, Konrad Rzeszutek Wilk wrote:
> On Wed, Sep 10, 2014 at 05:25:40PM +0100, David Vrabel wrote:
>> On 10/09/14 17:00, Konrad Rzeszutek Wilk wrote:
>>> On Wed, Sep 10, 2014 at 01:07:49PM +0100, David Vrabel wrote:
>>>> The DEFINE_XENBUS_DRIVER() macro looks a bit weird and causes sparse
>>>> errors.
>>>
>>> .. but it is also useful for downstream distros to bolt on Xen patches.
>>>
>>> Is this urgent? Could it wait until Novell/SuSE has switched over
>>> to using pvops and then this can go in?
>>
>> If the macro didn't cause sparse errors, I could be persuaded to wait.
> 
> I presume there is a going to be more of the 'sparse errors' fixes coming?

The addition of xen-scsiback and xen-scsifront added three new spare
errors that two or three automated build systems whinged to me about.

I think it is reasonable to require that new code does not introduce new
sparse errors.

Two of the errors are because of this bad DEFINE_XENBUS_DRIVER macro.
This macro is bad because:

1. It's a macro.  All macros that are not simple constants or
function-like are bad and reduce maintainability and readability,
particularly for developers unfamiliar with the code.

2. It adds a variable with a name constructed with ##.

3. It requires a variable in scope with a specific name.

4. The empty parameter just looks plain weird.

>> I do not think we should avoid fixing bugs or improving the readability
>> or maintainability of the code to help out someone still using
>> non-upstream Xen support.
> 
> That is not what I am saying. I am asking whether it could wait a bit.
> It is not that urgent is it? Why are sparse errors suddenly so important?
> 
>>
>> In general, the cost of maintaining non-upstream forks should not be
>> paid for by upstream users/developers.
> 
> Of course. However the downstream forks have nice QA departments that
> can catch upstream bugs as they rebase. Accomodating them and at the
> same time nudging them towards upstream I think is a small price to pay.

I think the time for nudging was 2-3 years ago.

A look at the history shows that Suse made zero Xen-related
contributions to upstream kernel in 2014 /until/ they started working on
the upstream kernel.  I do not think this reason for accommodating
Suse's kernel fork is applicable any more.

David