On Thu, 27 Jan 2011, Ian Jackson wrote:
> I wrote:
> > Hrm. Perhaps a utility function for testing valid guest domids would
> > be a good idea.
>
> How about this.
>
> Ian.
>
> commit 2036fda8474df55879ce8b7dd2b0e20fe7e8e3eb
> Author: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Date: Thu Jan 27 17:22:41 2011 +0000
>
> libxl, xl: fixes to domain creation cleanup logic (domid values)
>
> libxl__domain_make makes some assumptions about the way its caller
> treats its uint32_t *domid parameter: specifically, if it fails it may
> have partially created the domain and it does not every destroy it.
> But it does not initialise it. Document this assumption in a comment,
> and assert on entry that domid not a guest domain id, to ensure that
> the caller has properly initialised it.
>
> Introduce a function libxl_domid_valid_guest to help with this.
>
> This is not intended to produce any practical functional change in
> current code.
>
> Secondly, libxl_create_stubdom calls libxl__domain_make and has no
> code to tear down the domain again on error. This is complicated to
> fix (since it may even be possible for the the domain to be left in a
> state where it's not possible to tell that it was going to be a
> stubdom for some other domain). So for now simply leave a fixme
> comment.
>
> Finally, in 22739:d839631b6048 we introduced "-1" as a sentinel "no
> such domain" value for domid. However, domid is a uint32 so testing
> it with "if (domid > 0)" as we do in 22740:ce208811f540 is wrong
> because it always triggers. Instead use libxl_domid_valid_guest.
> This fix means that that if "xl create" fails, it will not try to
> destroy the domain "-1". Previously you'd see this message:
> libxl: error: libxl.c:697:libxl_domain_destroy non-existant domain -1
> whose "-1" many readers may have thought was an error code, but which
> is actually supposedly a domain id.
>
> Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
>
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index d608e99..0851e66 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -554,6 +554,12 @@ int libxl_cpupool_cpuremove(libxl_ctx *ctx, uint32_t
> poolid, int cpu);
> int libxl_cpupool_cpuremove_node(libxl_ctx *ctx, uint32_t poolid, int node,
> int *cpus);
> int libxl_cpupool_movedomain(libxl_ctx *ctx, uint32_t poolid, uint32_t
> domid);
>
> +static inline int libxl_domid_valid_guest(uint32_t domid) {
> + /* returns 1 if the value _could_ be a valid guest domid, 0 otherwise
> + * does not check whether the domain actually exists */
> + return domid > 0 && domid < DOMID_FIRST_RESERVED;
> +}
> +
better
> int libxl__domain_make(libxl_ctx *ctx, libxl_domain_create_info *info,
> - uint32_t *domid)
> + uint32_t *domid /* domid 0 or ~0 on entry; on exit
> + valid, perhaps >0 (even on error)
> */)
can you please change this comment too?
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home