Tests for this have completed successfully, so ACK'ing. However...
Now that we have a cimtest patch for this
(
),
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(a)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(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-cim
--
Chip Vincent
Open Virtualization
IBM Linux Technology Center
cvincent(a)linux.vnet.ibm.com