Re: [Xen-devel] [PATCH 3/6] libxl, xl: fixes to domain creation cleanup logic (domid values)

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 28 Jan 2011, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 3/6] libxl, xl: fixes to 
> domain creation cleanup logic (domid values)"):
> > domid 0 or domid > DOMID_FIRST_RESERVED on entry
> 
> How about this.  Also fixes Gianni's brace formatting complaint.
> 
> Ian.
> 
> commit b4a79d2621a7e21e7fcbbf605d650e106d8fc3d6
> 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>
> 

Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel