[Libvirt-cim] [PATCH] (#2) Fix the problem that libvirt-cim can't find cdrom device that do not have disk

[Chip Vincent]

Pushed.

On 10/02/2011 08:12 PM, Chip Vincent wrote:
> Tests for this have completed successfully, so ACK'ing. However...
> Now that we have a cimtest patch for this
> (https://www.redhat.com/archives/libvirt-cim/2011-September/msg00047.html),
> I think a follow-up tweak may be order. For example, I'd prefer to
> simply use NULL for 'no media' rather than a null_string to simplify the
> code further. We should also already have the libvirt-cim and Pegasus
> patches (no references, sorry) that allow us to un-set attributes like
> this. That is, set an actual value to NULL.
>
> On 09/20/2011 11:14 PM, Wayne Xia wrote:
>> # HG changeset patch
>> # User Wayne Xia<xiawenc at linux.vnet.ibm.com>
>> # Date 1316154275 -28800
>> # Node ID afee8d9b7214884ab74690b3ca9fd3d4f139f455
>> # Parent db809376d763493849c2a19f587969eaec619b75
>> (#2) Fix the problem that libvirt-cim can't find cdrom device that do
>> not have disk
>>
>> This patch would allow define a system with an empty CDROM device, and
>> allow method modify
>> resource settings to insert ISO files into an empty CDROM device.
>> Examples:
>> InvokeMethod(ModifyResourceSettings):
>> ResourceSettings: ['instance of KVM_DiskResourceAllocationSettingData
>> {\nResourceType = 17;\nInstanceID = "test/hdc";\nEmulatedType =
>> 1;\nVirtualDevice = "hdc";\nAddress = "";\n};']
>> InvokeMethod(ModifyResourceSettings):
>> ResourceSettings: ['instance of KVM_DiskResourceAllocationSettingData
>> {\nResourceType = 17;\nInstanceID = "test/hdc";\nEmulatedType =
>> 1;\nVirtualDevice = "hdc";\nAddress =
>> "/var/lib/libvirt/images/test-disk.iso";\n};']
>> Note that the Address property should be set to "", not None(not set),
>> to tell that user want
>> an ejection.
>>
>> (#2) Add comments that saying what the code does, and improved some
>> codes to avoid doing duplicated things.
>>
>> Signed-off-by: Wayne Xia<xiawenc at linux.vnet.ibm.com>
>>
>> diff -r db809376d763 -r afee8d9b7214 libxkutil/device_parsing.c
>> --- a/libxkutil/device_parsing.c Thu Jul 28 13:56:00 2011 -0300
>> +++ b/libxkutil/device_parsing.c Fri Sep 16 14:24:35 2011 +0800
>> @@ -287,6 +287,13 @@
>> ddev->shareable = true;
>> }
>> }
>> +
>> + /* handle the situation that a cdrom device have no disk in it, no
>> ISO file */
>> + if ((XSTREQ(ddev->device, "cdrom"))&& (ddev->source == NULL)) {
>> + ddev->source = strdup("");
>> + ddev->disk_type = DISK_FILE;
>> + }
>> +
>> if ((ddev->source == NULL) || (ddev->virtual_dev == NULL))
>> goto err;
>>
>> diff -r db809376d763 -r afee8d9b7214 libxkutil/xmlgen.c
>> --- a/libxkutil/xmlgen.c Thu Jul 28 13:56:00 2011 -0300
>> +++ b/libxkutil/xmlgen.c Fri Sep 16 14:24:35 2011 +0800
>> @@ -110,10 +110,18 @@
>> xmlNewProp(tmp, BAD_CAST "cache", BAD_CAST dev->cache);
>> }
>>
>> - tmp = xmlNewChild(disk, NULL, BAD_CAST "source", NULL);
>> - if (tmp == NULL)
>> - return XML_ERROR;
>> - xmlNewProp(tmp, BAD_CAST "file", BAD_CAST dev->source);
>> + if ((XSTREQ(dev->device, "cdrom"))&&
>> + (XSTREQ(dev->source, ""))) {
>> + /* This is the situation that user defined a cdrom device without
>> + disk in it, so skip generating a line saying "source", for that
>> + xml defination for libvirt should not have this defined in this
>> + situation. */
>> + } else {
>> + tmp = xmlNewChild(disk, NULL, BAD_CAST "source", NULL);
>> + if (tmp == NULL)
>> + return XML_ERROR;
>> + xmlNewProp(tmp, BAD_CAST "file", BAD_CAST dev->source);
>> + }
>>
>> tmp = xmlNewChild(disk, NULL, BAD_CAST "target", NULL);
>> if (tmp == NULL)
>> diff -r db809376d763 -r afee8d9b7214
>> src/Virt_VirtualSystemManagementService.c
>> --- a/src/Virt_VirtualSystemManagementService.c Thu Jul 28 13:56:00
>> 2011 -0300
>> +++ b/src/Virt_VirtualSystemManagementService.c Fri Sep 16 14:24:35
>> 2011 +0800
>> @@ -879,6 +879,17 @@
>> dev->dev.disk.device = strdup("disk");
>> else if (type == VIRT_DISK_TYPE_CDROM) {
>> dev->dev.disk.device = strdup("cdrom");
>> + /* following code is for the case that user defined cdrom device
>> + without disk in it, or a empty disk "" */
>> + if (XSTREQ(dev->dev.disk.source, "")) {
>> + dev->dev.disk.disk_type = DISK_FILE;
>> + }
>> + if (XSTREQ(dev->dev.disk.source, "/dev/null")) {
>> + dev->dev.disk.disk_type = DISK_FILE;
>> + free(dev->dev.disk.source);
>> + dev->dev.disk.source = strdup("");
>> + }
>> +
>> if (dev->dev.disk.disk_type == DISK_UNKNOWN)
>> dev->dev.disk.disk_type = DISK_PHY;
>> }
>>
>> _______________________________________________
>> Libvirt-cim mailing list
>> Libvirt-cim at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvirt-cim
>

-- 
Chip Vincent
Open Virtualization
IBM Linux Technology Center
cvincent at linux.vnet.ibm.com