[libvirt] [PATCH] virsh edit command

One thing which is very apparent is that sys admins using libvirt / virsh have a great deal of difficulty understanding "where the configuration files have gone" and how to edit them. This patch adds a "virsh edit <domain>" command which is basically the equivalent of: virsh dumpxml dom > /tmp/dom.xml $EDITOR /tmp/dom.xml && virsh define /tmp/dom.xml but with much more sanity checking. The editor is $EDITOR or vi, and it does the right thing if the user doesn't modify the file, or if another user edits the configuration at the same time. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v

On Tue, Jul 29, 2008 at 01:27:42PM +0100, Richard W.M. Jones wrote:
One thing which is very apparent is that sys admins using libvirt / virsh have a great deal of difficulty understanding "where the configuration files have gone" and how to edit them.
.py files and the like were supposed to be human editable. I'm not convinced that libvirt XML is (heck, I certainly can't remember it). Isn't the *right* solution to this problem to finally add property set/get interface for the things people actually want to modify, like boot flags? regards john

On Tue, Jul 29, 2008 at 02:48:26PM +0100, John Levon wrote:
On Tue, Jul 29, 2008 at 01:27:42PM +0100, Richard W.M. Jones wrote:
One thing which is very apparent is that sys admins using libvirt / virsh have a great deal of difficulty understanding "where the configuration files have gone" and how to edit them.
.py files and the like were supposed to be human editable. I'm not convinced that libvirt XML is (heck, I certainly can't remember it).
Isn't the *right* solution to this problem to finally add property set/get interface for the things people actually want to modify, like boot flags?
Do you mean things like the current 'virsh attach-device' / 'virsh detach-device' interface? I'm responding here to a need that sysadmins feel they have -- to edit the configuration file (even if it's XML). Witness an endless series of questions on this subject on the #virt channel yesterday. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v

On Tue, Jul 29, 2008 at 05:00:27PM +0100, Richard W.M. Jones wrote:
Isn't the *right* solution to this problem to finally add property set/get interface for the things people actually want to modify, like boot flags?
Do you mean things like the current 'virsh attach-device' / 'virsh detach-device' interface?
I hope not... that's just as bad as editing the domain XML.
I'm responding here to a need that sysadmins feel they have -- to edit the configuration file (even if it's XML). Witness an endless series of questions on this subject on the #virt channel yesterday.
Right. But to my mind you're fixing the symptom not the problem. *Why* do they need to edit the XML? I ask this of everybody who complains at me about having to edit XML: 99% of the time it's wanting to change boot flags, but it's also stuff like turning off ACPI, setting on_crash, etc. Editing XML is absolutely not user friendly, and adding 'edit' just papers over the real problems IMHO. regards john

On Tue, Jul 29, 2008 at 05:09:37PM +0100, John Levon wrote:
Right. But to my mind you're fixing the symptom not the problem. *Why* do they need to edit the XML? I ask this of everybody who complains at me about having to edit XML: 99% of the time it's wanting to change boot flags, but it's also stuff like turning off ACPI, setting on_crash, etc.
Editing XML is absolutely not user friendly, and adding 'edit' just papers over the real problems IMHO.
I actually started at one point on a graphical libvirt XML editor, although I fairly quickly realised it would be a Sisyphean task because the format isn't tremendously well defined[1] and it keeps changing. Also because there's a lot of overlap between virt-install and (potential) virt-config-editor. I do genuinely think that having 'virsh edit' is better than the current situation. Currently the advice that everyone gives is to do: virsh dumpxml foo > foo.xml vi foo.xml virsh define foo.xml which is of course precisely the same as what 'virsh edit' does :-) Rich. [1] Although it's a great deal better since Dan Berrange started to formalize the way drivers generate and parse the XML. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v

On Tue, Jul 29, 2008 at 05:17:55PM +0100, Richard W.M. Jones wrote:
On Tue, Jul 29, 2008 at 05:09:37PM +0100, John Levon wrote:
Right. But to my mind you're fixing the symptom not the problem. *Why* do they need to edit the XML? I ask this of everybody who complains at me about having to edit XML: 99% of the time it's wanting to change boot flags, but it's also stuff like turning off ACPI, setting on_crash, etc.
Editing XML is absolutely not user friendly, and adding 'edit' just papers over the real problems IMHO.
I actually started at one point on a graphical libvirt XML editor, although I fairly quickly realised it would be a Sisyphean task because the format isn't tremendously well defined[1] and it keeps changing. Also because there's a lot of overlap between virt-install and (potential) virt-config-editor.
I do genuinely think that having 'virsh edit' is better than the current situation. Currently the advice that everyone gives is to do:
virsh dumpxml foo > foo.xml vi foo.xml virsh define foo.xml
which is of course precisely the same as what 'virsh edit' does :-)
Yes, I think this command is worthwhile adding. We should also try to address the problem that John raises too, but I see that as a parallel task - and a far more involved piece of work to undertake :-) Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Tue, Jul 29, 2008 at 05:25:57PM +0100, Daniel P. Berrange wrote:
I actually started at one point on a graphical libvirt XML editor, although I fairly quickly realised it would be a Sisyphean task because the format isn't tremendously well defined[1] and it keeps changing. Also because there's a lot of overlap between virt-install and (potential) virt-config-editor.
I do genuinely think that having 'virsh edit' is better than the current situation. Currently the advice that everyone gives is to do:
virsh dumpxml foo > foo.xml vi foo.xml virsh define foo.xml
which is of course precisely the same as what 'virsh edit' does :-)
Yes, I think this command is worthwhile adding. We should also try to address the problem that John raises too, but I see that as a parallel task - and a far more involved piece of work to undertake :-)
Right, and it'll probably never happen if we band-aid over the problem. Ah well. john

On Tue, Jul 29, 2008 at 05:27:09PM +0100, John Levon wrote:
On Tue, Jul 29, 2008 at 05:25:57PM +0100, Daniel P. Berrange wrote:
I actually started at one point on a graphical libvirt XML editor, although I fairly quickly realised it would be a Sisyphean task because the format isn't tremendously well defined[1] and it keeps changing. Also because there's a lot of overlap between virt-install and (potential) virt-config-editor.
I do genuinely think that having 'virsh edit' is better than the current situation. Currently the advice that everyone gives is to do:
virsh dumpxml foo > foo.xml vi foo.xml virsh define foo.xml
which is of course precisely the same as what 'virsh edit' does :-)
Yes, I think this command is worthwhile adding. We should also try to address the problem that John raises too, but I see that as a parallel task - and a far more involved piece of work to undertake :-)
Right, and it'll probably never happen if we band-aid over the problem. Ah well.
Well, I take your point here, and if you really think 'virsh edit' is a bad idea then perhaps we can do something else (give it another name?). Do you think a graphical libvirt XML format editor is a good solution? Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v

On Tue, Jul 29, 2008 at 05:46:34PM +0100, Richard W.M. Jones wrote:
Right, and it'll probably never happen if we band-aid over the problem. Ah well.
Well, I take your point here, and if you really think 'virsh edit' is a bad idea then perhaps we can do something else (give it another name?).
Well, I've not really any objection to the proposal as it is.
Do you think a graphical libvirt XML format editor is a good solution?
This would help enormously. You could even start it from 'virsh edit' if $DISPLAY is set. I've no idea about the general tools out there for generating UI from XML, but there has to be something. regards john

On Tue, Jul 29, 2008 at 05:50:30PM +0100, John Levon wrote:
On Tue, Jul 29, 2008 at 05:46:34PM +0100, Richard W.M. Jones wrote:
Do you think a graphical libvirt XML format editor is a good solution?
This would help enormously. You could even start it from 'virsh edit' if $DISPLAY is set.
I've no idea about the general tools out there for generating UI from XML, but there has to be something.
DV?? Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v

On Tue, Jul 29, 2008 at 05:55:13PM +0100, Richard W.M. Jones wrote:
On Tue, Jul 29, 2008 at 05:50:30PM +0100, John Levon wrote:
On Tue, Jul 29, 2008 at 05:46:34PM +0100, Richard W.M. Jones wrote:
Do you think a graphical libvirt XML format editor is a good solution?
This would help enormously. You could even start it from 'virsh edit' if $DISPLAY is set.
I've no idea about the general tools out there for generating UI from XML, but there has to be something.
DV??
:-) I can probably be found on record in a few place suggesting to not inflict XML directly to users as what you frequently get is a very annoyed person and a not-wellformed instance in the end. Adding vi as the editor I really hope no journalist will ever try that feature :-) But this can be useful for those who know what they are doing. Unfortunately, general purpose XML editors, are very complex, usually large beast, and not a good solution for a the problem. There exist some specific XML formats editors but they hide the syntax based on the semantic of the XML format (e.g. the glade UI editor) but are not suitable as a result. I don't think there is a good option between editing as text using $EDITOR and a dedicated editing tool. Ideally this could be the configuration dialog of virt-manager separated as a standalone application for coherency and minimizing external dependancies. If not present fallback to $EDITOR, so IMHO having the edit command now even if the graphical editor is not here is just a first step, I agree with this approach. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

On Wed, Jul 30, 2008 at 04:14:30AM -0400, Daniel Veillard wrote:
On Tue, Jul 29, 2008 at 05:55:13PM +0100, Richard W.M. Jones wrote:
On Tue, Jul 29, 2008 at 05:50:30PM +0100, John Levon wrote:
On Tue, Jul 29, 2008 at 05:46:34PM +0100, Richard W.M. Jones wrote:
Do you think a graphical libvirt XML format editor is a good solution?
This would help enormously. You could even start it from 'virsh edit' if $DISPLAY is set.
I've no idea about the general tools out there for generating UI from XML, but there has to be something.
DV??
:-)
I can probably be found on record in a few place suggesting to not inflict XML directly to users as what you frequently get is a very annoyed person and a not-wellformed instance in the end. Adding vi as the editor I really hope no journalist will ever try that feature :-)
But this can be useful for those who know what they are doing.
Unfortunately, general purpose XML editors, are very complex, usually large beast, and not a good solution for a the problem. There exist some specific XML formats editors but they hide the syntax based on the semantic of the XML format (e.g. the glade UI editor) but are not suitable as a result.
I don't think there is a good option between editing as text using $EDITOR and a dedicated editing tool. Ideally this could be the configuration dialog of virt-manager separated as a standalone application for coherency and minimizing external dependancies. If not present fallback to $EDITOR, so IMHO having the edit command now even if the graphical editor is not here is just a first step, I agree with this approach.
Yes, virt-manager is getting ever expanding capabilities WRT to editing guest configuration settings, so I don't think we have a pressing need for another graphical editor for this yet. This virsh edit command is basically a nice convenience for a set of actions that many admins are already doing on a fairly regular basis, and its not a significantly complex piece of code for us to maintain. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Tue, Jul 29, 2008 at 05:09:37PM +0100, John Levon wrote:
On Tue, Jul 29, 2008 at 05:00:27PM +0100, Richard W.M. Jones wrote:
Isn't the *right* solution to this problem to finally add property set/get interface for the things people actually want to modify, like boot flags?
Do you mean things like the current 'virsh attach-device' / 'virsh detach-device' interface?
I hope not... that's just as bad as editing the domain XML.
I'm responding here to a need that sysadmins feel they have -- to edit the configuration file (even if it's XML). Witness an endless series of questions on this subject on the #virt channel yesterday.
Right. But to my mind you're fixing the symptom not the problem. *Why* do they need to edit the XML? I ask this of everybody who complains at me about having to edit XML: 99% of the time it's wanting to change boot flags, but it's also stuff like turning off ACPI, setting on_crash, etc.
Editing XML is absolutely not user friendly, and adding 'edit' just papers over the real problems IMHO.
When I did the storage APIs, I had the traditional commands 'create' and 'define' which took XML documents. I then also add a 'create-as' and 'define-as' command which took a list of named arguments. virsh then turned these into XML docs. eg, to create an LVM backed storage pool from /dev/hda2, you could do something like virsh pool-define-as --source-path /dev/hda2 MyVolGroup lvm And it'd create <pool type='lvm'> <name>MyVolGroup</name> <source> <device path='/dev/hda2'/> </source> </pool> This only deals with creation, or defining a new config, not updating an existing config - in the latter you'd only want to specify the bits which are actually changing - for that we'd want an 'edit-as' command which reads the XML, updates the bits that are changing, and saves the XML back into libvirt We could try todo a similar thing for domains too, though obviously we're going to have a huge number of options to handle to get decent coverage. We'd also need to have an 'edit-as' Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Tue, Jul 29, 2008 at 01:27:42PM +0100, Richard W.M. Jones wrote:
but with much more sanity checking. The editor is $EDITOR or vi, and it does the right thing if the user doesn't modify the file, or if another user edits the configuration at the same time. [...] + /* Compare original XML with edited. Has it changed at all? */ + if (STREQ (doc, doc_edited)) { + vshPrint(ctl, _("Domain %s XML configuration not changed.\n"), + virDomainGetName (dom)); + ret = TRUE; + goto cleanup; + }
Hum, I would check for basic well-formedness here because it just too easy to break the XML file while editing with a text editor http://xmlsoft.org/html/libxml-parser.html#xmlReadMemory if the call fails to build a parsed document suggest to reedit from scratch, based on the current buffer or just abort the edit. Numerous newbies will get trapped in vi, get lost and looking for a safe exit path :-) Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

On Wed, Jul 30, 2008 at 05:43:44AM -0400, Daniel Veillard wrote:
Hum, I would check for basic well-formedness here because it just too easy to break the XML file while editing with a text editor
But the subsequent call to virDomainDefineXML should fail if the XML isn't well-formed. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/

On Wed, Jul 30, 2008 at 10:56:48AM +0100, Richard W.M. Jones wrote:
On Wed, Jul 30, 2008 at 05:43:44AM -0400, Daniel Veillard wrote:
Hum, I would check for basic well-formedness here because it just too easy to break the XML file while editing with a text editor
But the subsequent call to virDomainDefineXML should fail if the XML isn't well-formed.
Right, but you're taking a risk and not giving a chance for the user to escape while being safe. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

On Wed, Jul 30, 2008 at 06:04:23AM -0400, Daniel Veillard wrote:
On Wed, Jul 30, 2008 at 10:56:48AM +0100, Richard W.M. Jones wrote:
On Wed, Jul 30, 2008 at 05:43:44AM -0400, Daniel Veillard wrote:
Hum, I would check for basic well-formedness here because it just too easy to break the XML file while editing with a text editor
But the subsequent call to virDomainDefineXML should fail if the XML isn't well-formed.
Right, but you're taking a risk and not giving a chance for the user to escape while being safe.
As it stands, this is the error message that users get if they edit the XML so that it is not well-formed: # virsh edit RHEL5U2 libvir: QEMU error : XML description not well formed or invalid and the XML isn't changed. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones Read my OCaml programming blog: http://camltastic.blogspot.com/ Fedora now supports 59 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

On Wed, Jul 30, 2008 at 12:49:20PM +0100, Richard W.M. Jones wrote:
On Wed, Jul 30, 2008 at 06:04:23AM -0400, Daniel Veillard wrote:
On Wed, Jul 30, 2008 at 10:56:48AM +0100, Richard W.M. Jones wrote:
On Wed, Jul 30, 2008 at 05:43:44AM -0400, Daniel Veillard wrote:
Hum, I would check for basic well-formedness here because it just too easy to break the XML file while editing with a text editor
But the subsequent call to virDomainDefineXML should fail if the XML isn't well-formed.
Right, but you're taking a risk and not giving a chance for the user to escape while being safe.
As it stands, this is the error message that users get if they edit the XML so that it is not well-formed:
# virsh edit RHEL5U2 libvir: QEMU error : XML description not well formed or invalid
and the XML isn't changed.
and they don't see the error information from libxml2 and the line number ? if that's the case that's one more argument for doing that separated well formedness checking. Could be a good time to check against the RNG too because those are not things easilly doable from the library itself. If you don't like this, don't do it, but i think it's wrong from an user POV Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

On Wed, Jul 30, 2008 at 07:54:46AM -0400, Daniel Veillard wrote:
and they don't see the error information from libxml2 and the line number ? if that's the case that's one more argument for doing that separated well formedness checking. Could be a good time to check against the RNG too because those are not things easilly doable from the library itself.
I think this should be fixed in vir*DefineXML functions, since at the moment 'virsh define' produces exactly the same error. I'll have a go at a separate patch to fix that later on. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/

On Wed, Jul 30, 2008 at 07:54:46AM -0400, Daniel Veillard wrote:
On Wed, Jul 30, 2008 at 12:49:20PM +0100, Richard W.M. Jones wrote:
On Wed, Jul 30, 2008 at 06:04:23AM -0400, Daniel Veillard wrote:
On Wed, Jul 30, 2008 at 10:56:48AM +0100, Richard W.M. Jones wrote:
On Wed, Jul 30, 2008 at 05:43:44AM -0400, Daniel Veillard wrote:
Hum, I would check for basic well-formedness here because it just too easy to break the XML file while editing with a text editor
But the subsequent call to virDomainDefineXML should fail if the XML isn't well-formed.
Right, but you're taking a risk and not giving a chance for the user to escape while being safe.
As it stands, this is the error message that users get if they edit the XML so that it is not well-formed:
# virsh edit RHEL5U2 libvir: QEMU error : XML description not well formed or invalid
and the XML isn't changed.
and they don't see the error information from libxml2 and the line number ?
Hmm, I seem to recall that you previously modified the Xen driver's XML parser to try and include that info. This could be something we need to add to the new XML parser for better error reporting. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (4)
-
Daniel P. Berrange
-
Daniel Veillard
-
John Levon
-
Richard W.M. Jones