[Libvirt-cim] [PATCH 7/8] VSMS: Support for domains with console devices
John Ferlan
jferlan at redhat.com
Tue Sep 10 22:05:25 UTC 2013
On 09/05/2013 11:36 AM, Viktor Mihajlovski wrote:
> From: Thilo Boehm <tboehm at linux.vnet.ibm.com>
>
> An instance of KVM_ConsoleResourceAllocationSettingData can be added to
> domain specification for VSMS DefineSystem() to define a console for a domain.
> A console definition can not be modified or deleted.
> It only can be added at system definition and deleted at system deletion.
> If a KVM_ConsoleRASD is specified on a system definition,
> no default graphics adapter definition is done.
>
> Signed-off-by: Thilo Boehm <tboehm at linux.vnet.ibm.com>
> Signed-off-by: Viktor Mihajlovski <mihajlov at linux.vnet.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
> ---
> src/Virt_VirtualSystemManagementService.c | 300 +++++++++++++++++++++++++++--
> 1 file changed, 279 insertions(+), 21 deletions(-)
>
> diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c
> index 6629b35..cd6ca9d 100644
> --- a/src/Virt_VirtualSystemManagementService.c
> +++ b/src/Virt_VirtualSystemManagementService.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright IBM Corp. 2007
> + * Copyright IBM Corp. 2007, 2013
> *
> * Authors:
> * Dan Smith <danms at us.ibm.com>
> @@ -626,7 +626,8 @@ static bool default_input_device(struct domain *domain)
>
> static bool add_default_devs(struct domain *domain)
> {
> - if (domain->dev_graphics_ct < 1) {
> + if (domain->dev_graphics_ct < 1 &&
> + domain->dev_console_ct < 1) {
> if (!default_graphics_device(domain))
> return false;
> }
> @@ -1339,47 +1340,284 @@ static int parse_sdl_address(const char *id,
> return ret;
> }
>
> -static int parse_vnc_address(const char *id,
> - char **ip,
> - char **port)
> +static int parse_ip_address(const char *id,
> + char **ip,
> + char **port)
> {
> int ret;
> char *tmp_ip = NULL;
> char *tmp_port = NULL;
>
> - CU_DEBUG("Entering parse_vnc_address, address is %s", id);
> + CU_DEBUG("Entering parse_ip_address, address is %s", id);
> if (strstr(id, "[") != NULL) {
> /* its an ipv6 address */
> ret = sscanf(id, "%a[^]]]:%as", &tmp_ip, &tmp_port);
> + tmp_ip = realloc(tmp_ip, strlen(tmp_ip) + 2);
What if tmp_ip == NULL? This one needs to be checked...
> strcat(tmp_ip, "]");
> } else {
> ret = sscanf(id, "%a[^:]:%as", &tmp_ip, &tmp_port);
> }
>
> - if (ret != 2) {
> + /* ret == 2: address and port, ret == 1: address only */
> + if (ret < 1) {
> ret = 0;
> goto out;
> }
>
> - if (ip)
> + if (ip) {
> *ip = strdup(tmp_ip);
> + CU_DEBUG("IP = '%s'",*ip);
strdup() could return NULL, although I suppose it's only important for
debugging...
> + }
>
> - if (port)
> + if (port && tmp_port) {
> *port = strdup(tmp_port);
> -
> - ret = 1;
> + CU_DEBUG("Port = '%s'",*port);
Again - strdup() could return NULL
> + }
>
> out:
> - if (ip && port)
> - CU_DEBUG("Exiting parse_vnc_address, ip is %s, port is %s",
> - *ip, *port);
> -
> free(tmp_ip);
> free(tmp_port);
>
> return ret;
> }
>
> +static bool parse_console_url(const char *url,
> + char **protocol,
> + char **host,
> + char **port)
> +{
> + bool success = false;
> + char *tmp_protocol = NULL;
> + char *tmp_address = NULL;
> +
> + CU_DEBUG("Entering parse_console_url:'%s'", url);
> +
> + if (sscanf(url,"%a[^:]://%as", &tmp_protocol, &tmp_address) != 2)
> + goto out;
> +
> + if (parse_ip_address(tmp_address, host, port) < 1)
> + goto out;
> +
> + if (protocol) {
> + *protocol = strdup(tmp_protocol);
> + CU_DEBUG("Protocol = '%s'", *protocol);
strdup() failure again
> + }
> +
> + success = true;
> +
> + out:
> + free(tmp_protocol);
> + free(tmp_address);
> +
> + return success;
> +}
> +
> +static const char *_unixsock_console_rasd_to_vdev(CMPIInstance *inst,
> + struct console_device *cdev)
> +{
> + const char *val = NULL;
> + const char *val2 = NULL;
> + char* protocol = NULL;
> +
> + cdev->source_dev.unixsock.mode = NULL;
> + if (cu_get_str_prop(inst,"ConnectURL", &val) == CMPI_RC_OK) {
> + CU_DEBUG("ConnectURL = '%s'", val);
> + cdev->source_dev.unixsock.mode = strdup("connect");
> + }
> +
> + if (cu_get_str_prop(inst, "BindURL", &val2) == CMPI_RC_OK) {
> + if (cdev->source_dev.unixsock.mode != NULL)
> + return "ConsoleRASD: Only ConnectURL or BindURL are allowed for "
> + "UNIX domain socket client/server.";
Not quite sure I understand the above check - are you checking that both
Connect and Bind aren't being used? Then perhaps the message should
state - cannot supply both ConnectURL and BindURL. Of course the above
strdup() could fail, but that's a different issue...
> + CU_DEBUG("BindURL = '%s'", val2);
> + cdev->source_dev.unixsock.mode = strdup("bind");
> + val = val2;
> + }
> +
> + if (val) {
> + if (!parse_console_url(val, &protocol, &cdev->source_dev.unixsock.path, NULL))
> + return "ConsoleRASD: Invalid ConnectURL or BindURL for "
> + "UNIX domain socket client/server.";
> +
> + if (!STREQC("file", protocol)) {
> + CU_DEBUG("Wrong ConsoleRASD protocol specified: '%s'",protocol);
> + free(protocol);
> + return "ConsoleRASD: Protocol 'file' was not specified for "
> + "ConnectURL or BindURL for UNIX domain socket client/server.";
> + }
> + free(protocol);
> + } else
Prefer to see "} else {" as it's cleaner and no one mistakenly adds a
line eventually. BTW: We could get here if strdup() fails...
> + return "ConsoleRASD: ConnectURL or BindURL not specified for "
> + "UNIX domain socket client/server.";
> +
> + return NULL;
> +}
> +
> +static const char *_udp_console_rasd_to_vdev(CMPIInstance *inst,
> + struct console_device *cdev)
> +{
> + const char *val = NULL;
> + char* protocol = NULL;
> +
> + if (cu_get_str_prop(inst, "ConnectURL", &val) != CMPI_RC_OK)
> + return "ConsoleRASD: ConnectURL not specified for UDP network console.";
> +
> + if (!parse_console_url(val, &protocol,
> + &cdev->source_dev.udp.connect_host,
> + &cdev->source_dev.udp.connect_service))
> + return "ConsoleRASD: Invalid ConnectURL specified for UDP network console.";
> +
> + if (!STREQC("udp" ,protocol)) {
> + CU_DEBUG("Wrong ConsoleRASD protocol specified: '%s'",protocol);
> + free(protocol);
> + return "ConsoleRASD: Protocol 'udp' was not specified at "
> + "ConnectURL for UDP network console.";
> + }
> +
> + free(protocol);
> +
> + if (cu_get_str_prop(inst, "BindURL", &val) != CMPI_RC_OK)
> + return "ConsoleRASD: BindURL not specified for UDP network console.";
> +
> + if (!parse_console_url(val,&protocol,
> + &cdev->source_dev.udp.bind_host,
> + &cdev->source_dev.udp.bind_service))
> + return "ConsoleRASD: Invalid BindURL specified for UDP network console.";
> +
> + if (!STREQC("udp", protocol)) {
> + CU_DEBUG("Wrong ConsoleRASD protocol specified: '%s'",protocol);
> + free(protocol);
> + return "ConsoleRASD: Protocol 'udp' was not specified at BindURL "
> + "for UDP network console.";
> + }
> +
> + free(protocol);
> + return NULL;
> +}
> +
> +static const char *_tcp_console_rasd_to_vdev(CMPIInstance *inst,
> + struct console_device *cdev)
> +{
> + const char *val = NULL;
> + const char *val2 = NULL;
> +
> + cdev->source_dev.tcp.mode = NULL;
> + if (cu_get_str_prop(inst, "ConnectURL", &val) == CMPI_RC_OK) {
> + CU_DEBUG("ConnectURL = '%s'",val);
> + cdev->source_dev.tcp.mode = strdup("connect");
> + }
> +
> + if (cu_get_str_prop(inst, "BindURL", &val2) == CMPI_RC_OK) {
> + if (cdev->source_dev.tcp.mode != NULL)
> + return "ConsoleRASD: Only ConnectURL or BindURL are allowed for "
> + "TCP client/server console.";
Similar comment from above - cannot supply both.
> + CU_DEBUG("BindURL = '%s'",val2);
> + cdev->source_dev.tcp.mode = strdup("bind");
> + val = val2;
> + }
> +
> + if (val) {
> + if (!parse_console_url(val,
> + &cdev->source_dev.tcp.protocol,
> + &cdev->source_dev.tcp.host,
> + &cdev->source_dev.tcp.service))
> + return "ConsoleRASD: Invalid ConnectURL or BindURL for "
> + "TCP client/server console.";
> + if (cdev->source_dev.tcp.service == NULL)
> + return "ConsoleRASD: Missing TCP port for TCP client/server console.";
> + } else
Again the "} else {"
> + return "ConsoleRASD: ConnectURL or BindURL not specified for "
> + "TCP client/server console.";
> +
> + if (STREQC("udp",cdev->source_dev.tcp.protocol)) {
> + CU_DEBUG("Wrong ConsoleRASD protocol specified: '%s'", cdev->source_dev.tcp.protocol);
> + return "ConsoleRASD: Invalid protocol 'udp' was specified at "
> + "TCP client/server console.";
> + }
> +
> + if (STREQC("file", cdev->source_dev.tcp.protocol)) {
> + CU_DEBUG("Wrong ConsoleRASD protocol specified: '%s'",cdev->source_dev.tcp.protocol);
> + return "ConsoleRASD: Invalid protocol 'file' was specified at "
> + "TCP client/server console.";
> + }
> +
> + return NULL;
> +}
> +
> +static const char *console_rasd_to_vdev(CMPIInstance *inst,
> + struct virt_device *dev)
> +{
> + int rc = 0;
> + const char *msg = NULL;
> + const char *val = NULL;
> + struct console_device *cdev = &dev->dev.console;
> + uint16_t tmp;
> +
> + rc = cu_get_u16_prop(inst, "SourceType", &tmp);
> + if (rc != CMPI_RC_OK)
> + return "ConsoleRASD: SourceType field not specified.";
> +
> + if (tmp < 0 || tmp >= CIM_CHARDEV_SOURCE_TYPE_INVALIDTYPE)
tmp is a uint16_t so a < 0 comparison is not possible.
> + return "ConsoleRASD: Invalid SourceType value";
> +
> + cdev->source_type = tmp;
> + CU_DEBUG("Processeing SourceType: %d", cdev->source_type);
> +
> + /* property not required */
> + if (cu_get_str_prop(inst, "TargetType", &val) == CMPI_RC_OK)
> + cdev->target_type = strdup(val);
> + CU_DEBUG("TargetType is '%s'", cdev->target_type);
More strdup/debug
Nothing overly serious - let me know how you want to proceed especially
with respect to realloc() failure and unsigned compare less than zero
John
> +
> + switch (cdev->source_type) {
> + case CIM_CHARDEV_SOURCE_TYPE_PTY:
> + /* property not required */
> + if (cu_get_str_prop(inst, "SourcePath", &val) == CMPI_RC_OK)
> + cdev->source_dev.pty.path = strdup(val);
> + break;
> + case CIM_CHARDEV_SOURCE_TYPE_DEV:
> + if (cu_get_str_prop(inst, "SourcePath", &val) != CMPI_RC_OK)
> + return "ConsoleRASD: SourcePath not specified for Host device proxy.";
> + cdev->source_dev.dev.path = strdup(val);
> + break;
> + case CIM_CHARDEV_SOURCE_TYPE_FILE:
> + if (cu_get_str_prop(inst, "SourcePath", &val) != CMPI_RC_OK)
> + return "ConsoleRASD: SourcePath not specified for Device logfile.";
> + cdev->source_dev.file.path = strdup(val);
> + break;
> + case CIM_CHARDEV_SOURCE_TYPE_PIPE:
> + if (cu_get_str_prop(inst, "SourcePath", &val) != CMPI_RC_OK)
> + return "ConsoleRASD: SourcePath not specified for Named pipe.";
> + cdev->source_dev.pipe.path = strdup(val);
> + break;
> + case CIM_CHARDEV_SOURCE_TYPE_UNIXSOCK:
> + msg = _unixsock_console_rasd_to_vdev(inst, cdev);
> + if (msg != NULL)
> + return msg;
> + break;
> + case CIM_CHARDEV_SOURCE_TYPE_UDP:
> + msg = _udp_console_rasd_to_vdev(inst, cdev);
> + if (msg != NULL)
> + return msg;
> + break;
> + case CIM_CHARDEV_SOURCE_TYPE_TCP:
> + msg = _tcp_console_rasd_to_vdev(inst, cdev);
> + if (msg != NULL)
> + return msg;
> + break;
> +
> + default:
> + /* Nothing to do for :
> + CIM_CHARDEV_SOURCE_TYPE_STDIO
> + CIM_CHARDEV_SOURCE_TYPE_NULL
> + CIM_CHARDEV_SOURCE_TYPE_VC
> + CIM_CHARDEV_SOURCE_TYPE_SPICEVMC
> + */
> + break;
> + }
> +
> + return NULL;
> +}
> +
> static const char *graphics_rasd_to_vdev(CMPIInstance *inst,
> struct virt_device *dev)
> {
> @@ -1411,10 +1649,10 @@ static const char *graphics_rasd_to_vdev(CMPIInstance *inst,
> val = "127.0.0.1:-1";
> }
>
> - ret = parse_vnc_address(val,
> + ret = parse_ip_address(val,
> &dev->dev.graphics.dev.vnc.host,
> &dev->dev.graphics.dev.vnc.port);
> - if (ret != 1) {
> + if (ret != 2) {
> msg = "GraphicsRASD field Address not valid";
> goto out;
> }
> @@ -1540,6 +1778,8 @@ static const char *_sysvirt_rasd_to_vdev(CMPIInstance *inst,
> return proc_rasd_to_vdev(inst, dev);
> } else if (type == CIM_RES_TYPE_GRAPHICS) {
> return graphics_rasd_to_vdev(inst, dev);
> + } else if (type == CIM_RES_TYPE_CONSOLE) {
> + return console_rasd_to_vdev(inst, dev);
> } else if (type == CIM_RES_TYPE_INPUT) {
> return input_rasd_to_vdev(inst, dev);
> }
> @@ -1601,7 +1841,7 @@ static const char *rasd_to_vdev(CMPIInstance *inst,
> return msg;
> }
>
> -static char *add_device_nodup(struct virt_device *dev,
> +static const char *add_device_nodup(struct virt_device *dev,
> struct virt_device *list,
> int max,
> int *index)
> @@ -1663,6 +1903,9 @@ static const char *classify_resources(CMPIArray *resources,
> if (!make_space(&domain->dev_graphics, domain->dev_graphics_ct, count))
> return "Failed to alloc graphics list";
>
> + if (!make_space(&domain->dev_console, domain->dev_console_ct, count))
> + return "Failed to alloc console list";
> +
> if (!make_space(&domain->dev_input, domain->dev_input_ct, count))
> return "Failed to alloc input list";
>
> @@ -1765,6 +2008,14 @@ static const char *classify_resources(CMPIArray *resources,
> domain->dev_graphics,
> gcount,
> &domain->dev_graphics_ct);
> + } else if (type == CIM_RES_TYPE_CONSOLE) {
> + msg = rasd_to_vdev(inst,
> + domain,
> + &domain->dev_console[domain->dev_console_ct],
> + ns,
> + p_error);
> + if (msg == NULL)
> + domain->dev_console_ct+=1;
> } else if (type == CIM_RES_TYPE_INPUT) {
> domain->dev_input_ct = 1;
> msg = rasd_to_vdev(inst,
> @@ -2570,6 +2821,9 @@ static struct virt_device **find_list(struct domain *dominfo,
> } else if (type == CIM_RES_TYPE_GRAPHICS) {
> list = &dominfo->dev_graphics;
> *count = &dominfo->dev_graphics_ct;
> + } else if (type == CIM_RES_TYPE_CONSOLE) {
> + list = &dominfo->dev_console;
> + *count = &dominfo->dev_console_ct;
> } else if (type == CIM_RES_TYPE_INPUT) {
> list = &dominfo->dev_input;
> *count = &dominfo->dev_input_ct;
> @@ -2693,7 +2947,8 @@ static CMPIStatus resource_del(struct domain *dominfo,
>
> if (STREQ(dev->id, devid)) {
> if ((type == CIM_RES_TYPE_GRAPHICS) ||
> - (type == CIM_RES_TYPE_INPUT))
> + (type == CIM_RES_TYPE_CONSOLE) ||
> + (type == CIM_RES_TYPE_INPUT))
> cu_statusf(_BROKER, &s, CMPI_RC_OK, "");
> else {
> s = _resource_dynamic(dominfo,
> @@ -2774,7 +3029,9 @@ static CMPIStatus resource_add(struct domain *dominfo,
> goto out;
> }
>
> - if ((type == CIM_RES_TYPE_GRAPHICS) || (type == CIM_RES_TYPE_INPUT)) {
> + if ((type == CIM_RES_TYPE_GRAPHICS) ||
> + (type == CIM_RES_TYPE_INPUT) ||
> + (type == CIM_RES_TYPE_CONSOLE)) {
> (*count)++;
> cu_statusf(_BROKER, &s, CMPI_RC_OK, "");
> goto out;
> @@ -2850,7 +3107,8 @@ static CMPIStatus resource_mod(struct domain *dominfo,
> }
>
> if ((type == CIM_RES_TYPE_GRAPHICS) ||
> - (type == CIM_RES_TYPE_INPUT))
> + (type == CIM_RES_TYPE_INPUT) ||
> + (type == CIM_RES_TYPE_CONSOLE))
> cu_statusf(_BROKER, &s, CMPI_RC_OK, "");
> else {
> #if LIBVIR_VERSION_NUMBER < 9000
>
More information about the Libvirt-cim
mailing list