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

Re: [virt-tools-list] RFC: virt-xml: command line tool for altering guest configuration



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

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