9:35 a.m.
# HG changeset patch
# User Chip Vincent <cvincent(a)us.ibm.com>
# Date 1301520765 14400
# Node ID 18793660dbc8f0755062a7f90902379288501f4c
# Parent a521a11eeec4b41399ca954ab17b874a708eb4b3
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(a)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,92 @@
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)
@@ -1059,8 +1091,6 @@
out:
CU_DEBUG("Exiting parse_vnc_address, ip is %s, port is %s", *ip,
*port);
- free(tmp_ip);
- free(tmp_port);
return ret;
}
@@ -1068,9 +1098,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 +1109,67 @@
}
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 +1310,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 +1421,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 +2389,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 */
2:19 p.m.
New subject: [PATCH] Add support for console/serial grahpics devices
Chip,
My comments are inline below at 2 places.
There are few lines in this patch that are longer than 80 characters,
please correct them.
Thanks,
Sharad Mishra
Open Virtualization
Linux Technology Center
IBM
libvirt-cim-bounces(a)redhat.com wrote on 04/05/2011 07:35:51 AM:
Chip Vincent <cvincent(a)linux.vnet.ibm.com>
Sent by: libvirt-cim-bounces(a)redhat.com
04/05/2011 07:35 AM
Please respond to
List for discussion and development of libvirt CIM
<libvirt-cim(a)redhat.com>
To
libvirt-cim(a)redhat.com
cc
Subject
[Libvirt-cim] [PATCH] Add support for console/serial grahpics devices
# HG changeset patch
# User Chip Vincent <cvincent(a)us.ibm.com>
# Date 1301520765 14400
# Node ID 18793660dbc8f0755062a7f90902379288501f4c
# Parent a521a11eeec4b41399ca954ab17b874a708eb4b3
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(a)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,92 @@
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");
Password is being set to "*****". According to libvirt.org, password
should appear in clear text.
I am not seeing password attribute in libvirt xml even when password is
being passed to libvirt-cim. Here is the libvirt xml generated -
<graphics type='vnc' port='5910' autoport='no'
listen='127.0.0.1'
keymap='en-us'/>
I was expecting something like this -
<graphics type="vnc" port="5910" autoport="no"
listen="127.0.0.1"
passwd="test0all" keymap="en-us"/>
Libvirt-cim log indicates that code knew about password but lost it
somewhere -
device_parsing.c(517): graphics device type = vnc
Virt_RASD.c(433): graphics Address = 127.0.0.1:5910
misc_util.c(75): Connecting to libvirt with uri `qemu:///system'
infostore.c(89): Path is /etc/libvirt/cim/QEMU_test_kvmredsap_dom
Virt_RASD.c(464): 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)
@@ -1059,8 +1091,6 @@
out:
CU_DEBUG("Exiting parse_vnc_address, ip is %s, port is %s",
*ip, *port);
- free(tmp_ip);
- free(tmp_port);
return ret;
}
@@ -1068,9 +1098,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 +1109,67 @@
}
dev->dev.graphics.type = strdup(val);
+ CU_DEBUG("graphics type = %s", dev->dev.graphics.type);
+
If graphics type is "sdl", then the logic below leads to error message
that "sdl" is unsupported
/* 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 +1310,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 +1421,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 +2389,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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-cim
7:46 p.m.
New subject: [PATCH] Add support for console/serial grahpics devices
Thanks for the comments. My responses below.
On 04/06/2011 03:19 PM, Sharad Mishra wrote:
Chip,
My comments are inline below at 2 places.
There are few lines in this patch that are longer than 80 characters,
please correct them.
Thanks,
Sharad Mishra
Open Virtualization
Linux Technology Center
IBM
libvirt-cim-bounces(a)redhat.com wrote on 04/05/2011 07:35:51 AM:
> Chip Vincent <cvincent(a)linux.vnet.ibm.com>
> Sent by: libvirt-cim-bounces(a)redhat.com
>
> + infostore = infostore_open(dom);
> + if (infostore != NULL)
> + has_passwd = infostore_get_bool(infostore,
> + "has_vnc_passwd");
> +
> + if (has_passwd) {
> + CU_DEBUG("has password");
Password is being set to "*****". According to libvirt.org, password
should appear in clear text.
I am not seeing password attribute in libvirt xml even when password is
being passed to libvirt-cim. Here is the libvirt xml generated -
<graphics type='vnc' port='5910' autoport='no'
listen='127.0.0.1'
keymap='en-us'/>
I was expecting something like this -
<graphics type="vnc" port="5910" autoport="no"
listen="127.0.0.1"
passwd="test0all" keymap="en-us"/>
Libvirt-cim log indicates that code knew about password but lost it
somewhere -
device_parsing.c(517): graphics device type = vnc
Virt_RASD.c(433): graphics Address = 127.0.0.1:5910
misc_util.c(75): Connecting to libvirt with uri `qemu:///system'
infostore.c(89): Path is /etc/libvirt/cim/QEMU_test_kvmredsap_dom
Virt_RASD.c(464): has password
The password is only clear text when the CIM instance is created and
passed to libvirt. Once the password is set, it cannot be fetched
(except by a secure libvirt connection, I think). The only was to change
the password today is delete the instance (or XML) and recreate with a
new password. That is why the existence of a password is determine via
an attribute in the libvirt-cim data store.
NOTE: This is the original logic and was not changed in this patch, just
re-factored to be contained within the "vnc" code path.
> @@ -1068,9 +1098,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 +1109,67 @@
> }
> dev->dev.graphics.type = strdup(val);
>
> + CU_DEBUG("graphics type = %s", dev->dev.graphics.type);
> +
If graphics type is "sdl", then the logic below leads to error message
that "sdl" is unsupported
The original code only handled vnc connections and would incorrectly
create other ResourceSubTypes, such as 'sdl'. For example, the original
code would create sdl objects with a internet address in the XML and
pass to libvirt, which would then reject the device. Since sdl did not
appear to be full supported, I added it to the explicitly not supported
list. We have other code that "tolerates" sdl, but more work is needed
to ensure complete support. I'll defer this work to another bug/patch.
> /* 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";
> + }
> +
_______________________________________________
Libvirt-cim mailing list
Libvirt-cim(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-cim
If there are no other issues, I'll resolve the line-length issues before
pushing. Agreed?
--
Chip Vincent
Open Virtualization
IBM Linux Technology Center
cvincent(a)linux.vnet.ibm.com
9:02 a.m.
New subject: [PATCH] Add support for console/serial grahpics devices
Chip,
Can you open new bugs to fix "sdl" support and "password" issues
?
Regards,
Sharad Mishra
Open Virtualization
Linux Technology Center
IBM
libvirt-cim-bounces(a)redhat.com wrote on 04/06/2011 05:46:23 PM:
Chip Vincent <cvincent(a)linux.vnet.ibm.com>
Sent by: libvirt-cim-bounces(a)redhat.com
04/06/2011 05:46 PM
Please respond to
cvincent(a)linux.vnet.ibm.com; Please respond to
List for discussion and development of libvirt CIM
<libvirt-cim(a)redhat.com>
To
libvirt-cim(a)redhat.com
cc
Subject
Re: [Libvirt-cim] [PATCH] Add support for console/serial grahpics devices
Thanks for the comments. My responses below.
On 04/06/2011 03:19 PM, Sharad Mishra wrote:
> Chip,
>
> My comments are inline below at 2 places.
> There are few lines in this patch that are longer than 80 characters,
> please correct them.
>
> Thanks,
> Sharad Mishra
> Open Virtualization
> Linux Technology Center
> IBM
>
> libvirt-cim-bounces(a)redhat.com wrote on 04/05/2011 07:35:51 AM:
>
> > Chip Vincent <cvincent(a)linux.vnet.ibm.com>
> > Sent by: libvirt-cim-bounces(a)redhat.com
> >
> > + infostore = infostore_open(dom);
> > + if (infostore != NULL)
> > + has_passwd = infostore_get_bool(infostore,
> > + "has_vnc_passwd");
> > +
> > + if (has_passwd) {
> > + CU_DEBUG("has password");
>
> Password is being set to "*****". According to libvirt.org, password
> should appear in clear text.
> I am not seeing password attribute in libvirt xml even when password is
> being passed to libvirt-cim. Here is the libvirt xml generated -
>
> <graphics type='vnc' port='5910' autoport='no'
listen='127.0.0.1'
> keymap='en-us'/>
>
> I was expecting something like this -
>
> <graphics type="vnc" port="5910" autoport="no"
listen="127.0.0.1"
> passwd="test0all" keymap="en-us"/>
>
> Libvirt-cim log indicates that code knew about password but lost it
> somewhere -
>
> device_parsing.c(517): graphics device type = vnc
> Virt_RASD.c(433): graphics Address = 127.0.0.1:5910
> misc_util.c(75): Connecting to libvirt with uri `qemu:///system'
> infostore.c(89): Path is /etc/libvirt/cim/QEMU_test_kvmredsap_dom
> Virt_RASD.c(464): has password
>
The password is only clear text when the CIM instance is created and
passed to libvirt. Once the password is set, it cannot be fetched
(except by a secure libvirt connection, I think). The only was to change
the password today is delete the instance (or XML) and recreate with a
new password. That is why the existence of a password is determine via
an attribute in the libvirt-cim data store.
NOTE: This is the original logic and was not changed in this patch, just
re-factored to be contained within the "vnc" code path.
> > @@ -1068,9 +1098,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 +1109,67 @@
> > }
> > dev->dev.graphics.type = strdup(val);
> >
> > + CU_DEBUG("graphics type = %s", dev->dev.graphics.type);
> > +
>
> If graphics type is "sdl", then the logic below leads to error message
> that "sdl" is unsupported
The original code only handled vnc connections and would incorrectly
create other ResourceSubTypes, such as 'sdl'. For example, the original
code would create sdl objects with a internet address in the XML and
pass to libvirt, which would then reject the device. Since sdl did not
appear to be full supported, I added it to the explicitly not supported
list. We have other code that "tolerates" sdl, but more work is needed
to ensure complete support. I'll defer this work to another bug/patch.
>
> > /* 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";
> > + }
> > +
>
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim
If there are no other issues, I'll resolve the line-length issues before
pushing. Agreed?
--
Chip Vincent
Open Virtualization
IBM Linux Technology Center
cvincent(a)linux.vnet.ibm.com
_______________________________________________
Libvirt-cim mailing list
Libvirt-cim(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-cim
12:27 p.m.
New subject: [PATCH] Add support for console/serial grahpics devices
Opened bug to add end-to-end support for sdl. The password logic is
working as designed (password is not saved by libvirt-cim, but does show
a password has been set).
On 04/07/2011 10:02 AM, Sharad Mishra wrote:
Chip,
Can you open new bugs to fix "sdl" support and "password" issues ?
Regards,
Sharad Mishra
Open Virtualization
Linux Technology Center
IBM
libvirt-cim-bounces(a)redhat.com wrote on 04/06/2011 05:46:23 PM:
> Chip Vincent <cvincent(a)linux.vnet.ibm.com>
> Sent by: libvirt-cim-bounces(a)redhat.com
>
> 04/06/2011 05:46 PM
>
> Please respond to
> cvincent(a)linux.vnet.ibm.com; Please respond to
> List for discussion and development of libvirt CIM
<libvirt-cim(a)redhat.com>
>
> To
>
> libvirt-cim(a)redhat.com
>
> cc
>
> Subject
>
> Re: [Libvirt-cim] [PATCH] Add support for console/serial grahpics devices
>
> Thanks for the comments. My responses below.
>
> On 04/06/2011 03:19 PM, Sharad Mishra wrote:
> > Chip,
> >
> > My comments are inline below at 2 places.
> > There are few lines in this patch that are longer than 80 characters,
> > please correct them.
> >
> > Thanks,
> > Sharad Mishra
> > Open Virtualization
> > Linux Technology Center
> > IBM
> >
> > libvirt-cim-bounces(a)redhat.com wrote on 04/05/2011 07:35:51 AM:
> >
> > > Chip Vincent <cvincent(a)linux.vnet.ibm.com>
> > > Sent by: libvirt-cim-bounces(a)redhat.com
> > >
> > > + infostore = infostore_open(dom);
> > > + if (infostore != NULL)
> > > + has_passwd = infostore_get_bool(infostore,
> > > + "has_vnc_passwd");
> > > +
> > > + if (has_passwd) {
> > > + CU_DEBUG("has password");
> >
> > Password is being set to "*****". According to libvirt.org,
password
> > should appear in clear text.
> > I am not seeing password attribute in libvirt xml even when password is
> > being passed to libvirt-cim. Here is the libvirt xml generated -
> >
> > <graphics type='vnc' port='5910' autoport='no'
listen='127.0.0.1'
> > keymap='en-us'/>
> >
> > I was expecting something like this -
> >
> > <graphics type="vnc" port="5910"
autoport="no" listen="127.0.0.1"
> > passwd="test0all" keymap="en-us"/>
> >
> > Libvirt-cim log indicates that code knew about password but lost it
> > somewhere -
> >
> > device_parsing.c(517): graphics device type = vnc
> > Virt_RASD.c(433): graphics Address = 127.0.0.1:5910
> > misc_util.c(75): Connecting to libvirt with uri `qemu:///system'
> > infostore.c(89): Path is /etc/libvirt/cim/QEMU_test_kvmredsap_dom
> > Virt_RASD.c(464): has password
> >
>
> The password is only clear text when the CIM instance is created and
> passed to libvirt. Once the password is set, it cannot be fetched
> (except by a secure libvirt connection, I think). The only was to change
> the password today is delete the instance (or XML) and recreate with a
> new password. That is why the existence of a password is determine via
> an attribute in the libvirt-cim data store.
>
> NOTE: This is the original logic and was not changed in this patch, just
> re-factored to be contained within the "vnc" code path.
>
>
> > > @@ -1068,9 +1098,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 +1109,67 @@
> > > }
> > > dev->dev.graphics.type = strdup(val);
> > >
> > > + CU_DEBUG("graphics type = %s", dev->dev.graphics.type);
> > > +
> >
> > If graphics type is "sdl", then the logic below leads to error
message
> > that "sdl" is unsupported
>
> The original code only handled vnc connections and would incorrectly
> create other ResourceSubTypes, such as 'sdl'. For example, the original
> code would create sdl objects with a internet address in the XML and
> pass to libvirt, which would then reject the device. Since sdl did not
> appear to be full supported, I added it to the explicitly not supported
> list. We have other code that "tolerates" sdl, but more work is needed
> to ensure complete support. I'll defer this work to another bug/patch.
>
> >
> > > /* 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";
> > > + }
> > > +
> >
> > _______________________________________________
> > Libvirt-cim mailing list
> > Libvirt-cim(a)redhat.com
> > https://www.redhat.com/mailman/listinfo/libvirt-cim
>
> If there are no other issues, I'll resolve the line-length issues before
> pushing. Agreed?
>
> --
> Chip Vincent
> Open Virtualization
> IBM Linux Technology Center
> cvincent(a)linux.vnet.ibm.com
>
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim
_______________________________________________
Libvirt-cim mailing list
Libvirt-cim(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-cim
--
Chip Vincent
Open Virtualization
IBM Linux Technology Center
cvincent(a)linux.vnet.ibm.com
5070
days inactive
5072
days old
4 comments
2 participants
participants (2)
-
Chip Vincent -
Sharad Mishra