Dan Smith wrote:
HE> Please can you explain why you set the status here and overwrite HE> it right afterwards ? I think the CIM_JOBSTATE is already COMPLETE HE> here.
I wanted this function to serve three roles: save, save-then-restore, and restore. In the save-then-restore scenario, we want a status update after the save is complete, of course. Ok, thanks - makes sense now for me. Because I like symmetric code, I added the same sort of status update to the end of restore. In the save-only case, this comes out to setting the save status and then changing it to "complete" right afterwards.
I don't know if this is a big deal or not, but I think that the code to make it avoid the extra set would be less pretty. Suggestions are welcome, of course :)
Well, I have no suggestion to reorder the code - as it would end up in less pretty code (as you already said). But maybe a little description to do_snapshot() would help to understand it better.
... Yes, removing at least one of these results in the behavior described above. I'll revisit and see which is the necessary one.
Please check if the CMPIInstance contains the namespace information. I guess this will be the leak.
...
HE> You could use a CMPIString and return its char* to the HE> caller. This avoids missing frees.
But then I have to pass a broker pointer, since this is used outside this file. I'll look to see if it makes sense to do that here or not.
Oh yes, I forgot about that. Mhh, then it might be better to not use CMPIString, as this would also break the common look and feel with similar functions in libvirt-cim. -- Regards Heidi Eckhart Software Engineer IBM Linux Technology Center - Open Hypervisor