1. Patchew
  2. Libvirt
  3. View series

[PATCH] domain_logcontext: Don't assume remote driver is always available

Michal Privoznik via Devel posted 1 patch 2 weeks, 3 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/4c917e91cb4524ccad87e536b672b8527d5ddcd8.1768485809.git.mprivozn@redhat.com
There is a newer version of this series
src/hypervisor/domain_logcontext.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
[PATCH] domain_logcontext: Don't assume remote driver is always available
Posted by Michal Privoznik via Devel 2 weeks, 3 days ago 
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
Re: [PATCH] domain_logcontext: Don't assume remote driver is always available
Posted by Daniel P. Berrangé via Devel 2 weeks, 3 days ago 
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 :|
Re: [PATCH] domain_logcontext: Don't assume remote driver is always available
Posted by Michal Prívozník via Devel 2 weeks, 3 days ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.