qemuBuildChardevCommand: Split creation of the command and setup of other objects

Completely seprate the creation of the commandline string from the setup
of other objects instantiated on the commandline.

'qemuBuildChardevCommand' will aggregate the setup of individual
parameters such as -add-fd and setup of TLS and the -chardev parameter
itself while the code formatting the commandline will be moved into
qemuBuildChardevStr.

'fdset' names are then stored in qemuDomainChrSourcePrivate.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 26cb25a70cc8cae90fc6d12bc7158566fea1f7cb..910508e725137a47b23679d911549120c1bbc4b7 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1318,16 +1318,15 @@ qemuBuildChrChardevReconnectStr(virBuffer *buf,
 }
 
 
-static int
-qemuBuildChardevCommand(virCommand *cmd,
-                        virQEMUDriverConfig *cfg,
-                        const virDomainChrSourceDef *dev,
-                        const char *charAlias,
-                        virQEMUCaps *qemuCaps)
+static char *
+qemuBuildChardevStr(const virDomainChrSourceDef *dev,
+                    const char *charAlias)
 {
+
     qemuDomainChrSourcePrivate *chrSourcePriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev);
     g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-    bool telnet;
+    const char *path;
+    virTristateSwitch append;
 
     switch ((virDomainChrType) dev->type) {
     case VIR_DOMAIN_CHR_TYPE_NULL:
@@ -1351,27 +1350,19 @@ qemuBuildChardevCommand(virCommand *cmd,
 
     case VIR_DOMAIN_CHR_TYPE_FILE:
         virBufferAsprintf(&buf, "file,id=%s", charAlias);
+        path = dev->data.file.path;
+        append = dev->data.file.append;
 
-        if (chrSourcePriv->fd != -1) {
-            g_autofree char *fdset = NULL;
-            size_t idx;
-
-            virCommandPassFDIndex(cmd, chrSourcePriv->fd,
-                                  VIR_COMMAND_PASS_FD_CLOSE_PARENT, &idx);
-            fdset = qemuBuildFDSet(chrSourcePriv->fd, idx);
-            chrSourcePriv->fd = -1;
-
-            virCommandAddArg(cmd, "-add-fd");
-            virCommandAddArg(cmd, fdset);
+        if (chrSourcePriv->fdset) {
+            path = chrSourcePriv->fdset;
+            append = VIR_TRISTATE_SWITCH_ON;
+        }
 
-            virBufferAsprintf(&buf, ",path=/dev/fdset/%zu,append=on", idx);
-        } else {
-            virBufferAddLit(&buf, ",path=");
-            virQEMUBuildBufferEscapeComma(&buf, dev->data.file.path);
-            if (dev->data.file.append != VIR_TRISTATE_SWITCH_ABSENT) {
-                virBufferAsprintf(&buf, ",append=%s",
-                                  virTristateSwitchTypeToString(dev->data.file.append));
-            }
+        virBufferAddLit(&buf, ",path=");
+        virQEMUBuildBufferEscapeComma(&buf, path);
+        if (append != VIR_TRISTATE_SWITCH_ABSENT) {
+            virBufferAsprintf(&buf, ",append=%s",
+                              virTristateSwitchTypeToString(append));
         }
         break;
 
@@ -1405,14 +1396,16 @@ qemuBuildChardevCommand(virCommand *cmd,
                           bindHost, bindService);
         break;
     }
+
     case VIR_DOMAIN_CHR_TYPE_TCP:
-        telnet = dev->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET;
         virBufferAsprintf(&buf,
-                          "socket,id=%s,host=%s,port=%s%s",
+                          "socket,id=%s,host=%s,port=%s",
                           charAlias,
                           dev->data.tcp.host,
-                          dev->data.tcp.service,
-                          telnet ? ",telnet=on" : "");
+                          dev->data.tcp.service);
+
+        if (dev->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET)
+            virBufferAddLit(&buf, ",telnet=on");
 
         if (dev->data.tcp.listen) {
             virBufferAddLit(&buf, ",server=on");
@@ -1422,6 +1415,77 @@ qemuBuildChardevCommand(virCommand *cmd,
 
         qemuBuildChrChardevReconnectStr(&buf, &dev->data.tcp.reconnect);
 
+        if (chrSourcePriv->tlsCredsAlias)
+            virBufferAsprintf(&buf, ",tls-creds=%s", chrSourcePriv->tlsCredsAlias);
+        break;
+
+    case VIR_DOMAIN_CHR_TYPE_UNIX:
+        virBufferAsprintf(&buf, "socket,id=%s", charAlias);
+        if (chrSourcePriv->passedFD != -1) {
+            virBufferAsprintf(&buf, ",fd=%d", chrSourcePriv->passedFD);
+        } else {
+            virBufferAddLit(&buf, ",path=");
+            virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path);
+        }
+
+        if (dev->data.nix.listen) {
+            virBufferAddLit(&buf, ",server=on");
+            if (!chrSourcePriv->wait)
+                virBufferAddLit(&buf, ",wait=off");
+        }
+
+        qemuBuildChrChardevReconnectStr(&buf, &dev->data.nix.reconnect);
+        break;
+
+    case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
+        virBufferAsprintf(&buf, "spicevmc,id=%s,name=%s", charAlias,
+                          virDomainChrSpicevmcTypeToString(dev->data.spicevmc));
+        break;
+
+    case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
+        virBufferAsprintf(&buf, "spiceport,id=%s,name=%s", charAlias,
+                          dev->data.spiceport.channel);
+        break;
+
+    case VIR_DOMAIN_CHR_TYPE_NMDM:
+    case VIR_DOMAIN_CHR_TYPE_LAST:
+    default:
+        break;
+    }
+
+    if (dev->logfile) {
+        path = dev->logfile;
+        append = dev->logappend;
+
+        if (chrSourcePriv->logFdset) {
+            path = chrSourcePriv->logFdset;
+            append = VIR_TRISTATE_SWITCH_ON;
+        }
+
+        virBufferAddLit(&buf, ",logfile=");
+        virQEMUBuildBufferEscapeComma(&buf, path);
+        if (append != VIR_TRISTATE_SWITCH_ABSENT) {
+            virBufferAsprintf(&buf, ",logappend=%s",
+                              virTristateSwitchTypeToString(append));
+        }
+    }
+
+    return virBufferContentAndReset(&buf);
+}
+
+
+static int
+qemuBuildChardevCommand(virCommand *cmd,
+                        virQEMUDriverConfig *cfg,
+                        const virDomainChrSourceDef *dev,
+                        const char *charAlias,
+                        virQEMUCaps *qemuCaps)
+{
+    qemuDomainChrSourcePrivate *chrSourcePriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev);
+    g_autofree char *charstr = NULL;
+
+    switch ((virDomainChrType) dev->type) {
+    case VIR_DOMAIN_CHR_TYPE_TCP:
         if (dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES) {
             g_autofree char *objalias = NULL;
             const char *tlsCertEncSecAlias = NULL;
@@ -1450,38 +1514,44 @@ qemuBuildChardevCommand(virCommand *cmd,
                 return -1;
             }
 
-            virBufferAsprintf(&buf, ",tls-creds=%s", objalias);
+            chrSourcePriv->tlsCredsAlias = g_steal_pointer(&objalias);
         }
         break;
 
-    case VIR_DOMAIN_CHR_TYPE_UNIX:
-        virBufferAsprintf(&buf, "socket,id=%s", charAlias);
+    case VIR_DOMAIN_CHR_TYPE_FILE:
         if (chrSourcePriv->fd != -1) {
-            virBufferAsprintf(&buf, ",fd=%d", chrSourcePriv->fd);
+            g_autofree char *fdset = NULL;
+            size_t idx;
 
-            virCommandPassFD(cmd, chrSourcePriv->fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+            virCommandPassFDIndex(cmd, chrSourcePriv->fd,
+                                  VIR_COMMAND_PASS_FD_CLOSE_PARENT, &idx);
+            fdset = qemuBuildFDSet(chrSourcePriv->fd, idx);
             chrSourcePriv->fd = -1;
-        } else {
-            virBufferAddLit(&buf, ",path=");
-            virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path);
-        }
-        if (dev->data.nix.listen) {
-            virBufferAddLit(&buf, ",server=on");
-            if (!chrSourcePriv->wait)
-                virBufferAddLit(&buf, ",wait=off");
-        }
 
-        qemuBuildChrChardevReconnectStr(&buf, &dev->data.nix.reconnect);
+            virCommandAddArg(cmd, "-add-fd");
+            virCommandAddArg(cmd, fdset);
+
+            chrSourcePriv->fdset = g_strdup_printf("/dev/fdset/%zu", idx);
+        }
         break;
 
-    case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
-        virBufferAsprintf(&buf, "spicevmc,id=%s,name=%s", charAlias,
-                          virDomainChrSpicevmcTypeToString(dev->data.spicevmc));
+    case VIR_DOMAIN_CHR_TYPE_UNIX:
+        if (chrSourcePriv->fd != -1) {
+            virCommandPassFD(cmd, chrSourcePriv->fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+            chrSourcePriv->passedFD = chrSourcePriv->fd;
+            chrSourcePriv->fd = -1;
+        }
         break;
 
+    case VIR_DOMAIN_CHR_TYPE_NULL:
+    case VIR_DOMAIN_CHR_TYPE_VC:
+    case VIR_DOMAIN_CHR_TYPE_PTY:
+    case VIR_DOMAIN_CHR_TYPE_DEV:
+    case VIR_DOMAIN_CHR_TYPE_PIPE:
+    case VIR_DOMAIN_CHR_TYPE_STDIO:
+    case VIR_DOMAIN_CHR_TYPE_UDP:
+    case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
     case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
-        virBufferAsprintf(&buf, "spiceport,id=%s,name=%s", charAlias,
-                          dev->data.spiceport.channel);
         break;
 
     case VIR_DOMAIN_CHR_TYPE_NMDM:
@@ -1493,31 +1563,25 @@ qemuBuildChardevCommand(virCommand *cmd,
         return -1;
     }
 
-    if (dev->logfile) {
-        if (chrSourcePriv->logfd != -1) {
-            g_autofree char *fdset = NULL;
-            size_t idx;
+    if (chrSourcePriv->logfd != -1) {
+        g_autofree char *fdset = NULL;
+        size_t idx;
 
-            virCommandPassFDIndex(cmd, chrSourcePriv->logfd,
-                                  VIR_COMMAND_PASS_FD_CLOSE_PARENT, &idx);
-            fdset = qemuBuildFDSet(chrSourcePriv->logfd, idx);
-            chrSourcePriv->logfd = -1;
+        virCommandPassFDIndex(cmd, chrSourcePriv->logfd,
+                              VIR_COMMAND_PASS_FD_CLOSE_PARENT, &idx);
+        fdset = qemuBuildFDSet(chrSourcePriv->logfd, idx);
+        chrSourcePriv->logfd = -1;
 
-            virCommandAddArg(cmd, "-add-fd");
-            virCommandAddArg(cmd, fdset);
+        virCommandAddArg(cmd, "-add-fd");
+        virCommandAddArg(cmd, fdset);
 
-            virBufferAsprintf(&buf, ",logfile=/dev/fdset/%zu,logappend=on", idx);
-        } else {
-            virBufferAddLit(&buf, ",logfile=");
-            virQEMUBuildBufferEscapeComma(&buf, dev->logfile);
-            if (dev->logappend != VIR_TRISTATE_SWITCH_ABSENT) {
-                virBufferAsprintf(&buf, ",logappend=%s",
-                                  virTristateSwitchTypeToString(dev->logappend));
-            }
-        }
+        chrSourcePriv->logFdset = g_strdup_printf("/dev/fdset/%zu", idx);
     }
 
-    virCommandAddArgList(cmd, "-chardev", virBufferCurrentContent(&buf), NULL);
+    if (!(charstr = qemuBuildChardevStr(dev, charAlias)))
+        return -1;
+
+    virCommandAddArgList(cmd, "-chardev", charstr, NULL);
 
     return 0;
 }

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e6d6bf10f105fc6da861a7120afda6ec18a8b09d..a2ee160128eb30c992da9141e680a16556564632 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -853,6 +853,8 @@ qemuDomainChrSourcePrivateNew(void)
     priv->fd = -1;
     priv->logfd = -1;
 
+    priv->passedFD = -1;
+
     return (virObject *) priv;
 }
 
@@ -865,6 +867,10 @@ qemuDomainChrSourcePrivateDispose(void *obj)
     VIR_FORCE_CLOSE(priv->fd);
     VIR_FORCE_CLOSE(priv->logfd);
 
+    g_free(priv->fdset);
+    g_free(priv->logFdset);
+    g_free(priv->tlsCredsAlias);
+
     g_clear_pointer(&priv->secinfo, qemuDomainSecretInfoFree);
 }
 

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 2d93cf2253e0cd627d8371fc34e08e6eb4cfe2df..d07def3d8506742415920fe76fde3d813ad6d14a 100644 (file)
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -345,6 +345,11 @@ struct _qemuDomainChrSourcePrivate {
     int fd; /* file descriptor of the chardev source */
     int logfd; /* file descriptor of the logging source */
     bool wait; /* wait for incomming connections on chardev */
+
+    char *fdset; /* fdset path corresponding to the passed filedescriptor */
+    char *logFdset; /* fdset path corresponding to the passed filedescriptor for logfile */
+    int passedFD; /* filedescriptor number when fdset passing it directly */
+    char *tlsCredsAlias; /* alias of the x509 tls credentials object */
 };