[Libvirt-cim] [PATCH V2 01/48] Add others member for saving unsupported tag and unknown device

[Xu Wang]

Since I started to work on libvirt-cim almost every bug is about new 
tags support. My first patch is about <shareable> in the <disk>. The 
second one is <dumpCore> in the <memory>. And other one coming soon is 
<model> in <cpu>, and endless like this. So I got a mission that is 
designing/implementing a long term solution to solve this category of 
bug. So I have a dream, even re-write all content of 
libxutil/device_parsing.c and libxutil/xmlgen.c to reach that goal. I'll 
continue to work on the issues you found. And update the design 
unsuitable. I'll make a new version after finished. Just rebasing work 
is a little boring. Thank you for taking so much time to review and test 
them.

Thanks,
   Xu Wang

于 2013/11/14 7:08, John Ferlan 写道:
> On 11/12/2013 10:27 PM, Xu Wang wrote:
>> 于 2013/11/13 8:59, John Ferlan 写道:
>> Dear John,
>> My idea is, using a data structure (a link list) to keep the elements
>> left after all kinds of
>> virtual devices fetched tags they needed from xml. And if I just mark a
>> node of others with
>> status instead of delete it, the whole structure of xml could be saved
>> as well.
>> The introduction of 'id' in 'others' structure is to identify several
>> <tags> with the same
>> name. The console device support new introduced and some tags in
>> 'unknown_device'
>> have to use 'id' to identify elements. It maybe a little coarse now so I
>> need any suggestion
>> from you to make it works better. If you have any better idea to solve
>> the unsupported
>> tags missing issue please share with me :-) Because so many patches
>> about it merged
>> and there will be more in the future.
>> These 48 patches just the first part in my designing. After that I want
>> to build the
>> management about all data (even in 'others' member). And all of these
>> updates are
>> compatitable with older version libvirt-cim. So the upper layer need
>> little change and
>> keep the original feature but could make libvirt-cim becomes more powerful.
>> XFAIL about hotplug disappeared after updated. About the warning you
>> mentioned above
>> I'll check and fix it in the new version.
>>
>> Thanks,
>> Xu Wang
> I don't have any thoughts on better ideas - it's just not my focus.
> There's a lot of changes being made in this series which I suppose in
> hindsight would have been nice to have been broken up a bit more to make
> it manageable to review, understand, and test.  I've started with the
> first 3 patches as it seems all they do is create infrastructure and
> "for the most part" no one should be calling them.
>
> I started with patch 1...  got to patch 2 and had issues.  I ran patch 2
> through coverity and it complained about the memset() in
> cleanup_virt_device().
>
> A recent patch changed that from "memset(&dev->dev, 0,
> sizeof(dev->dev));" to "memset(dev, 0, sizeof(*dev));".  Coverity
> complained about the path from "classify_resources()" for
> CIM_RES_TYPE_DISK into the rasd_to_vdev() call, then into
> _sysvirt_rasd_to_vdev(), disk_rasd_to_vdev(), and finally
> cleanup_virt_device().
>
> It seems the change to device_parsing.h, inclusion by
> Virt_VirtualSystemManagementService.c, and usage of virt_device as a
> static structure caused the "issue" as a "clean" build fixed things.
> Although it took a while for me to figure it out.  I suppose that's more
> a build/Makefile issue than anything else...
>
> The second issue I've run into is that by only applying the first two
> patches, I can get a test to cause cimprovagt to fail:
>
> --------------------------------------------------------------------
> VirtualSystemManagementService - 16_removeresource.py: FAIL
> ERROR 	- (1, u'CIM_ERR_FAILED: Lost connection with cimprovagt
> "libvirt-cim".')
> InvokeMethod(RemoveResourceSettings): CIM_ERR_FAILED: Lost connection
> with cimprovagt "libvirt-cim".
> --------------------------------------------------------------------
>
> This one's really frustrating as I'm really perplexed why a possible fix
> could work. It seems if I don't call cleanup_others() from
> cleanup_unknown_device(), then I don't get that core.  I have no idea
> why though. Is it possibly related to the static usage of virt_device? I
> see this even after a clean build.
>
> Maybe this "cleanup_unknown_device()" and it's counter-part
> "dev_unknown" should have been added after the others work. It seems
> there are two things being done at once - creating an 'others' list to
> store the read xml and creating a list of unknown devices.
>
> You've done a lot of work and it seems promising as a way handle new
> tags.  Of course I ask myself why is it so important to go through this
> exercise.  Are the libvirt API's deficient?  I would think libvirt
> should be the only thing handling XML format and the applications that
> use libvirt would have API's to create, define, get, set, start, stop,
> etc VM's.  Having to store the XML is "dangerous" insomuch as we've
> already found it can change and libvirt-cim cannot keep up.
>
> Anyway, as an exercise for me to learn and because I pushed a number of
> changes which conflict with what you've already done... I will try to
> help with the effort to compact each patch series into manageable parts.
> FWIW: I pushed the other patches because they were posted first - I just
> went in order.
>
> John