Re: [PATCH v1 1/5] xen/riscv: add stub for share_xen_page_with_guest()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.10.2024 15:10, oleksii.kurochko@xxxxxxxxx wrote:
> On Thu, 2024-10-17 at 16:51 +0200, Jan Beulich wrote:
>> On 16.10.2024 11:15, Oleksii Kurochko wrote:
>>> To avoid the following linkage fail the stub for
>>> share_xen_page_with_guest()
>>> is introduced:
>>
>> What do you intend to express with "is introduced"? Is there a
>> problem now?
>> Would there be a problem with subsequent changes? I'm entirely fine
>> with
>> adding that stub, but the description should make clear why /when
>> exactly
>> it's needed.
> I mentioned that in the cover letter:
> ```
>    Also, after patch 3 ("xen/riscv: introduce setup_mm()") of this
>    patch series,
>    the linkage error mentioned in patch 1 ("xen/riscv: add stub for
>    share_xen_page_with_guest()") began to occur, so patch 1 addresses
>    this issue.
> ```
> I thought it would be the better then just mention in the commit
> message that.

Mentioning in the cover letter is fine, but each patch needs to also
be self-contained.

> Will it be fine to mention instead:
> ```
>    Introduction of setup memory management function will require
>    share_xen_page_with_guest() defined, at least, as a stub otherwise
>    the following linkage error will occur...
> ```

Quoting the linker error is imo of limited use. What such an explanation
wants to say is why, all of the sudden, such an error occurs. After all
you don't change anything directly related to common/trace.c.

> Perhaps it would be better just to merge these changes with patch 3 and
> add an explanation in patch 3 commit message.

Perhaps, as you say.

Jan