Re: [libvirt] [PATCH 1/2] Revert "network: Check for QOS before blindly using it"
On 11/24/19 9:12 PM, Julio Faracco wrote: Em sex., 22 de nov. de 2019 às 14:09, Erik Skultety escreveu: This reverts commit f4db846c32c0a1e99a0f62b340273e48f8a98ed3. This patch results in the following error when trying to start essentially any VM with default network: This kind of issue should be caught by test driver, shouldn't it? Not really. The test driver doesn't call APIs of other drivers. It's only providing sensible outputs for APIs inputs without doing any actual work. An integration test would have caught this though. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Revert "network: Check for QOS before blindly using it"
On 11/22/19 5:08 PM, Erik Skultety wrote: This reverts commit f4db846c32c0a1e99a0f62b340273e48f8a98ed3. This patch results in the following error when trying to start essentially any VM with default network: unsupported configuration: QOS must be defined for network 'default' Coverity didn't see that the bandwidth == NULL it complained about in virNetDevBandwidthPlug was already checked properly in networkCheckBandwidth, thus causing networkPlugBandwidth to return 0 and finish before a call to virNetDevBandwidthPlug would have been even made. Signed-off-by: Erik Skultety --- src/network/bridge_driver.c | 14 -- 1 file changed, 14 deletions(-) Reviewed-by: Michal Privoznik Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Revert "network: Check for QOS before blindly using it"
On Sun, Nov 24, 2019 at 06:12:58PM -0200, Julio Faracco wrote: > Em sex., 22 de nov. de 2019 às 14:09, Erik Skultety > escreveu: > > > > This reverts commit f4db846c32c0a1e99a0f62b340273e48f8a98ed3. > > > > This patch results in the following error when trying to start > > essentially any VM with default network: > > This kind of issue should be caught by test driver, shouldn't it? Test driver is supposed to be completely isolated from the host. In this case, the error comes from the bridge_driver which does interact with the underlying host so no, test driver is of no help here. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Revert "network: Check for QOS before blindly using it"
On Fri, Nov 22, 2019 at 17:08:59 +0100, Erik Skultety wrote: > This reverts commit f4db846c32c0a1e99a0f62b340273e48f8a98ed3. > > This patch results in the following error when trying to start > essentially any VM with default network: > > unsupported configuration: QOS must be defined for network 'default' > > Coverity didn't see that the bandwidth == NULL it complained about in > virNetDevBandwidthPlug was already checked properly in > networkCheckBandwidth, thus causing networkPlugBandwidth to return 0 > and finish before a call to virNetDevBandwidthPlug would have been even > made. > > Signed-off-by: Erik Skultety > --- > src/network/bridge_driver.c | 14 -- > 1 file changed, 14 deletions(-) Reviewed-by: Jiri Denemark -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Revert "network: Check for QOS before blindly using it"
Em sex., 22 de nov. de 2019 às 14:09, Erik Skultety escreveu: > > This reverts commit f4db846c32c0a1e99a0f62b340273e48f8a98ed3. > > This patch results in the following error when trying to start > essentially any VM with default network: This kind of issue should be caught by test driver, shouldn't it? -- Julio Faracco > > unsupported configuration: QOS must be defined for network 'default' > > Coverity didn't see that the bandwidth == NULL it complained about in > virNetDevBandwidthPlug was already checked properly in > networkCheckBandwidth, thus causing networkPlugBandwidth to return 0 > and finish before a call to virNetDevBandwidthPlug would have been even > made. > > Signed-off-by: Erik Skultety > --- > src/network/bridge_driver.c | 14 -- > 1 file changed, 14 deletions(-) > > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c > index 9c49c70564..68bb916501 100644 > --- a/src/network/bridge_driver.c > +++ b/src/network/bridge_driver.c > @@ -4567,13 +4567,6 @@ networkAllocatePort(virNetworkObjPtr obj, > return -1; > } > > -if (!port->bandwidth) { > -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > - _("QOS must be defined for network '%s'"), > - netdef->name); > -return -1; > -} > - > if (networkPlugBandwidth(obj, >mac, port->bandwidth, > >class_id) < 0) > return -1; > break; > @@ -4640,13 +4633,6 @@ networkAllocatePort(virNetworkObjPtr obj, > } > } > > -if (!port->bandwidth) { > -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > - _("QOS must be defined for network '%s'"), > - netdef->name); > -return -1; > -} > - > if (networkPlugBandwidth(obj, >mac, port->bandwidth, > >class_id) < 0) > return -1; > break; > -- > 2.23.0 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Revert "network: Check for QOS before blindly using it"
On 11/22/19 1:08 PM, Erik Skultety wrote: This reverts commit f4db846c32c0a1e99a0f62b340273e48f8a98ed3. This patch results in the following error when trying to start essentially any VM with default network: unsupported configuration: QOS must be defined for network 'default' Coverity didn't see that the bandwidth == NULL it complained about in virNetDevBandwidthPlug was already checked properly in networkCheckBandwidth, thus causing networkPlugBandwidth to return 0 and finish before a call to virNetDevBandwidthPlug would have been even made. Signed-off-by: Erik Skultety --- Reviewed-by: Daniel Henrique Barboza -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Revert "network: Check for QOS before blindly using it"
This reverts commit f4db846c32c0a1e99a0f62b340273e48f8a98ed3. This patch results in the following error when trying to start essentially any VM with default network: unsupported configuration: QOS must be defined for network 'default' Coverity didn't see that the bandwidth == NULL it complained about in virNetDevBandwidthPlug was already checked properly in networkCheckBandwidth, thus causing networkPlugBandwidth to return 0 and finish before a call to virNetDevBandwidthPlug would have been even made. Signed-off-by: Erik Skultety --- src/network/bridge_driver.c | 14 -- 1 file changed, 14 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 9c49c70564..68bb916501 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4567,13 +4567,6 @@ networkAllocatePort(virNetworkObjPtr obj, return -1; } -if (!port->bandwidth) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("QOS must be defined for network '%s'"), - netdef->name); -return -1; -} - if (networkPlugBandwidth(obj, >mac, port->bandwidth, >class_id) < 0) return -1; break; @@ -4640,13 +4633,6 @@ networkAllocatePort(virNetworkObjPtr obj, } } -if (!port->bandwidth) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("QOS must be defined for network '%s'"), - netdef->name); -return -1; -} - if (networkPlugBandwidth(obj, >mac, port->bandwidth, >class_id) < 0) return -1; break; -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list