Re: [Xen-devel] [PATCH v4 3/3] golang/xenlight: Implement DomainCreateNew

[Nick Rosbrook <rosbrookn@gmail.com>]

Looks good, I just have two small comments:

> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> index 56fa31fd7b..808b4a327c 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -1111,3 +1111,24 @@ func (Ctx *Context) PrimaryConsoleGetTty(domid uint32) (path string, err error)
>         path = C.GoString(cpath)
>         return
>  }
> +
> +// int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config,
> +//                             uint32_t *domid,
> +//                             const libxl_asyncop_how *ao_how,
> +//                             const libxl_asyncprogress_how *aop_console_how)

Conventionally, we want to have comments for exported functions along
the lines of:

    // DomainCreateNew creates a new domain with config, and returns
its Domid on success.
    // A non-nil error is returned if config cannot be marshaled, or
an error occurs within libxl.

Besides being easier to read, it makes documentation more clear on
godoc/pkg.go.dev.

> +func (Ctx *Context) DomainCreateNew(config *DomainConfig) (Domid, error) {

Capitalizing "Ctx" here is a little weird to me. Since it's only the
receiver name, there's no effect, but since capitalized identifiers
have special-meaning in other contexts, I would avoid doing this.

I only point those out in case you want to change it on check-in. Besides that,

Reviewed-by: Nick Rosbrook <rosbrookn@ainfosec.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel