Re: [libvirt] [PATCH 1/2] Revert "network: Check for QOS before blindly using it"

2019-11-25 Thread Michal Privoznik

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"

2019-11-25 Thread Michal Privoznik

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"

2019-11-25 Thread Erik Skultety
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"

2019-11-25 Thread Jiri Denemark
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"

2019-11-24 Thread Julio Faracco
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"

2019-11-22 Thread Daniel Henrique Barboza




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"

2019-11-22 Thread Erik Skultety
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