[PATCHv2 0/3] Console Fixes and Enhancements
Enabling cimtest for the Console RASDs required a few improvements. The XML generation code was simplified, a logical device class for consoles had to be added and a bug in the KVM redirection service was fixed. V2 Changes: - Smaller change in Virt_Device.c (2/3) - Simpler console check in Virt_KVMRedirectionSAP.c (3/3) Viktor Mihajlovski (3): libxkutil: Simplify XML handling of consoles Virt_Device: Add a device class for consoles KVMRedirectionSAP: Only return redirection SAPs for VNC graphics libxkutil/device_parsing.c | 12 +----- libxkutil/xmlgen.c | 76 --------------------------------- schema/DisplayController.mof | 15 +++++++ schema/DisplayController.registration | 3 ++ src/Virt_Device.c | 6 ++- src/Virt_ElementSettingData.c | 3 ++ src/Virt_KVMRedirectionSAP.c | 26 ++++++----- src/Virt_SettingsDefineState.c | 6 +++ src/Virt_SystemDevice.c | 3 ++ src/Virt_VSSDComponent.c | 3 ++ 10 files changed, 53 insertions(+), 100 deletions(-) -- 1.7.9.5
The attempt to avoid duplication of console definitions was a bit too complicated and error prone. We move the generation of console XML entirely to the new console code. Further, it's incorrect to restrict PTY consoles being represented as GraphicsRASDs to virtio only. At least serial must be allowed, but it doesn't really hurt to map all PTYs to GraphicsRASD consoles. Further, the parsing code returned two instances for serial consoles, one from the /domain/devices/console and /domain/devices/serial both representing the same device. This confuses cimtest and, more problematic, will prevent DefineSystem to work, if a reference VSSD with a serial console is passed in. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Daniel Hansel <daniel.hansel@linux.vnet.ibm.com> --- libxkutil/device_parsing.c | 12 +------ libxkutil/xmlgen.c | 76 -------------------------------------------- 2 files changed, 1 insertion(+), 87 deletions(-) diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index 55c8d57..aecca4c 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -47,7 +47,7 @@ * still part of the graphics. */ #define GRAPHICS_XPATH (xmlChar *)"/domain/devices/graphics | "\ - "/domain/devices/console | /domain/devices/serial" + "/domain/devices/console" #define INPUT_XPATH (xmlChar *)"/domain/devices/input" #define DEFAULT_BRIDGE "xenbr0" @@ -948,16 +948,6 @@ static int parse_graphics_device(xmlNode *node, struct virt_device **vdevs) else if (XSTREQ(child->name, "target")) { gdev->dev.vnc.port = get_attr_value(child, "port"); - /* The graphics pty console can only be a - virtio console. If 'type' is not set in the - xml, the default of libvirt is virtio.*/ - char *t_type = get_attr_value(child, "type"); - if (t_type != NULL && !STREQC(t_type, "virtio")) { - CU_DEBUG("Not a pty-virtio graphics console"); - free(t_type); - goto err; - } - free(t_type); } } } diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c index 45bfb04..7e8801d 100644 --- a/libxkutil/xmlgen.c +++ b/libxkutil/xmlgen.c @@ -42,36 +42,11 @@ typedef const char *(*devfn_t)(xmlNodePtr node, struct domain *dominfo); typedef const char *(*poolfn_t)(xmlNodePtr node, struct virt_pool *pool); typedef const char *(*resfn_t)(xmlNodePtr node, struct virt_pool_res *res); -static int _count_graphics_console_definitions(struct domain *dominfo) -{ - int i; - int num = 0; - - for (i = 0; i < dominfo->dev_graphics_ct; i++) { - struct virt_device *_dev = &dominfo->dev_graphics[i]; - if (_dev->type == CIM_RES_TYPE_UNKNOWN) - continue; - - struct graphics_device *dev = &_dev->dev.graphics; - - if (STREQC(dev->type, "console")) { - num++; - } - } - CU_DEBUG("Found %d console defintions in graphics devices.",num); - return num; - -} - static const char *console_xml(xmlNodePtr root, struct domain *dominfo) { int i; xmlNodePtr console; xmlNodePtr tmp; - int num_graphics_consol_def = 0; - int num_suppressed_console_def = 0; - - num_graphics_consol_def = _count_graphics_console_definitions(dominfo); for (i = 0; i < dominfo->dev_console_ct; i++) { struct virt_device *_dev = &dominfo->dev_console[i]; @@ -80,25 +55,6 @@ static const char *console_xml(xmlNodePtr root, struct domain *dominfo) struct console_device *cdev = &_dev->dev.console; - /* Due to backward compatibility, the graphics device handling - is still parsing consoles: - source = pty, target = virtio (which is the default target) - But the console device handling processes these kind of - consoles too. This would lead to a duplication of these - default consoles in the domain xml definition. - This code prevents the console handling of writing xml for - duplicate pty/virtio consoles which are written by the - graphics device handling. */ - if (cdev->source_type == CIM_CHARDEV_SOURCE_TYPE_PTY && - (cdev->target_type == NULL || - STREQC(cdev->target_type, "virtio"))) { - if (num_suppressed_console_def < - num_graphics_consol_def) { - num_suppressed_console_def++; - continue; - } - } - console = xmlNewChild(root, NULL, BAD_CAST "console", NULL); if (console == NULL) return XML_ERROR; @@ -760,35 +716,6 @@ static const char *graphics_vnc_xml(xmlNodePtr root, 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->dev.vnc.host) - xmlNewProp(tmp, BAD_CAST "path", BAD_CAST dev->dev.vnc.host); - - tmp = xmlNewChild(pty, NULL, BAD_CAST "target", NULL); - if (tmp == NULL) - return XML_ERROR; - - if(dev->dev.vnc.port) - xmlNewProp(tmp, BAD_CAST "port", BAD_CAST dev->dev.vnc.port); - - return NULL; -} - static const char *graphics_xml(xmlNodePtr root, struct domain *dominfo) { const char *msg = NULL; @@ -803,9 +730,6 @@ static const char *graphics_xml(xmlNodePtr root, struct domain *dominfo) 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; -- 1.7.9.5
libvirt-cim has a very strict assumption about the presence of a CIM_LogicalDevice class being associated to a RASD. It is practically impossible to extend the cimtest framework for the ConsoleRASD class without having a matching device class. Adding a new ConsoleDisplayController class for this purpose. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Daniel Hansel <daniel.hansel@linux.vnet.ibm.com> --- schema/DisplayController.mof | 15 +++++++++++++++ schema/DisplayController.registration | 3 +++ src/Virt_Device.c | 6 ++++-- src/Virt_ElementSettingData.c | 3 +++ src/Virt_SettingsDefineState.c | 6 ++++++ src/Virt_SystemDevice.c | 3 +++ src/Virt_VSSDComponent.c | 3 +++ 7 files changed, 37 insertions(+), 2 deletions(-) diff --git a/schema/DisplayController.mof b/schema/DisplayController.mof index b57c9cc..8ccdc07 100644 --- a/schema/DisplayController.mof +++ b/schema/DisplayController.mof @@ -15,3 +15,18 @@ class LXC_DisplayController : CIM_DisplayController { }; +[ Provider("cmpi::Virt_Device") ] +class Xen_ConsoleDisplayController : CIM_DisplayController +{ +}; + +[ Provider("cmpi::Virt_Device") ] +class KVM_ConsoleDisplayController : CIM_DisplayController +{ +}; + +[ Provider("cmpi::Virt_Device") ] +class LXC_ConsoleDisplayController : CIM_DisplayController +{ +}; + diff --git a/schema/DisplayController.registration b/schema/DisplayController.registration index de8adc9..e7e9770 100644 --- a/schema/DisplayController.registration +++ b/schema/DisplayController.registration @@ -3,3 +3,6 @@ Xen_DisplayController root/virt Virt_Device Virt_Device instance KVM_DisplayController root/virt Virt_Device Virt_Device instance LXC_DisplayController root/virt Virt_Device Virt_Device instance +Xen_ConsoleDisplayController root/virt Virt_Device Virt_Device instance +KVM_ConsoleDisplayController root/virt Virt_Device Virt_Device instance +LXC_ConsoleDisplayController root/virt Virt_Device Virt_Device instance diff --git a/src/Virt_Device.c b/src/Virt_Device.c index aa47276..b93e592 100644 --- a/src/Virt_Device.c +++ b/src/Virt_Device.c @@ -263,12 +263,12 @@ static CMPIInstance *console_instance(const CMPIBroker *broker, conn = virDomainGetConnect(dom); inst = get_typed_instance(broker, pfx_from_conn(conn), - "DisplayController", + "ConsoleDisplayController", ns, true); if (inst == NULL) { - CU_DEBUG("Failed to get instance for DisplayController"); + CU_DEBUG("Failed to get instance for ConsoleDisplayController"); return NULL; } @@ -549,6 +549,8 @@ uint16_t res_type_from_device_classname(const char *classname) return CIM_RES_TYPE_MEM; else if (strstr(classname, "Processor")) return CIM_RES_TYPE_PROC; + else if (strstr(classname, "ConsoleDisplayController")) + return CIM_RES_TYPE_CONSOLE; else if (strstr(classname, "DisplayController")) return CIM_RES_TYPE_GRAPHICS; else if (strstr(classname, "PointingDevice")) diff --git a/src/Virt_ElementSettingData.c b/src/Virt_ElementSettingData.c index c257710..c088e49 100644 --- a/src/Virt_ElementSettingData.c +++ b/src/Virt_ElementSettingData.c @@ -128,18 +128,21 @@ static char* resource_allocation_setting_data[] = { "Xen_NetResourceAllocationSettingData", "Xen_ProcResourceAllocationSettingData", "Xen_GraphicsResourceAllocationSettingData", + "Xen_ConsoleResourceAllocationSettingData", "Xen_InputResourceAllocationSettingData", "KVM_DiskResourceAllocationSettingData", "KVM_MemResourceAllocationSettingData", "KVM_NetResourceAllocationSettingData", "KVM_ProcResourceAllocationSettingData", "KVM_GraphicsResourceAllocationSettingData", + "KVM_ConsoleResourceAllocationSettingData", "KVM_InputResourceAllocationSettingData", "LXC_DiskResourceAllocationSettingData", "LXC_MemResourceAllocationSettingData", "LXC_NetResourceAllocationSettingData", "LXC_ProcResourceAllocationSettingData", "LXC_GraphicsResourceAllocationSettingData", + "LXC_ConsoleResourceAllocationSettingData", "LXC_InputResourceAllocationSettingData", NULL }; diff --git a/src/Virt_SettingsDefineState.c b/src/Virt_SettingsDefineState.c index f30f45f..be2ded5 100644 --- a/src/Virt_SettingsDefineState.c +++ b/src/Virt_SettingsDefineState.c @@ -327,18 +327,21 @@ static char* logical_device[] = { "Xen_NetworkPort", "Xen_LogicalDisk", "Xen_DisplayController", + "Xen_ConsoleDisplayController", "Xen_PointingDevice", "KVM_Processor", "KVM_Memory", "KVM_NetworkPort", "KVM_LogicalDisk", "KVM_DisplayController", + "KVM_ConsoleDisplayController", "KVM_PointingDevice", "LXC_Processor", "LXC_Memory", "LXC_NetworkPort", "LXC_LogicalDisk", "LXC_DisplayController", + "LXC_ConsoleDisplayController", "LXC_PointingDevice", NULL }; @@ -350,18 +353,21 @@ static char* resource_allocation_setting_data[] = { "Xen_ProcResourceAllocationSettingData", "Xen_GraphicsResourceAllocationSettingData", "Xen_InputResourceAllocationSettingData", + "Xen_ConsoleResourceAllocationSettingData", "KVM_DiskResourceAllocationSettingData", "KVM_MemResourceAllocationSettingData", "KVM_NetResourceAllocationSettingData", "KVM_ProcResourceAllocationSettingData", "KVM_GraphicsResourceAllocationSettingData", "KVM_InputResourceAllocationSettingData", + "KVM_ConsoleResourceAllocationSettingData", "LXC_DiskResourceAllocationSettingData", "LXC_MemResourceAllocationSettingData", "LXC_NetResourceAllocationSettingData", "LXC_ProcResourceAllocationSettingData", "LXC_GraphicsResourceAllocationSettingData", "LXC_InputResourceAllocationSettingData", + "LXC_ConsoleResourceAllocationSettingData", NULL }; diff --git a/src/Virt_SystemDevice.c b/src/Virt_SystemDevice.c index 3a2f7ce..d2e526d 100644 --- a/src/Virt_SystemDevice.c +++ b/src/Virt_SystemDevice.c @@ -135,18 +135,21 @@ static char* part_component[] = { "Xen_NetworkPort", "Xen_LogicalDisk", "Xen_DisplayController", + "Xen_ConsoleDisplayController", "Xen_PointingDevice", "KVM_Processor", "KVM_Memory", "KVM_NetworkPort", "KVM_LogicalDisk", "KVM_DisplayController", + "KVM_ConsoleDisplayController", "KVM_PointingDevice", "LXC_Processor", "LXC_Memory", "LXC_NetworkPort", "LXC_LogicalDisk", "LXC_DisplayController", + "LXC_ConsoleDisplayController", "LXC_PointingDevice", NULL }; diff --git a/src/Virt_VSSDComponent.c b/src/Virt_VSSDComponent.c index 378de96..35bffde 100644 --- a/src/Virt_VSSDComponent.c +++ b/src/Virt_VSSDComponent.c @@ -132,6 +132,7 @@ static char* part_component[] = { "Xen_NetResourceAllocationSettingData", "Xen_ProcResourceAllocationSettingData", "Xen_GraphicsResourceAllocationSettingData", + "Xen_ConsoleResourceAllocationSettingData", "Xen_InputResourceAllocationSettingData", "KVM_DiskResourceAllocationSettingData", "KVM_MemResourceAllocationSettingData", @@ -139,12 +140,14 @@ static char* part_component[] = { "KVM_ProcResourceAllocationSettingData", "KVM_GraphicsResourceAllocationSettingData", "KVM_InputResourceAllocationSettingData", + "KVM_ConsoleResourceAllocationSettingData", "LXC_DiskResourceAllocationSettingData", "LXC_MemResourceAllocationSettingData", "LXC_NetResourceAllocationSettingData", "LXC_ProcResourceAllocationSettingData", "LXC_GraphicsResourceAllocationSettingData", "LXC_InputResourceAllocationSettingData", + "LXC_ConsoleResourceAllocationSettingData", NULL }; -- 1.7.9.5
Since pty consoles still show up as GraphicsRASDs the check for VNC redirection proved to be too rigid. Instead of failing the associator call, PTY consoles will just be ignored now. This is an old bug, but will be exposed when cimtest is extended to handle consoles. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Daniel Hansel <daniel.hansel@linux.vnet.ibm.com> --- src/Virt_KVMRedirectionSAP.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/Virt_KVMRedirectionSAP.c b/src/Virt_KVMRedirectionSAP.c index 708b0d1..1533e52 100644 --- a/src/Virt_KVMRedirectionSAP.c +++ b/src/Virt_KVMRedirectionSAP.c @@ -264,28 +264,31 @@ static CMPIStatus get_vnc_sessions(const CMPIBroker *broker, return s; } -static bool check_graphics(virDomainPtr dom, - struct domain **dominfo) +static int check_graphics(virDomainPtr dom, + struct domain **dominfo) { int ret = 0; + int i; ret = get_dominfo(dom, dominfo); if (!ret) { CU_DEBUG("Unable to get domain info"); - return false; + return -1; } if ((*dominfo)->dev_graphics == NULL) { CU_DEBUG("No graphics device associated with guest"); - return false; + return -1; } - if (!STREQC((*dominfo)->dev_graphics->dev.graphics.type, "vnc")) { - CU_DEBUG("Only vnc devices have console redirection sessions"); - return false; + for (i = 0; i < (*dominfo)->dev_graphics_ct; i++) { + if (STREQC((*dominfo)->dev_graphics[i].dev.graphics.type, "vnc")) { + return i; + } } - return true; + CU_DEBUG("Only vnc devices have console redirection sessions"); + return -1; } static CMPIStatus return_console_sap(const CMPIObjectPath *ref, @@ -362,12 +365,13 @@ CMPIStatus enum_console_sap(const CMPIBroker *broker, } for (i = 0; i < count; i++) { - if (!check_graphics(domain_list[i], &dominfo)) { + int pos = check_graphics(domain_list[i], &dominfo); + if (pos < 0) { cleanup_dominfo(&dominfo); continue; } - ret = sscanf(dominfo->dev_graphics->dev.graphics.dev.vnc.port, + ret = sscanf(dominfo->dev_graphics[pos].dev.graphics.dev.vnc.port, "%d", &lport); if (ret != 1) { @@ -449,7 +453,7 @@ CMPIStatus get_console_sap_by_name(const CMPIBroker *broker, goto out; } - if (!check_graphics(dom, &dominfo)) { + if (check_graphics(dom, &dominfo) < 0) { virt_set_status(broker, &s, CMPI_RC_ERR_FAILED, conn, -- 1.7.9.5
On 10/07/2013 10:02 AM, Viktor Mihajlovski wrote:
Enabling cimtest for the Console RASDs required a few improvements. The XML generation code was simplified, a logical device class for consoles had to be added and a bug in the KVM redirection service was fixed.
V2 Changes: - Smaller change in Virt_Device.c (2/3) - Simpler console check in Virt_KVMRedirectionSAP.c (3/3)
Viktor Mihajlovski (3): libxkutil: Simplify XML handling of consoles Virt_Device: Add a device class for consoles KVMRedirectionSAP: Only return redirection SAPs for VNC graphics
libxkutil/device_parsing.c | 12 +----- libxkutil/xmlgen.c | 76 --------------------------------- schema/DisplayController.mof | 15 +++++++ schema/DisplayController.registration | 3 ++ src/Virt_Device.c | 6 ++- src/Virt_ElementSettingData.c | 3 ++ src/Virt_KVMRedirectionSAP.c | 26 ++++++----- src/Virt_SettingsDefineState.c | 6 +++ src/Virt_SystemDevice.c | 3 ++ src/Virt_VSSDComponent.c | 3 ++ 10 files changed, 53 insertions(+), 100 deletions(-)
ACK I have pushed these changes (just now) John (I've been having some "issues" recently with network config and connectivity - it's been a rough stretch!)
On 10/10/2013 07:03 PM, John Ferlan wrote:
ACK
I have pushed these changes (just now)
cool, thanks! -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (2)
-
John Ferlan -
Viktor Mihajlovski