[PATCH 0/6] Add Virtual Controller Device
This is rework of the Controller patch series submitted by Xu Wang. I have built upon the constructs put in place by Xu with respect to reading and writing the XML, but for the remainder of the code I changed where the virtual controller lives. Xu's patches made a Controller RASD which had some issues regarding associations with other devices and pools. Rather than go with a RASD model, I have chosen to make a KVM_Controller virtual device to mimic a CIM_Controller device. This solution keeps things at a lower level and has been able to pass the cimtest as well as the update a defined domain with a changed value test. This second test is what fails for RHEL7. As it turns out, it seems as long as a 'pci' device with model 'pci-root' is created, then things seem to work. The code relies on the previous patch code Xu created to read/write the XML file with some adjustments. Perhaps the only controversial patch (for me at least) is 6/6. I figured that after we've read everything and just before we go to create or update the guest that we need to make sure that at least the 'pci' device with model 'pci-root' exists. This is similar to the add_default_devs() code, except that it's run after all that code prior to any add or update of the guest just before the "system_to_xml()" call is made. I figure this is the last gasp to ensure that at least the 'pci' device is there which seems to be used by a number of "silently added" libvirt devices. John Ferlan (4): Add virtual controller object defs Change static API to global API Controller Device Details VSMS: Determine if default controller exists for KVM Xu Wang (2): Add virtual controller device types Parse/Store controller XML tags Makefile.am | 2 + libvirt-cim.spec.in | 2 + libxkutil/device_parsing.c | 119 +++++++++++++++++++++++++++- libxkutil/device_parsing.h | 15 ++++ libxkutil/xmlgen.c | 72 +++++++++++++++++ schema/Controller.mof | 47 +++++++++++ schema/Controller.registration | 19 +++++ src/Virt_Device.c | 84 ++++++++++++++++++++ src/Virt_RASD.c | 21 +++-- src/Virt_RASD.h | 4 + src/Virt_VirtualSystemManagementService.c | 106 ++++++++++++++++++++++++- src/svpc_types.h | 127 ++++++++++++++++++++++++++++++ 12 files changed, 602 insertions(+), 16 deletions(-) create mode 100644 schema/Controller.mof create mode 100644 schema/Controller.registration -- 1.8.5.3
From: Xu Wang <gesaint@linux.vnet.ibm.com> Add data and strutures for a virtual controller device Signed-off-by: John Ferlan <jferlan@redhat.com> --- libxkutil/device_parsing.h | 15 ++++++ src/svpc_types.h | 127 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+) diff --git a/libxkutil/device_parsing.h b/libxkutil/device_parsing.h index 92427c1..545b46c 100644 --- a/libxkutil/device_parsing.h +++ b/libxkutil/device_parsing.h @@ -161,6 +161,17 @@ struct input_device { char *bus; }; +#define CONTROLLER_INDEX_NOT_SET -1 +struct controller_device { + uint16_t type; + uint64_t index; + char *model; + char *ports; + char *vectors; + char *queues; + struct device_address address; +}; + struct virt_device { uint16_t type; union { @@ -172,6 +183,7 @@ struct virt_device { struct graphics_device graphics; struct console_device console; struct input_device input; + struct controller_device controller; } dev; char *id; }; @@ -247,6 +259,9 @@ struct domain { struct virt_device *dev_vcpu; int dev_vcpu_ct; + + struct virt_device *dev_controller; + int dev_controller_ct; }; struct virt_device *virt_device_dup(struct virt_device *dev); diff --git a/src/svpc_types.h b/src/svpc_types.h index 404e428..b1add52 100644 --- a/src/svpc_types.h +++ b/src/svpc_types.h @@ -24,6 +24,9 @@ #define CIM_OPERATIONAL_STATUS 2 +/* From the ResourceType list for CIM_ResourceAllocationSettingData.html + * Found on http://schemas.dmtf.org/wbem/cim-html/2.31.0 + */ #define CIM_RES_TYPE_ALL 0 #define CIM_RES_TYPE_OTHER 1 #define CIM_RES_TYPE_PROC 3 @@ -33,9 +36,11 @@ #define CIM_RES_TYPE_GRAPHICS 24 #define CIM_RES_TYPE_INPUT 13 #define CIM_RES_TYPE_UNKNOWN 1000 +/* libvirt-cim specific values can start here */ #define CIM_RES_TYPE_IMAGE 32768 #define CIM_RES_TYPE_CONSOLE 32769 #define CIM_RES_TYPE_EMU 32770 +#define CIM_RES_TYPE_CONTROLLER 32771 #define CIM_RES_TYPE_COUNT 7 const static int cim_res_types[CIM_RES_TYPE_COUNT] = @@ -266,4 +271,126 @@ static inline const char* chardev_source_type_IDToStr(int type) return type_str; } +/* enum for Controller ProtocolSupported values + * + * From the ProtocolSupported list for CIM_Controller.html + * Found on http://schemas.dmtf.org/wbem/cim-html/2.31.0 + */ +enum CIM_controller_protocol_type { + CIM_CONTROLLER_PROTOCOL_TYPE_OTHER = 1, + CIM_CONTROLLER_PROTOCOL_TYPE_UNKNOWN = 2, + CIM_CONTROLLER_PROTOCOL_TYPE_EISA = 3, + CIM_CONTROLLER_PROTOCOL_TYPE_ISA = 4, + CIM_CONTROLLER_PROTOCOL_TYPE_PCI = 5, + CIM_CONTROLLER_PROTOCOL_TYPE_ATA = 6, + CIM_CONTROLLER_PROTOCOL_TYPE_FD = 7, + CIM_CONTROLLER_PROTOCOL_TYPE_1496 = 8, + CIM_CONTROLLER_PROTOCOL_TYPE_SCSI_PI = 9, + CIM_CONTROLLER_PROTOCOL_TYPE_SCSI_FC = 10, + CIM_CONTROLLER_PROTOCOL_TYPE_SCSI_SB = 11, + CIM_CONTROLLER_PROTOCOL_TYPE_SCSI_SB2 = 12, + CIM_CONTROLLER_PROTOCOL_TYPE_SCSI_SSA = 13, + CIM_CONTROLLER_PROTOCOL_TYPE_VESA = 14, + CIM_CONTROLLER_PROTOCOL_TYPE_PCMCIA = 15, + CIM_CONTROLLER_PROTOCOL_TYPE_USB = 16, + CIM_CONTROLLER_PROTOCOL_TYPE_PP = 17, + CIM_CONTROLLER_PROTOCOL_TYPE_ESCON = 18, + CIM_CONTROLLER_PROTOCOL_TYPE_DIAG = 19, + CIM_CONTROLLER_PROTOCOL_TYPE_I2C = 20, + CIM_CONTROLLER_PROTOCOL_TYPE_POWER = 21, + CIM_CONTROLLER_PROTOCOL_TYPE_HIPPI = 22, + CIM_CONTROLLER_PROTOCOL_TYPE_MULTIBUS = 23, + CIM_CONTROLLER_PROTOCOL_TYPE_VME = 24, + CIM_CONTROLLER_PROTOCOL_TYPE_IPI = 25, + CIM_CONTROLLER_PROTOCOL_TYPE_IEEE488 = 26, + CIM_CONTROLLER_PROTOCOL_TYPE_RS232 = 27, + CIM_CONTROLLER_PROTOCOL_TYPE_IEEE8023_10BASE5 = 28, + CIM_CONTROLLER_PROTOCOL_TYPE_IEEE8023_10BASE2 = 29, + CIM_CONTROLLER_PROTOCOL_TYPE_IEEE8023_1BASE5 = 30, + CIM_CONTROLLER_PROTOCOL_TYPE_IEEE8023_10BROAD36 = 31, + CIM_CONTROLLER_PROTOCOL_TYPE_IEEE8023_100BASEVG = 32, + CIM_CONTROLLER_PROTOCOL_TYPE_TOKENRING = 33, + CIM_CONTROLLER_PROTOCOL_TYPE_ANSIX3T9 = 34, + CIM_CONTROLLER_PROTOCOL_TYPE_MCA = 35, + CIM_CONTROLLER_PROTOCOL_TYPE_ESDI = 36, + CIM_CONTROLLER_PROTOCOL_TYPE_IDE = 37, + CIM_CONTROLLER_PROTOCOL_TYPE_CMD = 38, + CIM_CONTROLLER_PROTOCOL_TYPE_ST506 = 39, + CIM_CONTROLLER_PROTOCOL_TYPE_DSSI = 40, + CIM_CONTROLLER_PROTOCOL_TYPE_QIC2 = 41, + CIM_CONTROLLER_PROTOCOL_TYPE_ENH_ATA = 42, + CIM_CONTROLLER_PROTOCOL_TYPE_AGP = 43, + CIM_CONTROLLER_PROTOCOL_TYPE_TWIRP = 44, + CIM_CONTROLLER_PROTOCOL_TYPE_FIR = 45, + CIM_CONTROLLER_PROTOCOL_TYPE_SIR = 46, + CIM_CONTROLLER_PROTOCOL_TYPE_IRBUS = 47, + CIM_CONTROLLER_PROTOCOL_TYPE_SATA = 48, + /* libvirt specific */ + CIM_CONTROLLER_PROTOCOL_TYPE_CCID = 32678, + CIM_CONTROLLER_PROTOCOL_TYPE_VIRTIO_SERIAL = 32769, +}; + +static inline int controller_protocol_type_StrToID(const char *type_str) +{ + int rc = CIM_CONTROLLER_PROTOCOL_TYPE_UNKNOWN; + + if (type_str == NULL) + return rc; + + if (STREQC(type_str, "ide")) + rc = CIM_CONTROLLER_PROTOCOL_TYPE_IDE; + else if (STREQC(type_str, "fdc")) + rc = CIM_CONTROLLER_PROTOCOL_TYPE_FD; + else if (STREQC(type_str, "scsi")) + rc = CIM_CONTROLLER_PROTOCOL_TYPE_SCSI_SSA; /* REVISIT */ + else if (STREQC(type_str, "sata")) + rc = CIM_CONTROLLER_PROTOCOL_TYPE_SATA; + else if (STREQC(type_str, "ccid")) + rc = CIM_CONTROLLER_PROTOCOL_TYPE_CCID; + else if (STREQC(type_str, "virtio-serial")) + rc = CIM_CONTROLLER_PROTOCOL_TYPE_VIRTIO_SERIAL; + else if (STREQC(type_str, "pci")) + rc = CIM_CONTROLLER_PROTOCOL_TYPE_PCI; + else if (STREQC(type_str, "usb")) + rc = CIM_CONTROLLER_PROTOCOL_TYPE_USB; + + return rc; +} + +static inline const char* controller_protocol_type_IDToStr(int type) +{ + char *type_str = NULL; + + switch (type) + { + case CIM_CONTROLLER_PROTOCOL_TYPE_IDE: + type_str = "ide"; + break; + case CIM_CONTROLLER_PROTOCOL_TYPE_FD: + type_str = "fdc"; + break; + case CIM_CONTROLLER_PROTOCOL_TYPE_SCSI_SSA: + type_str = "scsi"; + break; + case CIM_CONTROLLER_PROTOCOL_TYPE_SATA: + type_str = "sata"; + break; + case CIM_CONTROLLER_PROTOCOL_TYPE_CCID: + type_str = "ccid"; + break; + case CIM_CONTROLLER_PROTOCOL_TYPE_VIRTIO_SERIAL: + type_str = "virtio-serial"; + break; + case CIM_CONTROLLER_PROTOCOL_TYPE_PCI: + type_str = "pci"; + break; + case CIM_CONTROLLER_PROTOCOL_TYPE_USB: + type_str = "usb"; + break; + default: + break; + } + return type_str; +} + #endif -- 1.8.5.3
Add the mofs and registration of the new virtual contoller device, which will be a subclass of the CIM_Controller class. Signed-off-by: John Ferlan <jferlan@redhat.com> --- Makefile.am | 2 ++ libvirt-cim.spec.in | 2 ++ schema/Controller.mof | 47 ++++++++++++++++++++++++++++++++++++++++++ schema/Controller.registration | 19 +++++++++++++++++ 4 files changed, 70 insertions(+) create mode 100644 schema/Controller.mof create mode 100644 schema/Controller.registration diff --git a/Makefile.am b/Makefile.am index 69b65cf..864047c 100644 --- a/Makefile.am +++ b/Makefile.am @@ -53,6 +53,7 @@ MOFS = \ $(top_srcdir)/schema/KVMRedirectionSAP.mof \ $(top_srcdir)/schema/DisplayController.mof \ $(top_srcdir)/schema/PointingDevice.mof \ + $(top_srcdir)/schema/Controller.mof \ $(top_srcdir)/schema/GraphicsPool.mof \ $(top_srcdir)/schema/InputPool.mof \ $(top_srcdir)/schema/HostedAccessPoint.mof \ @@ -143,6 +144,7 @@ REGS = \ $(top_srcdir)/schema/KVMRedirectionSAP.registration \ $(top_srcdir)/schema/DisplayController.registration \ $(top_srcdir)/schema/PointingDevice.registration \ + $(top_srcdir)/schema/Controller.registration \ $(top_srcdir)/schema/GraphicsPool.registration \ $(top_srcdir)/schema/InputPool.registration \ $(top_srcdir)/schema/HostedAccessPoint.registration \ diff --git a/libvirt-cim.spec.in b/libvirt-cim.spec.in index 01ee329..4e331d9 100644 --- a/libvirt-cim.spec.in +++ b/libvirt-cim.spec.in @@ -111,6 +111,7 @@ rm -fr $RPM_BUILD_ROOT %{_datadir}/%{name}/KVMRedirectionSAP.registration \\\ %{_datadir}/%{name}/DisplayController.registration \\\ %{_datadir}/%{name}/PointingDevice.registration \\\ + %{_datadir}/%{name}/Controller.registration \\\ %{_datadir}/%{name}/GraphicsPool.registration \\\ %{_datadir}/%{name}/InputPool.registration \\\ %{_datadir}/%{name}/HostedAccessPoint.registration \\\ @@ -172,6 +173,7 @@ rm -fr $RPM_BUILD_ROOT %{_datadir}/%{name}/KVMRedirectionSAP.mof \\\ %{_datadir}/%{name}/DisplayController.mof \\\ %{_datadir}/%{name}/PointingDevice.mof \\\ + %{_datadir}/%{name}/Controller.mof \\\ %{_datadir}/%{name}/GraphicsPool.mof \\\ %{_datadir}/%{name}/InputPool.mof \\\ %{_datadir}/%{name}/HostedAccessPoint.mof \\\ diff --git a/schema/Controller.mof b/schema/Controller.mof new file mode 100644 index 0000000..a8cb06a --- /dev/null +++ b/schema/Controller.mof @@ -0,0 +1,47 @@ + +// Copyright Red Hat Corp. 2014 +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Lesser General Public +// License as published by the Free Software Foundation; either +// version 2.1 of the License, or (at your option) any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public +// License along with this library. If not, see +// <http://www.gnu.org/licenses/>. +// + +[Description ("KVM virtual controller device. It is identified by: " + "CIM_Controller.ProtocolSupported which is set to the " + "libvirt controller 'type' value converted to an uint16. " + "Additionally, the libvirt 'model' field will be used to " + "set the CIM_Controller.ProtocolDescription."), + Provider("cmpi::Virt_Device") +] +class KVM_Controller : CIM_Controller +{ + [Description ("Order in which the bus controller is encountered. " + "The order is controller type scoped.")] + uint64 Index; + + [Description ("The 'virtio-serial' controller optionally uses the Ports " + "and Vectors to control how many devices can be connected " + "through the controller.")] + string Ports; + string Vectors; + + [Description ("Optional number of queues for the controller.")] + string Queues; + + [Description ("For controllers that are themselves devices on a " + "bus an optional element to specify the exact " + "relationship of the controller to its master bus. " + "Stored in the property and value arrays.")] + string AddressProperties[]; + string AddressValues[]; +}; diff --git a/schema/Controller.registration b/schema/Controller.registration new file mode 100644 index 0000000..7e06331 --- /dev/null +++ b/schema/Controller.registration @@ -0,0 +1,19 @@ +# Copyright Red Hat Corp. 2014 +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library. If not, see +# <http://www.gnu.org/licenses/>. +# + +# Classname Namespace ProviderName ProviderModule ProviderTypes +KVM_Controller root/virt Virt_Device Virt_Device instance -- 1.8.5.3
From: Xu Wang <gesaint@linux.vnet.ibm.com> Parse and store the <controller> XML tags. The generated DeviceID will be a combination of "controller" + XML device "name" + XML Index "value". This should be unique enough now and generates "controller:pci:0" or "controller:usb:0" DeviceID's. In the future, if support is added for the "<master>" XML child a new mechanism will need to be put in place since one of those controllers is designed to have the same name and index - what changes is the function for the device. Signed-off-by: John Ferlan <jferlan@redhat.com> --- libxkutil/device_parsing.c | 119 ++++++++++++++++++++++++++++++++++++++++++++- libxkutil/xmlgen.c | 72 +++++++++++++++++++++++++++ 2 files changed, 190 insertions(+), 1 deletion(-) diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index c9ae886..e68a950 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -1,5 +1,5 @@ /* - * Copyright IBM Corp. 2007, 2013 + * Copyright IBM Corp. 2007, 2014 * * Authors: * Dan Smith <danms@us.ibm.com> @@ -49,6 +49,7 @@ #define GRAPHICS_XPATH (xmlChar *)"/domain/devices/graphics | "\ "/domain/devices/console" #define INPUT_XPATH (xmlChar *)"/domain/devices/input" +#define CONTROLLER_XPATH (xmlChar *)"/domain/devices/controller" #define DEFAULT_BRIDGE "xenbr0" #define DEFAULT_NETWORK "default" @@ -69,6 +70,7 @@ static void cleanup_device_address(struct device_address *addr) if (addr == NULL) return; + CU_DEBUG("Cleanup %d addresses", addr->ct); for (i = 0; i < addr->ct; i++) { free(addr->key[i]); free(addr->value[i]); @@ -84,6 +86,7 @@ static void cleanup_disk_device(struct disk_device *dev) if (dev == NULL) return; + CU_DEBUG("Clean Disk type %s", dev->type); free(dev->type); free(dev->device); free(dev->driver); @@ -101,6 +104,7 @@ static void cleanup_vsi_device(struct vsi_device *dev) if (dev == NULL) return; + CU_DEBUG("Clean VSI type %s", dev->vsi_type); free(dev->vsi_type); free(dev->manager_id); free(dev->type_id); @@ -115,6 +119,7 @@ static void cleanup_net_device(struct net_device *dev) if (dev == NULL) return; + CU_DEBUG("Clean net type %s", dev->type); free(dev->type); free(dev->mac); free(dev->source); @@ -132,6 +137,7 @@ static void cleanup_emu_device(struct emu_device *dev) if (dev == NULL) return; + CU_DEBUG("Clean emu %s", dev->path); free(dev->path); } @@ -155,6 +161,7 @@ static void cleanup_graphics_device(struct graphics_device *dev) if (dev == NULL || dev->type == NULL) return; + CU_DEBUG("Clean graphics type %s", dev->type); if (STREQC(dev->type, "sdl")) cleanup_sdl_device(dev); else @@ -168,6 +175,7 @@ static void cleanup_path_device(struct path_device *dev) if (dev == NULL) return; + CU_DEBUG("Clean path device %s", dev->path); free(dev->path); } @@ -177,6 +185,7 @@ static void cleanup_unixsock_device(struct unixsock_device *dev) if (dev == NULL) return; + CU_DEBUG("Clean unixsock device"); free(dev->path); free(dev->mode); @@ -187,6 +196,7 @@ static void cleanup_tcp_device(struct tcp_device *dev) if (dev == NULL) return; + CU_DEBUG("Clean tcp device"); free(dev->mode); free(dev->protocol); free(dev->host); @@ -199,6 +209,7 @@ static void cleanup_udp_device(struct udp_device *dev) if (dev == NULL) return; + CU_DEBUG("Clean udb bind device"); free(dev->bind_host); free(dev->bind_service); free(dev->connect_host); @@ -210,6 +221,7 @@ static void cleanup_console_device(struct console_device *dev) if (dev == NULL) return; + CU_DEBUG("Clean console source_type %d", dev->source_type); switch (dev->source_type) { case CIM_CHARDEV_SOURCE_TYPE_PTY: @@ -302,10 +314,24 @@ static void cleanup_input_device(struct input_device *dev) if (dev == NULL) return; + CU_DEBUG("Clean input device %s", dev->type); free(dev->type); free(dev->bus); } +static void cleanup_controller_device(struct controller_device *dev) +{ + if (dev == NULL) + return; + + CU_DEBUG("Clean controller device %d", dev->type); + free(dev->model); + free(dev->queues); + free(dev->ports); + free(dev->vectors); + cleanup_device_address(&dev->address); +} + void cleanup_virt_device(struct virt_device *dev) { if (dev == NULL) @@ -323,6 +349,8 @@ void cleanup_virt_device(struct virt_device *dev) cleanup_input_device(&dev->dev.input); else if (dev->type == CIM_RES_TYPE_CONSOLE) cleanup_console_device(&dev->dev.console); + else if (dev->type == CIM_RES_TYPE_CONTROLLER) + cleanup_controller_device(&dev->dev.controller); free(dev->id); @@ -337,6 +365,7 @@ void cleanup_virt_devices(struct virt_device **_devs, int count) for (i = 0; i < count; i++) cleanup_virt_device(&devs[i]); + CU_DEBUG("All devices cleaned"); free(devs); *_devs = NULL; } @@ -1101,6 +1130,75 @@ static int parse_input_device(xmlNode *node, struct virt_device **vdevs) return 0; } +static int parse_controller_device(xmlNode *cnode, struct virt_device **vdevs) +{ + struct virt_device *vdev = NULL; + struct controller_device *cdev = NULL; + char *type_str = NULL; + xmlNode *child = NULL; + char *index = NULL; + int ret; + + vdev = calloc(1, sizeof(*vdev)); + if (vdev == NULL) + goto err; + + cdev = &(vdev->dev.controller); + + type_str = get_attr_value(cnode, "type"); + if (type_str == NULL) { + CU_DEBUG("No type"); + goto err; + } + CU_DEBUG("controller device type = %s", type_str); + cdev->type = controller_protocol_type_StrToID(type_str); + if (cdev->type == CIM_CONTROLLER_PROTOCOL_TYPE_UNKNOWN) { + CU_DEBUG("Unknown controller protocol type (%d)", cdev->type); + goto err; + } + + index = get_attr_value(cnode, "index"); + if (index != NULL) { + sscanf(index, "%" PRIu64, &cdev->index); + free(index); + } else { + CU_DEBUG("No index"); + goto err; + } + + cdev->model = get_attr_value(cnode, "model"); + cdev->ports = get_attr_value(cnode, "ports"); + cdev->vectors = get_attr_value(cnode, "vectors"); + + for (child = cnode->children; child != NULL; child = child->next) { + if (XSTREQ(child->name, "address")) { + parse_device_address(child, &cdev->address); + } else if (XSTREQ(child->name, "driver")) { + cdev->queues = get_attr_value(child, "queues"); + } + } + vdev->type = CIM_RES_TYPE_CONTROLLER; + + ret = asprintf(&vdev->id, "controller:%s:%" PRIu64, + type_str, cdev->index); + if (ret == -1) { + CU_DEBUG("Failed to create controller id string"); + goto err; + } + CU_DEBUG("Controller id is %s", vdev->id); + free(type_str); + + *vdevs = vdev; + + return 1; + err: + free(type_str); + cleanup_controller_device(cdev); + free(vdev); + + return 0; +} + static bool resize_devlist(struct virt_device **list, int newsize) { struct virt_device *_list; @@ -1224,6 +1322,11 @@ static int parse_devices(const char *xml, struct virt_device **_list, int type) func = &parse_input_device; break; + case CIM_RES_TYPE_CONTROLLER: + xpathstr = CONTROLLER_XPATH; + func = &parse_controller_device; + break; + default: CU_DEBUG("Unrecognized device type. Returning."); goto err1; @@ -1343,7 +1446,17 @@ struct virt_device *virt_device_dup(struct virt_device *_dev) } else if (dev->type == CIM_RES_TYPE_CONSOLE) { console_device_dup(&dev->dev.console, &_dev->dev.console); + } else if (dev->type == CIM_RES_TYPE_CONTROLLER) { + dev->dev.controller.type = _dev->dev.controller.type; + dev->dev.controller.index = _dev->dev.controller.index; + DUP_FIELD(dev, _dev, dev.controller.model); + DUP_FIELD(dev, _dev, dev.controller.ports); + DUP_FIELD(dev, _dev, dev.controller.vectors); + DUP_FIELD(dev, _dev, dev.controller.queues); + duplicate_device_address(&dev->dev.controller.address, + &_dev->dev.controller.address); } + return dev; } @@ -1723,6 +1836,9 @@ int get_dominfo_from_xml(const char *xml, struct domain **dominfo) (*dominfo)->dev_vcpu_ct = parse_devices(xml, &(*dominfo)->dev_vcpu, CIM_RES_TYPE_PROC); + (*dominfo)->dev_controller_ct = parse_devices(xml, + &(*dominfo)->dev_controller, + CIM_RES_TYPE_CONTROLLER); return ret; @@ -1811,6 +1927,7 @@ void cleanup_dominfo(struct domain **dominfo) cleanup_virt_devices(&dom->dev_graphics, dom->dev_graphics_ct); cleanup_virt_devices(&dom->dev_input, dom->dev_input_ct); cleanup_virt_devices(&dom->dev_console, dom->dev_console_ct); + cleanup_virt_devices(&dom->dev_controller, dom->dev_controller_ct); free(dom); diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c index 931f0c9..a8a00ad 100644 --- a/libxkutil/xmlgen.c +++ b/libxkutil/xmlgen.c @@ -794,6 +794,72 @@ static const char *input_xml(xmlNodePtr root, struct domain *dominfo) return NULL; } +static const char *controller_xml(xmlNodePtr root, struct domain *dominfo) +{ + int i; + const char *msg = NULL; + + CU_DEBUG("Found %d controllers", dominfo->dev_controller_ct); + for (i = 0; i < dominfo->dev_controller_ct; i++) { + xmlNodePtr ctlr; + xmlNodePtr tmp; + char *type_str; + + struct virt_device *_dev = &dominfo->dev_controller[i]; + if (_dev->type == CIM_RES_TYPE_UNKNOWN) + continue; + + struct controller_device *cdev = &_dev->dev.controller; + + ctlr = xmlNewChild(root, NULL, BAD_CAST "controller", NULL); + if (ctlr == NULL) + return XML_ERROR; + + + type_str = controller_protocol_type_IDToStr(cdev->type); + if (type_str == NULL) + return XML_ERROR; + + CU_DEBUG("Type=%s Index=%" PRIu64, type_str, cdev->index); + xmlNewProp(ctlr, BAD_CAST "type", + BAD_CAST type_str); + + /* If index is missing, let libvirt generate it */ + if (cdev->index != CONTROLLER_INDEX_NOT_SET) { + char *index; + if (asprintf(&index, "%" PRIu64, cdev->index) == -1) + return XML_ERROR; + xmlNewProp(ctlr, BAD_CAST "index", BAD_CAST index); + free(index); + } + + /* Optional */ + if (cdev->model) + xmlNewProp(ctlr, BAD_CAST "model", + BAD_CAST cdev->model); + if (cdev->ports) + xmlNewProp(ctlr, BAD_CAST "ports", + BAD_CAST cdev->ports); + if (cdev->vectors) + xmlNewProp(ctlr, BAD_CAST "vectors", + BAD_CAST cdev->vectors); + if (cdev->queues) { + tmp = xmlNewChild(ctlr, NULL, BAD_CAST "driver", NULL); + xmlNewProp(tmp, BAD_CAST "queueus", + BAD_CAST cdev->queues); + } + if (cdev->address.ct > 0) { + msg = device_address_xml(ctlr, &cdev->address); + if (msg != NULL) { + CU_DEBUG("Failed to set the address"); + return msg; + } + } + } + + return NULL; +} + static char *system_xml(xmlNodePtr root, struct domain *domain) { xmlNodePtr tmp; @@ -1125,6 +1191,11 @@ char *device_to_xml(struct virt_device *_dev) dominfo->dev_input_ct = 1; dominfo->dev_input = dev; break; + case CIM_RES_TYPE_CONTROLLER: + func = controller_xml; + dominfo->dev_controller_ct = 1; + dominfo->dev_controller = dev; + break; default: cleanup_virt_devices(&dev, 1); goto out; @@ -1163,6 +1234,7 @@ char *system_to_xml(struct domain *dominfo) &console_xml, &graphics_xml, &emu_xml, + &controller_xml, NULL }; -- 1.8.5.3
Change and rename the static set_rasd_device_address() to a global set_device_address() which can be shared with Virt_Device.c to handle controller addresses. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/Virt_RASD.c | 21 +++++++++------------ src/Virt_RASD.h | 4 ++++ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/Virt_RASD.c b/src/Virt_RASD.c index df1e921..a06714e 100644 --- a/src/Virt_RASD.c +++ b/src/Virt_RASD.c @@ -289,10 +289,9 @@ static bool get_vol_size(const CMPIBroker *broker, } #endif -static CMPIStatus set_rasd_device_address(const CMPIBroker *broker, - const CMPIObjectPath *ref, - const struct device_address *addr, - CMPIInstance *inst) +CMPIStatus set_device_address(const CMPIBroker *broker, + const struct device_address *addr, + CMPIInstance *inst) { int i; CMPIArray *arr_key; @@ -489,10 +488,9 @@ static CMPIStatus set_disk_rasd_params(const CMPIBroker *broker, CMPI_boolean); if (dev->dev.disk.address.ct > 0) - set_rasd_device_address(broker, - ref, - &dev->dev.disk.address, - inst); + set_device_address(broker, + &dev->dev.disk.address, + inst); virStoragePoolFree(pool); virStorageVolFree(vol); @@ -658,10 +656,9 @@ static CMPIStatus set_net_rasd_params(const CMPIBroker *broker, CMPI_chars); if (dev->dev.net.address.ct > 0) - set_rasd_device_address(broker, - ref, - &dev->dev.net.address, - inst); + set_device_address(broker, + &dev->dev.net.address, + inst); #if LIBVIR_VERSION_NUMBER < 9000 out: diff --git a/src/Virt_RASD.h b/src/Virt_RASD.h index 400143f..c96cf43 100644 --- a/src/Virt_RASD.h +++ b/src/Virt_RASD.h @@ -69,6 +69,10 @@ int list_rasds(virConnectPtr conn, const char *host, struct virt_device **list); +CMPIStatus set_device_address(const CMPIBroker *broker, + const struct device_address *addr, + CMPIInstance *inst); + CMPIInstance *rasd_from_vdev(const CMPIBroker *broker, struct virt_device *dev, const char *host, -- 1.8.5.3
Set the various fields for the KVM_Contoller instance based on the read XML from the virtual controller device structure. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/Virt_Device.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/src/Virt_Device.c b/src/Virt_Device.c index 12ae6bd..5f48fae 100644 --- a/src/Virt_Device.c +++ b/src/Virt_Device.c @@ -366,6 +366,83 @@ static CMPIInstance *input_instance(const CMPIBroker *broker, return inst; } +static int controller_set_attr(const CMPIBroker *broker, + CMPIInstance *instance, + struct controller_device *dev) +{ + const char *type_str; + + type_str = controller_protocol_type_IDToStr(dev->type); + if (type_str == NULL) { + CU_DEBUG("controller type=%d fails to return string", dev->type); + return 0; + } + + /* Use 0 for unknown or unlimited - although probably + * unnecessary since the struct is calloc'd + */ + CMSetProperty(instance, "MaxNumberControlled", + 0, CMPI_uint32); + + CMSetProperty(instance, "ProtocolSupported", + (CMPIValue *)dev->type, + CMPI_uint16); + + CMSetProperty(instance, "Index", + (CMPIValue *)dev->index, + CMPI_uint64); + + if (dev->model) + CMSetProperty(instance, "ProtocolDescription", + (CMPIValue *)dev->model, + CMPI_chars); + if (dev->ports) + CMSetProperty(instance, "Ports", + (CMPIValue *)dev->ports, + CMPI_chars); + if (dev->vectors) + CMSetProperty(instance, "Vectors", + (CMPIValue *)dev->vectors, + CMPI_chars); + if (dev->queues) + CMSetProperty(instance, "Queues", + (CMPIValue *)dev->queues, + CMPI_chars); + if (dev->address.ct > 0) + set_device_address(broker, &dev->address, instance); + + return 1; +} + +static CMPIInstance *controller_instance(const CMPIBroker *broker, + struct controller_device *dev, + const virDomainPtr dom, + const char *ns) +{ + CMPIInstance *inst; + virConnectPtr conn; + + conn = virDomainGetConnect(dom); + inst = get_typed_instance(broker, + pfx_from_conn(conn), + "Controller", + ns, + true); + if (inst == NULL) { + CU_DEBUG("Failed to get instance of %s_Controller", + pfx_from_conn(conn)); + return NULL; + } + + + if (!controller_set_attr(broker, inst, dev)) { + CU_DEBUG("Failed to set contoller attributes of %s_Controller", + pfx_from_conn(conn)); + return NULL; + } + + return inst; +} static int device_set_devid(CMPIInstance *instance, struct virt_device *dev, const virDomainPtr dom) @@ -521,6 +598,11 @@ static bool device_instances(const CMPIBroker *broker, &dev->dev.input, dom, ns); + else if (dev->type == CIM_RES_TYPE_CONTROLLER) + instance = controller_instance(broker, + &dev->dev.controller, + dom, + ns); else return false; @@ -555,6 +637,8 @@ uint16_t res_type_from_device_classname(const char *classname) return CIM_RES_TYPE_GRAPHICS; else if (strstr(classname, "PointingDevice")) return CIM_RES_TYPE_INPUT; + else if (strstr(classname, "Controller")) + return CIM_RES_TYPE_CONTROLLER; else return CIM_RES_TYPE_UNKNOWN; } -- 1.8.5.3
If a default controller for 'pci' with model 'pci-root' doesn't exist, create one for any add/modify type event. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/Virt_VirtualSystemManagementService.c | 106 +++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 3 deletions(-) diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 3e7785e..4610374 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -659,6 +659,82 @@ static bool default_input_device(struct domain *domain) return true; } +static bool check_default_controller_device(struct domain *domain) +{ + int i; + struct controller_device *cdev = NULL; + + CU_DEBUG("Domain '%s' type=%d", domain->name, domain->type); + + /* Only QEMU/KVM devices need controllers */ + if (domain->type != DOMAIN_KVM && domain->type != DOMAIN_QEMU) + return true; + + /* Do we find a 'pci' device with 'pci-root' model on the system? + * If not, create one. + */ + for (i = 0; i < domain->dev_controller_ct; i++) { + struct virt_device *_dev = &domain->dev_controller[i]; + if (_dev->type == CIM_RES_TYPE_UNKNOWN) { + CU_DEBUG("controller _dev->type is unknown") + continue; + } + + cdev = &_dev->dev.controller; + + CU_DEBUG("controller type=%d model=%s", + cdev->type, cdev->model ? cdev->model : "unknown"); + + if (cdev->type == CIM_CONTROLLER_PROTOCOL_TYPE_PCI && + cdev->model != NULL && + STREQC(cdev->model, "pci-root")) { + CU_DEBUG("Found 'pci-root' for %s", domain->name); + break; + } + cdev = NULL; + } + + /* Didn't find 'pci'/'pci-root', let's add it */ + if (cdev == NULL) { + char *model; + struct virt_device *_list; + struct virt_device *_dev; + + CU_DEBUG("Domain '%s' missing 'pci-root'", domain->name); + + model = strdup("pci-root"); + if (model == NULL) { + CU_DEBUG("Fail to allocate default controller model"); + return false; + } + + if (domain->dev_controller == NULL) + _list = calloc(1, sizeof(*domain->dev_controller)); + else + _list = realloc(domain->dev_controller, + (domain->dev_controller_ct + 1) * + sizeof(struct virt_device)); + if (_list == NULL) { + CU_DEBUG("Fail to allocate controller model"); + free(model); + return false; + } + domain->dev_controller = _list; + + _dev = &domain->dev_controller[domain->dev_controller_ct]; + _dev->type = CIM_RES_TYPE_CONTROLLER; + + cdev = &_dev->dev.controller; + memset(cdev, 0, sizeof(*cdev)); + cdev->type = CIM_CONTROLLER_PROTOCOL_TYPE_PCI; + cdev->index = 0; /* By definition it must be 0 */ + cdev->model = model; + domain->dev_controller_ct += 1; + } + + return true; +} + static bool add_default_devs(struct domain *domain) { if (domain->dev_graphics_ct < 1 && @@ -2545,8 +2621,16 @@ static CMPIInstance *create_system(const CMPIContext *context, goto out; } + if (!check_default_controller_device(domain)) { + CU_DEBUG("Failed to check default controller"); + cu_statusf(_BROKER, s, + CMPI_RC_ERR_FAILED, + "Failed to create default controller"); + goto out; + } + xml = system_to_xml(domain); - CU_DEBUG("System XML:\n%s", xml); + CU_DEBUG("create_system XML:\n%s", xml); inst = connect_and_create(xml, ref, s); if (inst != NULL) { @@ -2815,9 +2899,17 @@ static CMPIStatus update_system_settings(const CMPIContext *context, goto out; } + if (!check_default_controller_device(dominfo)) { + CU_DEBUG("Failed to check default controller"); + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Failed to create default controller"); + goto out; + } + xml = system_to_xml(dominfo); if (xml != NULL) { - CU_DEBUG("New XML is:\n%s", xml); + CU_DEBUG("update_system_settings XML:\n%s", xml); connect_and_create(xml, ref, &s); } @@ -3310,9 +3402,17 @@ static CMPIStatus _update_resources_for(const CMPIContext *context, goto out; } + if (!check_default_controller_device(dominfo)) { + CU_DEBUG("Failed to check default controller"); + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Failed to create default controller"); + goto out; + } + xml = system_to_xml(dominfo); if (xml != NULL) { - CU_DEBUG("New XML:\n%s", xml); + CU_DEBUG("_update_resources_for XML:\n%s", xml); connect_and_create(xml, ref, &s); if (inst_list_add(&list, rasd) == 0) { -- 1.8.5.3
On 03/18/2014 10:49 PM, John Ferlan wrote:
If a default controller for 'pci' with model 'pci-root' doesn't exist, create one for any add/modify type event.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/Virt_VirtualSystemManagementService.c | 106 +++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 3 deletions(-)
diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 3e7785e..4610374 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -659,6 +659,82 @@ static bool default_input_device(struct domain *domain) return true; }
+static bool check_default_controller_device(struct domain *domain) +{ + int i; + struct controller_device *cdev = NULL; + + CU_DEBUG("Domain '%s' type=%d", domain->name, domain->type); + + /* Only QEMU/KVM devices need controllers */ + if (domain->type != DOMAIN_KVM && domain->type != DOMAIN_QEMU) + return true; + + /* Do we find a 'pci' device with 'pci-root' model on the system? + * If not, create one. + */ + for (i = 0; i < domain->dev_controller_ct; i++) { + struct virt_device *_dev = &domain->dev_controller[i]; + if (_dev->type == CIM_RES_TYPE_UNKNOWN) { + CU_DEBUG("controller _dev->type is unknown") + continue; + } + + cdev = &_dev->dev.controller; + + CU_DEBUG("controller type=%d model=%s", + cdev->type, cdev->model ? cdev->model : "unknown"); + + if (cdev->type == CIM_CONTROLLER_PROTOCOL_TYPE_PCI && + cdev->model != NULL && + STREQC(cdev->model, "pci-root")) { + CU_DEBUG("Found 'pci-root' for %s", domain->name); + break; + } + cdev = NULL; + } + + /* Didn't find 'pci'/'pci-root', let's add it */ + if (cdev == NULL) { + char *model; + struct virt_device *_list; + struct virt_device *_dev; + + CU_DEBUG("Domain '%s' missing 'pci-root'", domain->name); + + model = strdup("pci-root"); + if (model == NULL) { + CU_DEBUG("Fail to allocate default controller model"); + return false; + } + + if (domain->dev_controller == NULL) + _list = calloc(1, sizeof(*domain->dev_controller)); + else + _list = realloc(domain->dev_controller, + (domain->dev_controller_ct + 1) * + sizeof(struct virt_device)); + if (_list == NULL) { + CU_DEBUG("Fail to allocate controller model"); + free(model); + return false; + } + domain->dev_controller = _list; + + _dev = &domain->dev_controller[domain->dev_controller_ct]; + _dev->type = CIM_RES_TYPE_CONTROLLER; + + cdev = &_dev->dev.controller; + memset(cdev, 0, sizeof(*cdev)); + cdev->type = CIM_CONTROLLER_PROTOCOL_TYPE_PCI; + cdev->index = 0; /* By definition it must be 0 */ + cdev->model = model; + domain->dev_controller_ct += 1; + } + + return true; +} + static bool add_default_devs(struct domain *domain) { if (domain->dev_graphics_ct < 1 && @@ -2545,8 +2621,16 @@ static CMPIInstance *create_system(const CMPIContext *context, goto out; }
+ if (!check_default_controller_device(domain)) { + CU_DEBUG("Failed to check default controller"); + cu_statusf(_BROKER, s, + CMPI_RC_ERR_FAILED, + "Failed to create default controller"); + goto out; + } + xml = system_to_xml(domain); - CU_DEBUG("System XML:\n%s", xml); + CU_DEBUG("create_system XML:\n%s", xml);
inst = connect_and_create(xml, ref, s); if (inst != NULL) { @@ -2815,9 +2899,17 @@ static CMPIStatus update_system_settings(const CMPIContext *context, goto out; }
+ if (!check_default_controller_device(dominfo)) { + CU_DEBUG("Failed to check default controller"); + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Failed to create default controller"); + goto out; + } + xml = system_to_xml(dominfo); if (xml != NULL) { - CU_DEBUG("New XML is:\n%s", xml); + CU_DEBUG("update_system_settings XML:\n%s", xml); connect_and_create(xml, ref, &s); }
@@ -3310,9 +3402,17 @@ static CMPIStatus _update_resources_for(const CMPIContext *context, goto out; }
+ if (!check_default_controller_device(dominfo)) { + CU_DEBUG("Failed to check default controller"); + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Failed to create default controller"); + goto out; + } + xml = system_to_xml(dominfo); if (xml != NULL) { - CU_DEBUG("New XML:\n%s", xml); + CU_DEBUG("_update_resources_for XML:\n%s", xml); connect_and_create(xml, ref, &s);
if (inst_list_add(&list, rasd) == 0) {
Running the changes through Coverity resulted in a simple change... Although totally unnecessary - Coverity complained that comparing dev_controller to NULL in order to decide whether to calloc or realloc, could result in a data overrun if dev_controller_ct was greater than 0. Changing the comparison to check dev_controller_ct kept Coverity quiet. Consider the following diff squashed in diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 4610374..7958537 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -708,7 +708,7 @@ static bool check_default_controller_device(struct domain *d return false; } - if (domain->dev_controller == NULL) + if (domain->dev_controller_ct == 0) _list = calloc(1, sizeof(*domain->dev_controller)); else _list = realloc(domain->dev_controller,
On 03/19/2014 03:49 AM, John Ferlan wrote:
This is rework of the Controller patch series submitted by Xu Wang. I have built upon the constructs put in place by Xu with respect to reading and writing the XML, but for the remainder of the code I changed where the virtual controller lives.
Xu's patches made a Controller RASD which had some issues regarding associations with other devices and pools.
Rather than go with a RASD model, I have chosen to make a KVM_Controller virtual device to mimic a CIM_Controller device. This solution keeps things In doing so you broke the DMTF Virtual System Profile! http://www.dmtf.org/sites/default/files/standards/documents/DSP1057_1.0.0_0.... Take a look at 7.3.1 which defines a Virtual System Configuration and a Virtual System Representation. It states the class concepts. You need to use RASDs.
at a lower level and has been able to pass the cimtest as well as the update a defined domain with a changed value test. This second test is what fails for RHEL7.
As it turns out, it seems as long as a 'pci' device with model 'pci-root' is created, then things seem to work. The code relies on the previous patch code Xu created to read/write the XML file with some adjustments.
Perhaps the only controversial patch (for me at least) is 6/6. I figured that after we've read everything and just before we go to create or update the guest that we need to make sure that at least the 'pci' device with model 'pci-root' exists. This is similar to the add_default_devs() code, Actually libvirt-cim should not be creating default devices but let libvirt do that. Reimplementing libvirt's default behavior would a overestimation of libvirt-cim's own capabilities and in the long run cause extra effort in maintaining default consistency.
except that it's run after all that code prior to any add or update of the guest just before the "system_to_xml()" call is made. I figure this is the last gasp to ensure that at least the 'pci' device is there which seems to be used by a number of "silently added" libvirt devices.
John Ferlan (4): Add virtual controller object defs Change static API to global API Controller Device Details VSMS: Determine if default controller exists for KVM
Xu Wang (2): Add virtual controller device types Parse/Store controller XML tags
Makefile.am | 2 + libvirt-cim.spec.in | 2 + libxkutil/device_parsing.c | 119 +++++++++++++++++++++++++++- libxkutil/device_parsing.h | 15 ++++ libxkutil/xmlgen.c | 72 +++++++++++++++++ schema/Controller.mof | 47 +++++++++++ schema/Controller.registration | 19 +++++ src/Virt_Device.c | 84 ++++++++++++++++++++ src/Virt_RASD.c | 21 +++-- src/Virt_RASD.h | 4 + src/Virt_VirtualSystemManagementService.c | 106 ++++++++++++++++++++++++- src/svpc_types.h | 127 ++++++++++++++++++++++++++++++ 12 files changed, 602 insertions(+), 16 deletions(-) create mode 100644 schema/Controller.mof create mode 100644 schema/Controller.registration
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski 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
On 03/21/2014 07:57 AM, Boris Fiuczynski wrote:
On 03/19/2014 03:49 AM, John Ferlan wrote:
This is rework of the Controller patch series submitted by Xu Wang. I have built upon the constructs put in place by Xu with respect to reading and writing the XML, but for the remainder of the code I changed where the virtual controller lives.
Xu's patches made a Controller RASD which had some issues regarding associations with other devices and pools.
Rather than go with a RASD model, I have chosen to make a KVM_Controller virtual device to mimic a CIM_Controller device. This solution keeps things In doing so you broke the DMTF Virtual System Profile! http://www.dmtf.org/sites/default/files/standards/documents/DSP1057_1.0.0_0.... Take a look at 7.3.1 which defines a Virtual System Configuration and a Virtual System Representation. It states the class concepts. You need to use RASDs.
Hmm... Way too many standards for me to know :-). Also, to say the least the docs are written for someone that lives/breaths CIM (as far as I'm concerned at least). I see a box for "LogicalDevice" which is what CIM_Controller extends - so it seems the reality is in addition to the CIM_Controller there needs to be a KVM_ControllerResourceAllocationSettingData, right? I see for other KVM_*RASD types there is a related KVM_*LogicalDevice, so it's "more or less" a matter of extending what I already have, I think. For now the pressure is off though - the issue won't be fixed/ready for the RHEL7.0 release, so at the earliest it'll be 7.1. There's *way too much* code to backport sanely to including in a 7.0.z stream - right now there's about 30 or so patches - basically anything that's been added since 0.6.3 was created (and inserted into RHEL7.0). For me that's mostly a process issue. Furthermore, the addition of a keyboard input device in libvirt 1.2.2 has complicated things, especially the cimtest results, see: http://www.redhat.com/archives/libvir-list/2014-February/msg01058.html I'm off trying to figure out the best way to add/represent this. The libvirt code considers it an input device and on the "devices/input" XML tree. Thus it's parsed by the device_parsing code. Of course cimtest blows up because 'keyboard' doesn't match 'mouse' or 'tablet' and all the associations and links off in the weeds. There is a CIM_Keyboard device; however, given the above comments - it seems that may not be the path to take... If I make a KVM_Keyboard device and include found keyboards in the "KVM_InputRASD" there's issues because quite a few of the cimtest tests assume a 1-to-1 relationship between a classname and the RASD. They are not very happy to find two KVM_InputRASD's that are different. Building references of the RASD by the test uses the classname as a key - thus whichever is last (eg, keyboard) gets the representation. Anyway different problem, but still I think a similar need for representation - so it'll be good to understand the requirements for how a controller needs to be or can be represented.
at a lower level and has been able to pass the cimtest as well as the update a defined domain with a changed value test. This second test is what fails for RHEL7.
As it turns out, it seems as long as a 'pci' device with model 'pci-root' is created, then things seem to work. The code relies on the previous patch code Xu created to read/write the XML file with some adjustments.
Perhaps the only controversial patch (for me at least) is 6/6. I figured that after we've read everything and just before we go to create or update the guest that we need to make sure that at least the 'pci' device with model 'pci-root' exists. This is similar to the add_default_devs() code, Actually libvirt-cim should not be creating default devices but let libvirt do that. Reimplementing libvirt's default behavior would a overestimation of libvirt-cim's own capabilities and in the long run cause extra effort in maintaining default consistency.
Right - so I wasn't sure "how" to handle the issue if someone didn't configure/add a controller at domain/guest creation time. Other devices see to use the add_default_devs() mechanism, but honestly I really don't understand it that well. John
except that it's run after all that code prior to any add or update of the guest just before the "system_to_xml()" call is made. I figure this is the last gasp to ensure that at least the 'pci' device is there which seems to be used by a number of "silently added" libvirt devices.
John Ferlan (4): Add virtual controller object defs Change static API to global API Controller Device Details VSMS: Determine if default controller exists for KVM
Xu Wang (2): Add virtual controller device types Parse/Store controller XML tags
Makefile.am | 2 + libvirt-cim.spec.in | 2 + libxkutil/device_parsing.c | 119 +++++++++++++++++++++++++++- libxkutil/device_parsing.h | 15 ++++ libxkutil/xmlgen.c | 72 +++++++++++++++++ schema/Controller.mof | 47 +++++++++++ schema/Controller.registration | 19 +++++ src/Virt_Device.c | 84 ++++++++++++++++++++ src/Virt_RASD.c | 21 +++-- src/Virt_RASD.h | 4 + src/Virt_VirtualSystemManagementService.c | 106 ++++++++++++++++++++++++- src/svpc_types.h | 127 ++++++++++++++++++++++++++++++ 12 files changed, 602 insertions(+), 16 deletions(-) create mode 100644 schema/Controller.mof create mode 100644 schema/Controller.registration
participants (2)
-
Boris Fiuczynski -
John Ferlan