[Libvirt-cim] [PATCH] (#2) Fix the problem that libvirt-cim can't find cdrom device that do not have disk
Gareth S Bestor
bestor at us.ibm.com
Mon Oct 3 02:47:44 UTC 2011
>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 at 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 at 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<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
_______________________________________________
Libvirt-cim mailing list
Libvirt-cim at redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-cim
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvirt-cim/attachments/20111002/2ec3598c/attachment.htm>
More information about the Libvirt-cim
mailing list