[virt-tools-list] [virt-manager PATCH 3/5] devicepanic: use model instead of address.type

Pavel Hrdina phrdina at redhat.com
Tue Sep 5 07:59:17 UTC 2017


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

  <panic model='isa'>
    <address iobase='0x505' type='isa'/>
  </panic>

2. there is only one parameter and it doesn't match any model:

  --panic 0x505

  <panic model='isa'>
    <address iobase='0x505' type='isa'/>
  </panic>

3. there are two parameters:

  --panic isa,iobase=0x505

  <panic model='isa'>
    <address iobase='0x505' type='isa'/>
  </panic>

Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
---
 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 ++++++++++++++++++++++
 .../compare/virt-install-singleton-config-1.xml    |  2 +-
 .../compare/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 3482e53b..f2a036a3 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<ht
 
 =back
 
-=item B<--panic> 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 11a0771b..852b63aa 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 @@
         <source mode="connect" host="127.0.0.1" service="8000"/>
       </backend>
     </rng>
-    <panic>
+    <panic model="isa">
       <address iobase="507" type="isa"/>
     </panic>
   </devices>
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 00000000..9b7ddfd5
--- /dev/null
+++ b/tests/cli-test-xml/compare/virt-install-panic-default.xml
@@ -0,0 +1,35 @@
+<domain type="kvm">
+  <name>foobar</name>
+  <uuid>00000000-1111-2222-3333-444444444444</uuid>
+  <memory>65536</memory>
+  <currentMemory>65536</currentMemory>
+  <vcpu>1</vcpu>
+  <os>
+    <type arch="x86_64">hvm</type>
+    <boot dev="hd"/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+  </features>
+  <cpu mode="custom" match="exact">
+    <model>Opteron_G4</model>
+  </cpu>
+  <clock offset="utc">
+    <timer name="rtc" tickpolicy="catchup"/>
+    <timer name="pit" tickpolicy="delay"/>
+    <timer name="hpet" present="no"/>
+  </clock>
+  <pm>
+    <suspend-to-mem enabled="no"/>
+    <suspend-to-disk enabled="no"/>
+  </pm>
+  <devices>
+    <emulator>/usr/bin/qemu-kvm</emulator>
+    <controller type="usb" index="0" model="none"/>
+    <console type="pty"/>
+    <panic model="isa">
+      <address type="isa" iobase="0x505"/>
+    </panic>
+  </devices>
+</domain>
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 00000000..714cb56b
--- /dev/null
+++ b/tests/cli-test-xml/compare/virt-install-panic-isa-iobase.xml
@@ -0,0 +1,35 @@
+<domain type="kvm">
+  <name>foobar</name>
+  <uuid>00000000-1111-2222-3333-444444444444</uuid>
+  <memory>65536</memory>
+  <currentMemory>65536</currentMemory>
+  <vcpu>1</vcpu>
+  <os>
+    <type arch="x86_64">hvm</type>
+    <boot dev="hd"/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+  </features>
+  <cpu mode="custom" match="exact">
+    <model>Opteron_G4</model>
+  </cpu>
+  <clock offset="utc">
+    <timer name="rtc" tickpolicy="catchup"/>
+    <timer name="pit" tickpolicy="delay"/>
+    <timer name="hpet" present="no"/>
+  </clock>
+  <pm>
+    <suspend-to-mem enabled="no"/>
+    <suspend-to-disk enabled="no"/>
+  </pm>
+  <devices>
+    <emulator>/usr/bin/qemu-kvm</emulator>
+    <controller type="usb" index="0" model="none"/>
+    <console type="pty"/>
+    <panic model="isa">
+      <address iobase="0x505" type="isa"/>
+    </panic>
+  </devices>
+</domain>
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 00000000..9b7ddfd5
--- /dev/null
+++ b/tests/cli-test-xml/compare/virt-install-panic-isa.xml
@@ -0,0 +1,35 @@
+<domain type="kvm">
+  <name>foobar</name>
+  <uuid>00000000-1111-2222-3333-444444444444</uuid>
+  <memory>65536</memory>
+  <currentMemory>65536</currentMemory>
+  <vcpu>1</vcpu>
+  <os>
+    <type arch="x86_64">hvm</type>
+    <boot dev="hd"/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+  </features>
+  <cpu mode="custom" match="exact">
+    <model>Opteron_G4</model>
+  </cpu>
+  <clock offset="utc">
+    <timer name="rtc" tickpolicy="catchup"/>
+    <timer name="pit" tickpolicy="delay"/>
+    <timer name="hpet" present="no"/>
+  </clock>
+  <pm>
+    <suspend-to-mem enabled="no"/>
+    <suspend-to-disk enabled="no"/>
+  </pm>
+  <devices>
+    <emulator>/usr/bin/qemu-kvm</emulator>
+    <controller type="usb" index="0" model="none"/>
+    <console type="pty"/>
+    <panic model="isa">
+      <address type="isa" iobase="0x505"/>
+    </panic>
+  </devices>
+</domain>
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 8bf23451..5c38036c 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 @@
     <rng model="virtio">
       <backend model="random">/dev/random</backend>
     </rng>
-    <panic>
+    <panic model="isa">
       <address type="isa" iobase="0x505"/>
     </panic>
   </devices>
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 914403e6..d7fbb49c 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 @@
         <source mode="connect" host="foo" service="708"/>
       </backend>
     </rng>
-    <panic>
+    <panic model="isa">
       <address iobase="0x506" type="isa"/>
     </panic>
   </devices>
@@ -294,7 +294,7 @@
         <source mode="connect" host="foo" service="708"/>
       </backend>
     </rng>
-    <panic>
+    <panic model="isa">
       <address iobase="0x506" type="isa"/>
     </panic>
   </devices>
diff --git a/tests/clitest.py b/tests/clitest.py
index 56148cc0..147c615c 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 caee0be6..eb476dab 100644
--- a/ui/addhardware.ui
+++ b/ui/addhardware.ui
@@ -1720,13 +1720,13 @@
                             <property name="row_spacing">6</property>
                             <property name="column_spacing">6</property>
                             <child>
-                              <object class="GtkLabel" id="panic-type-label">
+                              <object class="GtkLabel" id="panic-model-label">
                                 <property name="visible">True</property>
                                 <property name="can_focus">False</property>
                                 <property name="halign">start</property>
-                                <property name="label" translatable="yes">Address _Type:</property>
+                                <property name="label" translatable="yes">_Model:</property>
                                 <property name="use_underline">True</property>
-                                <property name="mnemonic_widget">panic-type</property>
+                                <property name="mnemonic_widget">panic-model</property>
                               </object>
                               <packing>
                                 <property name="left_attach">0</property>
@@ -1734,7 +1734,7 @@
                               </packing>
                             </child>
                             <child>
-                              <object class="GtkComboBox" id="panic-type">
+                              <object class="GtkComboBox" id="panic-model">
                                 <property name="visible">True</property>
                                 <property name="can_focus">False</property>
                               </object>
diff --git a/ui/details.ui b/ui/details.ui
index 04d2b213..f307f340 100644
--- a/ui/details.ui
+++ b/ui/details.ui
@@ -5660,13 +5660,13 @@ if you know what you are doing.</small></property>
                                     <property name="row_spacing">4</property>
                                     <property name="column_spacing">8</property>
                                     <child>
-                                      <object class="GtkLabel" id="label95">
+                                      <object class="GtkLabel" id="panic-model-label">
                                         <property name="visible">True</property>
                                         <property name="can_focus">False</property>
                                         <property name="halign">start</property>
                                         <property name="margin_top">3</property>
                                         <property name="margin_bottom">3</property>
-                                        <property name="label" translatable="yes">Address Type:</property>
+                                        <property name="label" translatable="yes">Model:</property>
                                       </object>
                                       <packing>
                                         <property name="left_attach">0</property>
@@ -5674,12 +5674,11 @@ if you know what you are doing.</small></property>
                                       </packing>
                                     </child>
                                     <child>
-                                    <child>
-                                      <object class="GtkLabel" id="panic-type">
+                                      <object class="GtkLabel" id="panic-model">
                                         <property name="visible">True</property>
                                         <property name="can_focus">False</property>
                                         <property name="halign">start</property>
-                                        <property name="label" translatable="yes">panic-address-type</property>
+                                        <property name="label" translatable="yes">panic-model</property>
                                       </object>
                                       <packing>
                                         <property name="left_attach">1</property>
diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py
index 415e7e90..e14e6bdf 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 9ec2d8de..6074654b 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 6bf121fc..2c8cbff4 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 8b9a161f..69f72888 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()
-- 
2.13.5




More information about the virt-tools-list mailing list