# 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

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(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

>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. FYI, libvirt-cim ModifyResourceSettings[] ignores properties in the modified RASD having no/null value when merging this 'new' instance with the existing one. Unfortuantely, this is a restriction from the DMTF SVPC profile itself. From DSP1042 System Virtualization Profile: " ...The execution of the ModifyResourceSettings( ) method shall effect the modification of resource alloca- tions or resource allocation requests, such that non-key and non-NULL values of instances of the CIM_ResourceAllocationSettingData class provided as values for elements of the ResourceSettings[ ] ar- ray parameter override respective values in instances identified through the InstanceID property. " - G Dr. Gareth S. Bestor IBM Senior Software Engineer Systems & Technology Group - Systems Management Standards 971-285-6375 (mobile) bestor(a)us.ibm.com

*Re: [Libvirt-cim] [PATCH] (#2) Fix the problem that libvirt-cim can't find cdrom device that do not have disk* *Chip Vincent * to: libvirt-cim 10/02/11 05:13 PM Sent by: *libvirt-cim-bounces(a)redhat.com* *Please respond to cvincent, List for discussion and development of libvirt CIM * 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&lt;xiawenc(a)linux.vnet.ibm.com&gt; > # 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&lt;xiawenc(a)linux.vnet.ibm.com&gt; > > 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/archives/libvirt-cim/2011-September/msg00047.html&... -- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent(a)linux.vnet.ibm.com _______________________________________________ Libvirt-cim mailing list Libvirt-cim(a)redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim _______________________________________________ Libvirt-cim mailing list Libvirt-cim(a)redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim