ZFS patch proposal: zfs_fuid.c fix

[Pawel Jakub Dawidek]

On Fri, Jul 23, 2010 at 12:27:35AM +0200, Martin Matuska wrote:
>  I have examined the opensolaris SMB code and would like to propose the
> attached patch. It fixes zfs_fuid.c and acts like OpenSolaris with an
> unresolved IDMAP user: uses nulldomain (no domain) and unknown SMB user
> (UID_NOBODY).
> 
> In addition, I would like to allow users to set/unsed the sharesmb
> property even if it does nothing. It is good for imported pools that
> have datasets with this property enabled, so that it can be disabled.
> 
> It should fix kern/145778 and kern/148709.

The patch looks good to me.

> Index: cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c
> ===================================================================
> --- cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c	(revision 210319)
> +++ cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c	(working copy)
> @@ -1265,7 +1265,6 @@
>  	case ZFS_PROP_XATTR:
>  	case ZFS_PROP_VSCAN:
>  	case ZFS_PROP_NBMAND:
> -	case ZFS_PROP_SHARESMB:
>  		(void) snprintf(errbuf, sizeof (errbuf),
>  		    "property '%s' not supported on FreeBSD", propname);
>  		ret = zfs_error(hdl, EZFS_PERM, errbuf);
> Index: sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_fuid.c
> ===================================================================
> --- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_fuid.c	(revision 210319)
> +++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_fuid.c	(working copy)
> @@ -410,7 +410,7 @@
>  	domain = zfs_fuid_find_by_idx(zfsvfs, index);
>  	ASSERT(domain != NULL);
>  
> -#ifdef TODO
> +#ifdef sun
>  	if (type == ZFS_OWNER || type == ZFS_ACE_USER) {
>  		(void) kidmap_getuidbysid(crgetzone(cr), domain,
>  		    FUID_RID(fuid), &id);
> @@ -418,9 +418,9 @@
>  		(void) kidmap_getgidbysid(crgetzone(cr), domain,
>  		    FUID_RID(fuid), &id);
>  	}
> -#else
> -	panic(__func__);
> -#endif
> +#else	/* sun */
> +	id = UID_NOBODY;
> +#endif	/* sun */
>  	return (id);
>  }
>  
> @@ -514,21 +514,21 @@
>  	if (!zfsvfs->z_use_fuids || !IS_EPHEMERAL(id))
>  		return ((uint64_t)id);
>  
> -#ifdef TODO
> +#ifdef sun
>  	ksid = crgetsid(cr, (type == ZFS_OWNER) ? KSID_OWNER : KSID_GROUP);
>  
>  	VERIFY(ksid != NULL);
>  	rid = ksid_getrid(ksid);
>  	domain = ksid_getdomain(ksid);
> -
> +#else	/* sun */
> +	rid = UID_NOBODY;
> +	domain = nulldomain;
> +#endif	/* sun */
>  	idx = zfs_fuid_find_by_domain(zfsvfs, domain, &kdomain, B_TRUE);
>  
>  	zfs_fuid_node_add(fuidp, kdomain, rid, idx, id, type);
>  
>  	return (FUID_ENCODE(idx, rid));
> -#else
> -	panic(__func__);
> -#endif
>  }
>  
>  /*
> @@ -597,7 +597,7 @@
>  		};
>  		domain = fuidp->z_domain_table[idx -1];
>  	} else {
> -#ifdef TODO
> +#ifdef sun
>  		if (type == ZFS_OWNER || type == ZFS_ACE_USER)
>  			status = kidmap_getsidbyuid(crgetzone(cr), id,
>  			    &domain, &rid);
> @@ -606,6 +606,7 @@
>  			    &domain, &rid);
>  
>  		if (status != 0) {
> +#endif	/* sun */
>  			/*
>  			 * When returning nobody we will need to
>  			 * make a dummy fuid table entry for logging
> @@ -613,10 +614,9 @@
>  			 */
>  			rid = UID_NOBODY;
>  			domain = nulldomain;
> +#ifdef sun
>  		}
> -#else
> -		panic(__func__);
> -#endif
> +#endif	/* sun */
>  	}
>  
>  	idx = zfs_fuid_find_by_domain(zfsvfs, domain, &kdomain, B_TRUE);

-- 
Pawel Jakub Dawidek                       http://www.wheelsystems.com
pjd at FreeBSD.org                           http://www.FreeBSD.org
FreeBSD committer                         Am I Evil? Yes, I Am!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/zfs-devel/attachments/20100722/17907958/attachment.pgp