mirror of
https://github.com/virt-manager/virt-manager.git
synced 2024-12-22 13:34:07 +03:00
virtxml: Add Action class to track action + option pairs
This adds an Action class which is a container for (most) of the
data we need to perform a virt-xml operation. Basically an instance
of the class represents a pairing like
--edit 1 --disk path=/foo or
--add-device --network wibble,model=virtio
etc
This is a functional no-op, but it moves the code closer to
an architecture that will allow us to perform multiple actions
with a single virt-xml command.
While doing this, we centralize most of the cli validation that is
scattered around, since it's now easier to do it in one place.
Signed-off-by: Cole Robinson <crobinso@redhat.com>
This commit is contained in:
Cole Robinson
parent
b7c72af73d
commit
455b38f724
@ -63,37 +63,122 @@ def defined_xml_is_unchanged(conn, domain, original_xml):
|
||||
# Action parsing #
|
||||
##################
|
||||
|
||||
def check_action_collision(options):
|
||||
actions = ["edit", "add-device", "remove-device", "build-xml"]
|
||||
class Action:
|
||||
"""
|
||||
Helper class tracking one pair of
|
||||
XML ACTION (ex. --edit) and
|
||||
XML OPTION (ex. --disk)
|
||||
"""
|
||||
def __init__(self, action_name, selector, parserclass, parservalue):
|
||||
# one of ["edit", "add-device", "remove-device", "build-xml"]
|
||||
self.action_name = action_name
|
||||
# ex. for `--edit 1` this is selector="1"
|
||||
self.selector = selector
|
||||
# ParserDisk, etc
|
||||
self.parserclass = parserclass
|
||||
# ex for `--disk path=/foo` this is "path=/foo"
|
||||
self.parservalue = parservalue
|
||||
|
||||
@property
|
||||
def is_edit(self):
|
||||
return self.action_name == "edit"
|
||||
|
||||
@property
|
||||
def is_add_device(self):
|
||||
return self.action_name == "add-device"
|
||||
|
||||
@property
|
||||
def is_remove_device(self):
|
||||
return self.action_name == "remove-device"
|
||||
|
||||
@property
|
||||
def is_build_xml(self):
|
||||
return self.action_name == "build-xml"
|
||||
|
||||
|
||||
def validate_action(action, conn, options):
|
||||
if options.os_variant is not None:
|
||||
if action.is_edit:
|
||||
fail(_("--os-variant/--osinfo is not supported with --edit"))
|
||||
if action.is_remove_device:
|
||||
fail(_("--os-variant/--osinfo is not supported with --remove-device"))
|
||||
if action.is_build_xml:
|
||||
fail(_("--os-variant/--osinfo is not supported with --build-xml"))
|
||||
|
||||
if not action.parserclass.guest_propname and action.is_build_xml:
|
||||
fail(_("--build-xml not supported for --%s") %
|
||||
action.parserclass.cli_arg_name)
|
||||
|
||||
if action.parserclass is cli.ParserXML and not action.is_edit:
|
||||
fail(_("--xml can only be used with --edit"))
|
||||
|
||||
stub_guest = Guest(conn)
|
||||
if not action.parserclass.prop_is_list(stub_guest):
|
||||
if action.is_remove_device:
|
||||
fail(_("Cannot use --remove-device with --%s") %
|
||||
action.parserclass.cli_arg_name)
|
||||
if action.is_add_device:
|
||||
fail(_("Cannot use --add-device with --%s") %
|
||||
action.parserclass.cli_arg_name)
|
||||
|
||||
if options.update and not action.parserclass.guest_propname:
|
||||
fail(_("Don't know how to --update for --%s") %
|
||||
(action.parserclass.cli_arg_name))
|
||||
|
||||
|
||||
def check_action_collision(options):
|
||||
collisions = []
|
||||
actions = ["edit", "add-device", "remove-device", "build-xml"]
|
||||
for cliname in actions:
|
||||
optname = cliname.replace("-", "_")
|
||||
if getattr(options, optname) not in [False, -1]:
|
||||
collisions.append(cliname)
|
||||
value = getattr(options, optname)
|
||||
if value not in [False, -1]:
|
||||
collisions.append((cliname, value))
|
||||
|
||||
if len(collisions) == 0:
|
||||
fail(_("One of %s must be specified.") %
|
||||
", ".join(["--" + c for c in actions]))
|
||||
if len(collisions) > 1:
|
||||
fail(_("Conflicting options %s") %
|
||||
", ".join(["--" + c for c in collisions]))
|
||||
", ".join(["--" + c[0] for c in collisions]))
|
||||
|
||||
action_name, selector = collisions[0]
|
||||
return action_name, selector
|
||||
|
||||
|
||||
def check_xmlopt_collision(options):
|
||||
collisions = []
|
||||
for parserclass in cli.VIRT_PARSERS + [cli.ParserXML]:
|
||||
if getattr(options, parserclass.cli_arg_name):
|
||||
collisions.append(parserclass)
|
||||
value = getattr(options, parserclass.cli_arg_name)
|
||||
if value:
|
||||
collisions.append((parserclass, value))
|
||||
|
||||
if len(collisions) == 0:
|
||||
fail(_("No change specified."))
|
||||
if len(collisions) != 1:
|
||||
fail(_("Only one change operation may be specified "
|
||||
"(conflicting options %s)") %
|
||||
[c.cli_flag_name() for c in collisions])
|
||||
[c[0].cli_flag_name() for c in collisions])
|
||||
|
||||
return collisions[0]
|
||||
parserclass, parservalue = collisions[0]
|
||||
return parserclass, parservalue
|
||||
|
||||
|
||||
def parse_action(conn, options):
|
||||
# Ensure there wasn't more than one device/xml config option
|
||||
# specified. So reject '--disk X --network X'
|
||||
parserclass, parservalue = check_xmlopt_collision(options)
|
||||
|
||||
# Ensure only one of these actions was specified
|
||||
# --edit
|
||||
# --remove-device
|
||||
# --add-device
|
||||
# --build-xml
|
||||
action_name, selector = check_action_collision(options)
|
||||
|
||||
action = Action(action_name, selector, parserclass, parservalue)
|
||||
validate_action(action, conn, options)
|
||||
return action
|
||||
|
||||
|
||||
################
|
||||
@ -146,33 +231,41 @@ def _find_objects_to_edit(guest, action_name, editval, parserclass):
|
||||
return inst
|
||||
|
||||
|
||||
def action_edit(guest, options, parserclass):
|
||||
def action_edit(action, guest, options):
|
||||
parserclass = action.parserclass
|
||||
selector = action.selector
|
||||
|
||||
if parserclass is cli.ParserXML:
|
||||
cli.parse_xmlcli(guest, options)
|
||||
return []
|
||||
|
||||
if parserclass.guest_propname:
|
||||
inst = _find_objects_to_edit(guest, "edit", options.edit, parserclass)
|
||||
inst = _find_objects_to_edit(guest, "edit",
|
||||
selector, parserclass)
|
||||
else:
|
||||
inst = guest
|
||||
if options.edit and options.edit != '1' and options.edit != 'all':
|
||||
if (selector and selector != '1' and selector != 'all'):
|
||||
fail(_("'--edit %(option)s' doesn't make sense with "
|
||||
"--%(objecttype)s, just use empty '--edit'") %
|
||||
{"option": options.edit,
|
||||
{"option": selector,
|
||||
"objecttype": parserclass.cli_arg_name})
|
||||
if options.os_variant is not None:
|
||||
fail(_("--os-variant/--osinfo is not supported with --edit"))
|
||||
|
||||
devs = []
|
||||
for editinst in xmlutil.listify(inst):
|
||||
devs += cli.run_parser(options, guest, parserclass, editinst=editinst)
|
||||
devs += cli.run_parser(options, guest, parserclass,
|
||||
editinst=editinst)
|
||||
return devs
|
||||
|
||||
|
||||
def action_add_device(guest, options, parserclass, devs):
|
||||
if not parserclass.prop_is_list(guest):
|
||||
fail(_("Cannot use --add-device with --%s") % parserclass.cli_arg_name)
|
||||
def action_add_device(action, guest, options, input_devs):
|
||||
parserclass = action.parserclass
|
||||
|
||||
set_os_variant(options, guest)
|
||||
|
||||
if devs:
|
||||
for dev in devs:
|
||||
if input_devs:
|
||||
for dev in input_devs:
|
||||
guest.add_device(dev)
|
||||
devs = input_devs
|
||||
else:
|
||||
devs = cli.run_parser(options, guest, parserclass)
|
||||
for dev in devs:
|
||||
@ -181,15 +274,12 @@ def action_add_device(guest, options, parserclass, devs):
|
||||
return devs
|
||||
|
||||
|
||||
def action_remove_device(guest, options, parserclass):
|
||||
if not parserclass.prop_is_list(guest):
|
||||
fail(_("Cannot use --remove-device with --%s") %
|
||||
parserclass.cli_arg_name)
|
||||
if options.os_variant is not None:
|
||||
fail(_("--os-variant/--osinfo is not supported with --remove-device"))
|
||||
def action_remove_device(action, guest):
|
||||
parserclass = action.parserclass
|
||||
parservalue = action.parservalue[-1]
|
||||
|
||||
devs = _find_objects_to_edit(guest, "remove-device",
|
||||
getattr(options, parserclass.cli_arg_name)[-1], parserclass)
|
||||
parservalue, parserclass)
|
||||
devs = xmlutil.listify(devs)
|
||||
|
||||
for dev in devs:
|
||||
@ -197,12 +287,8 @@ def action_remove_device(guest, options, parserclass):
|
||||
return devs
|
||||
|
||||
|
||||
def action_build_xml(options, parserclass, guest):
|
||||
if not parserclass.guest_propname:
|
||||
fail(_("--build-xml not supported for --%s") %
|
||||
parserclass.cli_arg_name)
|
||||
if options.os_variant is not None:
|
||||
fail(_("--os-variant/--osinfo is not supported with --build-xml"))
|
||||
def action_build_xml(action, guest, options):
|
||||
parserclass = action.parserclass
|
||||
|
||||
devs = cli.run_parser(options, guest, parserclass)
|
||||
for dev in devs:
|
||||
@ -210,6 +296,18 @@ def action_build_xml(options, parserclass, guest):
|
||||
return devs
|
||||
|
||||
|
||||
def perform_action(action, guest, options, input_devs):
|
||||
if action.is_add_device:
|
||||
return action_add_device(action, guest, options, input_devs)
|
||||
if action.is_remove_device:
|
||||
return action_remove_device(action, guest)
|
||||
if action.is_edit:
|
||||
return action_edit(action, guest, options)
|
||||
raise xmlutil.DevError(
|
||||
"perform_action() incorrectly called with action_name=%s" %
|
||||
action.action_name)
|
||||
|
||||
|
||||
def setup_device(dev):
|
||||
if getattr(dev, "DEVICE_TYPE", None) != "disk":
|
||||
return
|
||||
@ -224,7 +322,7 @@ def define_changes(conn, inactive_xmlobj, devs, action, confirm):
|
||||
_("Define '%s' with the changed XML?") % inactive_xmlobj.name):
|
||||
return False
|
||||
|
||||
if action == "hotplug":
|
||||
if action.is_add_device:
|
||||
for dev in devs:
|
||||
setup_device(dev)
|
||||
|
||||
@ -239,7 +337,7 @@ def start_domain_transient(conn, xmlobj, devs, action, confirm):
|
||||
_("Start '%s' with the changed XML?") % xmlobj.name):
|
||||
return False
|
||||
|
||||
if action == "hotplug":
|
||||
if action.is_add_device:
|
||||
for dev in devs:
|
||||
setup_device(dev)
|
||||
|
||||
@ -256,21 +354,25 @@ def start_domain_transient(conn, xmlobj, devs, action, confirm):
|
||||
|
||||
|
||||
def update_changes(domain, devs, action, confirm):
|
||||
if action == "hotplug":
|
||||
if action.is_add_device:
|
||||
msg_confirm = _("%(xml)s\n\nHotplug this device to the guest "
|
||||
"'%(domain)s'?")
|
||||
msg_success = _("Device hotplug successful.")
|
||||
msg_fail = _("Error attempting device hotplug: %(error)s")
|
||||
elif action == "hotunplug":
|
||||
elif action.is_remove_device:
|
||||
msg_confirm = _("%(xml)s\n\nHotunplug this device from the guest "
|
||||
"'%(domain)s'?")
|
||||
msg_success = _("Device hotunplug successful.")
|
||||
msg_fail = _("Error attempting device hotunplug: %(error)s")
|
||||
elif action == "update":
|
||||
elif action.is_edit:
|
||||
msg_confirm = _("%(xml)s\n\nUpdate this device for the guest "
|
||||
"'%(domain)s'?")
|
||||
msg_success = _("Device update successful.")
|
||||
msg_fail = _("Error attempting device update: %(error)s")
|
||||
else:
|
||||
raise xmlutil.DevError(
|
||||
"update_changes() incorrectly called with action=%s" %
|
||||
action.action_name)
|
||||
|
||||
for dev in devs:
|
||||
xml = dev.get_xml()
|
||||
@ -283,15 +385,15 @@ def update_changes(domain, devs, action, confirm):
|
||||
if not prompt_yes_or_no(msg):
|
||||
continue
|
||||
|
||||
if action == "hotplug":
|
||||
if action.is_add_device:
|
||||
setup_device(dev)
|
||||
|
||||
try:
|
||||
if action == "hotplug":
|
||||
if action.is_add_device:
|
||||
domain.attachDeviceFlags(xml, libvirt.VIR_DOMAIN_AFFECT_LIVE)
|
||||
elif action == "hotunplug":
|
||||
elif action.is_remove_device:
|
||||
domain.detachDeviceFlags(xml, libvirt.VIR_DOMAIN_AFFECT_LIVE)
|
||||
elif action == "update":
|
||||
elif action.is_edit:
|
||||
domain.updateDeviceFlags(xml, libvirt.VIR_DOMAIN_AFFECT_LIVE)
|
||||
except libvirt.libvirtError as e:
|
||||
if "VIRTXML_TESTSUITE_UPDATE_IGNORE_FAIL" not in os.environ:
|
||||
@ -302,37 +404,17 @@ def update_changes(domain, devs, action, confirm):
|
||||
print_stdout("")
|
||||
|
||||
|
||||
def prepare_changes(orig_xmlobj, options, parserclass, devs=None):
|
||||
def prepare_changes(orig_xmlobj, options, action, input_devs=None):
|
||||
"""
|
||||
Parse the command line device/XML arguments, and apply the changes to
|
||||
a copy of the passed in xmlobj.
|
||||
Perform requested XML edits locally, but don't submit them to libvirt.
|
||||
Optionally perform any XML printing per user request
|
||||
|
||||
:returns: (list of device objects, action string, altered xmlobj)
|
||||
:returns: (list of device objects, altered xmlobj)
|
||||
"""
|
||||
origxml = orig_xmlobj.get_xml()
|
||||
xmlobj = orig_xmlobj.__class__(conn=orig_xmlobj.conn, parsexml=origxml)
|
||||
has_edit = options.edit != -1
|
||||
is_xmlcli = parserclass is cli.ParserXML
|
||||
|
||||
if is_xmlcli and not has_edit:
|
||||
fail(_("--xml can only be used with --edit"))
|
||||
|
||||
if has_edit:
|
||||
if is_xmlcli:
|
||||
devs = []
|
||||
cli.parse_xmlcli(xmlobj, options)
|
||||
else:
|
||||
devs = action_edit(xmlobj, options, parserclass)
|
||||
action = "update"
|
||||
|
||||
elif options.add_device:
|
||||
devs = action_add_device(xmlobj, options, parserclass, devs)
|
||||
action = "hotplug"
|
||||
|
||||
elif options.remove_device:
|
||||
devs = action_remove_device(xmlobj, options, parserclass)
|
||||
action = "hotunplug"
|
||||
|
||||
devs = perform_action(action, xmlobj, options, input_devs)
|
||||
newxml = xmlobj.get_xml()
|
||||
diff = get_diff(origxml, newxml)
|
||||
|
||||
@ -346,7 +428,7 @@ def prepare_changes(orig_xmlobj, options, parserclass, devs=None):
|
||||
elif options.print_xml:
|
||||
print_stdout(newxml)
|
||||
|
||||
return devs, action, xmlobj
|
||||
return devs, xmlobj
|
||||
|
||||
|
||||
#######################
|
||||
@ -470,22 +552,8 @@ def main(conn=None):
|
||||
if options.confirm and not options.print_xml:
|
||||
options.print_diff = True
|
||||
|
||||
# Ensure only one of these actions wash specified
|
||||
# --edit
|
||||
# --remove-device
|
||||
# --add-device
|
||||
# --build-xml
|
||||
check_action_collision(options)
|
||||
|
||||
# Ensure there wasn't more than one device/xml config option
|
||||
# specified. So reject '--disk X --network X'
|
||||
parserclass = check_xmlopt_collision(options)
|
||||
|
||||
if options.update and not parserclass.guest_propname:
|
||||
fail(_("Don't know how to --update for --%s") %
|
||||
(parserclass.cli_arg_name))
|
||||
|
||||
conn = cli.getConnection(options.connect, conn)
|
||||
action = parse_action(conn, options)
|
||||
|
||||
domain = None
|
||||
active_xmlobj = None
|
||||
@ -497,22 +565,22 @@ def main(conn=None):
|
||||
inactive_xmlobj = Guest(conn, parsexml=options.stdinxml)
|
||||
vm_is_running = bool(active_xmlobj)
|
||||
|
||||
if options.build_xml:
|
||||
devs = action_build_xml(options, parserclass, inactive_xmlobj)
|
||||
for dev in devs:
|
||||
if action.is_build_xml:
|
||||
built_devs = action_build_xml(action, inactive_xmlobj, options)
|
||||
for dev in built_devs:
|
||||
# pylint: disable=no-member
|
||||
print_stdout(xmlutil.unindent_device_xml(dev.get_xml()))
|
||||
return 0
|
||||
|
||||
devs = None
|
||||
input_devs = None
|
||||
performed_update = False
|
||||
if options.update:
|
||||
if options.update and options.start:
|
||||
fail_conflicting("--update", "--start")
|
||||
if vm_is_running:
|
||||
devs, action, dummy = prepare_changes(
|
||||
active_xmlobj, options, parserclass)
|
||||
update_changes(domain, devs, action, options.confirm)
|
||||
input_devs, dummy = prepare_changes(
|
||||
active_xmlobj, options, action)
|
||||
update_changes(domain, input_devs, action, options.confirm)
|
||||
performed_update = True
|
||||
else:
|
||||
log.warning(
|
||||
@ -522,8 +590,8 @@ def main(conn=None):
|
||||
return 0
|
||||
|
||||
original_xml = inactive_xmlobj.get_xml()
|
||||
devs, action, xmlobj_to_define = prepare_changes(
|
||||
inactive_xmlobj, options, parserclass, devs=devs)
|
||||
devs, xmlobj_to_define = prepare_changes(
|
||||
inactive_xmlobj, options, action, input_devs=input_devs)
|
||||
if not options.define:
|
||||
if options.start:
|
||||
start_domain_transient(conn, xmlobj_to_define, devs,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user