| Message ID | 20200309144932.866097-3-george.dunlap@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,1/3] golang/xenlight: Don't try to marshall zero-length arrays in fromC | expand |
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>
On 3/11/20 2:59 PM, Nick Rosbrook wrote: > 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. Yes, absolutely, that's something we need to change before we declare "1.0". But that should probably be done in a series which changes all such comments together. >> +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 c&p'd this from another method and just changed the signature. It probably would be good to make all of them lower-case. I may change this one on check in though. > I only point those out in case you want to change it on check-in. Besides that, > > Reviewed-by: Nick Rosbrook <rosbrookn@ainfosec.com> Thanks! -George
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) +func (Ctx *Context) DomainCreateNew(config *DomainConfig) (Domid, error) { + var cdomid C.uint32_t + var cconfig C.libxl_domain_config + err := config.toC(&cconfig) + if err != nil { + return Domid(0), fmt.Errorf("converting domain config to C: %v", err) + } + defer C.libxl_domain_config_dispose(&cconfig) + + ret := C.libxl_domain_create_new(Ctx.ctx, &cconfig, &cdomid, nil, nil) + if ret != 0 { + return Domid(0), Error(ret) + } + + return Domid(cdomid), nil +}
This implements the wrapper around libxl_domain_create_new(). With the previous changes, it's now possible to create a domain using the golang bindings (although not yet to unpause it or harvest it after it shuts down). Signed-off-by: George Dunlap <george.dunlap@citrix.com> --- v4: - Remove hand-crafted constructor code, make non-RFC CC: Nick Rosbrook <rosbrookn@ainfosec.com> --- tools/golang/xenlight/xenlight.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)