On Thu, 15 Nov 2018 at 12:23, Michal Privoznik <mprivozn@redhat.com> wrote:
On 11/10/2018 01:56 PM, Radoslaw Biernacki wrote:
> The device xml parser code does not set "model" while parsing
> <interface type='hostdev'>
>   <source>
>     <address type='pci' domain='0x0002' bus='0x01' slot='0x00' function='0x2'/>
>   </source>
> </interface>
> virDomainDefPtr def->nets[i]->model can be NULL while latter compares strings with
> STREQ instead of STREQ_NULLABLE.
>
> Fixes: ac47e4a6225 (qemu: replace "def->nets[i]" with "net" and "def->sounds[i]" with "sound")
> Fixes: c7fc151eec7 (qemu: assign virtio devices to PCIe slot when appropriate)
> Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
> ---
>  src/qemu/qemu_domain_address.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index 27c9bfb946..15d25481d8 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -232,8 +232,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def)
>      for (i = 0; i < def->nnets; i++) {
>          virDomainNetDefPtr net = def->nets[i];

> -        if (net->model &&
> -            STREQ(net->model, "spapr-vlan")) {
> +        if (STREQ_NULLABLE(net->model, "spapr-vlan")) {
>              net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
>          }

We don't require curly braces around single line bodies. Actually the
opposite, our coding style says there should be none. This exception to
the rule was discussed many times but without any result. Anyway, 'make
syntax-check' would have caught this.

Sorry Michal, overlooked that.
Fixed in v2.
 


> @@ -325,7 +324,7 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def,
>          virDomainNetDefPtr net = def->nets[i];

>          if (net->model &&
> -            STREQ(net->model, "virtio") &&
> +            STREQ_NULLABLE(net->model, "virtio") &&

Looks like you've forgotten to remove net->model check ;-)

>              net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
>              net->info.type = type;
>          }
> @@ -634,14 +633,14 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
>           * addresses for other hostdev devices.
>           */
>          if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV ||
> -            STREQ(net->model, "usb-net")) {
> +            STREQ_NULLABLE(net->model, "usb-net")) {
>              return 0;
>          }

> -        if (STREQ(net->model, "virtio"))
> +        if (STREQ_NULLABLE(net->model, "virtio"))
>              return  virtioFlags;

> -        if (STREQ(net->model, "e1000e"))
> +        if (STREQ_NULLABLE(net->model, "e1000e"))
>              return pcieFlags;

>          return pciFlags;
>

The rest looks okay.

Michal