Description of problem: When using virt-install's virt-xml command, I've found it can fail to carry out an instruction, yet report it was successful doing so. Or rather, the message it provides is vague enough ("Domain '<domain name>' defined successfully.") that it's misleading, since virt-xml didn't do what was asked of it. Version-Release number of selected component (if applicable): 1.4.0-5.fc25 How reproducible: Always Steps to Reproduce: A sample demonstration: [disciple@arcusix ~]$ sudo virsh dumpxml netbsd-current >./netbsd-current-before.xml [disciple@arcusix ~]$ sudo virt-xml netbsd-current --remove-device --input mouse Domain 'netbsd-current' defined successfully. [disciple@arcusix ~]$ echo $? 0 [disciple@arcusix ~]$ sudo virsh dumpxml netbsd-current >./netbsd-current-after.xml [disciple@arcusix ~]$ diff -u ./netbsd-current-before.xml ./netbsd-current-after.xml --- ./netbsd-current-before.xml 2016-12-15 19:17:14.844829951 -0500 +++ ./netbsd-current-after.xml 2016-12-15 19:18:27.574065287 -0500 @@ -74,8 +74,8 @@ <target type='virtio' name='com.redhat.spice.0'/> <address type='virtio-serial' controller='0' bus='0' port='1'/> </channel> - <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> + <input type='mouse' bus='ps2'/> <graphics type='spice' autoport='yes'> <listen type='address'/> <image compression='off'/> Actual results: The mouse device in my example isn't removed, presumably because it cannot be. (I haven't explored why this is the case; that's not germane to this bug.) The user is not warned of this failure. Expected results: If virt-xml can't remove the device in my example, it should explicitly say so.
Yeah the ps2 mouse can't be removed from a qemu machine, there's no option to disable it when a graphical display is attached. Unfortunately there's no capability reported by libvirt that this is the case, we would have to hardcode the knowledge in virt-xml. Not sure if it's worth the effort though...
Alternately, virt-xml could simply validate that a change actually occurred. There's already sufficient functionality in its code to do so. It could take before and after copies of the XML representation of the guest it's working on and compare them using its get_diff() function as a diagnostic. If it's been asked to change something and there's no ultimate difference, that bears reporting. I just hacked on it as an example. --- ./virt-xml~ 2016-12-13 13:58:52.000000000 -0500 +++ ./virt-xml 2016-12-16 17:04:53.974011664 -0500 @@ -440,6 +440,8 @@ if options.domain: domain, inactive_xmlobj, active_xmlobj = get_domain_and_guest( conn, options.domain) + junk1, original_xmlobj, junk2 = get_domain_and_guest(conn, + options.domain) # XXX TESTING: Determine if a change was actually made elif not options.build_xml: inactive_xmlobj = _make_guest(conn, options.stdinxml) @@ -468,6 +470,14 @@ if not options.update and not options.define: prepare_changes(inactive_xmlobj, options, parserclass) + # XXX TESTING: Determine if a change was actually made + if options.domain: + junk1, final_xmlobj, junk2 = get_domain_and_guest(conn, options.domain) + diff = get_diff(original_xmlobj.get_xml_config(), + final_xmlobj.get_xml_config()) + if not diff: + print_stdout("Warning: there was no change applied to the domain!") + return 0 Of course, that might lead to more questions from users. Either way, it'd probably be a good idea to add a note to virt-xml's documentation explaining there are situations where it cannot actually make changes to VM definitions. I can submit a proposed change along these lines if you like.
(In reply to David H. Gutteridge from comment #2) > Of course, that might lead to more questions from users. Either way, it'd > probably be a good idea to add a note to virt-xml's documentation explaining > there are situations where it cannot actually make changes to VM > definitions. I can submit a proposed change along these lines if you like. I've submitted a GitHub pull request (#16) to address this bit, by adding a caveats section to the virt-xml man page with a general comment about this.
I've added a warning for this upstream now: commit 53f075ab76e1c372474ae0d88f202e487d9f213f (HEAD -> master, origin/master) Author: Cole Robinson <crobinso> Date: Sun Jun 16 17:01:32 2019 -0400 virt-xml: Warn if libvirt discards our defined changes This can happen if we try to remove a default device, like a ps2 mouse on x86, but it can happen for many other reasons as well https://bugzilla.redhat.com/show_bug.cgi?id=1405263