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

Re: [PATCH 1/2] xen: add domid_to_domain() helper





On 12/09/2022 09:42, Jan Beulich wrote:
On 12.09.2022 10:36, Julien Grall wrote:
On 12/09/2022 09:33, Juergen Gross wrote:
On 12.09.22 10:31, Jan Beulich wrote:
On 12.09.2022 10:23, Juergen Gross wrote:
On 12.09.22 10:19, Jan Beulich wrote:
On 12.09.2022 07:53, Juergen Gross wrote:
Add a helper domid_to_domain() returning the struct domain pointer for
a domain give by its domid and which is known not being able to be
released (its reference count isn't incremented and no
rcu_lock_domain()
is called for it).

In order to simplify coding add an internal helper for doing the
lookup
and call that from the new function and similar functions.

Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

I don't see an issue with adding such a helper (responding to your
concern
in the cover letter), but I think the constraints need to be empahsized
more: We already have get_knownalive_domain() and
get_domain_by_id(), so
how about naming the new helper get_knownalive_domain_by_id()? And
then ...

I explicitly didn't name it "get_...", as those helpers all increment
the
reference count of the domain. And this is NOT done by the new helper.

Hmm, agreed. But domid_to_domain() isn't expressing the "known alive"
aspect,
yet that's relevant to see when reviewing new uses of the function.
Such uses
aren't likely to make the reviewer go look at the function declaration
when
the function name is pretty "innocent".

Okay, what about domid_to_knownalive_domain()?

Or knownalive_domain_from_domid()?

FWIW, I am fine with either. However, please don't replace 'to' with '2'
if you need a internal helper. Just suffixed with 'locked' to make clear
this is a version where the caller should take the lock.

Hmm - personally I dislike "_locked" suffixes on functions. If the
"internal helper" aspect is to be made more explicit, then perhaps
by way of prefixing a single underscore?

I am OK with that. Although, from previous discussion, I would have expected that explicit name would have been your preference ("locked" is more explicit than "_").

Cheers,

--
Julien Grall



 


Rackspace

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