[PATCH 0 of 3] Add graphics device support to VSMS
Just graphics device support for now.. input device support will follow.
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1225319502 25200 # Node ID 793cbc81633c57f75a26766a3add3969c7ecc32e # Parent 087bd497d4ed6fbf1b2a00dac26a7c6f8baf91e2 Add count field to dominfo. Add keymap support for graphics devices. Not really needed, but allows VSMS to treat this device in the same manner the other devices are treated. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r 087bd497d4ed -r 793cbc81633c libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c Tue Nov 18 09:37:05 2008 -0800 +++ b/libxkutil/device_parsing.c Wed Oct 29 15:31:42 2008 -0700 @@ -926,8 +926,9 @@ goto err; parse_devices(xml, &(*dominfo)->dev_emu, CIM_RES_TYPE_EMU); - parse_devices(xml, &(*dominfo)->dev_graphics, CIM_RES_TYPE_GRAPHICS); - + (*dominfo)->dev_graphics_ct = parse_devices(xml, + &(*dominfo)->dev_graphics, + CIM_RES_TYPE_GRAPHICS); (*dominfo)->dev_input_ct = parse_devices(xml, &(*dominfo)->dev_input, CIM_RES_TYPE_INPUT); @@ -1000,6 +1001,7 @@ cleanup_virt_devices(&dom->dev_net, dom->dev_net_ct); cleanup_virt_devices(&dom->dev_disk, dom->dev_disk_ct); cleanup_virt_devices(&dom->dev_vcpu, dom->dev_vcpu_ct); + cleanup_virt_devices(&dom->dev_graphics, dom->dev_graphics_ct); cleanup_virt_devices(&dom->dev_input, dom->dev_input_ct); free(dom); diff -r 087bd497d4ed -r 793cbc81633c libxkutil/device_parsing.h --- a/libxkutil/device_parsing.h Tue Nov 18 09:37:05 2008 -0800 +++ b/libxkutil/device_parsing.h Wed Oct 29 15:31:42 2008 -0700 @@ -125,6 +125,8 @@ int on_crash; struct virt_device *dev_graphics; + int dev_graphics_ct; + struct virt_device *dev_emu; struct virt_device *dev_input; diff -r 087bd497d4ed -r 793cbc81633c libxkutil/xmlgen.c --- a/libxkutil/xmlgen.c Tue Nov 18 09:37:05 2008 -0800 +++ b/libxkutil/xmlgen.c Wed Oct 29 15:31:42 2008 -0700 @@ -703,7 +703,7 @@ if (dominfo->dev_graphics) concat_devxml(&devxml, dominfo->dev_graphics, - 1, + dominfo->dev_graphics_ct, graphics_to_xml); console_xml(dominfo, &devxml); diff -r 087bd497d4ed -r 793cbc81633c src/Virt_RASD.c --- a/src/Virt_RASD.c Tue Nov 18 09:37:05 2008 -0800 +++ b/src/Virt_RASD.c Wed Oct 29 15:31:42 2008 -0700 @@ -279,6 +279,9 @@ } CMSetProperty(inst, "Address", (CMPIValue *)addr_str, CMPI_chars); + CMSetProperty(inst, "ResourceSubType", + (CMPIValue *)dev->dev.graphics.keymap, CMPI_chars); + out: free(addr_str); diff -r 087bd497d4ed -r 793cbc81633c src/Virt_SettingsDefineCapabilities.c --- a/src/Virt_SettingsDefineCapabilities.c Tue Nov 18 09:37:05 2008 -0800 +++ b/src/Virt_SettingsDefineCapabilities.c Wed Oct 29 15:31:42 2008 -0700 @@ -718,6 +718,7 @@ { const char *id; const char *addr; + const char *keymap; CMPIInstance *inst; CMPIStatus s = {CMPI_RC_OK, NULL}; @@ -747,6 +748,9 @@ addr = "127.0.0.1:-1"; CMSetProperty(inst, "Address", (CMPIValue *)addr, CMPI_chars); + + keymap = "en-us"; + CMSetProperty(inst, "ResourceSubType", (CMPIValue *)keymap, CMPI_chars); inst_list_add(list, inst);
KR> diff -r 087bd497d4ed -r 793cbc81633c src/Virt_RASD.c KR> --- a/src/Virt_RASD.c Tue Nov 18 09:37:05 2008 -0800 KR> +++ b/src/Virt_RASD.c Wed Oct 29 15:31:42 2008 -0700 KR> @@ -279,6 +279,9 @@ KR> } KR> CMSetProperty(inst, "Address", (CMPIValue *)addr_str, CMPI_chars); KR> + CMSetProperty(inst, "ResourceSubType", KR> + (CMPIValue *)dev->dev.graphics.keymap, CMPI_chars); KR> + Hmm, it doesn't seem right to me to say that "en-us" further refines the graphics device to a more specific type. I would think that ResourceSubType here might be "Virtual Console" or "VNC Graphics Adapter" or some such. The keymap, while associated to the graphics device in the libvirt XML, isn't a scoping property IMHO. KR> @@ -747,6 +748,9 @@ KR> addr = "127.0.0.1:-1"; KR> CMSetProperty(inst, "Address", (CMPIValue *)addr, CMPI_chars); KR> + KR> + keymap = "en-us"; KR> + CMSetProperty(inst, "ResourceSubType", (CMPIValue *)keymap, CMPI_chars); I think you can get away with not declaring a new variable for this. You can pass the string constant directly to the CMSetProperty() call. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
KR> diff -r 087bd497d4ed -r 793cbc81633c src/Virt_RASD.c KR> --- a/src/Virt_RASD.c Tue Nov 18 09:37:05 2008 -0800 KR> +++ b/src/Virt_RASD.c Wed Oct 29 15:31:42 2008 -0700 KR> @@ -279,6 +279,9 @@ KR> }
KR> CMSetProperty(inst, "Address", (CMPIValue *)addr_str, CMPI_chars); KR> + CMSetProperty(inst, "ResourceSubType", KR> + (CMPIValue *)dev->dev.graphics.keymap, CMPI_chars); KR> +
Hmm, it doesn't seem right to me to say that "en-us" further refines the graphics device to a more specific type. I would think that ResourceSubType here might be "Virtual Console" or "VNC Graphics Adapter" or some such. The keymap, while associated to the graphics device in the libvirt XML, isn't a scoping property IMHO.
I struggled with which attribute to use to store the keymap info. When calling DefineSystem(), the user might want to specify a certain keymapping. I could extend the current GraphicsRASD schema to include a KeyMapping attribute or something similar. I considered using the caption attribute, but that doesn't seem appropriate either.
KR> @@ -747,6 +748,9 @@
KR> addr = "127.0.0.1:-1"; KR> CMSetProperty(inst, "Address", (CMPIValue *)addr, CMPI_chars); KR> + KR> + keymap = "en-us"; KR> + CMSetProperty(inst, "ResourceSubType", (CMPIValue *)keymap, CMPI_chars);
I think you can get away with not declaring a new variable for this. You can pass the string constant directly to the CMSetProperty() call.
Sure - I'll change this on the next revision. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
KR> I could extend the current GraphicsRASD schema to include a KR> KeyMapping attribute or something similar. I think that would be appropriate. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1225319502 25200 # Node ID 02650ebca65b14e9f243f11308d795371a8e740e # Parent 793cbc81633c57f75a26766a3add3969c7ecc32e Add graphics device support to VSMS. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r 793cbc81633c -r 02650ebca65b src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Wed Oct 29 15:31:42 2008 -0700 +++ b/src/Virt_VirtualSystemManagementService.c Wed Oct 29 15:31:42 2008 -0700 @@ -211,8 +211,7 @@ return 1; } -static bool default_graphics_device(CMPIInstance *inst, - struct domain *domain) +static bool default_graphics_device(struct domain *domain) { free(domain->dev_graphics); domain->dev_graphics = calloc(1, sizeof(*domain->dev_graphics)); @@ -223,6 +222,19 @@ domain->dev_graphics->dev.graphics.type = strdup("vnc"); domain->dev_graphics->dev.graphics.port = strdup("-1"); + domain->dev_graphics->dev.graphics.host = strdup("127.0.0.1"); + domain->dev_graphics->dev.graphics.keymap = strdup("en-us"); + domain->dev_graphics_ct = 1; + + return true; +} + +static bool add_default_devs(struct domain *domain) +{ + if (domain->dev_graphics_ct != 1) { + if (!default_graphics_device(domain)) + return false; + } return true; } @@ -275,9 +287,6 @@ else { CU_DEBUG("Unknown domain prefix: %s", pfx); } - - if (!default_graphics_device(inst, domain)) - ret = 0; out: free(pfx); @@ -554,6 +563,41 @@ return NULL; } +static const char *graphics_rasd_to_vdev(CMPIInstance *inst, + struct virt_device *dev) +{ + const char *val; + const char *msg; + const char *keymap; + char *addr; + char *port; + + dev->dev.graphics.type = strdup("vnc"); + + if (cu_get_str_prop(inst, "Address", &val) != CMPI_RC_OK) { + port = strdup("-1"); + addr = strdup("127.0.0.1"); + } else if (!parse_id(val, &addr, &port)) { + msg = "GraphicsRASD field Address not valid"; + goto out; + } + + /* FIXME: Add logic to prevent address:port collisions */ + dev->dev.graphics.port = strdup(port); + dev->dev.graphics.host = strdup(addr); + + if (cu_get_str_prop(inst, "ResourceSubType", &keymap) != CMPI_RC_OK) + keymap = "en-us"; + + dev->dev.graphics.keymap = strdup(keymap); + + out: + free(addr); + free(port); + + return NULL; +} + static const char *_sysvirt_rasd_to_vdev(CMPIInstance *inst, struct virt_device *dev, uint16_t type, @@ -567,6 +611,8 @@ return mem_rasd_to_vdev(inst, dev); } else if (type == CIM_RES_TYPE_PROC) { return proc_rasd_to_vdev(inst, dev); + } else if (type == CIM_RES_TYPE_GRAPHICS) { + return graphics_rasd_to_vdev(inst, dev); } return "Resource type not supported on this platform"; @@ -585,6 +631,8 @@ return net_rasd_to_vdev(inst, dev, ns); } else if (type == CIM_RES_TYPE_PROC) { return lxc_proc_rasd_to_vdev(inst, dev); + } else if (type == CIM_RES_TYPE_GRAPHICS) { + return graphics_rasd_to_vdev(inst, dev); } return "Resource type not supported on this platform"; @@ -688,6 +736,9 @@ if (!make_space(&domain->dev_net, domain->dev_net_ct, count)) return "Failed to alloc net list"; + if (!make_space(&domain->dev_graphics, domain->dev_graphics_ct, count)) + return "Failed to alloc graphics list"; + for (i = 0; i < count; i++) { CMPIObjectPath *op; CMPIData item; @@ -748,6 +799,12 @@ domain->dev_net, ncount, &domain->dev_net_ct); + } else if (type == CIM_RES_TYPE_GRAPHICS) { + domain->dev_graphics_ct = 1; + msg = rasd_to_vdev(inst, + domain, + &domain->dev_graphics[0], + ns); } if (msg != NULL) return msg; @@ -993,6 +1050,14 @@ goto out; } + if (!add_default_devs(domain)) { + CU_DEBUG("Failed to add default devices"); + cu_statusf(_BROKER, s, + CMPI_RC_ERR_FAILED, + "ResourceSettings Error"); + goto out; + } + xml = system_to_xml(domain); CU_DEBUG("System XML:\n%s", xml); @@ -1264,6 +1329,9 @@ } else if (type == CIM_RES_TYPE_MEM) { list = &dominfo->dev_mem; *count = &dominfo->dev_mem_ct; + } else if (type == CIM_RES_TYPE_GRAPHICS) { + list = &dominfo->dev_graphics; + *count = &dominfo->dev_graphics_ct; } return list;
KR> +static const char *graphics_rasd_to_vdev(CMPIInstance *inst, KR> + struct virt_device *dev) KR> +{ KR> + const char *val; KR> + const char *msg; KR> + const char *keymap; KR> + char *addr; KR> + char *port; KR> + KR> + dev->dev.graphics.type = strdup("vnc"); KR> + KR> + if (cu_get_str_prop(inst, "Address", &val) != CMPI_RC_OK) { KR> + port = strdup("-1"); KR> + addr = strdup("127.0.0.1"); KR> + } else if (!parse_id(val, &addr, &port)) { The parse_id() function returns an integer, not a boolean. KR> + msg = "GraphicsRASD field Address not valid"; KR> + goto out; KR> + } KR> + KR> + /* FIXME: Add logic to prevent address:port collisions */ KR> + dev->dev.graphics.port = strdup(port); KR> + dev->dev.graphics.host = strdup(addr); Can't you avoid the double-strdup()? -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1225319502 25200 # Node ID 70a6139bbe36c56ebec2a301468ff652c444520d # Parent 02650ebca65b14e9f243f11308d795371a8e740e Handle add/remove/modify support for graphics devices There's not dynamic update support for graphics devices. If a user add/removes/updates a graphics device while the guest is running, the changes aren't picked up until the guest is restarted. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r 02650ebca65b -r 70a6139bbe36 src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Wed Oct 29 15:31:42 2008 -0700 +++ b/src/Virt_VirtualSystemManagementService.c Wed Oct 29 15:31:42 2008 -0700 @@ -1448,11 +1448,16 @@ struct virt_device *dev = &list[i]; if (STREQ(dev->id, devid)) { - s = _resource_dynamic(dominfo, - dev, - RESOURCE_DEL, - CLASSNAME(op)); dev->type = CIM_RES_TYPE_UNKNOWN; + + if (type == CIM_RES_TYPE_GRAPHICS) + cu_statusf(_BROKER, &s, CMPI_RC_OK, ""); + else { + s = _resource_dynamic(dominfo, + dev, + RESOURCE_DEL, + CLASSNAME(op)); + } break; } } @@ -1497,6 +1502,13 @@ goto out; } + if ((type == CIM_RES_TYPE_GRAPHICS) && (*count > 0)) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "A resource already exiss for type %" PRIu16, type); + goto out; + } + list = realloc(*_list, ((*count)+1)*sizeof(struct virt_device)); if (list == NULL) { /* No memory */ @@ -1513,6 +1525,12 @@ dev->type = type; rasd_to_vdev(rasd, dominfo, dev, ns); + + if (type == CIM_RES_TYPE_GRAPHICS) { + (*count)++; + cu_statusf(_BROKER, &s, CMPI_RC_OK, ""); + goto out; + } s = _resource_dynamic(dominfo, dev, RESOURCE_ADD, CLASSNAME(op)); if (s.rc != CMPI_RC_OK) @@ -1570,10 +1588,15 @@ if (STREQ(dev->id, devid)) { rasd_to_vdev(rasd, dominfo, dev, ns); - s = _resource_dynamic(dominfo, - dev, - RESOURCE_MOD, - CLASSNAME(op)); + + if (type == CIM_RES_TYPE_GRAPHICS) + cu_statusf(_BROKER, &s, CMPI_RC_OK, ""); + else { + s = _resource_dynamic(dominfo, + dev, + RESOURCE_MOD, + CLASSNAME(op)); + } break; } }
KR> + "A resource already exiss for type %" PRIu16, type); ^^^^^ There's a teensy typo there :) -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
participants (2)
-
Dan Smith -
Kaitlin Rupert