[PATCH virt-install v1 3/3] virtxml: prevent defining same hostdev again

[Shalini Chellathurai Saroja]

Currently, it is possible to add same device multiple times, when the
guest domain is in shut-off state. This patch prevents this unexpected
behavior for all hostdev devices, including mdev devices.

This change impacts existing tests for usb devices, as the same device
already exists in the test domain and is added twice by the tests. So
changes were made to those tests as well.

Signed-off-by: Shalini Chellathurai Saroja <shalini at linux.ibm.com>
---
 .../cli/compare/virt-xml-add-host-device-start.xml |  2 +-
 .../data/cli/compare/virt-xml-add-host-device.xml  |  2 +-
 tests/data/testdriver/testdriver.xml               |  8 ++++----
 tests/data/testdriver/testsuite.xml                | 14 ++++++++++++++
 tests/test_cli.py                                  |  5 +++--
 virtinst/virtxml.py                                |  7 +++++++
 6 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/tests/data/cli/compare/virt-xml-add-host-device-start.xml b/tests/data/cli/compare/virt-xml-add-host-device-start.xml
index b3db5279..c31994e1 100644
--- a/tests/data/cli/compare/virt-xml-add-host-device-start.xml
+++ b/tests/data/cli/compare/virt-xml-add-host-device-start.xml
@@ -4,7 +4,7 @@
 +    <hostdev mode="subsystem" type="usb" managed="yes">
 +      <source>
 +        <vendor id="0x04b3"/>
-+        <product id="0x4485"/>
++        <product id="0x4486"/>
 +      </source>
 +    </hostdev>
    </devices>
diff --git a/tests/data/cli/compare/virt-xml-add-host-device.xml b/tests/data/cli/compare/virt-xml-add-host-device.xml
index 099fe853..64aeb216 100644
--- a/tests/data/cli/compare/virt-xml-add-host-device.xml
+++ b/tests/data/cli/compare/virt-xml-add-host-device.xml
@@ -4,7 +4,7 @@
 +    <hostdev mode="subsystem" type="usb" managed="yes">
 +      <source>
 +        <vendor id="0x04b3"/>
-+        <product id="0x4485"/>
++        <product id="0x4486"/>
 +      </source>
 +    </hostdev>
    </devices>
diff --git a/tests/data/testdriver/testdriver.xml b/tests/data/testdriver/testdriver.xml
index b8d67bac..5e3a2c72 100644
--- a/tests/data/testdriver/testdriver.xml
+++ b/tests/data/testdriver/testdriver.xml
@@ -3476,7 +3476,7 @@ ba</description>
 
 
 <device>
-  <name>usb_device_4b3_4485_noserial</name>
+  <name>usb_device_4b3_4486_noserial</name>
   <parent>usb_device_1d6b_2_0000_00_1a_7</parent>
   <driver>
     <name>usb</name>
@@ -3491,8 +3491,8 @@ ba</description>
 
 
 <device>
-  <name>usb_device_4b3_4485_noserial_if0</name>
-  <parent>usb_device_4b3_4485_noserial</parent>
+  <name>usb_device_4b3_4486_noserial_if0</name>
+  <parent>usb_device_4b3_4486_noserial</parent>
   <driver>
     <name>hub</name>
   </driver>
@@ -3507,7 +3507,7 @@ ba</description>
 
 <device>
   <name>usb_device_62a_1_noserial</name>
-  <parent>usb_device_4b3_4485_noserial</parent>
+  <parent>usb_device_4b3_4486_noserial</parent>
   <driver>
     <name>usb</name>
   </driver>
diff --git a/tests/data/testdriver/testsuite.xml b/tests/data/testdriver/testsuite.xml
index 41c20381..aee478f1 100644
--- a/tests/data/testdriver/testsuite.xml
+++ b/tests/data/testdriver/testsuite.xml
@@ -743,6 +743,20 @@
   </capability>
 </device>
 
+<device>
+  <name>usb_device_4b3_4486_noserial</name>
+  <parent>usb_device_1d6b_2_0000_00_1a_7</parent>
+  <driver>
+    <name>usb</name>
+  </driver>
+  <capability type='usb_device'>
+    <bus>1</bus>
+    <device>3</device>
+    <product id='0x4486'>Serial Converter</product>
+    <vendor id='0x04b3'>IBM Corp.</vendor>
+  </capability>
+</device>
+
 <device>
   <name>mdev_8e37ee90_2b51_45e3_9b25_bf8283c03110</name>
   <path>/sys/devices/css0/0.0.0023/8e37ee90-2b51-45e3-9b25-bf8283c03110</path>
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 467e9e3d..2dd4324c 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -1311,7 +1311,7 @@ c = vixml.add_category("add/rm devices", "test-for-virtxml --print-diff --define
 c.add_valid("--add-device --security model=dac")  # --add-device works for seclabel
 c.add_invalid("--add-device --pm suspend_to_disk=yes")  # --add-device without a device
 c.add_invalid("--remove-device --clock utc")  # --remove-device without a dev
-c.add_compare("--add-device --host-device usb_device_4b3_4485_noserial", "add-host-device")
+c.add_compare("--add-device --host-device usb_device_4b3_4486_noserial", "add-host-device")
 c.add_compare("--add-device --sound pcspk", "add-sound")
 c.add_compare("--add-device --disk %(EXISTIMG1)s,bus=virtio,target=vdf", "add-disk-basic")
 c.add_compare("--add-device --disk %(EXISTIMG1)s", "add-disk-notarget")  # filling in acceptable target
@@ -1324,13 +1324,14 @@ c.add_compare("--remove-device --video all", "remove-video-all")
 c.add_compare("--remove-device --host-device 0x04b3:0x4485", "remove-hostdev-name")
 c.add_compare("--remove-device --memballoon all", "remove-memballoon")
 c.add_compare("--add-device --hostdev mdev_8e37ee90_2b51_45e3_9b25_bf8283c03110", "add-hostdev-mdev")
+c.add_invalid("--add-device --hostdev mdev_b1ae8bf6_38b0_4c81_9d44_78ce3f520496")  # mdev exists in guest XML
 c.add_compare("--remove-device --hostdev mdev_b1ae8bf6_38b0_4c81_9d44_78ce3f520496", "remove-hostdev-mdev")
 
 c = vixml.add_category("add/rm devices and start", "test-state-shutoff --print-diff --start")
 c.add_invalid("--add-device --pm suspend_to_disk=yes")  # --add-device without a device
 c.add_invalid("--remove-device --clock utc")  # --remove-device without a dev
 # one test in combination with --define
-c.add_compare("--define --add-device --host-device usb_device_4b3_4485_noserial", "add-host-device-start")
+c.add_compare("--define --add-device --host-device usb_device_4b3_4486_noserial", "add-host-device-start")
 # all other test cases without
 c.add_compare("--add-device --disk %(EXISTIMG1)s,bus=virtio,target=vdf", "add-disk-basic-start")
 c.add_compare("--add-device --disk %(NEWIMG1)s,size=.01", "add-disk-create-storage-start")
diff --git a/virtinst/virtxml.py b/virtinst/virtxml.py
index 0c8da37e..333c8b28 100644
--- a/virtinst/virtxml.py
+++ b/virtinst/virtxml.py
@@ -323,6 +323,13 @@ def prepare_changes(xmlobj, options, parserclass):
         action = "update"
 
     elif options.add_device:
+        if options.hostdev:
+            device = getattr(options, parserclass.cli_arg_name)[-1]
+            parserobj = parserclass(device, guest=xmlobj)
+            if parserobj.lookup_child_from_option_string():
+                fail(_("Device {} already exists in guest"
+                       "".format(getattr(options, parserclass.cli_arg_name)[-1])))
+
         devs = action_add_device(xmlobj, options, parserclass)
         action = "hotplug"
 
-- 
2.26.2