[libvirt] [PATCH 1/2] Only parse custom vhost path for virtio interfaces

[John Ferlan]

On 02/05/2015 07:52 AM, Ján Tomko wrote:
> It is not used for other network interface models.

A little light here... Something along the lines of only supporting
backend vhost attribute for virtio adapters and quietly dropping it for
any other definition...

> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1147195
> ---
>  src/conf/domain_conf.c                             |  6 +++-
>  .../qemuxml2argv-tap-vhost-incorrect.xml           | 39 ++++++++++++++++++++++
>  .../qemuxml2xmlout-tap-vhost-incorrect.xml         | 38 +++++++++++++++++++++
>  tests/qemuxml2xmltest.c                            |  1 +
>  4 files changed, 83 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml
>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 4251b13..da3764f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7369,6 +7369,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>      char *vhostuser_path = NULL;
>      char *vhostuser_type = NULL;
>      char *trustGuestRxFilters = NULL;
> +    char *vhost_path = NULL;
>      virNWFilterHashTablePtr filterparams = NULL;
>      virDomainActualNetDefPtr actual = NULL;
>      xmlNodePtr oldnode = ctxt->node;
> @@ -7551,7 +7552,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>                  VIR_FREE(tmp);
>  
>                  if ((tmp = virXMLPropString(cur, "vhost")))
> -                    def->backend.vhost = virFileSanitizePath(tmp);
> +                    vhost_path = virFileSanitizePath(tmp);
>                  VIR_FREE(tmp);

Coverity got grumpy here - it believes "vhost" can be found twice
probably because there's other code (just above here) which fills in
vhostuser_* values, but first checks (!vhostuser_* &&...)

To keep Coverity quiet, adding a "!vhost_path &&" prior to the tmp =
does work.

Why this doesn't get flagged for def->backend.tap nor the former code
and some other allocations, I'm not quite sure.

Another option is to not use vhost_path and just do the following after
decoding def->model (prior to the if loop where you used vhost_path):

    if (def->backend.vhost && STRNEQ_NULLABLE(def->model, "virtio"))
        VIR_FREE(def->backend.vhost);

ACK - regardless of which method you choose.

John

>              }
>          }
> @@ -7992,6 +7993,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>              }
>              def->driver.virtio.guest.ufo = val;
>          }
> +        def->backend.vhost = vhost_path;
> +        vhost_path = NULL;
>      }
>  
>      def->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT;
> @@ -8061,6 +8064,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>      VIR_FREE(addrtype);
>      VIR_FREE(trustGuestRxFilters);
>      VIR_FREE(ips);
> +    VIR_FREE(vhost_path);
>      virNWFilterHashTableFree(filterparams);
>  
>      return def;
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml b/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml
> new file mode 100644
> index 0000000..2cf312f
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml
> @@ -0,0 +1,39 @@
> +<domain type='qemu'>
> +  <name>test</name>
> +  <uuid>bba65c0e-c049-934f-b6aa-4e2c0582acdf</uuid>
> +  <memory unit='KiB'>1048576</memory>
> +  <currentMemory unit='KiB'>1048576</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='x86_64' machine='pc-0.13'>hvm</type>
> +    <boot dev='cdrom'/>
> +    <boot dev='hd'/>
> +    <bootmenu enable='yes'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>restart</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu</emulator>
> +    <controller type='usb' index='0'/>
> +    <controller type='virtio-serial' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
> +    </controller>
> +    <controller type='ide' index='0'/>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <interface type='user'>
> +      <mac address='52:54:00:e5:48:58'/>
> +      <model type='definitely-not-virtio'/>
> +      <driver name='vhost' queues='5'/>
> +      <backend tap='/dev/null' vhost='/dev/zero'/>
> +    </interface>
> +    <serial type='pty'>
> +      <target port='0'/>
> +    </serial>
> +    <console type='pty'>
> +      <target type='serial' port='0'/>
> +    </console>
> +    <memballoon model='virtio'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml
> new file mode 100644
> index 0000000..266cbf0
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml
> @@ -0,0 +1,38 @@
> +<domain type='qemu'>
> +  <name>test</name>
> +  <uuid>bba65c0e-c049-934f-b6aa-4e2c0582acdf</uuid>
> +  <memory unit='KiB'>1048576</memory>
> +  <currentMemory unit='KiB'>1048576</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='x86_64' machine='pc-0.13'>hvm</type>
> +    <boot dev='cdrom'/>
> +    <boot dev='hd'/>
> +    <bootmenu enable='yes'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>restart</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu</emulator>
> +    <controller type='usb' index='0'/>
> +    <controller type='virtio-serial' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
> +    </controller>
> +    <controller type='ide' index='0'/>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <interface type='user'>
> +      <mac address='52:54:00:e5:48:58'/>
> +      <model type='definitely-not-virtio'/>
> +      <backend tap='/dev/null'/>
> +    </interface>
> +    <serial type='pty'>
> +      <target port='0'/>
> +    </serial>
> +    <console type='pty'>
> +      <target type='serial' port='0'/>
> +    </console>
> +    <memballoon model='virtio'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index a0a1cab..d3dfd9e 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -417,6 +417,7 @@ mymain(void)
>      DO_TEST("bios-nvram");
>  
>      DO_TEST("tap-vhost");
> +    DO_TEST_DIFFERENT("tap-vhost-incorrect");
>      DO_TEST("shmem");
>      DO_TEST("smbios");
>  
>