[PATCH 0 of 3] (#2) Add graphics device support to VSMS
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1225319502 25200 # Node ID b070e80c1c0fb2621b5d601c2aa5d57b6501e8b9 # Parent 0741392902634278daf88655883ce5f97f79ffea (#2) 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. Updates from 1 to 2: -Add KeyMapping attribute to GraphicsRASD - use this instead of ResourceSubType to store mapping. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r 074139290263 -r b070e80c1c0f libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c Wed Nov 19 13:49:35 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 074139290263 -r b070e80c1c0f libxkutil/device_parsing.h --- a/libxkutil/device_parsing.h Wed Nov 19 13:49:35 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 074139290263 -r b070e80c1c0f libxkutil/xmlgen.c --- a/libxkutil/xmlgen.c Wed Nov 19 13:49:35 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 074139290263 -r b070e80c1c0f schema/ResourceAllocationSettingData.mof --- a/schema/ResourceAllocationSettingData.mof Wed Nov 19 13:49:35 2008 -0800 +++ b/schema/ResourceAllocationSettingData.mof Wed Oct 29 15:31:42 2008 -0700 @@ -117,6 +117,8 @@ ] class Xen_GraphicsResourceAllocationSettingData : Xen_ResourceAllocationSettingData { + [Description ("Keyboard keymapping")] + string KeyMapping; }; [Description ("KVM virtual graphics device"), @@ -124,6 +126,8 @@ ] class KVM_GraphicsResourceAllocationSettingData : KVM_ResourceAllocationSettingData { + [Description ("Keyboard keymapping")] + string KeyMapping; }; [Description ("LXC virtual graphics device"), @@ -131,6 +135,8 @@ ] class LXC_GraphicsResourceAllocationSettingData : LXC_ResourceAllocationSettingData { + [Description ("Keyboard keymapping")] + string KeyMapping; }; [Description ("Xen virtual input device"), diff -r 074139290263 -r b070e80c1c0f src/Virt_RASD.c --- a/src/Virt_RASD.c Wed Nov 19 13:49:35 2008 -0800 +++ b/src/Virt_RASD.c Wed Oct 29 15:31:42 2008 -0700 @@ -279,6 +279,8 @@ } CMSetProperty(inst, "Address", (CMPIValue *)addr_str, CMPI_chars); + CMSetProperty(inst, "KeyMapping", + (CMPIValue *)dev->dev.graphics.keymap, CMPI_chars); out: free(addr_str); diff -r 074139290263 -r b070e80c1c0f src/Virt_SettingsDefineCapabilities.c --- a/src/Virt_SettingsDefineCapabilities.c Wed Nov 19 13:49:35 2008 -0800 +++ b/src/Virt_SettingsDefineCapabilities.c Wed Oct 29 15:31:42 2008 -0700 @@ -748,6 +748,8 @@ addr = "127.0.0.1:-1"; CMSetProperty(inst, "Address", (CMPIValue *)addr, CMPI_chars); + CMSetProperty(inst, "KeyMapping", (CMPIValue *)"en-us", CMPI_chars); + inst_list_add(list, inst); out:
KR> -Add KeyMapping attribute to GraphicsRASD - use this instead of KR> ResourceSubType to store mapping. I hate to pick nits here, but "KeyMapping" makes me think of mapping one key to another. Given the CIM term of "Key" it just seems wrong. Can we call it "KeyMap"? :) -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
KR> -Add KeyMapping attribute to GraphicsRASD - use this instead of KR> ResourceSubType to store mapping.
I hate to pick nits here, but "KeyMapping" makes me think of mapping one key to another. Given the CIM term of "Key" it just seems wrong.
Can we call it "KeyMap"? :)
Sure, that's fine by me. Fair thing to nitpick =) -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1225319502 25200 # Node ID 791b8e26b4ab4c2f1141f9bf807ece19fb08238c # Parent b070e80c1c0fb2621b5d601c2aa5d57b6501e8b9 (#2) Add graphics device support to VSMS. Updates from 1 to 2: -Remove double strdup in graphics_rasd_to_vdev() -parse_id() returns an int - check return appropriately Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r b070e80c1c0f -r 791b8e26b4ab 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 = NULL; + const char *keymap; + char *addr = NULL; + char *port = NULL; + + dev->dev.graphics.type = strdup("vnc"); + + /* FIXME: Add logic to prevent address:port collisions */ + if (cu_get_str_prop(inst, "Address", &val) != CMPI_RC_OK) { + dev->dev.graphics.port = strdup("-1"); + dev->dev.graphics.host = strdup("127.0.0.1"); + } else if (parse_id(val, &addr, &port) == 1) { + dev->dev.graphics.port = strdup(port); + dev->dev.graphics.host = strdup(addr); + } else { + msg = "GraphicsRASD field Address not valid"; + goto out; + } + + if (cu_get_str_prop(inst, "KeyMapping", &keymap) != CMPI_RC_OK) + keymap = "en-us"; + + dev->dev.graphics.keymap = strdup(keymap); + + out: + free(addr); + free(port); + + return msg; +} + 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 = NULL; KR> + const char *keymap; KR> + char *addr = NULL; KR> + char *port = NULL; KR> + KR> + dev->dev.graphics.type = strdup("vnc"); KR> + KR> + /* FIXME: Add logic to prevent address:port collisions */ KR> + if (cu_get_str_prop(inst, "Address", &val) != CMPI_RC_OK) { KR> + dev->dev.graphics.port = strdup("-1"); KR> + dev->dev.graphics.host = strdup("127.0.0.1"); KR> + } else if (parse_id(val, &addr, &port) == 1) { KR> + dev->dev.graphics.port = strdup(port); KR> + dev->dev.graphics.host = strdup(addr); What I meant was just make the call: parse_id(val, &dev->dev.graphics.port, &dev->dev.graphics.host); Since parse_id() does an alloc, we don't need to do another alloc and then free the first. If you want to keep the code cleaner, you can still use the char pointers, but no need to strdup() and free() them at the end. -- 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 2333214c2f14a08cc18e7028830b3fd6f2e0a38d # Parent 791b8e26b4ab4c2f1141f9bf807ece19fb08238c (#2) 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. Updates from 1 to 2: -Fix typo in error message Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r 791b8e26b4ab -r 2333214c2f14 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 exists 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; } }
participants (2)
-
Dan Smith -
Kaitlin Rupert