9:58 a.m.
On 09/18/2012 04:23 PM, Peter Krempa wrote:
On 09/18/12 15:52, Martin Kletzander wrote:
> Two changes are introduced in this patch. The first change removes
> ATTRIBUTE_RETURN_CHECK from virNetDevBandwidthClear, because it was
> called with ignore_value always, anyway. The function is used even
> when it's not necessary to call it, just for cleanup purposes. The
> second change is an added parameter "ignore_error" for the same
> function, which basically means "Ignore any errors from the commands
> that will be run". This parameter, when true, doesn't suppress any
> libvirt errors, just the exit value of commands ran.
> ---
> src/network/bridge_driver.c | 4 ++--
> src/util/virnetdevbandwidth.c | 15 ++++++++++-----
> src/util/virnetdevbandwidth.h | 6 +++---
> 3 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 0e38016..f153cdd 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -2161,7 +2161,7 @@ networkStartNetworkVirtual(struct network_driver
> *driver,
> return 0;
>
> err5:
> - ignore_value(virNetDevBandwidthClear(network->def->bridge));
> + virNetDevBandwidthClear(network->def->bridge, true);
>
> err4:
> if (!save_err)
> @@ -2206,7 +2206,7 @@ networkStartNetworkVirtual(struct network_driver
> *driver,
> static int networkShutdownNetworkVirtual(struct network_driver *driver,
> virNetworkObjPtr network)
> {
> - ignore_value(virNetDevBandwidthClear(network->def->bridge));
> + virNetDevBandwidthClear(network->def->bridge, true);
>
> if (network->radvdPid > 0) {
> char *radvdpidbase;
> diff --git a/src/util/virnetdevbandwidth.c
> b/src/util/virnetdevbandwidth.c
> index b23ccd3..98f6b68 100644
> --- a/src/util/virnetdevbandwidth.c
> +++ b/src/util/virnetdevbandwidth.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (C) 2009-2011 Red Hat, Inc.
> + * Copyright (C) 2009-2012 Red Hat, Inc.
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -69,7 +69,7 @@ virNetDevBandwidthSet(const char *ifname,
> goto cleanup;
> }
>
> - ignore_value(virNetDevBandwidthClear(ifname));
> + virNetDevBandwidthClear(ifname, true);
>
All 3 of the call paths for this function call it with ignore_error set
to true. I think that it's not necessary to have the feature to turn on
errors now, if nobody actually uses it.
> if (bandwidth->in) {
> if (virAsprintf(&average, "%llukbps",
> bandwidth->in->average) < 0)
> @@ -163,15 +163,19 @@ cleanup:
> * Return 0 on success, -1 otherwise.
> */
> int
> -virNetDevBandwidthClear(const char *ifname)
> +virNetDevBandwidthClear(const char *ifname, bool ignore_error)
> {
> int ret = 0;
> + int exitstatus, *es = NULL;
> virCommandPtr cmd = NULL;
>
> + if (ignore_error)
> + es = &exitstatus;
> +
> cmd = virCommandNew(TC);
> virCommandAddArgList(cmd, "qdisc", "del", "dev",
ifname, "root",
> NULL);
>
> - if (virCommandRun(cmd, NULL) < 0)
> + if (virCommandRun(cmd, es) < 0)
> ret = -1;
>
> virCommandFree(cmd);
> @@ -179,8 +183,9 @@ virNetDevBandwidthClear(const char *ifname)
> cmd = virCommandNew(TC);
> virCommandAddArgList(cmd, "qdisc", "del", "dev",
ifname,
> "ingress", NULL);
>
> - if (virCommandRun(cmd, NULL) < 0)
> + if (virCommandRun(cmd, es) < 0)
> ret = -1;
> +
> virCommandFree(cmd);
>
> return ret;
> diff --git a/src/util/virnetdevbandwidth.h
> b/src/util/virnetdevbandwidth.h
> index 18384ae..f15b489 100644
> --- a/src/util/virnetdevbandwidth.h
> +++ b/src/util/virnetdevbandwidth.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (C) 2009-2011 Red Hat, Inc.
> + * Copyright (C) 2009-2012 Red Hat, Inc.
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -43,8 +43,8 @@ void virNetDevBandwidthFree(virNetDevBandwidthPtr def);
>
> int virNetDevBandwidthSet(const char *ifname, virNetDevBandwidthPtr
> bandwidth)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
> -int virNetDevBandwidthClear(const char *ifname)
> - ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
> +int virNetDevBandwidthClear(const char *ifname, bool ignore_error)
> + ATTRIBUTE_NONNULL(1);
> int virNetDevBandwidthCopy(virNetDevBandwidthPtr *dest, const
> virNetDevBandwidthPtr src)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
>
ACK if you leave out the ignore_error argument.
Peter
Fixed as you requested and pushed, thanks.
Martin