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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel