| Message ID | 20181110125624.1168-4-radoslaw.biernacki@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | util: Fixing libvirt errors on cavium/thunder-nicvf | expand |
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. > > @@ -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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 > <div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, 15 Nov 2018 at 12:23, Michal Privoznik <<a href="mailto:mprivozn@redhat.com">mprivozn@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 11/10/2018 01:56 PM, Radoslaw Biernacki wrote:<br> > The device xml parser code does not set "model" while parsing<br> > <interface type='hostdev'><br> > <source><br> > <address type='pci' domain='0x0002' bus='0x01' slot='0x00' function='0x2'/><br> > </source><br> > </interface><br> > virDomainDefPtr def->nets[i]->model can be NULL while latter compares strings with<br> > STREQ instead of STREQ_NULLABLE.<br> > <br> > Fixes: ac47e4a6225 (qemu: replace "def->nets[i]" with "net" and "def->sounds[i]" with "sound")<br> > Fixes: c7fc151eec7 (qemu: assign virtio devices to PCIe slot when appropriate)<br> > Signed-off-by: Radoslaw Biernacki <<a href="mailto:radoslaw.biernacki@linaro.org" target="_blank">radoslaw.biernacki@linaro.org</a>><br> > ---<br> > src/qemu/qemu_domain_address.c | 11 +++++------<br> > 1 file changed, 5 insertions(+), 6 deletions(-)<br> > <br> > diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c<br> > index 27c9bfb946..15d25481d8 100644<br> > --- a/src/qemu/qemu_domain_address.c<br> > +++ b/src/qemu/qemu_domain_address.c<br> > @@ -232,8 +232,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def)<br> > for (i = 0; i < def->nnets; i++) {<br> > virDomainNetDefPtr net = def->nets[i];<br> > <br> > - if (net->model &&<br> > - STREQ(net->model, "spapr-vlan")) {<br> > + if (STREQ_NULLABLE(net->model, "spapr-vlan")) {<br> > net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;<br> > }<br> <br> We don't require curly braces around single line bodies. Actually the<br> opposite, our coding style says there should be none. This exception to<br> the rule was discussed many times but without any result. Anyway, 'make<br> syntax-check' would have caught this.<br></blockquote><div><br></div><div>Sorry Michal, overlooked that.</div><div>Fixed in v2.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br> > <br> > @@ -325,7 +324,7 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def,<br> > virDomainNetDefPtr net = def->nets[i];<br> > <br> > if (net->model &&<br> > - STREQ(net->model, "virtio") &&<br> > + STREQ_NULLABLE(net->model, "virtio") &&<br> <br> Looks like you've forgotten to remove net->model check ;-)<br> <br> > net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {<br> > net->info.type = type;<br> > }<br> > @@ -634,14 +633,14 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,<br> > * addresses for other hostdev devices.<br> > */<br> > if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV ||<br> > - STREQ(net->model, "usb-net")) {<br> > + STREQ_NULLABLE(net->model, "usb-net")) {<br> > return 0;<br> > }<br> > <br> > - if (STREQ(net->model, "virtio"))<br> > + if (STREQ_NULLABLE(net->model, "virtio"))<br> > return virtioFlags;<br> > <br> > - if (STREQ(net->model, "e1000e"))<br> > + if (STREQ_NULLABLE(net->model, "e1000e"))<br> > return pcieFlags;<br> > <br> > return pciFlags;<br> > <br> <br> The rest looks okay.<br> <br> Michal<br> </blockquote></div></div> -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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; } @@ -325,7 +324,7 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, virDomainNetDefPtr net = def->nets[i]; if (net->model && - STREQ(net->model, "virtio") && + STREQ_NULLABLE(net->model, "virtio") && 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 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(-) -- 2.14.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list