Re: [PATCH RFC 2/4] xen: add bitmap to indicate per-domain state changes

[Juergen Gross <jgross@xxxxxxxx>]

On 22.11.21 14:39, Jan Beulich wrote:
> On 22.11.2021 13:46, Juergen Gross wrote:
> > On 22.11.21 11:41, Jan Beulich wrote:
> > > On 14.09.2021 14:35, Juergen Gross wrote:
> > > > Add a bitmap with one bit per possible domid indicating the respective
> > > > domain has changed its state (created, deleted, dying, crashed,
> > > > shutdown).
> > > >
> > > > Registering the VIRQ_DOM_EXC event will result in setting the bits for
> > > > all existing domains and resetting all other bits.
> > > Generally I view VIRQ_DOM_EXC as overly restrictive already - what if both
> > > a xenstore domain and a control domain want respective notification? Hence
> > The general idea was that in this case the control domain should
> > register a related Xenstore watch.
> >
> > > similarly I'm not convinced a single, global instance will do here. Which
> > > in turn raises the question whether the approach chosen may not take us
> > > far enough, and we wouldn't instead want a more precise notification model
> > > (i.e. not just "something has changed").
> > Yes, that would be the job of Xenstore. It would provide the more
> > fine grained watches for that purpose.
> And the watch consumer still wouldn't have a way to distinguish two domain
> instances using the same ID, would it?
My further plans include new watches for domain creation/destroy
which will include the domid in the watch path reported with the
watch event. So anyone registering for domain creation watches
would no longer need another way to distinguish domains with the
same domid by other means.

The main question remaining here is whether we are okay to let
Xenstore be the instance providing that functionality to the rest
of the stack, or if we want that functionality in the hypervisor,
with all the related needed access control (Xenstore allows to
set the usability of the create/destroy watches for other domains
today).

> > > > @@ -1141,6 +1161,8 @@ static void complete_domain_destroy(struct rcu_head *head)
> > > >          xfree(d->vcpu);
> > > >    +    set_bit(d->domain_id, dom_state_changed);
> > > > +
> > > >        _domain_destroy(d);
> > > >          send_global_virq(VIRQ_DOM_EXC);
> > > Wouldn't this better be in domain_destroy() immediately after the domain
> > > has been taken off the list, and hence is no longer "discoverable"?
> > In this case the call of send_global_virq() should be moved, too,
> > I guess?
> Possibly, albeit I'd see this as a separate change to make. It doesn't
> seem outright wrong for the vIRQ to be sent late. But I agree with the
> idea of keeping sending and bit-setting together (ideally, even if this
> was to stay here, the two would better sit truly side by side).
Okay, I'll prepend another patch moving the call of send_global_virq()
to domain_destroy().


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature