Re: [libvirt PATCH 13/16] qemu: store logCtxt in domain private data

[Peter Krempa]

On Wed, Oct 06, 2021 at 09:15:19 +0200, Ján Tomko wrote:
> We might need it later for device hotplug.
> 
> Signed-off-by: Ján Tomko 
> ---
>  src/qemu/qemu_domain.h  | 11 +++
>  src/qemu/qemu_process.c |  5 -
>  2 files changed, 11 insertions(+), 5 deletions(-)

So from the code it looks like we will keep a connection to virtlogd
open for the lifetime of the VM rather than we do now for the startup of
the VM. The commit message doesn't justify or mention this and that is
IMO a substantial change which could have consequences.

Additionally there's no code whic would re-create the context on
libvirtd restart, so even the 'might need it later for device hotplug'
would not work properly.

[libvirt PATCH 13/16] qemu: store logCtxt in domain private data

[Ján Tomko]

We might need it later for device hotplug.

Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_domain.h  | 11 +++
 src/qemu/qemu_process.c |  5 -
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 7e717cf2ad..fef6a2f39f 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -101,6 +101,9 @@ struct _qemuDomainSecretInfo {
 char *ciphertext; /* encoded/encrypted secret */
 };
 
+#define QEMU_TYPE_DOMAIN_LOG_CONTEXT qemu_domain_log_context_get_type()
+G_DECLARE_FINAL_TYPE(qemuDomainLogContext, qemu_domain_log_context, QEMU, 
DOMAIN_LOG_CONTEXT, GObject);
+
 typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate;
 struct _qemuDomainObjPrivate {
 virQEMUDriver *driver;
@@ -193,6 +196,10 @@ struct _qemuDomainObjPrivate {
  * provided by QEMU. */
 virCPUDef *origCPU;
 
+/* Log context. After startup, it is also used for hotplugging devices
+ * with a log file */
+qemuDomainLogContext *logCtxt;
+
 /* If true virtlogd is used as stdio handler for character devices. */
 bool chardevStdioLogd;
 
@@ -418,10 +425,6 @@ struct qemuProcessEvent {
 };
 
 void qemuProcessEventFree(struct qemuProcessEvent *event);
-
-#define QEMU_TYPE_DOMAIN_LOG_CONTEXT qemu_domain_log_context_get_type()
-G_DECLARE_FINAL_TYPE(qemuDomainLogContext, qemu_domain_log_context, QEMU, 
DOMAIN_LOG_CONTEXT, GObject);
-
 typedef struct _qemuDomainSaveCookie qemuDomainSaveCookie;
 struct _qemuDomainSaveCookie {
 virObject parent;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1d0165af6d..4a1fd753ee 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7179,7 +7179,7 @@ qemuProcessLaunch(virConnectPtr conn,
 int ret = -1;
 int rv;
 int logfile = -1;
-g_autoptr(qemuDomainLogContext) logCtxt = NULL;
+qemuDomainLogContext *logCtxt = NULL;
 qemuDomainObjPrivate *priv = vm->privateData;
 g_autoptr(virCommand) cmd = NULL;
 struct qemuProcessHookData hookData;
@@ -7233,6 +7233,7 @@ qemuProcessLaunch(virConnectPtr conn,
 virLastErrorPrefixMessage("%s", _("can't connect to virtlogd"));
 goto cleanup;
 }
+priv->logCtxt = logCtxt;
 logfile = qemuDomainLogContextGetWriteFD(logCtxt);
 
 if (qemuProcessGenID(vm, flags) < 0)
@@ -7966,6 +7967,8 @@ void qemuProcessStop(virQEMUDriver *driver,
 
 qemuDBusStop(driver, vm);
 
+g_clear_object(&priv->logCtxt);
+
 vm->def->id = -1;
 
 /* Wake up anything waiting on domain condition */
-- 
2.31.1