On Fri, Jan 25, 2013 at 04:58:02PM +0100, Marc-André Lureau wrote: > On Fri, Jan 25, 2013 at 2:55 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote: > > Hey, > > > > On Wed, Jan 23, 2013 at 12:05:29AM +0100, Marc-André Lureau wrote: > >> Windows has a proper service for logging events and messages in a > >> structured way. It does many nice things, and "Event Viewer" allows > >> UI browsing / filtering of messages etc.. > >> > >> Note we don't really use any category or event ID but solely log level > >> and string. To make the Event Viewer happy, we still register a string > >> for our event. And MinGW doesn't seem to like linking to multiple > >> resource objects (apparently takes the first one and ignores the > >> rest?) > >> > >> The installer should add a registry key for the event source, however > >> it's not possible as a user, and the NSIS script is kinda user only > >> atm... The MSI will support those entries: > >> http://msdn.microsoft.com/en-us/library/windows/desktop/aa363661%28v=vs.85%29.aspx > >> > >> HKLM\SYSTEM\CurrentControlSet\services\eventlog\Application\VirtViewer > >> EventMessage $prefix\bin\remote-viewer.exe Oh, forgot to ask about this registry path in the commit log, is it a bad c&p? > >> > >> It's a minor annoyance if those entries are not in the registry > >> (basically, Event Viewer will complain a little, and it will be > >> impossible? to do create application filters) Is the '?' intentional? > >> + > >> +virt-viewer_rc.$(OBJEXT): $(VIRT_VIEWER_RES) win32-messages.rc $(ICONDIR)/virt-viewer.ico > >> $(AM_V_GEN)$(WINDRES) \ > >> -DICONDIR='\"$(ICONDIR)\"' \ > >> -DMANIFESTDIR='\"$(MANIFESTDIR)\"' \ > >> -i $< -o $@ > >> + > >> LDADD += virt-viewer_rc.$(OBJEXT) > >> -MAINTAINERCLEANFILES += virt-viewer_rc.$(OBJEXT) > >> +CLEANFILES += virt-viewer_rc.$(OBJEXT) > >> +CLEANFILES += win32-messages.rc Messages_*.bin > >> > >> bin_PROGRAMS += debug-helper > >> debug_helper_SOURCES = debug-helper.c > >> diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c > >> index 48a6978..7ee5e63 100644 > >> --- a/src/virt-viewer-util.c > >> +++ b/src/virt-viewer-util.c > >> @@ -30,6 +30,7 @@ > >> #ifdef G_OS_WIN32 > >> #include <windows.h> > >> #include <io.h> > >> +#include "win32-messages.h" > > > > Where is this file coming from? Is this autogenerated or did you forget a > > git add ? > > autogenerated, updating Makefile.am for deps Sorry, I realized that afterwards, and forgot to remove that comment. > >> +static void virt_viewer_log_handler (const gchar *log_domain, > >> + GLogLevelFlags log_level, > >> + const gchar *message, > >> + G_GNUC_UNUSED gpointer user_data) > >> +{ > >> + WORD logtype; > >> + > >> + switch (log_level) { > >> + case G_LOG_LEVEL_ERROR: > >> + case G_LOG_LEVEL_CRITICAL: > >> + logtype = EVENTLOG_ERROR_TYPE; > >> + break; > >> + case G_LOG_LEVEL_WARNING: > >> + logtype = EVENTLOG_WARNING_TYPE; > >> + break; > >> + default: > >> + logtype = EVENTLOG_INFORMATION_TYPE; > >> + } > >> + > >> + gchar *msg = g_strdup_printf("%s: %s", log_domain, message); > >> + ReportEventA(ms_eventlog, logtype, 0, EVENT_GLOG, NULL, 1, 0, > >> + (const char **)&msg, NULL); > >> + g_free(msg); > >> +} > >> +#endif > >> + > >> void virt_viewer_util_init(const char *appname) > >> { > >> #ifdef G_OS_WIN32 > >> + ms_eventlog = RegisterEventSourceA(NULL, "VirtViewer"); > > > > Using PACKAGE_NAME here would give us different log domains for virt-viewer > > and remote-viewer, dunno if that's something we want or not. > > Perhaps, although the event already has the executable name > associated. We will need to make sure the registry entries are created > / duplicated.. Ah ok, if there's already a way to differentiate between virtviewer and remoteviewer, and since that makes more work, I'm fine with keeping things as is. > > > > >> + if (ms_eventlog == NULL) > >> + g_printerr("can't open Windows event log\n"); > >> + else > >> + g_log_set_default_handler(virt_viewer_log_handler, NULL); > > > > Can you pass ms_eventlog as user_data instead of having it as a global > > variable? > > I don't see the point, it is global to the app. It feels cleaner to me to avoid having global variables at all. > >> + > >> +MessageId = 1 > >> +SymbolicName = CATEGORY_DUMMY > >> +Severity = Success > >> +Language = English > >> +A dummy category, as a reminder > > > > Is this needed at all? > > I am not familiar with that windows messages stuff and I was really > walking on thin ice, so I would really prefer to keep that template, > especailly if there is a big pitfall there. Ok, good for me if it has no side-effects that you noticed. Christophe
Attachment:
pgp_8Zkb7rWxP.pgp
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
