Re: [PATCH 10/12] xen/hypfs: add cpupool directories

[Dario Faggioli <dfaggioli@xxxxxxxx>]

On Mon, 2020-10-26 at 10:13 +0100, Juergen Gross wrote:
> Add /cpupool/<cpupool-id> directories to hypfs. Those are completely
> dynamic, so the related hypfs access functions need to be
> implemented.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>
So, I'm almost sold... Just one comment:

> --- a/xen/common/sched/cpupool.c
> +++ b/xen/common/sched/cpupool.c
> @@ -999,6 +1073,10 @@ static int __init cpupool_init(void)
>  
>      cpupool_gran_init();
>  
> +#ifdef CONFIG_HYPFS
> +    hypfs_add_dir(&hypfs_root, &cpupool_dir, true);
> +#endif
> +
What would you think about doing this in an helper function
(hypfs_cpupool_init() ?), implemented inside the above #ifdef and as an
empty stub if !CONFIG_HYPFS ?

That will save us from having the #ifdef-s again here.

I'm asking because it's certainly not critical and I don't have a too
strong opinion about it. But I do think the code would look better.

>      cpupool0 = cpupool_create(0, 0, &err);
>      BUG_ON(cpupool0 == NULL);
>      cpupool_put(cpupool0);

-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

Attachment: signature.asc
Description: This is a digitally signed message part