New subject: [PATCH] (#2) Add support for console/serial grahpics devices

7 Apr 2011

# 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 */

    

[PATCH] (#2) Add support for console/serial grahpics devices

Chip Vincent

Sharad Mishra

tags

participants (2)