[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2 of 5 V3] tools/hotplug: Remus network buffering setup scripts
On Thu, 2013-10-31 at 14:06 -0700, Shriram Rajagopalan wrote: > On Thu, Oct 31, 2013 at 1:21 PM, Ian Campbell > <Ian.Campbell@xxxxxxxxxx> wrote: > > + > > > +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. > > > > > I seem to get this wrong again and again.. > Did you mean the space trailing the second # ? or should the code > block inside setup/teardown > be indented with two tabs instead of one ? I meant "#foo" (1st line) vs "# foo" (2nd line) which just caught my eye. They should be the same. More normal would be a space after the # I think. I don't think we have a particularly strict coding style for shell scripts, but code being consistent with itself is a good start. > > > > > + _xenstore_write "$XENBUS_PATH/ifb" "$IFB" || > xs_write_failed "$vifname" "$IFB" > > + ;; > > + teardown) > > + : ${IFB?} > > > Do you mean log debug or something here? > > > > > This was to just make sure that the IFB variable was supplied as part > of the environment.. > Just like the two checks on top of this script.. > " > : ${vifname?} > : ${XENBUS_PATH?} > " Do these result in useful error reporting to the end user? Bearing in mind that the end user might be using libxl but not xl and therefore not see stdout/err (which might be going into some daemon's log file). Also, either I'm missing it or the bash manpage only documents ${parameter:?} and not ${parameter?}. Are both actually valid? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |