[v3,5/8] golang/xenlight: Default loglevel to DEBUG until we get everything working
diff mbox series

Message ID 20200117155734.1067550-5-george.dunlap@citrix.com
State New, archived
Headers show
Series
  • [v3,1/8] golang/xenlight: Don't try to marshall zero-length arrays in fromC
Related show

Commit Message

George Dunlap Jan. 17, 2020, 3:57 p.m. UTC
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---

The other option would be to expose the XTL logging levels and let the
caller set them somehow.

CC: Nick Rosbrook <rosbrookn@ainfosec.com>
---
 tools/golang/xenlight/xenlight.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Nick Rosbrook Jan. 20, 2020, 11:41 p.m. UTC | #1
> The other option would be to expose the XTL logging levels and let the
> caller set them somehow.
I think this is fine for now.

For the future, I like using the "functional option" pattern for this
sort of thing. That way, if a user wanted to set a non-default log
level, they could do:

ctx, err := xenlight.NewContext(xenlight.WithLogLevel(lvl))

but if they do not need to specify any options, it's still just:

ctx, err := xenlight.NewContext()

-NR
George Dunlap Jan. 21, 2020, 9:55 a.m. UTC | #2
> On Jan 20, 2020, at 11:41 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
> 
>> The other option would be to expose the XTL logging levels and let the
>> caller set them somehow.
> I think this is fine for now.
> 
> For the future, I like using the "functional option" pattern for this
> sort of thing. That way, if a user wanted to set a non-default log
> level, they could do:
> 
> ctx, err := xenlight.NewContext(xenlight.WithLogLevel(lvl))
> 
> but if they do not need to specify any options, it's still just:
> 
> ctx, err := xenlight.NewContext()

You know, I somehow remembered the “use a function to set options” pattern (and have a  mock-up for that in the “NewType()” patch later), but didn’t notice that such function were variadic.  That’s a lot nicer.

But really, we need a way to actually create a logger properly.  Apparently one thing libvirt does is to create a logger to a file for each guest.  That’s something our package users  might want to do at some point.

 -George
Nick Rosbrook Jan. 24, 2020, 7:51 p.m. UTC | #3
> But really, we need a way to actually create a logger properly.  Apparently one thing libvirt does is to create a logger to a file for each guest.  That’s something our package users  might want to do at some point.

Yes, I think package users will want logging to be pretty flexible. I
think we can cover most of those cases with Context options, or maybe
we make a Logger type that provides an abstraction for XTL.

-NR

Patch
diff mbox series

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index aa1e63a61a..0e71f6ca7d 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -82,7 +82,7 @@  type Context struct {
 func NewContext() (*Context, error) {
 	var ctx Context
 
-	ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)
+	ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_DEBUG, 0)
 
 	ret := C.libxl_ctx_alloc(&ctx.ctx, C.LIBXL_VERSION, 0,
 		(*C.xentoollog_logger)(unsafe.Pointer(ctx.logger)))