Hi Juergen,
On 10/11/2023 16:07, Juergen Gross wrote:
> Today domain_init() is called either just before calling dom0_init()
> in case no live update is being performed, or it is called after
> reading the global state from read_state_global(), as the event
> channel fd is needed.
>
> Split up domain_init() into a preparation part which can be called
> unconditionally, and in a part setting up the event channel handle.
Looking at the code, domain_init() may not be called if -D is passed to
Xen. With your change, part of it would not be called unconditionally.
So does -D actually make sense? Did it actually work before your change?
Should it be removed?
>
> Note that there is no chance that chk_domain_generation() can be
> called now before xc_handle has been setup, so there is no need for
> the related special case anymore.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> tools/xenstored/core.c | 2 ++
> tools/xenstored/domain.c | 12 ++++++------
> tools/xenstored/domain.h | 1 +
> 3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
> index 8e271e31f9..b9ec50b34c 100644
> --- a/tools/xenstored/core.c
> +++ b/tools/xenstored/core.c
> @@ -2960,6 +2960,8 @@ int main(int argc, char *argv[])
>
> init_pipe(reopen_log_pipe);
>
> + domain_static_init();
NIT: I find 'static' a bit confusing. How about using 'early' instead to
match 'early_init()'?
> +
> /* Listen to hypervisor. */
> if (!no_domain_init && !live_update) {
> domain_init(-1);
> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
> index 58b0942043..fa17f68618 100644
> --- a/tools/xenstored/domain.c
> +++ b/tools/xenstored/domain.c
> @@ -1224,10 +1224,8 @@ static int domeq_fn(const void *key1, const void *key2)
> return *(const unsigned int *)key1 == *(const unsigned int *)key2;
> }
>
> -void domain_init(int evtfd)
> +void domain_static_init(void)
> {
> - int rc;
> -
> /* Start with a random rather low domain count for the hashtable. */
> domhash = create_hashtable(NULL, "domains", domhash_fn, domeq_fn, 0);
> if (!domhash)
> @@ -1258,6 +1256,11 @@ void domain_init(int evtfd)
> xengnttab_set_max_grants(*xgt_handle, DOMID_FIRST_RESERVED);
>
> talloc_set_destructor(xgt_handle, close_xgt_handle);
> +}
> +
> +void domain_init(int evtfd)
> +{
> + int rc;
>
> if (evtfd < 0)
> xce_handle = xenevtchn_open(NULL, XENEVTCHN_NO_CLOEXEC);
> @@ -1291,9 +1294,6 @@ static bool chk_domain_generation(unsigned int domid, uint64_t gen)
> {
> struct domain *d;
>
> - if (!xc_handle && domid == dom0_domid)
> - return true;
> -
> d = find_domain_struct(domid);
>
> return d && d->generation <= gen;
> diff --git a/tools/xenstored/domain.h b/tools/xenstored/domain.h
> index 7625dca8cd..6c00540311 100644
> --- a/tools/xenstored/domain.h
> +++ b/tools/xenstored/domain.h
> @@ -82,6 +82,7 @@ int do_get_domain_path(const void *ctx, struct connection *conn,
> int do_reset_watches(const void *ctx, struct connection *conn,
> struct buffered_data *in);
>
> +void domain_static_init(void);
> void domain_init(int evtfd);
> void dom0_init(void);
> void domain_deinit(void);
Cheers,
--
Julien Grall