Justin, how does the below affect plans to modify/fix virsh in libvirt? On Wed, Apr 27, 2011 at 09:47:55AM -0400, Cole Robinson wrote:

Hi all,

Attached is a patch to virtinst.git that provides a new tool named virt-xml. virt-xml provides a scriptable command line interface for editing libvirt guest XML.

A couple examples:

Change boot order and boot menu for 'myguest': $ ./virt-xml --guest myguest --boot network,hd,cdrom,menu=on Requested changes: --- Original XML +++ New XML @@ -10,6 +10,8 @@ <os> <type arch="i686">hvm</type> <loader>/usr/lib/xen/boot/hvmloader</loader> + <boot dev="network"/> + <bootmenu enable="yes"/> <boot dev="hd"/> + <boot dev="cdrom"/> </os> <features> <acpi/>

Change disk #7 to use cache=none for 'myguest': $ ./virt-xml --guest myguest --device 7 --disk cache=none Requested changes: --- Original XML +++ New XML @@ -74,6 +74,7 @@ <disk type="file" device="disk"> <source file="/tmp/foobar3"/> <target dev="sdd" bus="usb"/> + <driver cache="none"/> </disk> <disk type="file" device="disk"> <driver name="tap" type="qcow" cache="writethrough"/>

Change watchdog action for 'myguest': $ ./virt-xml --guest myguest --device 1 --watchdog action=pause Requested changes: --- Original XML +++ New XML @@ -220,7 +220,7 @@ <address domain="0x0003" bus="0x00" slot="0x19" function="0x0"/> </source> </hostdev> - <watchdog model="ib700" action="poweroff"/> + <watchdog model="ib700" action="pause"/> <memballoon model="xen"/> </devices> </domain>

--boot, --disk, and --watchdog come straight from virt-install, see man virt-install for detailed docs of the option format. Those options shown are the only things wired up right now, but it's a straightforward task to wire up most of the relevant remaining virt-install opts. Also the tool doesn't actually apply the changes yet, but that is also a trivial addition. I think the interesting bit here is nailing down the scope of the tool and deciding on good CLI syntax.

Not sure if we should do hotplug/hotunplug as a subcommand or just another option like:

virt-xml ... --hotplug --disk /tmp/foo.img

Also not really sure how to hotunplug using the --device syntax of the above examples:

virt-xml ... --hotunplug --device 1 --disk <what goes here?>

We could make --device just an option to whatever device string the user gives

virt-xml ... --hotunplug --disk id=1

We can easily allow different XML input methods like:

virt-xml --guestxml file.xml ...

or even batch processing

virt-xml --guest * ...

Maybe even have allow more intelligence about what guests we edit, say we want to set cache=none for all disk devices, not CDROM or floppy (this would take a bit of work to sort out).

virt-xml --guest * --disk match,device=disk --disk cache=none

Then there is also the question of extending the tool to also edit other XML like storage, network, interface, etc.

Any recommendations appreciated: option syntax, future capabilities, or even a better name for the tool :)

A quick word about why I think we should do this with a separate tool rather than virsh. For starters, I think the need for this functionality on the command line is clear by now, so it's just a question of where to implement it.

Why not virsh? Text and command line processing in C is a serious pain compared to a higher level language like python. I don't think this is a controversial statement, in fact I think it's partly responsible for why the virsh XML building commands have typically been in a pretty sorry state. virsh could conceivably use the internal domain_conf API to make this task much easier, but it would still require a lot of command line options for every XML parameter.

In contrast, virt-install already needs to do most of this command line processing, and virt-manager needs to do all the XML building, parsing, and editing. Since this code already lives in one place (virtinst), it's almost trivial for us to reuse it. Supporting a new XML property on the command line is a very simple task and will immediately benefit both virt-install and virt-xml. We also already have over 75% of the domain XML schema available via existing CLI options that are already documented :)

Questions or comments appreciated!

Thanks, Cole

...

From cd821fe259734767619ccb4ca3001df0667c3aed Mon Sep 17 00:00:00 2001 From: Cole Robinson <crobinso@redhat.com> Date: Thu, 21 Apr 2011 13:13:49 -0400 Subject: [PATCH] virt-xml initial commit

--- tests/pylint-virtinst.sh | 2 +- todo.txt | 12 +++ virt-xml | 200 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 213 insertions(+), 1 deletions(-) create mode 100644 todo.txt create mode 100755 virt-xml

diff --git a/tests/pylint-virtinst.sh b/tests/pylint-virtinst.sh index 7c2a932..98c42fa 100755 --- a/tests/pylint-virtinst.sh +++ b/tests/pylint-virtinst.sh @@ -1,6 +1,6 @@ #!/bin/sh

-FILES="setup.py tests/ virt-install virt-image virt-clone virt-convert virtinst/ virtconv virtconv/parsers/*.py" +FILES="setup.py tests/ virt-install virt-image virt-clone virt-convert virt-xml virtinst/ virtconv virtconv/parsers/*.py"

# Don't print pylint config warning NO_PYL_CONFIG=".*No config file found.*" diff --git a/todo.txt b/todo.txt new file mode 100644 index 0000000..9705a43 --- /dev/null +++ b/todo.txt @@ -0,0 +1,12 @@ + +virt-xml: + change commands: + add device + remove device + edit device + targets: + --conn URI --guest GUESTNAME + --conn URI --guestfile file.xml + other opts: + --diff (just print diff) + --print-xml (just print new XML) diff --git a/virt-xml b/virt-xml new file mode 100755 index 0000000..9369991 --- /dev/null +++ b/virt-xml @@ -0,0 +1,200 @@ +#!/usr/bin/python -tt +# +# Copyright 2011 Red Hat, Inc. +# Cole Robinson <crobinso@redhat.com> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, +# MA 02110-1301 USA. + +import sys +import logging +import optparse + +import libvirt +import virtinst +import virtinst.CapabilitiesParser +import virtinst.cli as cli +import virtinst.support as support +from virtinst.VirtualCharDevice import VirtualCharDevice +from virtinst.VirtualDevice import VirtualDevice +from virtinst.cli import fail, print_stdout, print_stderr +import difflib + +cli.setupGettext() + +def diff(orig, new): + """ + Return a unified diff string between the passed strings + """ + return "".join(difflib.unified_diff(orig.splitlines(1), + new.splitlines(1), + fromfile="Original XML", + tofile="New XML")) + +######################### +# XML <device> altering # +######################### + +def change_devices(guest, options): + devmap = { + VirtualDevice.VIRTUAL_DEV_DISK : options.diskopts, + VirtualDevice.VIRTUAL_DEV_WATCHDOG : options.watchdog, + } + + if not any(devmap.values()): + return + + if sum(map(int, map(bool, devmap.values()))) > 1: + fail(_("Only one device can be changed at a time")) + + if options.device is None: + fail(_("A --device must be specified if altering an existing device")) + if options.device < 1: + fail(_("--device must be greater than 1")) + + dev = None + try: + for devtype, devstr in devmap.items(): + if devstr: + dev = guest.get_devices(devtype)[options.device - 1] + break + except Exception, e: + cli.log_exception() + fail(_("Didn't find requested device: %s" % e)) + + if options.diskopts: + cli.parse_disk(guest, options.diskopts, dev) + elif options.watchdog: + cli.parse_watchdog(guest, options.watchdog, dev) + + +################## +# main() helpers # +################## + +def get_xml_flags(vm): + flags = 0 + if support.check_domain_support(vm, support.SUPPORT_DOMAIN_XML_INACTIVE): + flags |= libvirt.VIR_DOMAIN_XML_INACTIVE + else: + logging.debug("Domain XML inactive flag not supported.") + + if support.check_domain_support(vm, support.SUPPORT_DOMAIN_XML_SECURE): + flags |= libvirt.VIR_DOMAIN_XML_SECURE + else: + logging.debug("Domain XML secure flag not supported.") + + return flags + +def build_guest(conn, guestname): + try: + vm = conn.lookupByName(guestname) + except Exception, e: + fail(_("Error fetching domain '%s': %s") % (guestname, e)) + + try: + flags = get_xml_flags(vm) + except Exception, e: + cli.log_exception() + fail(_("Error determining XML flags for '%s': %s") % (guestname, e)) + + try: + xml = vm.XMLDesc(flags) + except Exception, e: + fail(_("Error fetching XML for '%s': %s") % (guestname, e)) + + logging.debug("Original XML:\n%s" % xml) + + try: + return virtinst.Guest(connection=conn, parsexml=xml) + except Exception, e: + cli.log_exception() + fail(_("Error parsing '%s' XML: %s") % (guestname, e)) + +def parse_args(): + usage = "%prog " + parser = cli.setupParser(usage) + cli.add_connect_option(parser) + + parser.add_option("-g", "--guest", dest="guest", + help=_("Name of guest to alter")) + + + gstg = optparse.OptionGroup(parser, _("Guest Configuration")) + gstg.add_option("", "--boot", dest="bootopts", + help=_("Configure boot order, menu, permanent kernel " + "boot, etc.")) + parser.add_option_group(gstg) + + devg = optparse.OptionGroup(parser, _("Device Configuration")) + devg.add_option("", "--device", type="int", dest="device", + help=_("Device index")) + + devg.add_option("", "--disk", dest="diskopts", + help=_("Specify disk device with various options. Ex.\n" + "--disk path=/my/existing/disk\n" + "--disk path=/my/new/disk,size=5 (in gigabytes)\n" + "--disk vol=poolname:volname,device=cdrom,bus=scsi,...")) + + devg.add_option("", "--watchdog", dest="watchdog", + help=_("Configure a watchdog device")) + parser.add_option_group(devg) + + misc = optparse.OptionGroup(parser, _("Miscellaneous Options")) + misc.add_option("-d", "--debug", action="store_true", dest="debug", + help=_("Print debugging information")) + parser.add_option_group(misc) + + (options, cliargs) = parser.parse_args() + return options, cliargs + +def main(): + cli.earlyLogging() + options, cliargs = parse_args() + cli.setupLogging("virt-xml", options.debug) + + if cliargs: + fail(_("Unknown argument '%s'") % cliargs[0]) + + if not options.guest: + fail(_("--guest must be specified")) + + conn = cli.getConnection(options.connect) + guest = build_guest(conn, options.guest) + origxml = guest.get_xml_config() + + change_devices(guest, options) + cli.parse_boot(guest, options.bootopts) + + newxml = guest.get_xml_config() + diffstr = diff(origxml, newxml) + + if diffstr: + print_stdout(_("Requested changes:") + "\n" + diffstr) + else: + print_stdout(_("No configuration changes were generated")) + + return 0 + +if __name__ == "__main__": + try: + sys.exit(main()) + except SystemExit, sys_e: + sys.exit(sys_e.code) + except KeyboardInterrupt: + cli.log_exception() + print_stderr(_("Installation aborted at user request")) + except Exception, main_e: + fail(main_e) -- 1.7.4

Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw