[PATCH v2 0/3] chardev handling cleanups (chardev refactors part 1)
All patches except for 6/16 are now pushed. Pathces 1 and 2 in this series are new, split of 6/16 as requested in the review. Patch 3 is the rebase of 6/16 on top of the split out stuff and fix to the [B] point in the original review. Peter Krempa (3): qemuMonitorJSONAttachCharDevGetProps: Rename 'backend_type' and 'data' qemuMonitorJSONBuildChrChardevReconnect: Unify with qemuBuildChrChardevReconnectStr qemuMonitorJSONAttachCharDevGetProps: Modernize construction of JSON objects src/qemu/qemu_monitor_json.c | 192 +++++++++++++++++------------------ 1 file changed, 96 insertions(+), 96 deletions(-) -- 2.31.1
Rename 'data' to 'backendData' so that it's more clear what the object represents and 'backend_type' to 'backendType' to go with the common camel case notation. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 60 ++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 1ced942161..0c66086598 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6614,9 +6614,9 @@ qemuMonitorJSONAttachCharDevGetProps(const char *chrID, { g_autoptr(virJSONValue) props = NULL; g_autoptr(virJSONValue) backend = virJSONValueNewObject(); - g_autoptr(virJSONValue) data = virJSONValueNewObject(); + g_autoptr(virJSONValue) backendData = virJSONValueNewObject(); g_autoptr(virJSONValue) addr = NULL; - const char *backend_type = NULL; + const char *backendType = NULL; const char *host; const char *port; g_autofree char *tlsalias = NULL; @@ -6624,72 +6624,72 @@ qemuMonitorJSONAttachCharDevGetProps(const char *chrID, switch ((virDomainChrType)chr->type) { case VIR_DOMAIN_CHR_TYPE_NULL: - backend_type = "null"; + backendType = "null"; break; case VIR_DOMAIN_CHR_TYPE_VC: - backend_type = "vc"; + backendType = "vc"; break; case VIR_DOMAIN_CHR_TYPE_PTY: - backend_type = "pty"; + backendType = "pty"; break; case VIR_DOMAIN_CHR_TYPE_FILE: - backend_type = "file"; - if (virJSONValueObjectAppendString(data, "out", chr->data.file.path) < 0) + backendType = "file"; + if (virJSONValueObjectAppendString(backendData, "out", chr->data.file.path) < 0) return NULL; - if (virJSONValueObjectAdd(&data, + if (virJSONValueObjectAdd(&backendData, "T:append", chr->data.file.append, NULL) < 0) return NULL; break; case VIR_DOMAIN_CHR_TYPE_DEV: - backend_type = STRPREFIX(chrID, "parallel") ? "parallel" : "serial"; - if (virJSONValueObjectAppendString(data, "device", + backendType = STRPREFIX(chrID, "parallel") ? "parallel" : "serial"; + if (virJSONValueObjectAppendString(backendData, "device", chr->data.file.path) < 0) return NULL; break; case VIR_DOMAIN_CHR_TYPE_TCP: - backend_type = "socket"; + backendType = "socket"; addr = qemuMonitorJSONBuildInetSocketAddress(chr->data.tcp.host, chr->data.tcp.service); if (!addr || - virJSONValueObjectAppend(data, "addr", &addr) < 0) + virJSONValueObjectAppend(backendData, "addr", &addr) < 0) return NULL; telnet = chr->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET; if (chr->data.tcp.listen && - virJSONValueObjectAppendBoolean(data, "wait", false) < 0) + virJSONValueObjectAppendBoolean(backendData, "wait", false) < 0) return NULL; - if (virJSONValueObjectAppendBoolean(data, "telnet", telnet) < 0 || - virJSONValueObjectAppendBoolean(data, "server", chr->data.tcp.listen) < 0) + if (virJSONValueObjectAppendBoolean(backendData, "telnet", telnet) < 0 || + virJSONValueObjectAppendBoolean(backendData, "server", chr->data.tcp.listen) < 0) return NULL; if (chr->data.tcp.tlscreds) { if (!(tlsalias = qemuAliasTLSObjFromSrcAlias(chrID))) return NULL; - if (virJSONValueObjectAppendString(data, "tls-creds", tlsalias) < 0) + if (virJSONValueObjectAppendString(backendData, "tls-creds", tlsalias) < 0) return NULL; } - if (qemuMonitorJSONBuildChrChardevReconnect(data, &chr->data.tcp.reconnect) < 0) + if (qemuMonitorJSONBuildChrChardevReconnect(backendData, &chr->data.tcp.reconnect) < 0) return NULL; break; case VIR_DOMAIN_CHR_TYPE_UDP: - backend_type = "udp"; + backendType = "udp"; host = chr->data.udp.connectHost; if (!host) host = ""; addr = qemuMonitorJSONBuildInetSocketAddress(host, chr->data.udp.connectService); if (!addr || - virJSONValueObjectAppend(data, "remote", &addr) < 0) + virJSONValueObjectAppend(backendData, "remote", &addr) < 0) return NULL; host = chr->data.udp.bindHost; @@ -6701,34 +6701,34 @@ qemuMonitorJSONAttachCharDevGetProps(const char *chrID, port = ""; addr = qemuMonitorJSONBuildInetSocketAddress(host, port); if (!addr || - virJSONValueObjectAppend(data, "local", &addr) < 0) + virJSONValueObjectAppend(backendData, "local", &addr) < 0) return NULL; } break; case VIR_DOMAIN_CHR_TYPE_UNIX: - backend_type = "socket"; + backendType = "socket"; addr = qemuMonitorJSONBuildUnixSocketAddress(chr->data.nix.path); if (!addr || - virJSONValueObjectAppend(data, "addr", &addr) < 0) + virJSONValueObjectAppend(backendData, "addr", &addr) < 0) return NULL; if (chr->data.nix.listen && - virJSONValueObjectAppendBoolean(data, "wait", false) < 0) + virJSONValueObjectAppendBoolean(backendData, "wait", false) < 0) return NULL; - if (virJSONValueObjectAppendBoolean(data, "server", chr->data.nix.listen) < 0) + if (virJSONValueObjectAppendBoolean(backendData, "server", chr->data.nix.listen) < 0) return NULL; - if (qemuMonitorJSONBuildChrChardevReconnect(data, &chr->data.nix.reconnect) < 0) + if (qemuMonitorJSONBuildChrChardevReconnect(backendData, &chr->data.nix.reconnect) < 0) return NULL; break; case VIR_DOMAIN_CHR_TYPE_SPICEVMC: - backend_type = "spicevmc"; + backendType = "spicevmc"; - if (virJSONValueObjectAppendString(data, "type", + if (virJSONValueObjectAppendString(backendData, "type", virDomainChrSpicevmcTypeToString(chr->data.spicevmc)) < 0) return NULL; break; @@ -6749,14 +6749,14 @@ qemuMonitorJSONAttachCharDevGetProps(const char *chrID, } if (chr->logfile && - virJSONValueObjectAdd(&data, + virJSONValueObjectAdd(&backendData, "s:logfile", chr->logfile, "T:logappend", chr->logappend, NULL) < 0) return NULL; - if (virJSONValueObjectAppendString(backend, "type", backend_type) < 0 || - virJSONValueObjectAppend(backend, "data", &data) < 0) + if (virJSONValueObjectAppendString(backend, "type", backendType) < 0 || + virJSONValueObjectAppend(backend, "data", &backendData) < 0) return NULL; if (virJSONValueObjectAdd(&props, -- 2.31.1
When formatting the commandline we explicitly set the reconnect timeout to 0 when it's disabled even when that's the default. Do the same in the monitor/hotplug code. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 0c66086598..508d56994c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6602,10 +6602,22 @@ static int qemuMonitorJSONBuildChrChardevReconnect(virJSONValue *object, const virDomainChrSourceReconnectDef *def) { - if (def->enabled != VIR_TRISTATE_BOOL_YES) + int timeout = 0; + + switch (def->enabled) { + case VIR_TRISTATE_BOOL_ABSENT: + case VIR_TRISTATE_BOOL_LAST: return 0; - return virJSONValueObjectAppendNumberUint(object, "reconnect", def->timeout); + case VIR_TRISTATE_BOOL_YES: + timeout = def->timeout; + break; + + case VIR_TRISTATE_BOOL_NO: + break; + } + + return virJSONValueObjectAppendNumberUint(object, "reconnect", timeout); } static virJSONValue * -- 2.31.1
Use 'virJSONValueObjectAdd' instead of the step-by-step manual JSON object building. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 190 ++++++++++++++++------------------- 1 file changed, 89 insertions(+), 101 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 508d56994c..75b1209cc5 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6598,151 +6598,136 @@ int qemuMonitorJSONGetTPMTypes(qemuMonitor *mon, return qemuMonitorJSONGetStringArray(mon, "query-tpm-types", tpmtypes); } -static int -qemuMonitorJSONBuildChrChardevReconnect(virJSONValue *object, - const virDomainChrSourceReconnectDef *def) -{ - int timeout = 0; - - switch (def->enabled) { - case VIR_TRISTATE_BOOL_ABSENT: - case VIR_TRISTATE_BOOL_LAST: - return 0; - - case VIR_TRISTATE_BOOL_YES: - timeout = def->timeout; - break; - - case VIR_TRISTATE_BOOL_NO: - break; - } - - return virJSONValueObjectAppendNumberUint(object, "reconnect", timeout); -} static virJSONValue * qemuMonitorJSONAttachCharDevGetProps(const char *chrID, const virDomainChrSourceDef *chr) { g_autoptr(virJSONValue) props = NULL; - g_autoptr(virJSONValue) backend = virJSONValueNewObject(); + g_autoptr(virJSONValue) backend = NULL; g_autoptr(virJSONValue) backendData = virJSONValueNewObject(); - g_autoptr(virJSONValue) addr = NULL; const char *backendType = NULL; - const char *host; - const char *port; - g_autofree char *tlsalias = NULL; - bool telnet; switch ((virDomainChrType)chr->type) { case VIR_DOMAIN_CHR_TYPE_NULL: - backendType = "null"; - break; - case VIR_DOMAIN_CHR_TYPE_VC: - backendType = "vc"; - break; - case VIR_DOMAIN_CHR_TYPE_PTY: - backendType = "pty"; + backendType = virDomainChrTypeToString(chr->type); break; case VIR_DOMAIN_CHR_TYPE_FILE: backendType = "file"; - if (virJSONValueObjectAppendString(backendData, "out", chr->data.file.path) < 0) - return NULL; if (virJSONValueObjectAdd(&backendData, + "s:out", chr->data.file.path, "T:append", chr->data.file.append, NULL) < 0) return NULL; + break; case VIR_DOMAIN_CHR_TYPE_DEV: - backendType = STRPREFIX(chrID, "parallel") ? "parallel" : "serial"; - if (virJSONValueObjectAppendString(backendData, "device", - chr->data.file.path) < 0) + if (STRPREFIX(chrID, "parallel")) + backendType = "parallel"; + else + backendType = "serial"; + + if (virJSONValueObjectAdd(&backendData, + "s:device", chr->data.file.path, + NULL) < 0) return NULL; + break; - case VIR_DOMAIN_CHR_TYPE_TCP: + case VIR_DOMAIN_CHR_TYPE_UNIX: + case VIR_DOMAIN_CHR_TYPE_TCP: { + g_autofree char *tlsalias = NULL; + g_autoptr(virJSONValue) addr = NULL; + virTristateBool waitval = VIR_TRISTATE_BOOL_ABSENT; + virTristateBool telnet = VIR_TRISTATE_BOOL_ABSENT; + bool server = false; + int reconnect = -1; + backendType = "socket"; - addr = qemuMonitorJSONBuildInetSocketAddress(chr->data.tcp.host, - chr->data.tcp.service); - if (!addr || - virJSONValueObjectAppend(backendData, "addr", &addr) < 0) - return NULL; - telnet = chr->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET; + if (chr->type == VIR_DOMAIN_CHR_TYPE_TCP) { + telnet = virTristateBoolFromBool(chr->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET); - if (chr->data.tcp.listen && - virJSONValueObjectAppendBoolean(backendData, "wait", false) < 0) - return NULL; + if (chr->data.tcp.listen) { + server = true; + waitval = VIR_TRISTATE_BOOL_NO; + } - if (virJSONValueObjectAppendBoolean(backendData, "telnet", telnet) < 0 || - virJSONValueObjectAppendBoolean(backendData, "server", chr->data.tcp.listen) < 0) - return NULL; - if (chr->data.tcp.tlscreds) { - if (!(tlsalias = qemuAliasTLSObjFromSrcAlias(chrID))) + if (chr->data.tcp.tlscreds && + !(tlsalias = qemuAliasTLSObjFromSrcAlias(chrID))) return NULL; - if (virJSONValueObjectAppendString(backendData, "tls-creds", tlsalias) < 0) + if (!(addr = qemuMonitorJSONBuildInetSocketAddress(chr->data.tcp.host, + chr->data.tcp.service))) return NULL; - } - if (qemuMonitorJSONBuildChrChardevReconnect(backendData, &chr->data.tcp.reconnect) < 0) - return NULL; - break; - - case VIR_DOMAIN_CHR_TYPE_UDP: - backendType = "udp"; - host = chr->data.udp.connectHost; - if (!host) - host = ""; - addr = qemuMonitorJSONBuildInetSocketAddress(host, - chr->data.udp.connectService); - if (!addr || - virJSONValueObjectAppend(backendData, "remote", &addr) < 0) - return NULL; + if (chr->data.tcp.reconnect.enabled == VIR_TRISTATE_BOOL_YES) + reconnect = chr->data.tcp.reconnect.timeout; + else if (chr->data.tcp.reconnect.enabled == VIR_TRISTATE_BOOL_NO) + reconnect = 0; + } else { + if (chr->data.nix.listen) { + server = true; + waitval = VIR_TRISTATE_BOOL_NO; + } - host = chr->data.udp.bindHost; - port = chr->data.udp.bindService; - if (host || port) { - if (!host) - host = ""; - if (!port) - port = ""; - addr = qemuMonitorJSONBuildInetSocketAddress(host, port); - if (!addr || - virJSONValueObjectAppend(backendData, "local", &addr) < 0) + if (!(addr = qemuMonitorJSONBuildUnixSocketAddress(chr->data.nix.path))) return NULL; + + if (chr->data.nix.reconnect.enabled == VIR_TRISTATE_BOOL_YES) + reconnect = chr->data.tcp.reconnect.timeout; + else if (chr->data.nix.reconnect.enabled == VIR_TRISTATE_BOOL_NO) + reconnect = 0; } + + if (virJSONValueObjectAdd(&backendData, + "a:addr", &addr, + "T:wait", waitval, + "T:telnet", telnet, + "b:server", server, + "S:tls-creds", tlsalias, + "k:reconnect", reconnect, + NULL) < 0) + return NULL; + } break; - case VIR_DOMAIN_CHR_TYPE_UNIX: - backendType = "socket"; - addr = qemuMonitorJSONBuildUnixSocketAddress(chr->data.nix.path); + case VIR_DOMAIN_CHR_TYPE_UDP: { + g_autoptr(virJSONValue) local = NULL; + g_autoptr(virJSONValue) remote = NULL; - if (!addr || - virJSONValueObjectAppend(backendData, "addr", &addr) < 0) - return NULL; + backendType = "udp"; - if (chr->data.nix.listen && - virJSONValueObjectAppendBoolean(backendData, "wait", false) < 0) + if (!(remote = qemuMonitorJSONBuildInetSocketAddress(NULLSTR_EMPTY(chr->data.udp.connectHost), + chr->data.udp.connectService))) return NULL; - if (virJSONValueObjectAppendBoolean(backendData, "server", chr->data.nix.listen) < 0) - return NULL; + if (chr->data.udp.bindHost || chr->data.udp.bindService) { + if (!(local = qemuMonitorJSONBuildInetSocketAddress(NULLSTR_EMPTY(chr->data.udp.bindHost), + NULLSTR_EMPTY(chr->data.udp.bindService)))) + return NULL; + } - if (qemuMonitorJSONBuildChrChardevReconnect(backendData, &chr->data.nix.reconnect) < 0) + if (virJSONValueObjectAdd(&backendData, + "a:remote", &remote, + "A:local", &local, + NULL) < 0) return NULL; + } break; case VIR_DOMAIN_CHR_TYPE_SPICEVMC: backendType = "spicevmc"; - if (virJSONValueObjectAppendString(backendData, "type", - virDomainChrSpicevmcTypeToString(chr->data.spicevmc)) < 0) + if (virJSONValueObjectAdd(&backendData, + "s:type", virDomainChrSpicevmcTypeToString(chr->data.spicevmc), + NULL) < 0) return NULL; + break; case VIR_DOMAIN_CHR_TYPE_SPICEPORT: @@ -6760,15 +6745,18 @@ qemuMonitorJSONAttachCharDevGetProps(const char *chrID, return NULL; } - if (chr->logfile && - virJSONValueObjectAdd(&backendData, - "s:logfile", chr->logfile, - "T:logappend", chr->logappend, - NULL) < 0) - return NULL; + if (chr->logfile) { + if (virJSONValueObjectAdd(&backendData, + "s:logfile", chr->logfile, + "T:logappend", chr->logappend, + NULL) < 0) + return NULL; + } - if (virJSONValueObjectAppendString(backend, "type", backendType) < 0 || - virJSONValueObjectAppend(backend, "data", &backendData) < 0) + if (virJSONValueObjectAdd(&backend, + "s:type", backendType, + "A:data", &backendData, + NULL) < 0) return NULL; if (virJSONValueObjectAdd(&props, -- 2.31.1
On a Friday in 2021, Peter Krempa wrote:
All patches except for 6/16 are now pushed.
Pathces 1 and 2 in this series are new, split of 6/16 as requested in the review.
Patch 3 is the rebase of 6/16 on top of the split out stuff and fix to the [B] point in the original review.
Peter Krempa (3): qemuMonitorJSONAttachCharDevGetProps: Rename 'backend_type' and 'data' qemuMonitorJSONBuildChrChardevReconnect: Unify with qemuBuildChrChardevReconnectStr qemuMonitorJSONAttachCharDevGetProps: Modernize construction of JSON objects
src/qemu/qemu_monitor_json.c | 192 +++++++++++++++++------------------ 1 file changed, 96 insertions(+), 96 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa