Re: [libvirt] [PATCH] virsh: Allow users to reedit rejected XML

On 05/11/2012 02:07 AM, Michal Privoznik wrote: > On 10.05.2012 19:10, Laine Stump wrote: >> On 05/09/2012 09:36 AM, Michal Privoznik wrote: >>> If users {net-,pool-,}edit but make a mistake in XML all changes >>> are permanently lost. However, if virsh is running in interactive >>> mode we can as user if he wants to re-edit the file and correct >>> the mistakes. >> >> This all reminds me that I so much disliked the idea of creating a .c >> file by running sed against virsh.c and then #include'ing that .c file >> into virsh.c that I instead made a separate function for >> cmdInterfaceEdit rather than adding to it. I had thought that someone >> refactored that long ago so that the main bit of code was a helper >> function called by all (thus removing the sed trickery), but apparently >> we only talked about it. (*adds a line to todo list*) > > Heh, thanks for pointing that out. I've completely forgotten about > iface-edit. So my patch is incomplete. :( > On the other hand, seems like sed script is doing its work well. I've > taken look at generated code and it was good. Even when I've introduced > this dom_edited variable, it was renamed to network_edited and > pool_edited respectively in those generated functions. So honestly, > unless we are doing some different magic in cmdInterfaceEdit (quick look > doesn't say so) I think we can believe the sed script and make > cmdInterfaceEdit being generated as well. The advantage is - if somebody > (this time it's me) introduce any feature, fix a bug, all other *-edit > commands benefit from it immediately. Still, I can't help but wonder if we can refactor this into a generic helper function that takes a callback pointer for the specific API to call, rather than a sed script. In addition to iface-edit not using the sed script, we also have save-image-edit, nwfilter-edit, and snapshot-edit that all need fixing.