xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: George Dunlap <George.Dunlap@citrix.com>
To: Nick Rosbrook <rosbrookn@gmail.com>
Cc: "xen-devel@lists.xenproject.prg" <xen-devel@lists.xenproject.prg>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Nick Rosbrook <rosbrookn@ainfosec.com>,
	Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>
Subject: Re: [RESEND PATCH 07/12] golang/xenlight: add logging conveniences for within xenlight
Date: Fri, 18 Jun 2021 16:30:40 +0000	[thread overview]
Message-ID: <41F44BB8-0C20-44CA-AD7D-895C886115D0@citrix.com> (raw)
In-Reply-To: <YMy7NVTpaIlT+KWJ@FED-nrosbr-BE.crux.rad.ainfosec.com>



> On Jun 18, 2021, at 4:26 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
> 
> On Fri, Jun 18, 2021 at 01:21:40PM +0000, George Dunlap wrote:
>> 
>> 
>>> On Jun 18, 2021, at 2:17 PM, George Dunlap <George.Dunlap@citrix.com> wrote:
>>> 
>>> 
>>> 
>>>> On May 24, 2021, at 9:36 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
>>>> 
>>>> Add some logging methods to Context to provide easy use of the
>>>> Contenxt's xentoollog_logger. These are not exported, but the LogLevel
>>>> type is so that a later commit can allow the Context's log level to be
>>>> configurable.
>>>> 
>>>> Becuase cgo does not support calling C functions with variable
>>>> arguments, e.g. xtl_log, add an xtl_log_wrap function to the cgo preamble
>>>> that accepts an already formatted string, and handle the formatting in
>>>> Go.
>>>> 
>>>> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
>>> 
>>> Looks good.  One comment:
>> 
>> Er, sorry, turns out I had rather more than one comment.  Here’s one more:
>> 
>> Is there any particular reason not to export the Ctx.Log[X]() functions?
>> 
> No reason other than I tend to only export functions when I know they
> need to be exported. My motivation for adding these at the time was to
> help debug development. Would you prefer to export them then?

I don’t have a super-strong preference.  I was just thinking that xl and libxl both use the same logger, so it would make sense for whatever was built on top of xenlight to use the same logger.

But I guess we’d want the exported version to be able to pass in its own “context”; since it’s more work than just capitalizing the method names, I’d say go ahead and postpone that for now.

Thanks,
 -George

  reply	other threads:[~2021-06-18 16:32 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-24 20:36 [RESEND PATCH 00/12] golang/xenlight: domain life cycle support Nick Rosbrook
2021-05-24 20:36 ` [RESEND PATCH 01/12] golang/xenlight: update generated code Nick Rosbrook
2021-06-18 10:30   ` George Dunlap
2021-05-24 20:36 ` [RESEND PATCH 02/12] golang/xenlight: fix StringList toC conversion Nick Rosbrook
2021-06-18 10:51   ` George Dunlap
2021-05-24 20:36 ` [RESEND PATCH 03/12] golang/xenlight: fix string conversion in generated toC functions Nick Rosbrook
2021-06-18 11:00   ` George Dunlap
2021-06-21 16:11     ` Nick Rosbrook
2021-07-01 14:09       ` George Dunlap
2021-07-07 19:29         ` Nick Rosbrook
2021-05-24 20:36 ` [RESEND PATCH 04/12] golang/xenlight: export keyed union interface types Nick Rosbrook
2021-06-18 11:19   ` George Dunlap
2021-05-24 20:36 ` [RESEND PATCH 05/12] golang/xenlight: use struct pointers in keyed union fields Nick Rosbrook
2021-06-18 11:26   ` George Dunlap
2021-05-24 20:36 ` [RESEND PATCH 06/12] golang/xenlight: rename Ctx receivers to ctx Nick Rosbrook
2021-06-18 11:39   ` George Dunlap
2021-05-24 20:36 ` [RESEND PATCH 07/12] golang/xenlight: add logging conveniences for within xenlight Nick Rosbrook
2021-06-18 13:17   ` George Dunlap
2021-06-18 13:21     ` George Dunlap
2021-06-18 15:26       ` Nick Rosbrook
2021-06-18 16:30         ` George Dunlap [this message]
2021-06-18 15:17     ` Nick Rosbrook
2021-06-18 16:28       ` George Dunlap
2021-05-24 20:36 ` [RESEND PATCH 08/12] golang/xenlight: add functional options to configure Context Nick Rosbrook
2021-06-18 14:44   ` George Dunlap
2021-06-18 15:08     ` Nick Rosbrook
2021-06-18 16:18       ` George Dunlap
2021-06-18 17:00         ` Nick Rosbrook
2021-06-18 18:12           ` George Dunlap
2021-05-24 20:36 ` [RESEND PATCH 09/12] golang/xenlight: add DomainDestroy wrapper Nick Rosbrook
2021-06-18 14:47   ` George Dunlap
2021-05-24 20:36 ` [RESEND PATCH 10/12] golang/xenlight: add SendTrigger wrapper Nick Rosbrook
2021-06-18 14:54   ` George Dunlap
2021-05-24 20:36 ` [RESEND PATCH 11/12] golang/xenlight: do not negate ret when converting to Error Nick Rosbrook
2021-06-18 15:13   ` George Dunlap
2021-06-18 15:32     ` Nick Rosbrook
2021-05-24 20:36 ` [RESEND PATCH 12/12] golang/xenlight: add NotifyDomainDeath method to Context Nick Rosbrook
2021-06-18 18:28   ` George Dunlap
2021-06-18 19:31     ` George Dunlap
2021-06-21 21:41       ` Nick Rosbrook
2021-06-17 15:29 ` [RESEND PATCH 00/12] golang/xenlight: domain life cycle support Nick Rosbrook
2021-06-21 15:53 ` George Dunlap
2021-06-21 16:19   ` Nick Rosbrook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=41F44BB8-0C20-44CA-AD7D-895C886115D0@citrix.com \
    --to=george.dunlap@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=rosbrookn@ainfosec.com \
    --cc=rosbrookn@gmail.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    --cc=xen-devel@lists.xenproject.prg \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).