[virt-tools-list] [virt-manager PATCH] virtinst: do not add default channels if one is already present

[Cole Robinson]

On 01/09/2015 06:29 AM, Giuseppe Scrivano wrote:
> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1179680
> 

I don't think that test case would have triggered the original behavior, you'd
need to use the fake KVM URI, grep clitest.py for xml-comparison

That said, I think the problem is elsewhere? That reproducing command from the
bug works fine on F21. The idea was that --channel none will turn off all
default channels, but we only skip adding the spicevmc defaults if a user has
specified --channel spicevmc. So if someone is adding their own custom
--channel pty, they don't have to re-specify the spicevmc one as well.

Maybe it's non-intuitive but that's what the code does at the moment (we
really need some explicit fine grained way of turning off individual defaults
but that's a larger effort).

I'm not really sure where the error is coming from on RHEL... be useful to see
the full XML and qemu command line that is being generated.

- Cole

> Signed-off-by: Giuseppe Scrivano <gscrivan at redhat.com>
> ---
>  .../compare/virt-install-channel-only-pty.xml      | 91 ++++++++++++++++++++++
>  tests/clitest.py                                   |  1 +
>  virtinst/cli.py                                    |  7 +-
>  3 files changed, 96 insertions(+), 3 deletions(-)
>  create mode 100644 tests/cli-test-xml/compare/virt-install-channel-only-pty.xml
> 
> diff --git a/tests/cli-test-xml/compare/virt-install-channel-only-pty.xml b/tests/cli-test-xml/compare/virt-install-channel-only-pty.xml
> new file mode 100644
> index 0000000..faaae47
> --- /dev/null
> +++ b/tests/cli-test-xml/compare/virt-install-channel-only-pty.xml
> @@ -0,0 +1,91 @@
> +<domain type="test">
> +  <name>foobar</name>
> +  <uuid>00000000-1111-2222-3333-444444444444</uuid>
> +  <memory>65536</memory>
> +  <currentMemory>65536</currentMemory>
> +  <vcpu>1</vcpu>
> +  <os>
> +    <type arch="i686">hvm</type>
> +    <boot dev="cdrom"/>
> +  </os>
> +  <features>
> +    <acpi/>
> +    <apic/>
> +    <pae/>
> +  </features>
> +  <clock offset="utc"/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>destroy</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/test-hv</emulator>
> +    <disk type="file" device="cdrom">
> +      <source file="/dev/default-pool/testvol1.img"/>
> +      <target dev="hda" bus="ide"/>
> +      <readonly/>
> +    </disk>
> +    <controller type="usb" index="0" model="ich9-ehci1"/>
> +    <controller type="usb" index="0" model="ich9-uhci1">
> +      <master startport="0"/>
> +    </controller>
> +    <controller type="usb" index="0" model="ich9-uhci2">
> +      <master startport="2"/>
> +    </controller>
> +    <controller type="usb" index="0" model="ich9-uhci3">
> +      <master startport="4"/>
> +    </controller>
> +    <interface type="user">
> +      <mac address="00:11:22:33:44:55"/>
> +    </interface>
> +    <input type="mouse" bus="ps2"/>
> +    <console type="pty"/>
> +    <channel type="pty">
> +      <target type="virtio"/>
> +    </channel>
> +  </devices>
> +</domain>
> +<domain type="test">
> +  <name>foobar</name>
> +  <uuid>00000000-1111-2222-3333-444444444444</uuid>
> +  <memory>65536</memory>
> +  <currentMemory>65536</currentMemory>
> +  <vcpu>1</vcpu>
> +  <os>
> +    <type arch="i686">hvm</type>
> +    <boot dev="hd"/>
> +  </os>
> +  <features>
> +    <acpi/>
> +    <apic/>
> +    <pae/>
> +  </features>
> +  <clock offset="utc"/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>restart</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/test-hv</emulator>
> +    <disk type="block" device="cdrom">
> +      <target dev="hda" bus="ide"/>
> +      <readonly/>
> +    </disk>
> +    <controller type="usb" index="0" model="ich9-ehci1"/>
> +    <controller type="usb" index="0" model="ich9-uhci1">
> +      <master startport="0"/>
> +    </controller>
> +    <controller type="usb" index="0" model="ich9-uhci2">
> +      <master startport="2"/>
> +    </controller>
> +    <controller type="usb" index="0" model="ich9-uhci3">
> +      <master startport="4"/>
> +    </controller>
> +    <interface type="user">
> +      <mac address="00:11:22:33:44:55"/>
> +    </interface>
> +    <input type="mouse" bus="ps2"/>
> +    <console type="pty"/>
> +    <channel type="pty">
> +      <target type="virtio"/>
> +    </channel>
> +  </devices>
> +</domain>
> diff --git a/tests/clitest.py b/tests/clitest.py
> index b35fd43..6c65622 100644
> --- a/tests/clitest.py
> +++ b/tests/clitest.py
> @@ -723,6 +723,7 @@ c.add_compare("", "noargs-fail", auto_printarg=False)  # No arguments
>  c.add_valid("--panic help --disk=?")  # Make sure introspection doesn't blow up
>  c.add_invalid("--hvm --nodisks --pxe foobar")  # Positional arguments error
>  c.add_invalid("--nodisks --pxe --name test")  # Colliding name
> +c.add_compare("--nodisks --cdrom %(EXISTIMG1)s --channel pty,target_type=virtio", "channel-only-pty")  # Only one channel
>  
>  
>  
> diff --git a/virtinst/cli.py b/virtinst/cli.py
> index 00294fb..4748e07 100644
> --- a/virtinst/cli.py
> +++ b/virtinst/cli.py
> @@ -1,7 +1,7 @@
>  #
>  # Utility functions for the command line drivers
>  #
> -# Copyright 2006-2007, 2013, 2014 Red Hat, Inc.
> +# Copyright 2006-2007, 2013, 2014, 2015 Red Hat, Inc.
>  # Jeremy Katz <katzj at redhat.com>
>  #
>  # This program is free software; you can redistribute it and/or modify
> @@ -2002,9 +2002,10 @@ class _ParserChar(VirtCLIParser):
>          if opts.fullopts == "none" and inst.virtual_device_type == "console":
>              self.guest.skip_default_console = True
>              return
> -        if opts.fullopts == "none" and inst.virtual_device_type == "channel":
> +        if inst.virtual_device_type == "channel":
>              self.guest.skip_default_channel = True
> -            return
> +            if opts.fullopts == "none":
> +                return
>  
>          return VirtCLIParser._parse(self, opts, inst)
>  
>