> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 20 March 2018 16:37 > To: Paul Durrant <paul.durr...@citrix.com> > Cc: Andrew Cooper <andrew.coop...@citrix.com>; xen- > de...@lists.xenproject.org > Subject: Re: [PATCH v2 2/4] x86/hvm: take a reference on ioreq server > emulating domain > > >>> On 16.03.18 at 17:58, <paul.durr...@citrix.com> wrote: > > @@ -305,6 +305,7 @@ static int hvm_ioreq_server_add_vcpu(struct > hvm_ioreq_server *s, > > bool is_default, struct vcpu *v) > > { > > struct hvm_ioreq_vcpu *sv; > > + domid_t domid; > > int rc; > > > > sv = xzalloc(struct hvm_ioreq_vcpu); > > @@ -315,7 +316,9 @@ static int hvm_ioreq_server_add_vcpu(struct > hvm_ioreq_server *s, > > > > spin_lock(&s->lock); > > > > - rc = alloc_unbound_xen_event_channel(v->domain, v->vcpu_id, s- > >domid, > > + domid = s->emulator->domain_id; > > + > > + rc = alloc_unbound_xen_event_channel(v->domain, v->vcpu_id, > domid, > > NULL); > > if ( rc < 0 ) > > goto fail2; > > @@ -324,9 +327,9 @@ static int hvm_ioreq_server_add_vcpu(struct > hvm_ioreq_server *s, > > > > if ( v->vcpu_id == 0 && s->bufioreq.va != NULL ) > > { > > - struct domain *d = s->domain; > > + struct domain *d = s->target; > > > > - rc = alloc_unbound_xen_event_channel(v->domain, 0, s->domid, > NULL); > > + rc = alloc_unbound_xen_event_channel(v->domain, 0, domid, NULL); > > Especially for this second call site it would probably be more clear > at the first glance which domain is meant if you didn't latch > s->emulator->domain_id into a local variable.
Ok, I can get rid of that. > > > @@ -602,12 +605,17 @@ static int hvm_ioreq_server_init(struct > hvm_ioreq_server *s, > > struct domain *d, bool is_default, > > int bufioreq_handling, ioservid_t id) > > { > > + struct domain *currd = current->domain; > > struct vcpu *v; > > int rc; > > > > s->id = id; > > - s->domain = d; > > - s->domid = current->domain->domain_id; > > + s->target = d; > > + > > + if ( !get_domain(currd) ) > > + return -EACCES; > > Use get_knownalive_domain() here, which cannot fail? > Oh, didn't know about that... sounds better. > > @@ -651,6 +660,8 @@ static void hvm_ioreq_server_deinit(struct > hvm_ioreq_server *s, > > hvm_ioreq_server_remove_all_vcpus(s); > > hvm_ioreq_server_unmap_pages(s, is_default); > > hvm_ioreq_server_free_rangesets(s, is_default); > > + > > + put_domain(s->emulator); > > } > > Could you clarify in the description that this runs in the context of > XEN_DOMCTL_destroydomain and hence there's no risk of target > and emulator preventing each other from being cleaned up? > Ok, sure. Paul > Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel