Re: [PATCH 2 of 5 V3] tools/hotplug: Remus network buffering setup scripts

[Ian Campbell <Ian.Campbell@citrix.com>]

On Sun, 2013-10-20 at 22:58 -0700, Shriram Rajagopalan wrote:

> +check_libnl_tools() {
> +    if ! command -v nl-qdisc-list > /dev/null 2>&1; then
> +        fatal "Unable to find nl-qdisc-list tool"
> +    fi
> +    if ! command -v nl-qdisc-add > /dev/null 2>&1; then
> +        fatal "Unable to find nl-qdisc-add tool"
> +    fi
> +    if ! command -v nl-qdisc-delete > /dev/null 2>&1; then
> +        fatal "Unable to find nl-qdisc-delete tool"
> +    fi
> +}

This probably suffices in place of the configure check I commented on
earlier.

> +add_plug_qdisc() {
> +    local vif=$1
> +    local ifb=$2
> +
> +    nl-qdisc-add --dev="$ifb" --parent root plug >/dev/null 2>&1
> +    if [ $? -ne 0 ]
> +    then
> +	do_without_error tc qdisc del dev "$vif" ingress
> +	fatal "Failed to add plug qdisc to $ifb"
> +    fi
> +
> +    #set ifb buffering limit in bytes. Its okay if this command

... "fails" ?

> +
> +case "$command" in
> +    setup)
> +	check_libnl_tools
> +	check_modules
> +
> +	claim_lock "pickifb"
> +	setup_ifb
> +	redirect_vif_traffic "$vifname" "$IFB"
> +	add_plug_qdisc "$vifname" "$IFB"
> +	release_lock "pickifb"
> +
> +	#not using xenstore_write that automatically exits on error
> +	# because we need to cleanup

whitespace inconsistency.


> +	_xenstore_write "$XENBUS_PATH/ifb" "$IFB" || xs_write_failed "$vifname" "$IFB"
> +	;;
> +    teardown)
> +	: ${IFB?}

Do you mean log debug or something here?

> +	teardown_netbuf "$vifname" "$IFB"
> +	;;
> +esac
> +
> +log debug "Successful remus-netbuf-setup $command for $vifname, ifb $IFB."
> +
> +if [ "$command" = "setup" ]
> +then
> +  success
> +fi

Why not put this in the case for setup?

Ian.