+1

Thanks
Sharad Mishra
Open Virtualization
Linux Technology Center
IBM

libvirt-cim-bounces@redhat.com wrote on 04/07/2011 02:57:28 PM:

> Chip Vincent <cvincent@linux.vnet.ibm.com>

> Sent by: libvirt-cim-bounces@redhat.com
>

> 04/07/2011 02:57 PM
>
> Please respond to
> List for discussion and development of libvirt CIM <libvirt-cim@redhat.com>

>
> To

>
> libvirt-cim@redhat.com

>
> cc

>
> Subject

>
> [Libvirt-cim] [PATCH] (#2) Add support for console/serial grahpics devices

>
> # HG changeset patch
> # User Chip Vincent <cvincent@us.ibm.com>
> # Date 1301520765 14400
> # Node ID 5f1131a99b0846f0e960b4a0f643056b7559fcab
> # Parent  a521a11eeec4b41399ca954ab17b874a708eb4b3
> (#2) Add support for console/serial grahpics devices
>
> Add support for Graphics RASD ResourceSubType = console | serial.
> This includes
> support for allowing more than a single graphics RASD instance. Instances can
> be created externally or during DefineSystem. No changes to current level of
> hotswapping.
>
> Signed-off-by: Chip Vincent <cvincent@us.ibm.com>
>
> diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c
> --- a/libxkutil/device_parsing.c
> +++ b/libxkutil/device_parsing.c
> @@ -40,7 +40,8 @@
>  #define NET_XPATH       (xmlChar *)"/domain/devices/interface"
>  #define EMU_XPATH       (xmlChar *)"/domain/devices/emulator"
>  #define MEM_XPATH       (xmlChar *)"/domain/memory | /domain/currentMemory"
> -#define GRAPHICS_XPATH  (xmlChar *)"/domain/devices/graphics"
> +#define GRAPHICS_XPATH  (xmlChar *)"/domain/devices/graphics | "\
> +        "/domain/devices/console | /domain/devices/serial"
>  #define INPUT_XPATH     (xmlChar *)"/domain/devices/input"
>  
>  #define DEFAULT_BRIDGE "xenbr0"
> @@ -501,6 +502,7 @@
>  {
>          struct virt_device *vdev = NULL;
>          struct graphics_device *gdev = NULL;
> +        xmlNode *child = NULL;
>  
>          vdev = calloc(1, sizeof(*vdev));
>          if (vdev == NULL)
> @@ -509,24 +511,51 @@
>          gdev = &(vdev->dev.graphics);
>  
>          gdev->type = get_attr_value(node, "type");
> -        gdev->port = get_attr_value(node, "port");
> -        gdev->host = get_attr_value(node, "listen");
> -        gdev->keymap = get_attr_value(node, "keymap");
> -
>          if (gdev->type == NULL)
>                  goto err;
>  
> +        CU_DEBUG("graphics device type = %s", gdev->type);
> +
>          if (STREQC(gdev->type, "vnc")) {
> -                if (gdev->port == NULL)
> +                gdev->port = get_attr_value(node, "port");
> +                gdev->host = get_attr_value(node, "listen");
> +                gdev->keymap = get_attr_value(node, "keymap");
> +        
> +                if (gdev->port == NULL || gdev->host == NULL)
> +                        goto err;
> +        }
> +        else if (STREQC(gdev->type, "pty")) {
> +                if (node->name == NULL)
>                          goto err;
>  
> -                if (gdev->host == NULL)
> -                        goto err;
> +                /* Change type to serial, console, etc. It will be converted
> +                 * back in xmlgen.c */
> +                free(gdev->type);
> +                gdev->type = strdup((char *)node->name);
> +
> +                for (child = node->children; child != NULL;
> +                        child = child->next) {
> +                        if (XSTREQ(child->name, "source"))
> +                                gdev->host = get_attr_value(child, "path");
> +                        else if (XSTREQ(child->name, "target"))
> +                                gdev->port = get_attr_value(child, "port");
> +                }
> +        }
> +        else {
> +                CU_DEBUG("Unknown graphics type %s", gdev->type);
> +                goto err;
>          }
>  
>          vdev->type = CIM_RES_TYPE_GRAPHICS;
>          vdev->id = strdup("graphics");
>  
> +        /* FIXME: IDs should be unique, but that breaks existing tests.
> +        ret = asprintf(&vdev->id, "graphics:%s", gdev->type);
> +        if(ret == -1) {
> +                CU_DEBUG("Failed to create graphics is string");
> +                goto err;
> +        } */
> +
>          *vdevs = vdev;
>  
>          return 1;
> diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c
> --- a/libxkutil/xmlgen.c
> +++ b/libxkutil/xmlgen.c
> @@ -401,43 +401,93 @@
>          return NULL;
>  }
>  
> +static const char *graphics_vnc_xml(xmlNodePtr root,
> +                       struct graphics_device *dev)
> +{
> +        xmlNodePtr tmp = NULL;
> +      
> +        tmp = xmlNewChild(root, NULL, BAD_CAST "graphics", NULL);
> +        if (tmp == NULL)
> +                return XML_ERROR;
> +
> +        xmlNewProp(tmp, BAD_CAST "type", BAD_CAST dev->type);
> +
> +        if (STREQC(dev->type, "sdl"))
> +               return NULL;
> +
> +        if (dev->port) {
> +                xmlNewProp(tmp, BAD_CAST "port", BAD_CAST dev->port);
> +                if (STREQC(dev->port, "-1"))
> +                        xmlNewProp(tmp, BAD_CAST "autoport", BAD_CAST "yes");
> +                else
> +                        xmlNewProp(tmp, BAD_CAST "autoport", BAD_CAST "no");
> +        }
> +
> +        if (dev->host)        
> +                xmlNewProp(tmp, BAD_CAST "listen", BAD_CAST dev->host);
> +
> +        if (dev->passwd)
> +                xmlNewProp(tmp, BAD_CAST "passwd", BAD_CAST dev->passwd);
> +
> +        if (dev->keymap)
> +                xmlNewProp(tmp, BAD_CAST "keymap", BAD_CAST dev->keymap);
> +
> +        return NULL;
> +}
> +
> +static const char *graphics_pty_xml(xmlNodePtr root,
> +                       struct graphics_device *dev)
> +{
> +        xmlNodePtr pty = NULL;
> +        xmlNodePtr tmp = NULL;
> +      
> +        pty = xmlNewChild(root, NULL, BAD_CAST dev->type, NULL);
> +        if (pty == NULL)
> +                return XML_ERROR;
> +
> +        xmlNewProp(pty, BAD_CAST "type", BAD_CAST "pty");
> +        
> +        tmp = xmlNewChild(pty, NULL, BAD_CAST "source", NULL);
> +        if (tmp == NULL)
> +                return XML_ERROR;
> +      
> +        if(dev->host)
> +                xmlNewProp(tmp, BAD_CAST "path", BAD_CAST dev->host);
> +      
> +        tmp = xmlNewChild(pty, NULL, BAD_CAST "target", NULL);
> +        if (tmp == NULL)
> +                return XML_ERROR;
> +      
> +        if(dev->port)
> +                xmlNewProp(tmp, BAD_CAST "port", BAD_CAST dev->port);
> +
> +        return NULL;
> +}
> +
>  static const char *graphics_xml(xmlNodePtr root, struct domain *dominfo)
>  {
> +        const char *msg = NULL;
>          int i;
>  
>          for (i = 0; i < dominfo->dev_graphics_ct; i++) {
> -                xmlNodePtr tmp;
>                  struct virt_device *_dev = &dominfo->dev_graphics[i];
>                  if (_dev->type == CIM_RES_TYPE_UNKNOWN)
>                          continue;
>  
>                  struct graphics_device *dev = &_dev->dev.graphics;
>  
> -                tmp = xmlNewChild(root, NULL, BAD_CAST "graphics", NULL);
> -                if (tmp == NULL)
> -                        return XML_ERROR;
> -
> -                xmlNewProp(tmp, BAD_CAST "type", BAD_CAST dev->type);
> -
> -                if (STREQC(dev->type, "sdl"))
> -                        goto out;
> -
> -                if (STREQC(dev->port, "-1"))
> -                        xmlNewProp(tmp, BAD_CAST "autoport", BAD_CAST "yes");
> -                else {
> -                        xmlNewProp(tmp, BAD_CAST "autoport", BAD_CAST "no");
> -                        xmlNewProp(tmp, BAD_CAST "port", BAD_CAST dev->port);
> -                }
> -                xmlNewProp(tmp, BAD_CAST "listen", BAD_CAST dev->host);
> -                xmlNewProp(tmp, BAD_CAST "keymap", BAD_CAST dev->keymap);
> -
> -                if (dev->passwd != NULL)
> -                        xmlNewProp(tmp,
> -                                   BAD_CAST "passwd",
> -                                   BAD_CAST dev->passwd);
> +                if (STREQC(dev->type, "vnc") || STREQC(dev->type, "sdl"))
> +                        msg = graphics_vnc_xml(root, dev);
> +                else if (STREQC(dev->type, "console") ||
> +                        STREQC(dev->type, "serial"))
> +                        msg = graphics_pty_xml(root, dev);
> +                else
> +                        continue;
> +                
> +                if(msg != NULL)
> +                        return msg;
>          }
>  
> - out:
>          return NULL;
>  }
>  
> diff --git a/schema/ResourceAllocationSettingData.mof b/schema/
> ResourceAllocationSettingData.mof
> --- a/schema/ResourceAllocationSettingData.mof
> +++ b/schema/ResourceAllocationSettingData.mof
> @@ -216,8 +216,10 @@
>  ]
>  class Xen_GraphicsResourceAllocationSettingData :
> Xen_ResourceAllocationSettingData
>  {
> -      [Description ("VNC Address. IPv4 in a.b.c.d:port or"
> -       "IPv6 in [ip]:port format")]
> +      [Description ("If ResourceSubType is 'vnc', this is a VNC Address. "
> +       "IPv4 in a.b.c.d:port or IPv6 in [ip]:port format. If
> ResourceSubType "
> +       "is 'console', this is a character device path in "
> +       "path:port format (e.g., '/dev/pts/3:0'\)")]
>        string Address;
>  
>        [Description ("Keyboard keymapping")]
> @@ -235,8 +237,10 @@
>  ]
>  class KVM_GraphicsResourceAllocationSettingData :
> KVM_ResourceAllocationSettingData
>  {
> -      [Description ("VNC Address. IPv4 in a.b.c.d:port or"
> -       "IPv6 in [ip]:port format")]
> +      [Description ("If ResourceSubType is 'vnc', this is a VNC Address. "
> +       "IPv4 in a.b.c.d:port or IPv6 in [ip]:port format. If
> ResourceSubType "
> +       "is 'console', this is a character device path in "
> +       "path:port format (e.g., '/dev/pts/3:0'\)")]
>        string Address;
>  
>        [Description ("Keyboard keymapping")]
> @@ -254,8 +258,10 @@
>  ]
>  class LXC_GraphicsResourceAllocationSettingData :
> LXC_ResourceAllocationSettingData
>  {
> -      [Description ("VNC Address. IPv4 in a.b.c.d:port or"
> -       "IPv6 in [ip]:port format")]
> +      [Description ("If ResourceSubType is 'vnc', this is a VNC Address. "
> +       "IPv4 in a.b.c.d:port or IPv6 in [ip]:port format. If
> ResourceSubType "
> +       "is 'console', this is a character device path in "
> +       "path:port format (e.g., '/dev/pts/3:0'\)")]
>        string Address;
>  
>        [Description ("Keyboard keymapping")]
> diff --git a/src/Virt_Device.c b/src/Virt_Device.c
> --- a/src/Virt_Device.c
> +++ b/src/Virt_Device.c
> @@ -189,14 +189,13 @@
>          int rc;
>          char *vp_str = NULL;
>  
> -        if (STREQC(dev->type, "vnc"))
> +        if (STREQC(dev->type, "sdl"))
> +                rc = asprintf(&vp_str, "%s", dev->type);
> +        else
>                  rc = asprintf(&vp_str, "%s/%s:%s",
>                                dev->type,
>                                dev->host,
>                                dev->port);
> -        else
> -                rc = asprintf(&vp_str, "%s", dev->type);
> -
>          if (rc == -1)
>                  return 0;
>  
> diff --git a/src/Virt_RASD.c b/src/Virt_RASD.c
> --- a/src/Virt_RASD.c
> +++ b/src/Virt_RASD.c
> @@ -421,44 +421,56 @@
>          CMSetProperty(inst, "ResourceSubType",
>                         (CMPIValue *)dev->dev.graphics.type, CMPI_chars);
>          
> -        if (STREQC(dev->dev.graphics.type, "vnc")) {
> +        if (STREQC(dev->dev.graphics.type, "sdl"))
> +                rc = asprintf(&addr_str, "%s", dev->dev.graphics.type);
> +        else {
>                  rc = asprintf(&addr_str,
>                                "%s:%s",
>                                dev->dev.graphics.host,
>                                dev->dev.graphics.port);
> -                if (rc == -1)
> +        }
> +      
> +        CU_DEBUG("graphics Address = %s", addr_str);
> +        
> +        if (rc == -1)
> +                goto out;
> +
> +        CMSetProperty(inst, "Address",
> +                (CMPIValue *)addr_str, CMPI_chars);
> +
> +        if (STREQC(dev->dev.graphics.type, "vnc")) {
> +                CMSetProperty(inst, "KeyMap",
> +                             (CMPIValue *)dev->dev.graphics.keymap,
> CMPI_chars);        
> +                
> +                conn = connect_by_classname(_BROKER, classname, &s);
> +                if (conn == NULL)
>                          goto out;
>  
> -                CMSetProperty(inst, "Address",
> -                              (CMPIValue *)addr_str, CMPI_chars);
> +                dom = virDomainLookupByName(conn, name);
> +                if (dom == NULL) {
> +                        cu_statusf(_BROKER, &s,
> +                                   CMPI_RC_ERR_NOT_FOUND,
> +                                   "Domain %s not found",
> +                                   name);
> +                        goto out;
> +                }
>  
> -                CMSetProperty(inst, "KeyMap",
> -                             (CMPIValue *)dev->dev.graphics.keymap,
> CMPI_chars);
> +                infostore = infostore_open(dom);
> +                if (infostore != NULL)
> +                        has_passwd = infostore_get_bool(infostore,
> +                                "has_vnc_passwd");
> +
> +                if (has_passwd) {
> +                        CU_DEBUG("has password");
> +                        CMSetProperty(inst, "Password",
> +                                      (CMPIValue *)"********", CMPI_chars);
> +                }
> +
> +                infostore_close(infostore);
> +
> +                /* FIXME: Populate the IsIPv6Only */
>          }
>  
> -        conn = connect_by_classname(_BROKER, classname, &s);
> -        if (conn == NULL)
> -                goto out;
> -
> -        dom = virDomainLookupByName(conn, name);
> -        if (dom == NULL) {
> -                cu_statusf(_BROKER, &s,
> -                           CMPI_RC_ERR_NOT_FOUND,
> -                           "Domain %s not found",
> -                           name);
> -                goto out;
> -        }
> -
> -        infostore = infostore_open(dom);
> -        if (infostore != NULL)
> -                has_passwd = infostore_get_bool(infostore, "has_vnc_passwd");
> -
> -        if (has_passwd)
> -                CMSetProperty(inst, "Password",
> -                              (CMPIValue *)"********", CMPI_chars);
> -
> -        infostore_close(infostore);
> -
>   out:
>          free(addr_str);
>          virDomainFree(dom);
> diff --git a/src/Virt_SettingsDefineCapabilities.c b/src/
> Virt_SettingsDefineCapabilities.c
> --- a/src/Virt_SettingsDefineCapabilities.c
> +++ b/src/Virt_SettingsDefineCapabilities.c
> @@ -1694,10 +1694,9 @@
>          inst = sdc_rasd_inst(&s, ref, CIM_RES_TYPE_GRAPHICS, DEVICE_RASD);
>  
>          CMSetProperty(inst, "InstanceID", (CMPIValue *)id, CMPI_chars);
> -
> +        CMSetProperty(inst, "Address", (CMPIValue *)addr, CMPI_chars);
> +        
>          if (STREQC(type, "vnc")) {
> -                CMSetProperty(inst, "Address", (CMPIValue *)addr,
> CMPI_chars);
> -
>                  CMSetProperty(inst, "KeyMap", (CMPIValue *)"en-us",
> CMPI_chars);
>          }
>  
> diff --git a/src/Virt_VirtualSystemManagementService.c b/src/
> Virt_VirtualSystemManagementService.c
> --- a/src/Virt_VirtualSystemManagementService.c
> +++ b/src/Virt_VirtualSystemManagementService.c
> @@ -405,7 +405,7 @@
>  
>  static bool add_default_devs(struct domain *domain)
>  {
> -        if (domain->dev_graphics_ct != 1) {
> +        if (domain->dev_graphics_ct < 1) {
>                  if (!default_graphics_device(domain))        
>                          return false;
>          }
> @@ -1027,6 +1027,38 @@
>          return NULL;
>  }
>  
> +static int parse_console_address(const char *id,
> +                        char **path,
> +                        char **port)
> +{
> +        int ret;
> +        char *tmp_path = NULL;
> +        char *tmp_port = NULL;
> +
> +        CU_DEBUG("Entering parse_console_address, address is %s", id);
> +
> +        ret = sscanf(id, "%a[^:]:%as", &tmp_path, &tmp_port);
> +
> +        if (ret != 2) {
> +                ret = 0;
> +                goto out;
> +        }
> +
> +        if (path)
> +                *path = strdup(tmp_path);
> +
> +        if (port)
> +                *port = strdup(tmp_port);
> +
> +        ret = 1;
> +
> + out:        
> +        CU_DEBUG("Exiting parse_console_address, ip is %s, port is %s",
> +               *path, *port);
> +
> +        return ret;
> +}
> +
>  static int parse_vnc_address(const char *id,
>                        char **ip,
>                        char **port)
> @@ -1058,9 +1090,8 @@
>          ret = 1;
>  
>   out:
> -        CU_DEBUG("Exiting parse_vnc_address, ip is %s, port is %s",
> *ip, *port);
> -        free(tmp_ip);
> -        free(tmp_port);
> +        CU_DEBUG("Exiting parse_vnc_address, ip is %s, port is %s",
> +                *ip, *port);
>  
>          return ret;
>  }
> @@ -1068,9 +1099,8 @@
>  static const char *graphics_rasd_to_vdev(CMPIInstance *inst,
>                                           struct virt_device *dev)
>  {
> -        const char *val;
> +        const char *val = NULL;
>          const char *msg = NULL;
> -        const char *keymap;
>          bool ipv6 = false;
>          int ret;
>  
> @@ -1080,36 +1110,69 @@
>          }
>          dev->dev.graphics.type = strdup(val);
>  
> +        CU_DEBUG("graphics type = %s", dev->dev.graphics.type);
> +
>          /* FIXME: Add logic to prevent address:port collisions */
> -        if (cu_get_str_prop(inst, "Address", &val) != CMPI_RC_OK) {
> -                CU_DEBUG("no graphics port defined, giving default");
> -                if (cu_get_bool_prop(inst, "IsIPv6Only", &ipv6) !=
> CMPI_RC_OK)
> -                        ipv6 = false;
> -                if (ipv6)
> -                        dev->dev.graphics.host = strdup("[::1]");
> -                else
> -                        dev->dev.graphics.host = strdup("127.0.0.1");
> -                dev->dev.graphics.port = strdup("-1");
> -        } else {
> -               ret = parse_vnc_address(val,
> -                              &dev->dev.graphics.host,
> -                              &dev->dev.graphics.port);
> +        if (STREQC(dev->dev.graphics.type, "vnc")) {
> +                if (cu_get_str_prop(inst, "Address", &val) != CMPI_RC_OK) {
> +                        CU_DEBUG("graphics Address empty, using default");
> +
> +                        if (cu_get_bool_prop(inst, "IsIPV6Only", &ipv6) !=
> +                                CMPI_RC_OK)
> +                                ipv6 = false;
> +
> +                        if(ipv6)
> +                                val = "[::1]:-1";
> +                        else
> +                                val = "127.0.0.1:-1";
> +                }
> +
> +                ret = parse_vnc_address(val,
> +                                &dev->dev.graphics.host,
> +                                &dev->dev.graphics.port);
>                  if (ret != 1) {
>                          msg = "GraphicsRASD field Address not valid";
>                          goto out;
>                  }
> +                
> +                if (cu_get_str_prop(inst, "KeyMap", &val) != CMPI_RC_OK)
> +                        dev->dev.graphics.keymap = strdup("en-us");
> +                else
> +                        dev->dev.graphics.keymap = strdup(val);
> +        
> +                if (cu_get_str_prop(inst, "Password", &val) != CMPI_RC_OK) {
> +                        CU_DEBUG("vnc password is not set");
> +                        dev->dev.graphics.passwd = NULL;
> +                } else {
> +                        CU_DEBUG("vnc password is set");
> +                        dev->dev.graphics.passwd = strdup(val);
> +                }
>          }
> -
> -        if (cu_get_str_prop(inst, "KeyMap", &keymap) != CMPI_RC_OK)
> -                keymap = "en-us";
> -        
> -        dev->dev.graphics.keymap = strdup(keymap);
> -
> -        if (cu_get_str_prop(inst, "Password", &val) != CMPI_RC_OK) {
> -                dev->dev.graphics.passwd = NULL;
> -        } else {
> -                dev->dev.graphics.passwd = strdup(val);
> -        }
> +        else if (STREQC(dev->dev.graphics.type, "console") ||
> +                STREQC(dev->dev.graphics.type, "serial")) {
> +                if (cu_get_str_prop(inst, "Address", &val) != CMPI_RC_OK) {
> +                        CU_DEBUG("graphics Address empty, using default");
> +                        val = "/dev/pts/0:0";
> +                }
> +
> +                ret = parse_console_address(val,
> +                                &dev->dev.graphics.host,
> +                                &dev->dev.graphics.port);
> +                if (ret != 1) {
> +                         msg = "GraphicsRASD field Address not valid";
> +                         goto out;
> +                }
> +       } else {
> +                CU_DEBUG("Unsupported graphics type %s",
> +                        dev->dev.graphics.type);
> +                msg = "Unsupported graphics type";
> +                goto out;
> +       }
> +
> +        CU_DEBUG("graphics = %s:%s:%s",
> +                dev->dev.graphics.type,
> +                dev->dev.graphics.host,
> +                dev->dev.graphics.port);
>  
>   out:
>          return msg;
> @@ -1250,6 +1313,9 @@
>                                 "DiskResourceAllocationSettingData
> in a single "
>                                 "guest";
>  
> +                if (dev->type == CIM_RES_TYPE_GRAPHICS)
> +                        continue;
> +
>                  if (STREQC(ptr->id, dev->id)) {
>                          CU_DEBUG("Overriding device %s from
> refconf", ptr->id);
>                          cleanup_virt_device(ptr);
> @@ -1358,11 +1424,19 @@
>                                                         ncount,
>                                                         &domain->dev_net_ct);
>                  } else if (type == CIM_RES_TYPE_GRAPHICS) {
> -                        domain->dev_graphics_ct = 1;
> +                        struct virt_device dev;
> +                        int ncount = count + domain->dev_graphics_ct;
> +
> +                        memset(&dev, 0, sizeof(dev));
>                          msg = rasd_to_vdev(inst,
>                                             domain,
> -                                           &domain->dev_graphics[0],
> +                                           &dev,
>                                             ns);
> +                        if (msg == NULL)
> +                                msg = add_device_nodup(&dev,
> +                                                domain->dev_graphics,
> +                                                ncount,
> +                                                &domain->dev_graphics_ct);
>                  } else if (type == CIM_RES_TYPE_INPUT) {
>                          domain->dev_input_ct = 1;
>                          msg = rasd_to_vdev(inst,
> @@ -2318,13 +2392,6 @@
>                  goto out;
>          }
>  
> -        if ((type == CIM_RES_TYPE_GRAPHICS) && (*count > 0)) {
> -                cu_statusf(_BROKER, &s,
> -                           CMPI_RC_ERR_FAILED,
> -                           "A resource already exists for type %"
> PRIu16, type);
> -                goto out;
> -        }
> -
>          list = realloc(*_list, ((*count)+1)*sizeof(struct virt_device));
>          if (list == NULL) {
>                  /* No memory */
>
> _______________________________________________
> Libvirt-cim mailing list

> Libvirt-cim@redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim