[Libvirt-cim] [PATCH 00/20] REWORK/PARIAL: Changes to solve unsupported tag issue

[John Ferlan]

On 11/20/2013 03:36 AM, Xu Wang wrote:

> Dear John and Boris,
> I am considering pros/cons of suggestion from Boris. If the parsing
> process likes this,
> 1. Traversal the whole xml and save all data into data structure Boris
> describe above;

I hope the issue Viktor ran into with running out of memory shows that a
duplicitous design is problematic. If you have to - rather than strdup()
value, set INACTIVE, and return (unchecked) strdup()'d memory - it would
have been just as easy to return the memory already allocated, set
tmp->value = NULL, and set INACTIVE.  That way you know it's "managed"
by something else. The free(NULL) is a no-op anyway...

Be sure to use/change the "xml_parse_test" in order to validate.  In
fact that print function you created could be called by xml_parse_test -
at least temporarily. At first everything would be unknown.  As you
update parse functions, you should be able to prove that you've gone
from unknown to known...  Eventually your known should be the same
output as what one could get 'today' if they ran the test (I forget the
exact switches, but you'll get to know them).

> 2. Calling ***_parse() functions in device_parsing.c fetch data from the
> data structure
> generated in step 1.

I think 2 & 4/5 need to be intermingled.

That is when you update 'parse_fs_device', also update 'disk_fs_xml'.

I'd change 'parse_block_device', 'disk_block_xml', and 'disk_file_xml'
all at once.

Change all the network functions together.

Etc.

> 3. All kinds of resource operations.
> (Here I want to enhance the power of libvirt-cim after these patches
> merged. Some
> new name and value pairs could be passed from API and Management could
> be done.
> The work next step maybe more complex and I started to think about it)

I'm not exactly sure what this means, but it seems like this would be
after step 5.  Although it cannot be ignored or forgotten when devising
the mechanism to parse/generate XML

The change validity concern when there's unknown

> 4. Restore xml after operation, ***_xml() restore all data in the
> virt_devices fetched from
> data structure
> 5. Based on data structure restored, new xml re-generated.

What's the difference between 4 & 5?
> 
> I think code could be more tiny and maintained more easily.
> Do you have any suggestion or think the original one is better? I'll
> start to update all
> patches once we have a better plan:-)

Be conservative - I know it's hard to not make all the changes at once,
but let's get the infrastructure right first.  Updating the various
_parse() and _xml() modules is a rote operation, but splatting out 50
patches at a time is tough to review/test...

John

> 
> Other points,
> 1. Macros is a good idea. It could make code more clear.
> 2. I am not fans of 'others', too. It's just a 'historical issue' from
> my early design.

It's the "unmanaged" or "unrealized" list...

John
> 
> Thanks,
> Xu Wang