On Thu, Jun 06, 2013 at 14:30:01 +0200, Michal Privoznik wrote:
> The function being introduced is responsible for preparing and
> executing 'chardev-add' qemu monitor command. Moreover, in case
> of PTY chardev, the corresponding pty path is updated.
> ---
> src/qemu/qemu_monitor.c | 21 +++++
> src/qemu/qemu_monitor.h | 3 +
> src/qemu/qemu_monitor_json.c | 189 +++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor_json.h | 3 +
> 4 files changed, 216 insertions(+)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index ad326c5..d11b97e 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -3626,3 +3626,24 @@ int qemuMonitorGetTPMTypes(qemuMonitorPtr mon,
>
> return qemuMonitorJSONGetTPMTypes(mon, tpmtypes);
> }
> +
> +int qemuMonitorAttachCharDev(qemuMonitorPtr mon,
> + const char *chrID,
> + virDomainChrSourceDefPtr chr)
> +{
> + VIR_DEBUG("mon=%p chrID=%s chr=%p", mon, chrID, chr);
> +
> + if (!mon) {
> + virReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("monitor must not be NULL"));
> + return -1;
> + }
> +
> + if (!mon->json) {
> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> + _("JSON monitor is required"));
> + return -1;
> + }
> +
> + return qemuMonitorJSONAttachCharDev(mon, chrID, chr);
> +}
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index a607712..3e7b4cb 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -696,6 +696,9 @@ int qemuMonitorGetTPMModels(qemuMonitorPtr mon,
> int qemuMonitorGetTPMTypes(qemuMonitorPtr mon,
> char ***tpmtypes);
>
> +int qemuMonitorAttachCharDev(qemuMonitorPtr mon,
> + const char *chrID,
> + virDomainChrSourceDefPtr chr);
> /**
> * When running two dd process and using <> redirection, we need a
> * shell that will not truncate files. These two strings serve that
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index b87b8b5..0399963 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -4760,6 +4760,30 @@ error:
> return NULL;
> }
>
> +static virJSONValuePtr
> +qemuMonitorJSONBuildUnixSocketAddress(const char *path)
> +{
> + virJSONValuePtr addr = NULL;
> + virJSONValuePtr data = NULL;
> +
> + if (!(data = virJSONValueNewObject()) ||
> + !(addr = virJSONValueNewObject()))
> + goto error;
> +
> + /* port is really expected as a string here by qemu */
That's a bit surprising when unix sockets have no ports :-)
> + if (virJSONValueObjectAppendString(data, "path", path) < 0 ||
> + virJSONValueObjectAppendString(addr, "type", "unix") < 0 ||
> + virJSONValueObjectAppend(addr, "data", data) < 0)
> + goto error;
> +
> + return addr;
> +error:
> + virReportOOMError();
> + virJSONValueFree(data);
> + virJSONValueFree(addr);
> + return NULL;
> +}
> +
> int
> qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon,
> const char *host,
> @@ -4939,3 +4963,168 @@ int qemuMonitorJSONGetTPMTypes(qemuMonitorPtr mon,
> {
> return qemuMonitorJSONGetStringArray(mon, "query-tpm-types", tpmtypes);
> }
> +
> +static virJSONValuePtr
> +qemuMonitorJSONAttachCharDevCommand(const char *chrID,
> + const virDomainChrSourceDefPtr chr)
> +{
> + virJSONValuePtr ret;
> + virJSONValuePtr backend;
> + virJSONValuePtr data = NULL;
> + virJSONValuePtr addr = NULL;
> + const char *backend_type;
> + bool telnet;
> +
> + if (!(backend = virJSONValueNewObject()) ||
> + !(data = virJSONValueNewObject())) {
> + goto no_memory;
> + }
> +
> + switch ((enum virDomainChrType) chr->type) {
> + case VIR_DOMAIN_CHR_TYPE_NULL:
> + case VIR_DOMAIN_CHR_TYPE_VC:
> + backend_type = "null";
To avoid adding an empty data object, I'd add:
virJSONValueFree(data);
data = NULL;
> + break;
> +
> + case VIR_DOMAIN_CHR_TYPE_PTY:
> + backend_type = "pty";
and here as well:
virJSONValueFree(data);
data = NULL;
> + break;
> +
> + case VIR_DOMAIN_CHR_TYPE_FILE:
> + backend_type = "file";
> + if (virJSONValueObjectAppendString(data, "out", chr->data.file.path) < 0)
> + goto no_memory;
> + break;
> +
> + case VIR_DOMAIN_CHR_TYPE_DEV:
> + backend_type = STRPREFIX(chrID, "parallel") ? "parallel" : "serial";
> + if (virJSONValueObjectAppendString(data, "device",
> + chr->data.file.path) < 0)
> + goto no_memory;
> + break;
> +
> + case VIR_DOMAIN_CHR_TYPE_TCP:
> + backend_type = "socket";
> + addr = qemuMonitorJSONBuildInetSocketAddress(chr->data.tcp.host,
> + chr->data.tcp.service);
> + if (!addr ||
> + virJSONValueObjectAppend(data, "addr", addr) < 0)
> + goto no_memory;
> + addr = NULL;
> +
> + telnet = chr->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET;
> +
> + if (virJSONValueObjectAppendBoolean(data, "wait", false) < 0 ||
> + virJSONValueObjectAppendBoolean(data, "telnet", telnet) < 0 ||
> + virJSONValueObjectAppendBoolean(data, "server", chr->data.tcp.listen) < 0)
> + goto no_memory;
> + break;
> +
> + case VIR_DOMAIN_CHR_TYPE_UDP:
> + backend_type = "socket";
> + addr = qemuMonitorJSONBuildInetSocketAddress(chr->data.udp.connectHost,
> + chr->data.udp.connectService);
> + if (!addr ||
> + virJSONValueObjectAppend(data, "addr", addr) < 0)
> + goto no_memory;
> + addr = NULL;
> + break;
> +
> + case VIR_DOMAIN_CHR_TYPE_UNIX:
> + backend_type = "socket";
> + addr = qemuMonitorJSONBuildUnixSocketAddress(chr->data.nix.path);
> +
> + if (!addr ||
> + virJSONValueObjectAppend(data, "addr", addr) < 0)
> + goto no_memory;
> + addr = NULL;
> +
> + if (virJSONValueObjectAppendBoolean(data, "wait", false) < 0 ||
> + virJSONValueObjectAppendBoolean(data, "server", chr->data.nix.listen) < 0)
> + goto no_memory;
> + break;
> +
> + case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
> + case VIR_DOMAIN_CHR_TYPE_PIPE:
> + case VIR_DOMAIN_CHR_TYPE_STDIO:
> + case VIR_DOMAIN_CHR_TYPE_LAST:
> + default:
Remove the "default:" line.
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("Unsupported char device type '%d'"),
> + chr->type);
> + goto error;
> + }
> +
> + if (virJSONValueObjectAppendString(backend, "type", backend_type) < 0 ||
> + virJSONValueObjectAppend(backend, "data", data) < 0)
Only append the data object if it's non-NULL (after freeing it when it's
not required).
> + goto no_memory;
> + data = NULL;
> +
> + if (!(ret = qemuMonitorJSONMakeCommand("chardev-add",
> + "s:id", chrID,
> + "a:backend", backend,
> + NULL)))
> + goto error;
> +
> + return ret;
> +
> +no_memory:
> + virReportOOMError();
> +error:
> + virJSONValueFree(addr);
> + virJSONValueFree(data);
> + virJSONValueFree(backend);
> + return NULL;
> +}
> +
> +static int
> +qemuMonitorJSONAttachCharDevInfo(virJSONValuePtr reply,
> + virDomainChrSourceDefPtr chr)
The name of this function is quite confusing since it doesn't really
attach anything, it just parses the result of previous chardev-add
command. I think it could actually be inlined in
qemuMonitorJSONAttachCharDev to avoid the need for coming up with a
better name :-)
> +{
> + virJSONValuePtr data;
> + const char *path;
> +
> + if (chr->type != VIR_DOMAIN_CHR_TYPE_PTY)
> + return 0;
> +
> + if (!(data = virJSONValueObjectGet(reply, "return"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("chardev-add reply was missing return data"));
> + return -1;
> + }
> +
> + if (!(path = virJSONValueObjectGetString(data, "pty"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("chardev-add reply was missing pty path"));
> + return -1;
> + }
> +
> + if (VIR_STRDUP(chr->data.file.path, path) < 0)
> + return -1;
> + return 0;
> +}
> +
> +int
> +qemuMonitorJSONAttachCharDev(qemuMonitorPtr mon,
> + const char *chrID,
> + virDomainChrSourceDefPtr chr)
> +{
> + int ret = -1;
> + virJSONValuePtr cmd;
> + virJSONValuePtr reply = NULL;
> +
> + if (!(cmd = qemuMonitorJSONAttachCharDevCommand(chrID, chr)))
> + return ret;
> +
> + ret = qemuMonitorJSONCommand(mon, cmd, &reply);
> +
> + if (ret == 0)
> + ret = qemuMonitorJSONCheckError(cmd, reply);
> +
> + if (ret == 0)
> + ret = qemuMonitorJSONAttachCharDevInfo(reply, chr);
> +
> + virJSONValueFree(cmd);
> + virJSONValueFree(reply);
> + return ret;
> +}
ACK with the small issues addressed.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list