[Libvirt-cim] [PATCH] (#2) Fix the problem that libvirt-cim can't find cdrom device that do not have disk
Wayne Xia
xiawenc at linux.vnet.ibm.com
Tue Oct 11 09:35:50 UTC 2011
Back to work now. :)
于 2011-10-3 10:47, Gareth S Bestor 写道:
>
> >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
>
>
from my understanding, the pegasus need to tell libvirt-cim a special
value such as '' (empty string) or 'NULL', to show that user want a
empty media setting in the calling of method ModifyResourceSettings,
For that property not specified should not be merged, otherwise in
some case libvirt-cim may overwrite some properties to empty ones that
user do not want to.
So I think a string 'NULL' or '' representing an empty media setting
make sense.
>
> *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/archives/libvirt-cim/2011-September/msg00047.html>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
>
>
>
>
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim
--
Best Regards
Wayne Xia
mail:xiawenc at linux.vnet.ibm.com
tel:86-010-82450803
More information about the Libvirt-cim
mailing list