> -----Original Message-----
> From: Ian Jackson <ian.jack...@citrix.com>
> Sent: 18 February 2020 11:48
> To: Durrant, Paul <pdurr...@amazon.co.uk>
> Cc: xen-devel@lists.xenproject.org; Wei Liu <w...@xen.org>; Anthony Perard
> <anthony.per...@citrix.com>; Andrew Cooper <andrew.coop...@citrix.com>;
> George Dunlap <george.dun...@citrix.com>; Jan Beulich <jbeul...@suse.com>;
> Julien Grall <jul...@xen.org>; Konrad Rzeszutek Wilk
> <konrad.w...@oracle.com>; Stefano Stabellini <sstabell...@kernel.org>;
> Jason Andryuk <jandr...@gmail.com>
> Subject: RE: [PATCH v5 5/7] libxl: allow creation of domains with a
> specified or random domid
>
> Durrant, Paul writes ("RE: [PATCH v5 5/7] libxl: allow creation of domains
> with a specified or random domid"):
> > Ian Jackson <ian.jack...@citrix.com>
> > > Sorry if I was confused; I will read this again.
> >
> > It is hard to follow the error paths. Early on in development I ended up
> with domains getting destroyed when I didn't want them to be (when
> xc_domain_create() failed due to a duplicate domid).
>
> Having read the patch again, I suggest the following discipline (which
> is along the lines contemplated by CODYING_STYLE):
>
> The local variable `domid' contains only a domid we are trying to
> create and does not constitute a "local [variable] referring to
> resources which might need cleaning up" (in the words of
> CODING_STYLE). Therefore it should never be passed to destroy.
> Maybe it should be called `prospective_domid'.
>
> The variable *domid _is_ a "local [variable] referring to resources
> which might need cleaning up". Therefore it must only ever contain a
> domain which actually exists. It should be set from prospective_domid
> when xc_domain_create succeeds, and cleared (set back to INVALID) when
> xc_domain_destroy succeeds in our retry loop.
>
> That way any `goto out' anywhere will clear up a domain iff there is
> one to clear up.
>
> There is a hunk in this patch which I think is incompatible with this
> discipline:
>
> - assert(soft_reset || *domid == INVALID_DOMID);
> -
>
> I don't understand what this hunk is for. If we adopt the discipline
> I suggest, can it go away ?
Ok, I'll give that a try. It's possible things are sufficiently complex that a
sub-function may be appropriate, which should also achieve the localization.
Paul
>
> Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
