On Fri, May 11, 2012 at 06:15:29AM -0600, Eric Blake wrote:
> 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.
Or just #define the block of code for editing the XML and pass in
the separate API names / types as macro parameters
I'd love to see the back of the sed script.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list