[PATCH 0 of 2] Add KVM CDROM support
This set adds some KVM CDROM support to model these device types differently, using the CIM_CDROMDrive class. Also, support changing the Address property to eject or uneject the virtual drive as appropriate. These have been in my queue for a long time, but after finishing up some bits, they seem to work for me. We'll definitely want some test suite coverage for this.
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1224018018 25200 # Node ID 9e4f5b57f4125904121d650cc03866b6fbf45084 # Parent b9a93806b73aaaccef8ade2cc43170e77fe01755 Add KVM_CDROMDrive class This patch makes the Device provider return a KVM_CDROMDrive class where appropriate. All the associations that mention LogicalDisk have been updated, but it would be good to ensure that the test suite is properly checking the links, despite the classnames of the devices. Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r b9a93806b73a -r 9e4f5b57f412 libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c Tue Oct 14 12:11:00 2008 -0700 +++ b/libxkutil/device_parsing.c Tue Oct 14 14:00:18 2008 -0700 @@ -82,7 +82,8 @@ if (dev == NULL) return; /* free()-like semantics */ - if (dev->type == CIM_RES_TYPE_DISK) + if ((dev->type == CIM_RES_TYPE_DISK) || + (dev->type == CIM_RES_TYPE_CDROM)) cleanup_disk_device(&dev->dev.disk); else if (dev->type == CIM_RES_TYPE_NET) cleanup_net_device(&dev->dev.net); @@ -235,10 +236,16 @@ goto err; } } - if ((ddev->source == NULL) || (ddev->virtual_dev == NULL)) + if (ddev->virtual_dev == NULL) goto err; - vdev->type = CIM_RES_TYPE_DISK; + if ((ddev->source == NULL) && (!STREQC(ddev->device, "cdrom"))) + goto err; + + if (STREQC(ddev->device, "cdrom")) + vdev->type = CIM_RES_TYPE_CDROM; + else + vdev->type = CIM_RES_TYPE_DISK; vdev->id = strdup(ddev->virtual_dev); *vdevs = vdev; @@ -479,7 +486,8 @@ /* point to correct parser function according to type */ if (type == CIM_RES_TYPE_NET) do_real_parse = &parse_net_device; - else if (type == CIM_RES_TYPE_DISK) + else if ((type == CIM_RES_TYPE_DISK) || + (type == CIM_RES_TYPE_CDROM)) do_real_parse = &parse_disk_device; else if (type == CIM_RES_TYPE_PROC) do_real_parse = parse_vcpu_device; @@ -547,7 +555,8 @@ if (type == CIM_RES_TYPE_NET) xpathstr = NET_XPATH; - else if (type == CIM_RES_TYPE_DISK) + else if ((type == CIM_RES_TYPE_DISK) || + (type == CIM_RES_TYPE_CDROM)) xpathstr = DISK_XPATH; else if (type == CIM_RES_TYPE_PROC) xpathstr = VCPU_XPATH; @@ -605,7 +614,8 @@ DUP_FIELD(dev, _dev, dev.net.mac); DUP_FIELD(dev, _dev, dev.net.type); DUP_FIELD(dev, _dev, dev.net.source); - } else if (dev->type == CIM_RES_TYPE_DISK) { + } else if ((dev->type == CIM_RES_TYPE_DISK) || + (dev->type == CIM_RES_TYPE_CDROM)) { DUP_FIELD(dev, _dev, dev.disk.type); DUP_FIELD(dev, _dev, dev.disk.device); DUP_FIELD(dev, _dev, dev.disk.driver); diff -r b9a93806b73a -r 9e4f5b57f412 schema/LogicalDisk.mof --- a/schema/LogicalDisk.mof Tue Oct 14 12:11:00 2008 -0700 +++ b/schema/LogicalDisk.mof Tue Oct 14 14:00:18 2008 -0700 @@ -19,6 +19,15 @@ }; [Description ( + "A class derived from CIM_CDROMDrive to represent " + "the KVM virtual CDROM drive in a guest."), + Provider("cmpi::Virt_Device") +] +class KVM_CDROMDrive : CIM_CDROMDrive +{ +}; + +[Description ( "A class derived from CIM_LogicalDisk to represent " "the KVM virtual disks on the system."), Provider("cmpi::Virt_Device") diff -r b9a93806b73a -r 9e4f5b57f412 schema/LogicalDisk.registration --- a/schema/LogicalDisk.registration Tue Oct 14 12:11:00 2008 -0700 +++ b/schema/LogicalDisk.registration Tue Oct 14 14:00:18 2008 -0700 @@ -2,4 +2,5 @@ # Classname Namespace ProviderName ProviderModule ProviderTypes Xen_LogicalDisk root/virt Virt_Device Virt_Device instance KVM_LogicalDisk root/virt Virt_Device Virt_Device instance +KVM_CDROMDrive root/virt Virt_Device Virt_Device instance LXC_LogicalDisk root/virt Virt_Device Virt_Device instance diff -r b9a93806b73a -r 9e4f5b57f412 src/Virt_Device.c --- a/src/Virt_Device.c Tue Oct 14 12:11:00 2008 -0700 +++ b/src/Virt_Device.c Tue Oct 14 14:00:18 2008 -0700 @@ -124,11 +124,26 @@ { CMPIInstance *inst; virConnectPtr conn; + const char *conntype; + const char *basetype; conn = virDomainGetConnect(dom); + conntype = virConnectGetType(conn); + if (conntype == NULL) { + CU_DEBUG("Unable to get connection type"); + return NULL; + } + + if (STREQ(conntype, "QEMU") && STREQC(dev->device, "cdrom")) + basetype = "CDROMDrive"; + else + basetype = "LogicalDisk"; + + CU_DEBUG("DISK BASE TYPE: %s, device is %s", basetype, dev->device); + inst = get_typed_instance(broker, pfx_from_conn(conn), - "LogicalDisk", + basetype, ns); if (!disk_set_name(inst, dev)) @@ -301,7 +316,8 @@ &dev->dev.net, dom, ns); - else if (dev->type == CIM_RES_TYPE_DISK) + else if ((dev->type == CIM_RES_TYPE_DISK) || + (dev->type == CIM_RES_TYPE_CDROM)) instance = disk_instance(broker, &dev->dev.disk, dom, @@ -346,6 +362,8 @@ return CIM_RES_TYPE_MEM; else if (strstr(classname, "Processor")) return CIM_RES_TYPE_PROC; + else if (strstr(classname, "CDROMDrive")) + return CIM_RES_TYPE_CDROM; else return CIM_RES_TYPE_UNKNOWN; } diff -r b9a93806b73a -r 9e4f5b57f412 src/Virt_DevicePool.c --- a/src/Virt_DevicePool.c Tue Oct 14 12:11:00 2008 -0700 +++ b/src/Virt_DevicePool.c Tue Oct 14 14:00:18 2008 -0700 @@ -476,7 +476,8 @@ poolid = strdup("MemoryPool/0"); else if (type == CIM_RES_TYPE_NET) poolid = netpool_member_of(broker, id, refcn); - else if (type == CIM_RES_TYPE_DISK) + else if ((type == CIM_RES_TYPE_DISK) || + (type == CIM_RES_TYPE_CDROM)) poolid = diskpool_member_of(broker, id, refcn); else return NULL; diff -r b9a93806b73a -r 9e4f5b57f412 src/Virt_ElementAllocatedFromPool.c --- a/src/Virt_ElementAllocatedFromPool.c Tue Oct 14 12:11:00 2008 -0700 +++ b/src/Virt_ElementAllocatedFromPool.c Tue Oct 14 14:00:18 2008 -0700 @@ -191,6 +191,7 @@ "KVM_Memory", "KVM_NetworkPort", "KVM_LogicalDisk", + "KVM_CDROMDrive", "LXC_Processor", "LXC_Memory", "LXC_NetworkPort", diff -r b9a93806b73a -r 9e4f5b57f412 src/Virt_RASD.c --- a/src/Virt_RASD.c Tue Oct 14 12:11:00 2008 -0700 +++ b/src/Virt_RASD.c Tue Oct 14 14:00:18 2008 -0700 @@ -247,7 +247,8 @@ char *id; const char *keys[] = {"InstanceID", NULL}; - if (dev->type == CIM_RES_TYPE_DISK) { + if ((dev->type == CIM_RES_TYPE_DISK) || + (dev->type == CIM_RES_TYPE_CDROM)) { type = CIM_RES_TYPE_DISK; base = "DiskResourceAllocationSettingData"; } else if (dev->type == CIM_RES_TYPE_NET) { diff -r b9a93806b73a -r 9e4f5b57f412 src/Virt_SettingsDefineState.c --- a/src/Virt_SettingsDefineState.c Tue Oct 14 12:11:00 2008 -0700 +++ b/src/Virt_SettingsDefineState.c Tue Oct 14 14:00:18 2008 -0700 @@ -330,6 +330,7 @@ "KVM_Memory", "KVM_NetworkPort", "KVM_LogicalDisk", + "KVM_CDROMDrive", "LXC_Processor", "LXC_Memory", "LXC_NetworkPort", diff -r b9a93806b73a -r 9e4f5b57f412 src/Virt_SystemDevice.c --- a/src/Virt_SystemDevice.c Tue Oct 14 12:11:00 2008 -0700 +++ b/src/Virt_SystemDevice.c Tue Oct 14 14:00:18 2008 -0700 @@ -138,6 +138,7 @@ "KVM_Memory", "KVM_NetworkPort", "KVM_LogicalDisk", + "KVM_CDROMDrive", "LXC_Processor", "LXC_Memory", "LXC_NetworkPort", diff -r b9a93806b73a -r 9e4f5b57f412 src/svpc_types.h --- a/src/svpc_types.h Tue Oct 14 12:11:00 2008 -0700 +++ b/src/svpc_types.h Tue Oct 14 14:00:18 2008 -0700 @@ -29,14 +29,16 @@ #define CIM_RES_TYPE_DISK 17 #define CIM_RES_TYPE_EMU 1 #define CIM_RES_TYPE_GRAPHICS 24 +#define CIM_RES_TYPE_CDROM 15 #define CIM_RES_TYPE_UNKNOWN 1000 -#define CIM_RES_TYPE_COUNT 4 +#define CIM_RES_TYPE_COUNT 5 const static int cim_res_types[CIM_RES_TYPE_COUNT] = {CIM_RES_TYPE_NET, CIM_RES_TYPE_DISK, CIM_RES_TYPE_MEM, CIM_RES_TYPE_PROC, + CIM_RES_TYPE_CDROM, }; #define CIM_VSSD_RECOVERY_NONE 2
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1224018094 25200 # Node ID 41bb42ca279660963a392d91ea3d720f66eab6e5 # Parent 9e4f5b57f4125904121d650cc03866b6fbf45084 Add the CDROM-specific RASD pieces This makes the RASD provider expose the correct ResourceType, VSMS honor the CDROM type, and xmlgen create valid CDROM XML when asked. If you do a ModifyResources call and omit the Address field from the CDROM RASD, you will effectively "eject" the CDROM. Calling it again with a value for this field will "uneject" the CDROM for the guest. Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r 9e4f5b57f412 -r 41bb42ca2796 libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c Tue Oct 14 14:00:18 2008 -0700 +++ b/libxkutil/device_parsing.c Tue Oct 14 14:01:34 2008 -0700 @@ -1067,6 +1067,8 @@ return change_memory(dom, dev); else if (dev->type == CIM_RES_TYPE_PROC) return change_vcpus(dom, dev); + else if (dev->type == CIM_RES_TYPE_CDROM) + return _change_device(dom, dev, true); CU_DEBUG("Unhandled device type %i", dev->type); diff -r 9e4f5b57f412 -r 41bb42ca2796 libxkutil/xmlgen.c --- a/libxkutil/xmlgen.c Tue Oct 14 14:00:18 2008 -0700 +++ b/libxkutil/xmlgen.c Tue Oct 14 14:01:34 2008 -0700 @@ -113,43 +113,52 @@ return 1; } -static char *disk_block_xml(const char *path, const char *vdev) +static char *_disk_xml(struct disk_device *vdev) { char *xml; int ret; + const char *sourcetype; + const char *disktype; + const char *devtype = "disk"; - ret = asprintf(&xml, - "<disk type='block' device='disk'>\n" - " <source dev='%s'/>\n" - " <target dev='%s'/>\n" - "</disk>\n", - path, - vdev); + if (vdev->disk_type == DISK_PHY) { + sourcetype = "dev"; + disktype = "block"; + } else if (vdev->disk_type == DISK_FILE) { + sourcetype = "file"; + disktype="file"; + } else + return NULL; + + if ((vdev->device != NULL) && STREQC(vdev->device, "cdrom")) + devtype = "cdrom"; + + if (STREQ(devtype, "cdrom") && (vdev->source == NULL)) + ret = asprintf(&xml, + "<disk type='%s' device='%s'>\n" + " <target dev='%s'/>\n" + "</disk>\n", + disktype, + devtype, + vdev->virtual_dev); + else + ret = asprintf(&xml, + "<disk type='%s' device='%s'>\n" + " <source %s='%s'/>\n" + " <target dev='%s'/>\n" + "</disk>\n", + disktype, + devtype, + sourcetype, + vdev->source, + vdev->virtual_dev); if (ret == -1) xml = NULL; return xml; } -static char *disk_file_xml(const char *path, const char *vdev) -{ - char *xml; - int ret; - - ret = asprintf(&xml, - "<disk type='file' device='disk'>\n" - " <source file='%s'/>\n" - " <target dev='%s'/>\n" - "</disk>\n", - path, - vdev); - if (ret == -1) - xml = NULL; - - return xml; -} - -static char *disk_fs_xml(const char *path, const char *vdev) +static char *_disk_fs_xml(const char *path, const char *vdev) { char *xml; int ret; @@ -172,14 +181,11 @@ char *_xml = NULL; struct disk_device *disk = &dev->dev.disk; - if (disk->disk_type == DISK_PHY) - _xml = disk_block_xml(disk->source, disk->virtual_dev); - else if (disk->disk_type == DISK_FILE) - /* If it's not a block device, we assume a file, - which should be a reasonable fail-safe */ - _xml = disk_file_xml(disk->source, disk->virtual_dev); + if ((disk->disk_type == DISK_PHY) || + (disk->disk_type == DISK_FILE)) + _xml = _disk_xml(disk); else if (disk->disk_type == DISK_FS) - _xml = disk_fs_xml(disk->source, disk->virtual_dev); + _xml = _disk_fs_xml(disk->source, disk->virtual_dev); else return false; diff -r 9e4f5b57f412 -r 41bb42ca2796 src/Virt_RASD.c --- a/src/Virt_RASD.c Tue Oct 14 14:00:18 2008 -0700 +++ b/src/Virt_RASD.c Tue Oct 14 14:01:34 2008 -0700 @@ -247,18 +247,16 @@ char *id; const char *keys[] = {"InstanceID", NULL}; + type = dev->type; + if ((dev->type == CIM_RES_TYPE_DISK) || (dev->type == CIM_RES_TYPE_CDROM)) { - type = CIM_RES_TYPE_DISK; base = "DiskResourceAllocationSettingData"; } else if (dev->type == CIM_RES_TYPE_NET) { - type = CIM_RES_TYPE_NET; base = "NetResourceAllocationSettingData"; } else if (dev->type == CIM_RES_TYPE_PROC) { - type = CIM_RES_TYPE_PROC; base = "ProcResourceAllocationSettingData"; } else if (dev->type == CIM_RES_TYPE_MEM) { - type = CIM_RES_TYPE_MEM; base = "MemResourceAllocationSettingData"; } else { return NULL; @@ -284,7 +282,8 @@ CMSetProperty(inst, "ResourceType", (CMPIValue *)&type, CMPI_uint16); - if (dev->type == CIM_RES_TYPE_DISK) { + if ((dev->type == CIM_RES_TYPE_DISK) || + (dev->type == CIM_RES_TYPE_CDROM)) { s = set_disk_rasd_params(broker, ref, dev, inst); } else if (dev->type == CIM_RES_TYPE_NET) { CMSetProperty(inst, diff -r 9e4f5b57f412 -r 41bb42ca2796 src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Tue Oct 14 14:00:18 2008 -0700 +++ b/src/Virt_VirtualSystemManagementService.c Tue Oct 14 14:01:34 2008 -0700 @@ -402,6 +402,7 @@ struct virt_device *dev) { const char *val = NULL; + uint16_t rtype; if (cu_get_str_prop(inst, "VirtualDevice", &val) != CMPI_RC_OK) return "Missing `VirtualDevice' property"; @@ -409,12 +410,38 @@ free(dev->dev.disk.virtual_dev); dev->dev.disk.virtual_dev = strdup(val); - if (cu_get_str_prop(inst, "Address", &val) != CMPI_RC_OK) - val = "/dev/null"; + if (cu_get_u16_prop(inst, "ResourceType", &rtype) != CMPI_RC_OK) + rtype = CIM_RES_TYPE_DISK; - free(dev->dev.disk.source); - dev->dev.disk.source = strdup(val); - dev->dev.disk.disk_type = disk_type_from_file(val); + free(dev->dev.disk.device); + if (rtype == CIM_RES_TYPE_DISK) + dev->dev.disk.device = strdup("disk"); + else if (rtype == CIM_RES_TYPE_CDROM) + dev->dev.disk.device = strdup("cdrom"); + else + return "Unknown disk type"; + + if (cu_get_str_prop(inst, "Address", &val) == CMPI_RC_OK) { + free(dev->dev.disk.source); + dev->dev.disk.source = strdup(val); + } else if (rtype == CIM_RES_TYPE_CDROM) { + free(dev->dev.disk.source); + dev->dev.disk.source = NULL; + } else { + return "Missing `Address' property"; + } + + if (dev->dev.disk.source == NULL) { + /* If this is a removable media device (and it must be + * if we got to this point), don't freak out if they + * didn't specify any media right now. The type is + * immaterial until they attach it anyway, so just + * assume a default value. + */ + dev->dev.disk.disk_type = DISK_FILE; + } else { + dev->dev.disk.disk_type = disk_type_from_file(val); + } free(dev->id); dev->id = strdup(dev->dev.disk.virtual_dev); @@ -548,7 +575,8 @@ uint16_t type, const char *ns) { - if (type == CIM_RES_TYPE_DISK) { + if ((type == CIM_RES_TYPE_DISK) || + (type == CIM_RES_TYPE_CDROM)) { return disk_rasd_to_vdev(inst, dev); } else if (type == CIM_RES_TYPE_NET) { return net_rasd_to_vdev(inst, dev, ns); @@ -693,9 +721,10 @@ if (op == NULL) return "Unknown resource instance type"; - if (res_type_from_rasd_classname(CLASSNAME(op), &type) != - CMPI_RC_OK) - return "Unable to determine resource type"; + if (cu_get_u16_prop(inst, "ResourceType", &type) != CMPI_RC_OK) + if (res_type_from_rasd_classname(CLASSNAME(op), + &type) != CMPI_RC_OK) + return "Unable to determine resource type"; if (type == CIM_RES_TYPE_PROC) { domain->dev_vcpu_ct = 1; @@ -709,7 +738,8 @@ domain, &domain->dev_mem[0], ns); - } else if (type == CIM_RES_TYPE_DISK) { + } else if ((type == CIM_RES_TYPE_DISK) || + (type == CIM_RES_TYPE_CDROM)) { struct virt_device dev; int dcount = count + domain->dev_disk_ct;
participants (1)
-
Dan Smith