[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] virsh: Improve editing



On Fri, May 13, 2011 at 02:57:08PM +0200, Jiri Denemark wrote:
> On Fri, May 13, 2011 at 12:34:25 +0200, Michal Privoznik wrote:
> > When users (pool-/net-)edit and they make a mistake, temporary file
> > and thus all changes are gone. Better way is to let them decide if
> > they want to get back to edit and correct what's wrong.
> > However, this is suitable only in interactive mode.
> > ---
> >  tools/virsh.c |   42 ++++++++++++++++++++++++++++++++++++++++--
> >  1 files changed, 40 insertions(+), 2 deletions(-)
> 
> I like the idea (not sure if that's because I suggested it :-P) but, I have a
> few comments. The patch doesn't touch iface-edit, which is not generated from
> cmdEdit, though the right fix might turn out to be generating cmdIfaceEdit
> together with cmdPoolEdit and cmdNetworkEdit.
> 
> And IMHO *edit commands are always interactive even when not called in virsh
> interactive mode so I'd prefer this behaviour to be unconditional. I can't
> imagine anyone sane to call virsh edit from a script, it's much easier to just
> call virsh dumpxml, change the xml and virsh define it back.

There are two common problems with virsh edit & friends

  - Invalid XML syntax, causes error report & lost changes
  - User add unsupported/unknown XML attributes/elements which
    are silently discarded by libvirt

This patch only fixes the first problem. It would be nice to
fix the second two, by running the XML through the RNG schema
validator.

Rather than do this in virsh though, I'd add some flags to
the virXXXXDefine/Create  APIs, eg

  VIR_DOMAIN_XML_VALIDATE

virsh can set this flag by default, and if the XML fails
validation, it could prompt the user, asking if they want
to proceed anyway (in which case recall the same API but
without the validate flag set), or re-edit the XML


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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]