From 36230c9a187cba81cea7ecef23013feaee4124e5 Mon Sep 17 00:00:00 2001 From: Pavel Hrdina Date: Mon, 4 Sep 2017 18:40:34 +0200 Subject: [PATCH] devicepanic: use model instead of address.type There are multiple models of the panic device, the address type is only one and is valid only for "isa" model. To not break the virt-install/virt-xml the command line parser needs to be updated. Before this patch there was only one parameter that configured the "iobase". Now the first parameter configures a model but to keep it backward compatible it follows these rules: 1. there is only one parameter and it matches known model: --panic isa
2. there is only one parameter and it doesn't match any model: --panic 0x505
3. there are two parameters: --panic isa,iobase=0x505
Signed-off-by: Pavel Hrdina --- man/virt-install.pod | 2 +- .../compare/virt-install-many-devices.xml | 2 +- .../compare/virt-install-panic-default.xml | 35 +++++++++++++++++++ .../compare/virt-install-panic-isa-iobase.xml | 35 +++++++++++++++++++ .../compare/virt-install-panic-isa.xml | 35 +++++++++++++++++++ .../virt-install-singleton-config-1.xml | 2 +- .../virt-install-singleton-config-2.xml | 4 +-- tests/clitest.py | 10 ++++++ ui/addhardware.ui | 8 ++--- ui/details.ui | 9 +++-- virtManager/addhardware.py | 20 +++++------ virtManager/details.py | 4 +-- virtinst/cli.py | 23 ++++++++---- virtinst/devicepanic.py | 25 +++++++++---- 14 files changed, 175 insertions(+), 39 deletions(-) create mode 100644 tests/cli-test-xml/compare/virt-install-panic-default.xml create mode 100644 tests/cli-test-xml/compare/virt-install-panic-isa-iobase.xml create mode 100644 tests/cli-test-xml/compare/virt-install-panic-isa.xml diff --git a/man/virt-install.pod b/man/virt-install.pod index 3482e53bb..f2a036a3e 100644 --- a/man/virt-install.pod +++ b/man/virt-install.pod @@ -1589,7 +1589,7 @@ Use --rng=? to see a list of all available sub options. Complete details at L OPTS +=item B<--panic> MODEL[,OPTS] Attach a panic notifier device to the guest. For the recommended settings, use: diff --git a/tests/cli-test-xml/compare/virt-install-many-devices.xml b/tests/cli-test-xml/compare/virt-install-many-devices.xml index 11a0771b4..852b63aa8 100644 --- a/tests/cli-test-xml/compare/virt-install-many-devices.xml +++ b/tests/cli-test-xml/compare/virt-install-many-devices.xml @@ -369,7 +369,7 @@ - +
diff --git a/tests/cli-test-xml/compare/virt-install-panic-default.xml b/tests/cli-test-xml/compare/virt-install-panic-default.xml new file mode 100644 index 000000000..9b7ddfd5d --- /dev/null +++ b/tests/cli-test-xml/compare/virt-install-panic-default.xml @@ -0,0 +1,35 @@ + + foobar + 00000000-1111-2222-3333-444444444444 + 65536 + 65536 + 1 + + hvm + + + + + + + + Opteron_G4 + + + + + + + + + + + + /usr/bin/qemu-kvm + + + +
+ + + diff --git a/tests/cli-test-xml/compare/virt-install-panic-isa-iobase.xml b/tests/cli-test-xml/compare/virt-install-panic-isa-iobase.xml new file mode 100644 index 000000000..714cb56b0 --- /dev/null +++ b/tests/cli-test-xml/compare/virt-install-panic-isa-iobase.xml @@ -0,0 +1,35 @@ + + foobar + 00000000-1111-2222-3333-444444444444 + 65536 + 65536 + 1 + + hvm + + + + + + + + Opteron_G4 + + + + + + + + + + + + /usr/bin/qemu-kvm + + + +
+ + + diff --git a/tests/cli-test-xml/compare/virt-install-panic-isa.xml b/tests/cli-test-xml/compare/virt-install-panic-isa.xml new file mode 100644 index 000000000..9b7ddfd5d --- /dev/null +++ b/tests/cli-test-xml/compare/virt-install-panic-isa.xml @@ -0,0 +1,35 @@ + + foobar + 00000000-1111-2222-3333-444444444444 + 65536 + 65536 + 1 + + hvm + + + + + + + + Opteron_G4 + + + + + + + + + + + + /usr/bin/qemu-kvm + + + +
+ + + diff --git a/tests/cli-test-xml/compare/virt-install-singleton-config-1.xml b/tests/cli-test-xml/compare/virt-install-singleton-config-1.xml index 8bf23451f..5c38036c8 100644 --- a/tests/cli-test-xml/compare/virt-install-singleton-config-1.xml +++ b/tests/cli-test-xml/compare/virt-install-singleton-config-1.xml @@ -63,7 +63,7 @@ /dev/random - +
diff --git a/tests/cli-test-xml/compare/virt-install-singleton-config-2.xml b/tests/cli-test-xml/compare/virt-install-singleton-config-2.xml index 914403e68..d7fbb49c4 100644 --- a/tests/cli-test-xml/compare/virt-install-singleton-config-2.xml +++ b/tests/cli-test-xml/compare/virt-install-singleton-config-2.xml @@ -140,7 +140,7 @@ - +
@@ -294,7 +294,7 @@ - +
diff --git a/tests/clitest.py b/tests/clitest.py index 56148cc0d..147c615c2 100644 --- a/tests/clitest.py +++ b/tests/clitest.py @@ -645,6 +645,15 @@ c.add_invalid("--disk source_pool=default-pool,source_volume=idontexist") # try c.add_invalid("--disk size=1 --security model=foo,type=bar") # Libvirt will error on the invalid security params, which should trigger the code path to clean up the disk images we created. +################ +# Panic device # +################ + +c = vinst.add_category("panic", "--connect %(URI-KVM)s --noautoconsole --import --disk none --graphics none --controller usb,model=none --network none") +c.add_compare("--panic default", "panic-default") +c.add_compare("--panic isa", "panic-isa") +c.add_compare("--panic isa,iobase=0x505", "panic-isa-iobase") + ################################################ # Invalid devices that hit virtinst code paths # @@ -838,6 +847,7 @@ c.add_valid("--bridge mybr0 --mac 22:22:33:44:55:AF") # Old bridge w/ mac c.add_valid("--network bridge:mybr0,model=e1000") # --network bridge: c.add_valid("--network network:default --mac RANDOM") # VirtualNetwork with a random macaddr c.add_valid("--vnc --keymap=local") # --keymap local +c.add_valid("--panic 0x505") # ISA panic with iobase specified c.add_invalid("--nonetworks") # no networks c.add_invalid("--graphics vnc --vnclisten 1.2.3.4") # mixing old and new c.add_invalid("--network=FOO") # Nonexistent network diff --git a/ui/addhardware.ui b/ui/addhardware.ui index caee0be6d..eb476dab1 100644 --- a/ui/addhardware.ui +++ b/ui/addhardware.ui @@ -1720,13 +1720,13 @@ 6 6 - + True False start - Address _Type: + _Model: True - panic-type + panic-model 0 @@ -1734,7 +1734,7 @@ - + True False diff --git a/ui/details.ui b/ui/details.ui index 04d2b213b..f307f340a 100644 --- a/ui/details.ui +++ b/ui/details.ui @@ -5660,13 +5660,13 @@ if you know what you are doing.</small> 4 8 - + True False start 3 3 - Address Type: + Model: 0 @@ -5674,12 +5674,11 @@ if you know what you are doing.</small> - - + True False start - panic-address-type + panic-model 1 diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py index 415e7e901..e14e6bdf4 100644 --- a/virtManager/addhardware.py +++ b/virtManager/addhardware.py @@ -317,8 +317,8 @@ class vmmAddHardware(vmmGObjectUI): self._build_rng_backend_mode_combo(combo) # Panic widgets - combo = self.widget("panic-type") - self._build_panic_address_type(combo) + combo = self.widget("panic-model") + self._build_panic_models(combo) # Controller widgets combo = self.widget("controller-type") @@ -976,13 +976,13 @@ class vmmAddHardware(vmmGObjectUI): self._build_combo_with_values(combo, types, default) - def _build_panic_address_type(self, combo): - types = [] - for t in virtinst.VirtualPanicDevice.TYPES: - types.append([t, virtinst.VirtualPanicDevice.get_pretty_type(t)]) + def _build_panic_models(self, combo): + models = [] + for m in virtinst.VirtualPanicDevice.MODELS: + models.append([m, virtinst.VirtualPanicDevice.get_pretty_model(m)]) - self._build_combo_with_values(combo, types, - virtinst.VirtualPanicDevice.ADDRESS_TYPE_ISA) + self._build_combo_with_values(combo, models, + virtinst.VirtualPanicDevice.MODEL_ISA) ######################### @@ -1755,11 +1755,11 @@ class vmmAddHardware(vmmGObjectUI): def _validate_page_panic(self): conn = self.conn.get_backend() - type = uiutil.get_list_selection(self.widget("panic-type")) + model = uiutil.get_list_selection(self.widget("panic-model")) try: self._dev = VirtualPanicDevice(conn) - self._dev.type = type + self._dev.model = model except Exception as e: return self.err.val_err(_("Panic device parameter error"), e) diff --git a/virtManager/details.py b/virtManager/details.py index 9ec2d8de8..6074654b6 100644 --- a/virtManager/details.py +++ b/virtManager/details.py @@ -2795,8 +2795,8 @@ class vmmDetails(vmmGObjectUI): if not dev: return - ptyp = virtinst.VirtualPanicDevice.get_pretty_type(dev.type) - self.widget("panic-type").set_text(ptyp) + pmodel = virtinst.VirtualPanicDevice.get_pretty_model(dev.model) + self.widget("panic-model").set_text(pmodel) def refresh_rng_page(self): dev = self.get_hw_selection(HW_LIST_COL_DEVICE) diff --git a/virtinst/cli.py b/virtinst/cli.py index 6bf121fc4..2c8cbff42 100644 --- a/virtinst/cli.py +++ b/virtinst/cli.py @@ -2518,15 +2518,26 @@ ParserMemballoon.add_arg("model", "model") class ParserPanic(VirtCLIParser): cli_arg_name = "panic" objclass = VirtualPanicDevice - remove_first = "iobase" + remove_first = "model" + compat_mode = False - def set_iobase_cb(self, inst, val, virtarg): - if val == "default": - return - inst.iobase = val + def set_model_cb(self, inst, val, virtarg): + if self.compat_mode and val.startswith("0x"): + inst.model = VirtualPanicDevice.MODEL_ISA + inst.iobase = val + else: + inst.model = val + + def _parse(self, inst): + if (len(self.optstr.split(",")) == 1 and + not self.optstr.startswith("model=")): + self.compat_mode = True + return VirtCLIParser._parse(self, inst) _register_virt_parser(ParserPanic) -ParserPanic.add_arg(None, "iobase", cb=ParserPanic.set_iobase_cb) +ParserPanic.add_arg(None, "model", cb=ParserPanic.set_model_cb, + ignore_default=True) +ParserPanic.add_arg("iobase", "iobase") ###################################################### diff --git a/virtinst/devicepanic.py b/virtinst/devicepanic.py index 8b9a161f1..69f728889 100644 --- a/virtinst/devicepanic.py +++ b/virtinst/devicepanic.py @@ -24,20 +24,31 @@ from .xmlbuilder import XMLProperty class VirtualPanicDevice(VirtualDevice): virtual_device_type = VirtualDevice.VIRTUAL_DEV_PANIC - ADDRESS_TYPE_ISA = "isa" - TYPES = [ADDRESS_TYPE_ISA] + + MODEL_DEFAULT = "default" + MODEL_ISA = "isa" + MODELS = [MODEL_ISA] + + ISA_ADDRESS_TYPE = "isa" IOBASE_DEFAULT = "0x505" @staticmethod - def get_pretty_type(panic_type): - if panic_type == VirtualPanicDevice.ADDRESS_TYPE_ISA: + def get_pretty_model(panic_model): + if panic_model == VirtualPanicDevice.MODEL_ISA: return _("ISA") - return panic_type + return panic_model + def _get_default_address_type(self): + if self.iobase: + return VirtualPanicDevice.ISA_ADDRESS_TYPE + return None + model = XMLProperty("./@model", + default_cb=lambda s: VirtualPanicDevice.MODEL_ISA, + default_name=MODEL_DEFAULT) type = XMLProperty("./address/@type", - default_cb=lambda s: s.ADDRESS_TYPE_ISA) + default_cb=_get_default_address_type) iobase = XMLProperty("./address/@iobase", - default_cb=lambda s: s.IOBASE_DEFAULT) + default_cb=lambda s: s.IOBASE_DEFAULT) VirtualPanicDevice.register_type()