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

Re: [PATCH v3 32/39] xen/riscv: save and clear the old IMSIC VS-file on vCPU migration



On 2026-11-06 10:52 +0100, Oleksii Kurochko wrote:
> [ ... 25 lines skipped ... ]
> > I think it's worth mentioning that this MRIF structure could be reused
> > for software interrupt files later.
> 
> I'm fine with making that explicit. I tried to cover it with "... a vCPU
> which doesn't own a guest interrupt file ...", and adding "hardware"
> should make it clearer: "... a vCPU which doesn't own a hardware guest
> interrupt file ...". I think that covers your suggestion as well, so
> I'll add it.
> 
> To be honest, I'm a bit unsure how much to say about software interrupt 
> files at this point: some earlier commit messages mentioned them, and I 
> was asked to drop that, as they are neither introduced nor used yet.
> That's why I'd rather keep the wording implicit as above, but if you
> still prefer them to be named explicitly, I can add a sentence.

Agreed, keep it implicit.

> 
> At this point, I'm honestly getting a bit confused about how commit 
> messages are supposed to be written.

Sorry for the confusion. I'm generally not a fan of mentioning things
which don't exist yet, but here the commit message explains why the MRIF
layout is used, and the spec ties MRIFs closely to s/w interrupt files,
so I wondered whether a short hint would help. The implicit wording is
fine too.

I should have marked it as a nit. As I said before, your commit messages
are great, and this was just my subjective opinion, possibly partly
because I'm not a native English speaker. Feel free to discard such
remarks whenever you don't find them accurate ;)
> [ ... 49 lines skipped ... ]
> > Nit: is this blank line really needed?
> > 
> 
> No real need. Lets drop it.
> 
> Thanks for review!
You're wecome!

-- 
Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>



 


Rackspace

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