Re: Xen 4.18 release: Reminder about code freeze

[George Dunlap <george.dunlap@xxxxxxxxx>]

> > Stop tinkering in the hope that it hides the problem.  You're only
> > making it harder to fix properly.
>
> Making it harder to fix properly would be a valid reason not to commit
> the (maybe partial) fix. But looking at the fix again:
>
> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
> index a6cd199fdc..9cd6678015 100644
> --- a/tools/xenstored/domain.c
> +++ b/tools/xenstored/domain.c
> @@ -989,6 +989,7 @@ static struct domain *introduce_domain(const void *ctx,
>                 talloc_steal(domain->conn, domain);
>
>                 if (!restore) {
> +                       domain_conn_reset(domain);
>                         /* Notify the domain that xenstore is available */
>                         interface->connection = XENSTORE_CONNECTED;
>                         xenevtchn_notify(xce_handle, domain->port);
> @@ -1031,8 +1032,6 @@ int do_introduce(const void *ctx, struct connection 
> *conn,
>         if (!domain)
>                 return errno;
>
> -       domain_conn_reset(domain);
> -
>         send_ack(conn, XS_INTRODUCE);
>
> It is a 1-line movement. Textually small. Easy to understand and to
> revert. It doesn't seem to be making things harder to fix? We could
> revert it any time if a better fix is offered.
>
> Maybe we could have a XXX note in the commit message or in-code
> comment?

It moves a line from one function (do_domain_introduce()) into a
completely different function (introduce_domain()), nested inside two
if() statements; with no analysis on how the change will impact
things.

Are there any paths through do_domain_introduce() that now *won't* get
a domain_conn_reset() call?  Is that OK?

Is introduce_domain() called in other places?  Will those places now
get extra domain_conn_reset() calls they weren't expecting?  Is that
OK?

I mean, it certainly seems strange to set the state to CONNECTED, send
off an event channel, and then after that delete all watches /
transactions / buffered data and so on; but we need at least a basic
understanding of what's going on to know that this change isn't going
to break comething.

Not knowing much about the xenstore protocol: In the
(!domain->introduced) case, will there be anything to actually delete?
 It seems like it would only be necessary / useful on the
(domain->introduced) case.

 -George