From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH 3/6] libxl, xl: fixes to domain creation cleanup logic (domid values) Date: Fri, 28 Jan 2011 12:22:56 +0000 Message-ID: References: <1296150064-31991-1-git-send-email-ian.jackson@eu.citrix.com> <1296150064-31991-2-git-send-email-ian.jackson@eu.citrix.com> <1296150064-31991-3-git-send-email-ian.jackson@eu.citrix.com> <1296150064-31991-4-git-send-email-ian.jackson@eu.citrix.com> <19777.49322.747849.408538@mariner.uk.xensource.com> <19777.53024.392960.80045@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Return-path: In-Reply-To: <19777.53024.392960.80045@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Ian Jackson Cc: "xen-devel@lists.xensource.com" , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org 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 > 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 > > 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?