Fw: Errors in zfs / zpool compdef

Fw: Errors in zfs / zpool compdef

[Peter Stephenson]

Someone was doing something with these not long ago, I can't remember who.

pws



Begin forwarded message:

Date: Wed, 21 Jan 2009 17:00:08 +0100
From: Dag-Erling Smørgrav <des@des.no>
To: pws@zsh.org
Subject: Errors in zfs / zpool compdef


The contents of Completion/Unix/Command/_z{fs,pool} are duplicated, +
there is a stray comment in the code for the zfs clone command (line 102
in _zfs) that breaks it.

DES
-- 
Dag-Erling Smørgrav - des@des.no

Re: Fw: Errors in zfs / zpool compdef

[Danek Duvall]

On Wed, Jan 21, 2009 at 04:24:43PM +0000, Peter Stephenson wrote:

> Dav-Erling Smørgrav wrote:
>
> > The contents of Completion/Unix/Command/_z{fs,pool} are duplicated, +
> > there is a stray comment in the code for the zfs clone command (line 102
> > in _zfs) that breaks it.
>
> Someone was doing something with these not long ago, I can't remember who.

That's me.  I know about the comment; that's filed at bugs.opensolaris.org
as

    6767865 tab-completion for "zfs clone" fails

which I can fix in the Solaris bits reasonably easily (I've been meaning to
do an upgrade to 4.3.9, but haven't had the time), but would need help in
getting the changes upstream (would you want a patch, or given the
simplicity of it, to fix it yourself?).

I'm not aware of the duplication, though.  The copies of these files we
ship on Solaris aren't the same, so I'd need more information about what's
going on there.

Thanks,
Danek

Re: Fw: Errors in zfs / zpool compdef

[Peter Stephenson]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]

Danek Duvall wrote:
> On Wed, Jan 21, 2009 at 04:24:43PM +0000, Peter Stephenson wrote:
> 
> > Dav-Erling Smørgrav wrote:
> >
> > > The contents of Completion/Unix/Command/_z{fs,pool} are duplicated, +
> > > there is a stray comment in the code for the zfs clone command (line 102
> > > in _zfs) that breaks it.
> >
> > Someone was doing something with these not long ago, I can't remember who.
> 
> That's me.  I know about the comment; that's filed at bugs.opensolaris.org
> as
> 
>     6767865 tab-completion for "zfs clone" fails
> 
> which I can fix in the Solaris bits reasonably easily (I've been meaning to
> do an upgrade to 4.3.9, but haven't had the time), but would need help in
> getting the changes upstream (would you want a patch, or given the
> simplicity of it, to fix it yourself?).

In general, a patch for anything that needs fixing helps---it's
getting beyond the point where I've got time even to track down simple
things myself.  I need every ounce of help I can get.

Thanks.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: [26391] Re: Fw: Errors in zfs / zpool compdef

[Danek Duvall]

[-- Attachment #1: Type: text/plain, Size: 696 bytes --]

On Wed, Jan 21, 2009 at 04:52:10PM +0000, Peter Stephenson wrote:

> In general, a patch for anything that needs fixing helps---it's
> getting beyond the point where I've got time even to track down simple
> things myself.  I need every ounce of help I can get.

Attached are the patches I applied to 4.3.9 for integration into Solaris.
The first one adds the Solaris directory to the set of completion
directories, and the second one fixes the errors that Dag-Erling mentioned,
as well as one in _ptree.  Also some small updates to the SMF scripts, and
the introduction of _dladm.

If you have any problems applying them, let me know and I'll try to get
something up against CVS.

Thanks,
Danek

[-- Attachment #2: 00-solaris-completion-dir.patch --]
[-- Type: text/plain, Size: 765 bytes --]

--- Src/Zle/complete.mdd.orig	Tue Jul 22 11:32:02 2008
+++ Src/Zle/complete.mdd	Tue Jul 22 11:32:23 2008
@@ -1,7 +1,7 @@
 name=zsh/complete
 link=either
 load=yes
-functions='Completion/*comp* Completion/AIX/*/* Completion/BSD/*/* Completion/Base/*/* Completion/Cygwin/*/* Completion/Darwin/*/* Completion/Debian/*/* Completion/Linux/*/* Completion/Mandriva/*/* Completion/Redhat/*/* Completion/Unix/*/* Completion/X/*/* Completion/Zsh/*/*'
+functions='Completion/*comp* Completion/AIX/*/* Completion/BSD/*/* Completion/Base/*/* Completion/Cygwin/*/* Completion/Darwin/*/* Completion/Debian/*/* Completion/Linux/*/* Completion/Mandriva/*/* Completion/Redhat/*/* Completion/Solaris/*/* Completion/Unix/*/* Completion/X/*/* Completion/Zsh/*/*'
 
 moddeps="zsh/zle"
 

[-- Attachment #3: 01-completions.patch --]
[-- Type: text/plain, Size: 28981 bytes --]

--- /dev/null	Wed Feb 11 19:57:01 2009
+++ Completion/Solaris/Command/_dladm	Mon Feb  9 20:55:17 2009
@@ -0,0 +1,393 @@
+#compdef dladm
+
+_dladm_links() {
+	compadd "$@" - $(dladm show-link -p -o link)
+}
+
+_dladm_devs() {
+	compadd "$@" - $(dladm show-phys -p -o device)
+}
+
+_dladm_aggrs() {
+	compadd "$@" - $(dladm show-aggr -p -o link)
+}
+
+_dladm_aggr_ports() {
+	compadd "$@" - $(dladm show-aggr -p -x -o ports)
+}
+
+_dladm_vlans() {
+	compadd "$@" - $(dladm show-vlan -p -o link)
+}
+
+_dladm_wifi_links() {
+	compadd "$@" - $(dladm show-wifi -p -o link)
+}
+
+_dladm_wifi_nets() {
+	compadd "$@" - ${(f)"$(dladm scan-wifi -p -o essid)"}
+}
+
+_dladm_secobjs() {
+	compadd "$@" - ${(f)"$(dladm show-secobj -p -o object)"}
+}
+
+_dladm_ethers() {
+	compadd "$@" - $(dladm show-ether -p -o link)
+}
+
+_dladm_linkprops() {
+	# TODO: complete property values, when available
+	compadd "$@" - $(dladm show-linkprop -c -o property)
+}
+
+_dladm_vnics() {
+	compadd "$@" - $(dladm show-vnic -p -o link)
+}
+
+_dladm_etherstubs() {
+	compadd "$@" - $(dladm show-etherstub)
+}
+
+_dladm() {
+	local context state line expl
+	typeset -A opt_args
+	local -a subcmds rw_properties rw_propnames ro_properties
+	local -a link_properties link_stats_properties
+	local -a aggr_properties aggr_lacp_properties aggr_ext_properties
+	local -a vlan_properties wifi_properties wifi_connect_properties
+	local -a ether_properties linkprop_properties secobj_properties
+
+	# TODO: some subcommands can take multiple comma-separated targets
+	# TODO: some option sets may be different based on other commandline flags
+	# TODO: some subcommands may take different arguments based on options
+
+	subcmds=(
+		"show-ether" "show-usage"
+		{"rename","show"}"-link"
+		{"add","create","delete","modify","remove","show"}"-aggr"
+		{"connect","disconnect","scan","show"}"-wifi"
+		{"reset","set","show"}"-linkprop"
+		{"create","delete","show"}"-secobj"
+		{"create","delete","show"}"-vlan"
+		{"delete","show"}"-phys"
+		{"create","delete","show"}"-vnic"
+		{"create","delete","show"}"-etherstub"
+	)
+
+	if [[ $service == "dladm" ]]; then
+		_arguments -C -A "-*" \
+			'-\?[Help]' \
+			'*::command:->subcmd' && return 0
+
+		if (( CURRENT == 1 )); then
+			_wanted commands expl "dladm subcommand" compadd -a subcmds
+			return
+		fi
+		service="$words[1]"
+		curcontext="${curcontext%:*}=$service:"
+	fi
+
+	link_properties=( "link" "class" "mtu" "state" "over" )
+	link_stats_properties=( "link" "ipackets" "rbytes" "ierrors" "opackets" "obytes" "oerrors" )
+
+	aggr_properties=( "link" "policy" "addrpolicy" "lacpactivity" "lacptimer" "flags" )
+	aggr_lacp_properties=( "link" "port" "aggregatable" "sync" "coll" "dist" "defaulted" "expired" )
+	aggr_ext_properties=( "link" "port" "speed" "duplex" "state" "address" "portstate" )
+
+	vlan_properties=( "link" "vid" "over" "flags" )
+
+	wifi_connect_properties=( "link" "essid" "bssid" "sec" "mode" "strength" "speed" "bsstype" )
+	wifi_properties=( $wifi_connect_properties "status" "auth" )
+
+	ether_properties=( "link" "ptype" "state" "auto" "speed-duplex" "pause" "rem_fault" )
+
+	linkprop_properties=( "link" "property" "value" "default" "possible" )
+
+	secobj_properties=( "object" "class" )
+
+	vnic_properties=( "link" "over" "speed" "macaddr" "macaddrtype" )
+
+	case $service in
+	("show-link")
+		_arguments -A "-*" \
+			'(-P --persistent)'{-P,--persistent}'[Display persistent link configuration]' \
+			'(-p --parseable)'{-p,--parseable}'[Parseable output]' \
+			- set1 \
+			'(-o --output)'{-o,--output}'[Properties to display]:property:_values -s , "property" $link_properties' \
+			- set2 \
+			'(-s --statistics)'{-s,--statistics}'[Display link statistics]' \
+			'(-i --interval)'{-i,--interval}'[Specify an interval]:interval:' \
+			'(-o --output)'{-o,--output}'[Properties to display]:property:_values -s , "property" $link_stats_properties' \
+			':link name:_dladm_links' \
+		;;
+
+	("rename-link")
+		_arguments -A "-*" \
+			'-R[Root directory]:directory:_path_files -/' \
+			':old link name:_dladm_links' \
+			':new link name:' \
+		;;
+
+	("show-phys")
+		_arguments -A "-*" \
+			'(-P --persistent)'{-P,--persistent}'[Display persistent link configuration]' \
+			'(-p --parseable)'{-p,--parseable}'[Parseable output]' \
+			'(-s --statistics)'{-s,--statistics}'[Display link statistics]' \
+			'(-i --interval)'{-i,--interval}'[Specify an interval]:interval:' \
+			'(-o --output)'{-o,--output}'[Properties to display]:property:_values -s , "property" link media state speed duplex device' \
+			':physical link name:_dladm_devs'
+		;;
+
+	("delete-phys")
+		_arguments -A "-*" \
+			':physical link name:_dladm_devs'
+		;;
+
+	("create-aggr")
+		_arguments -A "-*" \
+			'(-t --temporary)'{-t,--temporary}'[Aggregation should be temporary]' \
+			'(-R --root)'{-R,--root}'[Root directory]:directory:_path_files -/' \
+			'(-l --link)'{-l,--link}'[Component link]:link:_dladm_links' \
+			'(-P --policy)'{-P,--policy}'[Port selection policy]:policy:_values -s , "policy" L2 L3 L4' \
+			'(-L --lacp-mode)'{-L,--lacp-mode}'[LACP mode]:lacp mode:(off active passive)' \
+			'(-T --lacp-timer)'{-T,--lacp-timer}'[LACP timer]:lacp timer:(short long)' \
+			'(-u --unicast)'{-u,--unicast}'[Unicast address]:unicast address:' \
+			':aggregate link name:'
+		;;
+
+	("modify-aggr")
+		_arguments -A "-*" \
+			'(-t --temporary)'{-t,--temporary}'[Aggregation should be temporary]' \
+			'(-R --root)'{-R,--root}'[Root directory]:directory:_path_files -/' \
+			'(-l --link)'{-l,--link}'[Component link]:link:_dladm_links' \
+			'(-P --policy)'{-P,--policy}'[Port selection policy]:policy:_values -s , "policy" L2 L3 L4' \
+			'(-L --lacp-mode)'{-L,--lacp-mode}'[LACP mode]:lacp mode:(off active passive)' \
+			'(-T --lacp-timer)'{-T,--lacp-timer}'[LACP timer]:lacp timer:(short long)' \
+			'(-u --unicast)'{-u,--unicast}'[Unicast address]:unicast address:' \
+			':aggregate link name:_dladm_aggrs'
+		;;
+
+	("delete-aggr")
+		_arguments -A "-*" \
+			'(-t --temporary)'{-t,--temporary}'[Deletion should be temporary]' \
+			'(-R --root)'{-R,--root}'[Root directory]:directory:_path_files -/' \
+			':aggregate link name:_dladm_aggrs'
+		;;
+
+	("add-aggr")
+		_arguments -A "-*" \
+			'(-t --temporary)'{-t,--temporary}'[Aggregation should be temporary]' \
+			'(-R --root)'{-R,--root}'[Root directory]:directory:_path_files -/' \
+			'(-l --link)'{-l,--link}'[Component link]:link:_dladm_links' \
+			':aggregate link name:_dladm_aggrs'
+		;;
+
+	("remove-aggr")
+		_arguments -A "-*" \
+			'(-t --temporary)'{-t,--temporary}'[Aggregation should be temporary]' \
+			'(-R --root)'{-R,--root}'[Root directory]:directory:_path_files -/' \
+			'(-l --link)'{-l,--link}'[Component link]:link:_dladm_aggr_ports' \
+			':aggregate link name:_dladm_aggrs'
+		;;
+
+	("show-aggr")
+		_arguments -A "-*" \
+			'(-P --persistent)'{-P,--persistent}'[Display persistent link configuration]' \
+			'(-p --parseable)'{-p,--parseable}'[Parseable output]' \
+			'(-s --statistics)'{-s,--statistics}'[Display link statistics]' \
+			'(-i --interval)'{-i,--interval}'[Specify an interval]:interval:' \
+			':aggregate link name:_dladm_links' \
+			- set1 \
+			'(-o --output)'{-o,--output}'[Properties to display]:property:_values -s , "property" $aggr_properties' \
+			- lacp \
+			'(-L --lacp)'{-L,--lacp}'[LACP information]' \
+			'(-o --output)'{-o,--output}'[Properties to display]:property:_values -s , "property" $aggr_lacp_properties' \
+			- extended \
+			'(-x --extended)'{-x,--extended}'[Extended information]' \
+			'(-o --output)'{-o,--output}'[Properties to display]:property:_values -s , "property" $aggr_ext_properties' \
+		;;
+
+	("create-vlan")
+		_arguments -A "-*" \
+			'(-t --temporary)'{-t,--temporary}'[VLAN should be temporary]' \
+			'(-f --force)'{-f,--force}'[Force VLAN creation]' \
+			'(-R --root)'{-R,--root}'[Root directory]:directory:_path_files -/' \
+			'(-l --link)'{-l,--link}'[Component link]:link:_dladm_links' \
+			'-v[VLAN ID]:id:' \
+			':VLAN link name:'
+		;;
+
+	("delete-vlan")
+		_arguments -A "-*" \
+			'(-t --temporary)'{-t,--temporary}'[Deletion should be temporary]' \
+			'(-R --root)'{-R,--root}'[Root directory]:directory:_path_files -/' \
+			':VLAN link name:_dladm_vlans'
+		;;
+
+	("show-vlan")
+		_arguments -A "-*" \
+			'(-P --persistent)'{-P,--persistent}'[Display persistent link configuration]' \
+			'(-p --parseable)'{-p,--parseable}'[Parseable output]' \
+			'(-o --output)'{-o,--output}'[Properties to display]:property:_values -s , "property" $vlan_properties' \
+			':VLAN link name:_dladm_vlans'
+		;;
+
+	("scan-wifi")
+		_arguments -A "-*" \
+			'(-p --parseable)'{-p,--parseable}'[Parseable output]' \
+			'(-o --output)'{-o,--output}'[Properties to display]:property:_values -s , "property" $wifi_connect_properties' \
+			':wireless link name:_dladm_wifi_links'
+		;;
+
+	("connect-wifi")
+		_arguments -A "-*" \
+			'(-e --essid)'{-e,--essid}'[ESSID name]:network:_dladm_wifi_nets' \
+			'(-b --bsstype)'{-b,--bsstype}'[BSS type]:' \
+			'(-m --mode)'{-m,--mode}'[802.11 mode]:mode:(a b g)' \
+			'(-k --key)'{-k,--key}'[Key name]:key:_dladm_secobjs' \
+			'(-s --sec)'{-s,--sec}'[Security mode]:(none wep wpa)' \
+			'(-a --auth)'{-a,--auth}'[Authentication mode]:mode:(open shared)' \
+			'(-c --create-ibss)'{-c,--create-ibss}'[Create an ad-hoc network]' \
+			'(-T --timeout)'{-T,--timeout}'[Association timeout]:(forever)' \
+			':wireless link name:_dladm_wifi_links'
+		;;
+
+	("disconnect-wifi")
+		_arguments -A "-*" \
+			- set1 \
+			'(-a --all-links)'{-a,--all-links}'[All links]' \
+			- set2 \
+			':wireless link name:_dladm_wifi_links'
+		;;
+
+	("show-wifi")
+		_arguments -A "-*" \
+			'(-p --parseable)'{-p,--parseable}'[Parseable output]' \
+			'(-o --output)'{-o,--output}'[Properties to display]:property:_values -s , "property" $wifi_properties' \
+			':wireless link name:_dladm_wifi_links'
+		;;
+
+	("show-ether")
+		_arguments -A "-*" \
+			'(-p --parseable)'{-p,--parseable}'[Parseable output]' \
+			'(-x --extended)'{-x,--extended}'[Extended output]' \
+			'(-o --output)'{-o,--output}'[Properties to display]:property:_values -s , "property" $ether_properties' \
+			':ethernet link name:_dladm_ethers'
+		;;
+
+	("set-linkprop")
+		_arguments -A "-*" \
+			'(-t --temporary)'{-t,--temporary}'[Change should be temporary]' \
+			'(-R --root)'{-R,--root}'[Root directory]:directory:_path_files -/' \
+			'(-p --prop)'{-p,--prop}'[Properties]:property:_dladm_linkprops' \
+			':link name:_dladm_links'
+		;;
+
+	("reset-linkprop")
+		_arguments -A "-*" \
+			'(-t --temporary)'{-t,--temporary}'[Change should be temporary]' \
+			'(-R --root)'{-R,--root}'[Root directory]:directory:_path_files -/' \
+			'(-p --prop)'{-p,--prop}'[Properties]:property:_dladm_linkprops' \
+			':link name:_dladm_links'
+		;;
+
+	("show-linkprop")
+		_arguments -A "-*" \
+			'(-P --persistent)'{-P,--persistent}'[Display persistent link properties]' \
+			'(-c --parseable)'{-c,--parseable}'[Parseable output]' \
+			'(-o --output)'{-o,--output}'[Properties to display]:property:_values -s , "property" $linkprop_properties' \
+			'(-p --prop)'{-p,--prop}'[Properties]:property:_dladm_linkprops' \
+			':link name:_dladm_links'
+		;;
+
+	("create-secobj")
+		_arguments -A "-*" \
+			'(-t --temporary)'{-t,--temporary}'[Creation should be temporary]' \
+			'(-R --root)'{-R,--root}'[Root directory]:directory:_path_files -/' \
+			'(-c --class)'{-c,--class}'[Class]:class:(wep wpa)' \
+			'(-f --file)'{-f,--file}'[File containing object value]:file:_path_files' \
+			':object name:'
+		;;
+
+	("delete-secobj")
+		_arguments -A "-*" \
+			'(-t --temporary)'{-t,--temporary}'[Deletion should be temporary]' \
+			'(-R --root)'{-R,--root}'[Root directory]:directory:_path_files -/' \
+			':object name:_dladm_secobjs'
+		;;
+
+	("show-secobj")
+		_arguments -A "-*" \
+			'(-P --persistent)'{-P,--persistent}'[Display persistent object information]' \
+			'(-p --parseable)'{-p,--parseable}'[Parseable output]' \
+			'(-o --output)'{-o,--output}'[Properties to display]:property:_values -s , "property" $secobj_properties' \
+			':object name:_dladm_secobjs'
+		;;
+
+	("create-vnic")
+		# TODO: MAC address completion could be richer
+		_arguments -A "-*" \
+			'(-t --temporary)'{-t,--temporary}'[Creation should be temporary]' \
+			'(-R --root)'{-R,--root}'[Root directory]:directory:_path_files -/' \
+			'(-l --link)'{-l,--link}'[Component link]:link:_dladm_links' \
+			'(-m --mac-address)'{-m,--mac-address}'[MAC address]:address:(factory random auto)' \
+			'-v[VLAN ID]:id:' \
+			'(-p --prop)'{-p,--prop}'[Property values]:value:' \
+			':VNIC name:'
+		;;
+
+	("delete-vnic")
+		_arguments -A "-*" \
+			'(-t --temporary)'{-t,--temporary}'[Deletion should be temporary]' \
+			'(-R --root)'{-R,--root}'[Root directory]:directory:_path_files -/' \
+			':VNIC name:_dladm_vnics'
+		;;
+
+	("show-vnic")
+		_arguments -A "-*" \
+			'(-P --persistent)'{-P,--persistent}'[Display persistent object information]' \
+			'(-p --parseable)'{-p,--parseable}'[Parseable output]' \
+			'(-o --output)'{-o,--output}'[Properties to display]:property:_values -s , "property" $vnic_properties' \
+			'(-l --link)'{-l,--link}'[Limit to VNICs on link]:link:_dladm_links' \
+			'(-s --statistics)'{-s,--statistics}'[Display VNIC statistics]' \
+			'(-i --interval)'{-i,--interval}'[Specify an interval]:interval:' \
+			':VNIC name:_dladm_vnics'
+		;;
+
+	("create-etherstub")
+		_arguments -A "-*" \
+			'(-t --temporary)'{-t,--temporary}'[Creation should be temporary]' \
+			'(-R --root)'{-R,--root}'[Root directory]:directory:_path_files -/' \
+			':etherstub name:'
+		;;
+
+	("delete-etherstub")
+		_arguments -A "-*" \
+			'(-t --temporary)'{-t,--temporary}'[Deletion should be temporary]' \
+			'(-R --root)'{-R,--root}'[Root directory]:directory:_path_files -/' \
+			':etherstub name:_dladm_etherstubs'
+		;;
+
+	("show-etherstub")
+		_arguments -A "-*" \
+			':etherstub name:_dladm_etherstubs'
+		;;
+
+	("show-usage")
+		_arguments -A "-*" \
+			'(-f --file)'{-f,--file}'[Read records from file]:_path_files' \
+			'(-F --format)'{-f,--format}'[Plotfile format]:(gnuplot)' \
+			'(-p --plot)'{-p,--plot}'[Write plot to file]:' \
+			'(-e --start)'{-e,--start}'[Start time]:date/time (MM/DD/YYYY,hh\:mm\:ss)' \
+			'(-s --stop)'{-s,--stop}'[Stop time]:date/time (MM/DD/YYYY,hh\:mm\:ss)' \
+			':link name:_dladm_links'
+		;;
+
+	(*)
+		_message "unknown dladm subcommand: $service"
+		;;
+	esac
+}
+
+_dladm "$@"
--- Completion/Solaris/Command/_ptree.orig	Mon Jul 21 12:15:24 2008
+++ Completion/Solaris/Command/_ptree	Sun Feb  8 19:14:43 2009
@@ -12,7 +12,7 @@
 
 	if [[ $ret -ne 0 ]]; then
 		_alternative \
-			'users:logged-in user:(${$(who -q)\:#(users=*|\#)})' \
+			'users:logged-in user:compadd ${$(who -q)\:#(users=*|\#)}' \
 			'pids:pid:_pids'
 		ret=$?
 	fi
--- Completion/Solaris/Command/_svccfg.orig	Mon Jul 21 12:15:24 2008
+++ Completion/Solaris/Command/_svccfg	Sun Feb  8 18:28:00 2009
@@ -1,9 +1,27 @@
 #compdef svccfg
 
+_svccfg_properties() {
+	local -a props fmris
+	local fmri="$argv[$#]"
+
+	# If There's more than one possible FMRI, bail
+	fmris=( ${(f)"$(svcs -H -o fmri $fmri)"} )
+	if [[ $#fmris -gt 1 ]]; then
+		_message "'$fmri' is ambiguous"
+		return 1
+	fi
+
+	# Get all the property names for the FMRI
+	props=( ${${${(f)"$(svccfg -s $fmri describe)"}:# *}%% *} )
+
+	_multi_parts "$expl[@]" - / props
+}
+
 _svccfg() {
 	local context state line subcmds
+	typeset -A opt_args
 
-	subcmds=( apply extract import validate export inventory delete addpropvalue delpropvalue )
+	subcmds=( apply describe extract import validate export inventory delete addpropvalue delpropvalue )
 
 	if [[ $service == "svccfg" ]]; then
 		_arguments -s \
@@ -30,10 +48,14 @@
 			':file:_files'
 		;;
 
-	(apply|validate|inventory)
+	(apply|inventory)
 		_files
 		;;
 
+	(validate)
+		_alternative 'files:file:_files' 'fmris:fmri:_svc_fmri -c'
+		;;
+
 	(export)
 		_svcs_fmri -c
 		;;
@@ -44,6 +66,14 @@
 			'*:FMRI:_svcs_fmri -c'
 		;;
 
+	(describe)
+		local fmri=$opt_args[-s]
+		_arguments -A "-*" \
+			'-v[Give all information]' \
+			'-t[Show only template data]' \
+			":property group or property:_svccfg_properties $fmri"
+		;;
+
 	(extract)
 		;;
 
--- Completion/Solaris/Command/_svcs.orig	Mon Jul 21 12:15:24 2008
+++ Completion/Solaris/Command/_svcs	Sun Feb  8 18:18:44 2009
@@ -15,13 +15,13 @@
 		'(-l -x -D -a -R)-d[List dependencies]' \
 		'(-l -x -d -a -R)-D[List dependents]' \
 		'(-l -x)-H[Suppress header line]' \
-		'(-)-l[Print detailed status about services and instances]' \
+		'(-x -d -D -a -R -s)-l[Print detailed status about services and instances]' \
 		'(-l -x)-o[Display specific columns]:column:_values -s , "column" ${^cols/\:/[}\]' \
 		'(-l -x)-p[List processes]' \
 		'(-l -x -d -D)-R[List services with the given restarter]:instance FMRI:_svcs_fmri -i' \
 		'(-l -x)-s[Sort by a column]:column:(($cols))' \
 		'(-l -x)-S[Reverse sort by a column]:column:(($cols))' \
-		'(-l)-v[verbose columns]' \
+		'-v[verbose columns]' \
 		'(-l)-x[Display explanation for service states]' \
 		'(-l -x)-\?[Print help]' \
 		'*:FMRI or pattern:_svcs_fmri -i'
--- Completion/Unix/Command/_zfs.orig	Tue Aug 12 03:28:11 2008
+++ Completion/Unix/Command/_zfs	Sun Feb  8 18:11:38 2009
@@ -97,249 +97,11 @@
 		;;
 
 	("clone")
+		# XXX needs to bail if there are no snapshots
 		_arguments -A "-*" \
 			'-p[Create parent datasets]' \
-			# XXX needs to bail if there are no snapshots
 			':snapshot:_zfs_dataset -t snap' \
 			':filesystem/volume:'
-		;;
-
-	("promote")
-		# XXX complete only cloned filesystems
-		_arguments \
-			':filesystem:_zfs_dataset -t fs' \
-		;;
-
-	("rename")
-		_arguments -A "-*" \
-			- set1 \
-			'-p[Create parent datasets]' \
-			':filesystem/volume/snapshot:_zfs_dataset' \
-			':filesystem/volume/snapshot:' \
-			- set2 \
-			'-r[Recursively rename snapshots of all descendent datasets]' \
-			':snapshot:_zfs_dataset -t snap' \
-			':snapshot:'
-		;;
-
-	("list")
-		_arguments -A "-*" \
-			'-r[Recursively display children]' \
-			'-H[Scripting mode]' \
-			'-o[Properties to list]:property:_values -s , "property" $ro_properties $rw_propnames' \
-			'*-s[Sort key (ascending)]:property:_values "property" $ro_properties $rw_propnames' \
-			'*-S[Sort key (descending)]:property:_values "property" $ro_properties $rw_propnames' \
-			'-t[Dataset types to list]:dataset type:_values -s , "dataset type" filesystem snapshot volume' \
-			'*:filesystem/volume/snapshot:_zfs_dataset'
-		;;
-
-	("set")
-		_arguments \
-			':property:_values -s , "property" $rw_properties' \
-			'*:filesystem/volume:_zfs_dataset -t fs -t vol'
-		;;
-
-	("get")
-		_arguments -A "-*" \
-			"-r[Recursively display children's properties]" \
-			'-H[Scripting mode]' \
-			'-p[Display numbers exactly]' \
-			'-s[Specify sources]:source:_values -s , "source" local default inherited temporary none' \
-			'-o[Specify fields]:field:_values -s , "field" name property value source' \
-			':property:_values -s , "property" $ro_properties $rw_propnames all' \
-			'*:filesystem/volume/snapshot:_zfs_dataset'
-		;;
-
-	("inherit")
-		_arguments -A "-*" \
-			'-r[Recursively inherit property for all children]' \
-			':property:_values -s , "property" $ro_properties $rw_properties' \
-			'*:filesystem/volume:_zfs_dataset -t fs -t vol'
-		;;
-
-	("mount")
-		_arguments -A "-*" \
-			'-o[Mount options]:mount options:_values -s , "option" {,no}{devices,exec,setuid} ro rw' \
-			'-O[Overlay mount]' \
-			'-v[Report mount progress]' \
-			- set1 \
-			':filesystem:_zfs_dataset -t fs' \
-			- set2 \
-			'-a[Mount all available ZFS filesystems]'
-		;;
-
-	("unmount")
-		_arguments -A "-*" \
-			- set1 \
-			'-f[Force unmount]' \
-			':filesystem:_zfs_dataset -t fs -t mtpt' \
-			- set2 \
-			'-a[Unmount all ZFS filesystems]'
-		;;
-
-	("share")
-		_arguments -A "-*" \
-			- set1 \
-			'-a[Share all available ZFS filesystems]' \
-			- set2 \
-			':filesystem:_zfs_dataset -t fs'
-		;;
-
-	("unshare")
-		_arguments -A "-*" \
-			- set1 \
-			'-a[Unshare all shared ZFS filesystems]' \
-			- set2 \
-			'-F[Force unshare]' \
-			':filesystem:_zfs_dataset -t fs -t mtpt'
-		;;
-
-	("send")
-		_arguments -A "-*" \
-			'-i[Generate an incremental stream]:snapshot:_zfs_dataset -t snap' \
-			':snapshot:_zfs_dataset -t snap'
-		;;
-
-	("receive")
-		_arguments -A "-*" \
-			'-v[Verbose]' \
-			'-n[Do not receive the stream]' \
-			'-F[Force a rollback if necessary]' \
-			- set1 \
-			':filesystem/volume/snapshot:_zfs_dataset' \
-			- set2 \
-			'-d[Set path prefix]:filesystem:_zfs_dataset -t fs'
-		;;
-
-	("allow"|"unallow")
-		_message "unimplemented zfs subcommand: $service"
-		;;
-
-	("upgrade")
-		_arguments -A "-*" \
-			- set1 \
-			'-v[Verbose]' \
-			- set2 \
-			'-a[Upgrade all filesystems on all pools]' \
-			'-r[Upgrade descendent filesystems, too]' \
-			'-V[Upgrade to specified version]:version:(1 2)' \
-			- set3 \
-			'-r[Upgrade descendent filesystems, too]' \
-			'-V[Upgrade to specified version]:version:(1 2)' \
-			':filesystem:_zfs_dataset -t fs' \
-		;;
-
-	(*)
-		_message "unknown zfs subcommand: $service"
-		;;
-	esac
-}
-
-_zfs "$@"
-#compdef zfs
-
-_zfs() {
-	local context state line expl
-	typeset -A opt_args
-	local -a subcmds rw_properties rw_propnames ro_properties
-
-	subcmds=(
-		"create" "destroy" "clone" "promote" "rename" "snapshot"
-		"rollback" "list" "set" "get" "inherit" "mount" "unmount"
-		"share" "unshare" "send" "receive" "allow" "unallow"
-		"upgrade"
-	)
-
-	ro_properties=(
-		"name" "type" "creation" "used" "available" "referenced"
-		"compressratio" "mounted" "origin"
-	)
-
-	# TODO: Be cleverer about what values can be set.  Is there any way to
-	# set the sorting for *size properties to false by default?
-	rw_properties=(
-		"aclinherit:value:(discard noallow secure passthrough)"
-		"aclmode:value:(discard groupmask passthrough)"
-		"atime:value:(on off)"
-		"canmount:value:(on off)"
-		"checksum:value:(on off fletcher2 fletcher4 sha256)"
-		"compression:value:(on off lzjb gzip gzip-{1..9})"
-		"copies:value:(1 2 3)"
-		"devices:value:(on off)"
-		"exec:value:(on off)"
-		"mountpoint:value:{if [[ -prefix /* ]]; then _path_files -/; else _wanted mountpoints expl 'mountpoint (type \"/\" to start completing paths)' compadd legacy none; fi}"
-		"quota:number or 'none':{if [[ -prefix [0-9]## ]]; then _message -e 'number'; elif [[ $PREFIX == quota= ]]; then _wanted none expl 'number or none' compadd none; else _wanted none expl 'quota' compadd none; fi}"
-		"readonly:value:(on off)"
-		"recordsize:value:(512 1K 2K 4K 8K 16K 32K 64K 128K)"
-		"reservation:value:" # <size>, "none"
-		"setuid:value:(on off)"
-		"shareiscsi:value:(on off)" # or "type=<type>"
-		"sharenfs:value:(on off)" # or share(1M) options
-		"snapdir:value:(hidden visible)"
-		"version:value:(1 2 current)"
-		"volblocksize:value:(512 1K 2K 4K 8K 16K 32K 64K 128K)"
-		"volsize:value:" # <size>
-		"xattr:value:(on off)"
-		"zoned:value:(on off)"
-	)
-
-	rw_propnames=( ${rw_properties%%:*} )
-
-	if [[ $service == "zfs" ]]; then
-		_arguments -C -A "-*" \
-			'-\?[Help]' \
-			'*::command:->subcmd' && return 0
-
-		if (( CURRENT == 1 )); then
-			_wanted commands expl "zfs subcommand" compadd -a subcmds
-			return
-		fi
-		service="$words[1]"
-		curcontext="${curcontext%:*}=$service:"
-	fi
-
-	case $service in
-	("create")
-		_arguments -A "-*" \
-			'-p[Create parent datasets]' \
-			'-o[Set initial properties]:property:_values -s , "property" $rw_properties' \
-			- set1 \
-			':filesystem:' \
-			- set2 \
-			'-s[Create sparse volume]' \
-			'-b[Set volblocksize]:blocksize:' \
-			'-V[Set size]:size:' \
-			':volume:'
-		;;
-
-	("destroy")
-		_arguments -A "-*" \
-			'-r[Recursively destroy all children]' \
-			'-R[Recursively destroy all dependents]' \
-			'-f[Force unmounts]' \
-			':filesystem/volume/snapshot:_zfs_dataset'
-		;;
-
-	("snapshot")
-		_arguments -A "-*" \
-			'-r[Recursively snapshot all descendant datasets]' \
-			':filesystem/volume:_zfs_dataset -t fs -t vol -S@'
-		;;
-
-	("rollback")
-		_arguments -A "-*" \
-			'-r[Recursively destroy more recent snapshots]' \
-			'-R[Recursively destroy more recent snapshots and clones]' \
-			'-f[Force unmounts]' \
-			':snapshot:_zfs_dataset -t snap'
-		;;
-
-	("clone")
-		_arguments -A "-*" \
-			'-p[Create parent datasets]' \
-			# XXX needs to bail if there are no snapshots
-			':snapshot:_zfs_dataset -t snap' \
-			':filesystem/volume:'
 		;;
 
 	("promote")
--- Completion/Unix/Command/_zpool.orig	Tue Aug 12 03:28:11 2008
+++ Completion/Unix/Command/_zpool	Sun Feb  8 18:13:12 2009
@@ -199,204 +199,3 @@
 }
 
 _zpool "$@"
-#compdef zpool
-
-_zpool() {
-	local -a subcmds fields
-
-	subcmds=(
-		  create destroy add remove list iostat status online
-		  offline clear attach detach replace scrub import export 
-		  upgrade history get set
-	)
-
-	fields=(
-		name\:"Pool name" size\:"Total size" used\:"Space used"
-		available\:"Space available" capacity\:"Space used (percentage)"
-		health\:"Health status"
-	)
-
-	properties=(
-		"bootfs:value:"
-		"autoreplace:value:(on off)"
-		"delegation:value:(on off)"
-	)
-	propnames=( ${properties%%:*} )
-
-	import_properties=(
-		xattr
-		copies
-		shareiscsi
-		canmount
-		share
-		sharenfs
-		userprop
-		mount
-	)
-
-	if [[ $service == "zpool" ]]; then
-		_arguments -C -A "-*" \
-			'-\?[Help]' \
-			'*::command:->subcmd' && return 0
-
-		if (( CURRENT == 1 )); then
-			_wanted commands expl "zpool subcommand" compadd -a subcmds
-			return
-		fi
-		service="$words[1]"
-		curcontext="${curcontext%:*}=$service:"
-	fi
-
-	case $service in
-	(clear)
-		_arguments \
-			':pool name:_zfs_pool' \
-			'*:virtual device:_files'
-		;;
-
-	(create)
-		# TODO: investigate better vdev handling
-		_arguments -A "-*" \
-			'-f[Force use of in-use devices]' \
-			'-n[Display configuration without creating pool]' \
-			'-R[Use alternate root]:alternate root:_files -/' \
-			'-m[Set mountpoint for root dataset]:mountpoint:' \
-			':pool name:' \
-			'*:virtual device:_files'
-		;;
-
-	(destroy)
-		_arguments -A "-*" \
-			'-f[Force active datasets to be unmounted]' \
-			':pool name:_zfs_pool'
-		;;
-
-	(add)
-		_arguments -A "-*" \
-			'-f[Force use of in-use devices]' \
-			'-n[Display configuration without modifying pool]' \
-			':pool name:_zfs_pool' \
-			'*:virtual device:_files'
-		;;
-
-	(list)
-		_arguments \
-			'-H[Scripted mode]' \
-			'-o[Fields to list]:field:_values -s , "field" ${^fields/\:/[}\]' \
-			'::pool name:_zfs_pool'
-		;;
-
-	(iostat)
-		_arguments -A "-*" \
-			'-v[Verbose statistics]' \
-			'*::pool name:_zfs_pool' \
-			'::interval:' \
-			'::count:'
-		;;
-
-	(status)
-		_arguments -A "-*" \
-			'-v[Verbose information]' \
-			'-x[Show only unhealthy pools]' \
-			'*::pool name:_zfs_pool'
-		;;
-
-	(offline)
-		_arguments -A "-*" \
-			'-t[Offline until next reboot]' \
-			':pool name:_zfs_pool' \
-			'*:virtual device:_files'
-		;;
-
-	(online)
-		_arguments \
-			':pool name:_zfs_pool' \
-			'*:virtual device:_files'
-		;;
-
-	(attach)
-		# TODO: first device should choose first from existing.
-		_arguments \
-			'-f[Force attach, even if in use]' \
-			':pool name:_zfs_pool' \
-			':virtual device:_files' \
-			':virtual device:_files'
-		;;
-
-	(detach)
-		_arguments \
-			':pool name:_zfs_pool' \
-			':virtual device:_files'
-		;;
-
-	(replace)
-		_arguments -A "-*" \
-			'-f[Force attach, even if in use]' \
-			':pool name:_zfs_pool' \
-			':virtual device:_files' \
-			'::virtual device:_files'
-		;;
-
-	(scrub)
-		_arguments -A "-*" \
-			'-s[Stop scrubbing]' \
-			'*:pool name:_zfs_pool'
-		;;
-
-	(export)
-		_arguments -A "-*" \
-			'-f[Forcefully unmount all datasets]' \
-			'*:pool name:_zfs_pool'
-		;;
-
-	(import)
-		# TODO: -o should complete options
-		_arguments -A "-*" \
-			'*-d[Search for devices or files in directory]:_files -/' \
-			'-D[Destroyed pools]' \
-			'-f[Force import]' \
-			- set1 \
-			'-o[Mount options]' \
-			'-p[Set property]:property:_values -s , "property" $import_properties' \
-			'-R[Alternate root]:_files -/' \
-			'*:pool name or id:_zfs_pool' \
-			'::new pool name:' \
-			- set2 \
-			'-a[All pools]'
-		;;
-
-	(get)
-		_arguments -A "-*" \
-			':property:_values -s , "property" $propnames all' \
-			'*:pool name:_zfs_pool'
-		;;
-
-	(set)
-		_arguments -A "-*" \
-			':property:_values -s , "property" $properties' \
-			'*:pool name:_zfs_pool'
-		;;
-
-	(upgrade)
-		_arguments -A "-*" \
-			- set1 \
-			'-v[Display ZFS versions and descriptions]' \
-			- set2 \
-			'-a[Upgrade all pools]' \
-			'*:pool name:_zfs_pool'
-		;;
-
-	(history)
-		_arguments -A "-*" \
-			'-i[Display internal events]' \
-			'-l[Long format]' \
-			'*:pool name:_zfs_pool'
-		;;
-
-	(*)
-		_message "unknown zpool subcommand: $service"
-		;;
-	esac
-}
-
-_zpool "$@"