[PATCH] domain_logcontext: Don't assume remote driver is always available
From: Michal Privoznik <mprivozn@redhat.com> Some functions inside of domain_logcontext call virLogManager APIs. But that one is available only when remote driver is enabled (from src/logging/meson.build): if conf.has('WITH_REMOTE') log_driver_lib = static_library( 'virt_log_driver', [ log_driver_sources, log_protocol_generated, ], ... Guard calls to virLogManager APIs with #ifdef WITH_REMOTE. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/842 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/hypervisor/domain_logcontext.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/hypervisor/domain_logcontext.c b/src/hypervisor/domain_logcontext.c index 41d1bbdf64..92f942eb81 100644 --- a/src/hypervisor/domain_logcontext.c +++ b/src/hypervisor/domain_logcontext.c @@ -71,7 +71,9 @@ domainLogContextFinalize(GObject *object) domainLogContext *ctxt = DOMAIN_LOG_CONTEXT(object); VIR_DEBUG("ctxt=%p", ctxt); +#ifdef WITH_REMOTE virLogManagerFree(ctxt->manager); +#endif VIR_FREE(ctxt->path); VIR_FORCE_CLOSE(ctxt->writefd); VIR_FORCE_CLOSE(ctxt->readfd); @@ -82,8 +84,8 @@ domainLogContextFinalize(GObject *object) domainLogContext * domainLogContextNew(bool stdioLogD, char *logDir, - const char *driver_name, - virDomainObj *vm, + const char *driver_name G_GNUC_UNUSED, + virDomainObj *vm G_GNUC_UNUSED, bool privileged, const char *basename) { @@ -96,6 +98,7 @@ domainLogContextNew(bool stdioLogD, ctxt->path = g_strdup_printf("%s/%s.log", logDir, basename); if (stdioLogD) { +#ifdef WITH_REMOTE ctxt->manager = virLogManagerNew(privileged); if (!ctxt->manager) goto error; @@ -110,6 +113,11 @@ domainLogContextNew(bool stdioLogD, &ctxt->pos); if (ctxt->writefd < 0) goto error; +#else /* !WITH_REMOTE */ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("logd stdio handler is not supported")); + goto error; +#endif /* !WITH_REMOTE */ } else { if ((ctxt->writefd = open(ctxt->path, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR)) < 0) { virReportSystemError(errno, _("failed to create logfile %1$s"), @@ -202,6 +210,7 @@ domainLogContextRead(domainLogContext *ctxt, (unsigned long long)ctxt->pos); if (ctxt->manager) { +#ifdef WITH_REMOTE buf = virLogManagerDomainReadLogFile(ctxt->manager, ctxt->path, ctxt->inode, @@ -211,6 +220,11 @@ domainLogContextRead(domainLogContext *ctxt, if (!buf) return -1; buflen = strlen(buf); +#else /* !WITH_REMOTE */ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("logd stdio handler is not supported")); + return -1; +#endif /* !WITH_REMOTE */ } else { ssize_t got; @@ -315,14 +329,17 @@ domainLogContextGetWriteFD(domainLogContext *ctxt) void domainLogContextMarkPosition(domainLogContext *ctxt) { - if (ctxt->manager) + if (ctxt->manager) { +#ifdef WITH_REMOTE virLogManagerDomainGetLogFilePosition(ctxt->manager, ctxt->path, 0, &ctxt->inode, &ctxt->pos); - else +#endif /* WITH_REMOTE */ + } else { ctxt->pos = lseek(ctxt->writefd, 0, SEEK_END); + } } -- 2.52.0
On Thu, Jan 15, 2026 at 03:03:29PM +0100, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
Some functions inside of domain_logcontext call virLogManager APIs. But that one is available only when remote driver is enabled (from src/logging/meson.build):
if conf.has('WITH_REMOTE') log_driver_lib = static_library( 'virt_log_driver', [ log_driver_sources, log_protocol_generated, ], ...
Guard calls to virLogManager APIs with #ifdef WITH_REMOTE.
Should we #ifdef the entire file ? It seems like its functionality is only going to be applicable to server side drivers.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/842 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/hypervisor/domain_logcontext.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/src/hypervisor/domain_logcontext.c b/src/hypervisor/domain_logcontext.c index 41d1bbdf64..92f942eb81 100644 --- a/src/hypervisor/domain_logcontext.c +++ b/src/hypervisor/domain_logcontext.c @@ -71,7 +71,9 @@ domainLogContextFinalize(GObject *object) domainLogContext *ctxt = DOMAIN_LOG_CONTEXT(object); VIR_DEBUG("ctxt=%p", ctxt);
+#ifdef WITH_REMOTE virLogManagerFree(ctxt->manager); +#endif VIR_FREE(ctxt->path); VIR_FORCE_CLOSE(ctxt->writefd); VIR_FORCE_CLOSE(ctxt->readfd); @@ -82,8 +84,8 @@ domainLogContextFinalize(GObject *object) domainLogContext * domainLogContextNew(bool stdioLogD, char *logDir, - const char *driver_name, - virDomainObj *vm, + const char *driver_name G_GNUC_UNUSED, + virDomainObj *vm G_GNUC_UNUSED, bool privileged, const char *basename) { @@ -96,6 +98,7 @@ domainLogContextNew(bool stdioLogD, ctxt->path = g_strdup_printf("%s/%s.log", logDir, basename);
if (stdioLogD) { +#ifdef WITH_REMOTE ctxt->manager = virLogManagerNew(privileged); if (!ctxt->manager) goto error; @@ -110,6 +113,11 @@ domainLogContextNew(bool stdioLogD, &ctxt->pos); if (ctxt->writefd < 0) goto error; +#else /* !WITH_REMOTE */ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("logd stdio handler is not supported")); + goto error; +#endif /* !WITH_REMOTE */ } else { if ((ctxt->writefd = open(ctxt->path, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR)) < 0) { virReportSystemError(errno, _("failed to create logfile %1$s"), @@ -202,6 +210,7 @@ domainLogContextRead(domainLogContext *ctxt, (unsigned long long)ctxt->pos);
if (ctxt->manager) { +#ifdef WITH_REMOTE buf = virLogManagerDomainReadLogFile(ctxt->manager, ctxt->path, ctxt->inode, @@ -211,6 +220,11 @@ domainLogContextRead(domainLogContext *ctxt, if (!buf) return -1; buflen = strlen(buf); +#else /* !WITH_REMOTE */ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("logd stdio handler is not supported")); + return -1; +#endif /* !WITH_REMOTE */ } else { ssize_t got;
@@ -315,14 +329,17 @@ domainLogContextGetWriteFD(domainLogContext *ctxt) void domainLogContextMarkPosition(domainLogContext *ctxt) { - if (ctxt->manager) + if (ctxt->manager) { +#ifdef WITH_REMOTE virLogManagerDomainGetLogFilePosition(ctxt->manager, ctxt->path, 0, &ctxt->inode, &ctxt->pos); - else +#endif /* WITH_REMOTE */ + } else { ctxt->pos = lseek(ctxt->writefd, 0, SEEK_END); + } }
-- 2.52.0
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 1/15/26 15:09, Daniel P. Berrangé wrote:
On Thu, Jan 15, 2026 at 03:03:29PM +0100, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
Some functions inside of domain_logcontext call virLogManager APIs. But that one is available only when remote driver is enabled (from src/logging/meson.build):
if conf.has('WITH_REMOTE') log_driver_lib = static_library( 'virt_log_driver', [ log_driver_sources, log_protocol_generated, ], ...
Guard calls to virLogManager APIs with #ifdef WITH_REMOTE.
Should we #ifdef the entire file ? It seems like its functionality is only going to be applicable to server side drivers.
Fair enough. The old log handler is still available, but no driver that cares is compiled when remote driver is disabled. V2 on its way. Michal
participants (3)
-
Daniel P. Berrangé -
Michal Privoznik -
Michal Prívozník