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 > > 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) > > https://bugzilla.redhat.com/show_bug.cgi?id=895919 > --- > configure.ac | 6 ++++++ > src/Makefile.am | 23 +++++++++++++++++------ > src/virt-viewer-util.c | 38 ++++++++++++++++++++++++++++++++++++++ > src/win32-messages.mc | 36 ++++++++++++++++++++++++++++++++++++ > 4 files changed, 97 insertions(+), 6 deletions(-) > create mode 100644 src/win32-messages.mc > > diff --git a/configure.ac b/configure.ac > index 339acbe..8419e85 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -46,6 +46,12 @@ AS_IF([test "x$os_win32" = "xyes"], [ > if test -z "$WINDRES" ; then > AC_MSG_ERROR("windres is required to compile virt-viewer on this platform") > fi > + > + AC_CHECK_TOOL(WINDMC, [windmc]) > + > + if test -z "$WINDMC" ; then > + AC_MSG_ERROR("windmc is required to compile virt-viewer on this platform") > + fi > ]) > > AC_CONFIG_LIBOBJ_DIR([src]) > diff --git a/src/Makefile.am b/src/Makefile.am > index 05e20b2..d5e98b1 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -1,5 +1,6 @@ > NULL = > LDADD = > +CLEANFILES = > MAINTAINERCLEANFILES = Setting MAINTAINERCLEANFILES is no longer needed here > bin_PROGRAMS = > > @@ -141,23 +142,33 @@ desktop_DATA = remote-viewer.desktop > > EXTRA_DIST += $(desktop_DATA) > > -VIRT_VIEWER_RES = virt-viewer.rc virt-viewer.manifest > -ICONDIR = $(top_builddir)/icons > -MANIFESTDIR = $(srcdir) > -EXTRA_DIST += $(VIRT_VIEWER_RES) > > if OS_WIN32 > bin_PROGRAMS += windows-cmdline-wrapper > windows_cmdline_wrapper_SOURCES = windows-cmdline-wrapper.c > windows_cmdline_wrapper_LDFLAGS = -lpsapi > > -virt-viewer_rc.$(OBJEXT): $(VIRT_VIEWER_RES) $(ICONDIR)/virt-viewer.ico > +VIRT_VIEWER_RES = virt-viewer.rc virt-viewer.manifest > +EXTRA_DIST += $(VIRT_VIEWER_RES) win32-messages.mc > + > +ICONDIR = $(top_builddir)/icons > +MANIFESTDIR = $(srcdir) > + > +win32-messages.rc: win32-messages.mc > + $(AM_V_GEN)$(WINDMC) $< As I understand it, this will generate win32-message.h which is then used in virt-viewer-util.c, do we need to encode this dependency somehow? > + > +win32-messages_rc.$(OBJEXT): win32-messages.rc > + $(AM_V_GEN)$(WINDRES) -i $< -o $@ It seems this object is not used, is this rule needed? > + > +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 ? > #endif > > #include <sys/types.h> > @@ -261,9 +262,46 @@ gulong virt_viewer_signal_connect_object(gpointer instance, > return ctx->handler_id; > } > > +#ifdef G_OS_WIN32 > +static HANDLE ms_eventlog = NULL; > + > +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. > + 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? > + > + g_message(PACKAGE_STRING " started on Windows"); > + > /* > * This named mutex will be kept around by Windows until the > * process terminates. This allows other instances to check if it > diff --git a/src/win32-messages.mc b/src/win32-messages.mc > new file mode 100644 > index 0000000..2c283ca > --- /dev/null > +++ b/src/win32-messages.mc > @@ -0,0 +1,36 @@ > +;#ifndef __MESSAGES_H__ > +;#define __MESSAGES_H__ > +; > + > +LanguageNames = > + ( > + English = 0x0409:Messages_ENU > + ) > + > + > +;//////////////////////////////////////// > +;// Eventlog categories > +;// > +;// Categories always have to be the first entries in a message file! > +;// Hmm this is contradicted by the LanguageNames entry just above this... > + > +MessageId = 1 > +SymbolicName = CATEGORY_DUMMY > +Severity = Success > +Language = English > +A dummy category, as a reminder Is this needed at all? Christophe
Attachment:
pgpapYBe1uiAG.pgp
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
