[PATCH] nfs: set security label when revalidating inode

[PATCH] nfs: set security label when revalidating inode

[Jeff Layton]

Currently, we fetch the security label when revalidating an inode's
attributes, but don't apply it. This is in contrast to the readdir()
codepath where we do apply label changes.

Cc: Dave Quigley <dpquigl@davequigley.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/nfs/inode.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 4bc7538..6ae6160 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -923,6 +923,8 @@ __nfs_revalidate_inode(struct nfs_server *server, struct inode *inode)
 	if (nfsi->cache_validity & NFS_INO_INVALID_ACL)
 		nfs_zap_acl_cache(inode);
 
+	nfs_setsecurity(inode, fattr, label);
+
 	dfprintk(PAGECACHE, "NFS: (%s/%Ld) revalidation complete\n",
 		inode->i_sb->s_id,
 		(long long)NFS_FILEID(inode));
-- 
1.8.3.1

Re: [PATCH] nfs: set security label when revalidating inode

[Dave Quigley]

On 11/2/2013 6:57 AM, Jeff Layton wrote:
> Currently, we fetch the security label when revalidating an inode's
> attributes, but don't apply it. This is in contrast to the readdir()
> codepath where we do apply label changes.
>
> Cc: Dave Quigley <dpquigl@davequigley.com>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>   fs/nfs/inode.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 4bc7538..6ae6160 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -923,6 +923,8 @@ __nfs_revalidate_inode(struct nfs_server *server, struct inode *inode)
>   	if (nfsi->cache_validity & NFS_INO_INVALID_ACL)
>   		nfs_zap_acl_cache(inode);
>
> +	nfs_setsecurity(inode, fattr, label);
> +
>   	dfprintk(PAGECACHE, "NFS: (%s/%Ld) revalidation complete\n",
>   		inode->i_sb->s_id,
>   		(long long)NFS_FILEID(inode));
>


I'm pretty sure (almost 100000%) that this was originally in the patch 
set and it got lost somewhere. So You have an ACK from me on it.

Dave

Re: [PATCH] nfs: set security label when revalidating inode

[Myklebust, Trond]

On Nov 2, 2013, at 6:57, Jeff Layton <jlayton@redhat.com> wrote:

> Currently, we fetch the security label when revalidating an inode's
> attributes, but don't apply it. This is in contrast to the readdir()
> codepath where we do apply label changes.

OK. Why should we not just throw out the code that fetches the security label here?

IOW: what is the caching model that is being implemented in this patch; is it just “fetch label at random intervals” or is there real method to the madness?

Trond

Re: [PATCH] nfs: set security label when revalidating inode

[Jeff Layton]

On Sun, 3 Nov 2013 02:23:29 +0000
"Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:

> 
> On Nov 2, 2013, at 6:57, Jeff Layton <jlayton@redhat.com> wrote:
> 
> > Currently, we fetch the security label when revalidating an inode's
> > attributes, but don't apply it. This is in contrast to the readdir()
> > codepath where we do apply label changes.
> 
> OK. Why should we not just throw out the code that fetches the security label here?
> 
> IOW: what is the caching model that is being implemented in this patch; is it just “fetch label at random intervals” or is there real method to the madness?
> 
> Trond

I think that we should apply the new security label as soon as we
realize that it has changed. Why should we treat the security label
differently from any other inode attribute (e.g. ownership or mode)?

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH] nfs: set security label when revalidating inode

[Jeff Layton]

On Sun, 3 Nov 2013 05:14:38 -0500
Jeff Layton <jlayton@redhat.com> wrote:

> On Sun, 3 Nov 2013 02:23:29 +0000
> "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> 
> > 
> > On Nov 2, 2013, at 6:57, Jeff Layton <jlayton@redhat.com> wrote:
> > 
> > > Currently, we fetch the security label when revalidating an inode's
> > > attributes, but don't apply it. This is in contrast to the readdir()
> > > codepath where we do apply label changes.
> > 
> > OK. Why should we not just throw out the code that fetches the security label here?
> > 
> > IOW: what is the caching model that is being implemented in this patch; is it just “fetch label at random intervals” or is there real method to the madness?
> > 
> > Trond
> 
> I think that we should apply the new security label as soon as we
> realize that it has changed. Why should we treat the security label
> differently from any other inode attribute (e.g. ownership or mode)?
> 

Ok, I think I understand what you're getting at now that I've had a cup
of coffee ;)

I guess you're pointing out a problem with the overall model, given that
the current implementation doesn't send anything in the RPC to denote
the security context of the client's task?

It's a fair point, and not one I have a great answer for. I think that
you're correct that for the most part that they won't change. But when
they do, what's to be gained by ignoring that?

They'll never be permanent anyway...as soon as the inode gets tossed
out of the cache or the client reboots then you'll see the change on
the next access of it.

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH] nfs: set security label when revalidating inode

[Jeff Layton]

On Sun, 3 Nov 2013 16:40:12 +0000
"Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:

> 
> On Nov 3, 2013, at 6:01, Jeff Layton <jlayton@redhat.com<mailto:jlayton@redhat.com>> wrote:
> 
> On Sun, 3 Nov 2013 05:14:38 -0500
> Jeff Layton <jlayton@redhat.com<mailto:jlayton@redhat.com>> wrote:
> 
> On Sun, 3 Nov 2013 02:23:29 +0000
> "Myklebust, Trond" <Trond.Myklebust@netapp.com<mailto:Trond.Myklebust@netapp.com>> wrote:
> 
> 
> On Nov 2, 2013, at 6:57, Jeff Layton <jlayton@redhat.com<mailto:jlayton@redhat.com>> wrote:
> 
> Currently, we fetch the security label when revalidating an inode's
> attributes, but don't apply it. This is in contrast to the readdir()
> codepath where we do apply label changes.
> 
> OK. Why should we not just throw out the code that fetches the security label here?
> 
> IOW: what is the caching model that is being implemented in this patch; is it just “fetch label at random intervals” or is there real method to the madness?
> 
> Trond
> 
> I think that we should apply the new security label as soon as we
> realize that it has changed. Why should we treat the security label
> differently from any other inode attribute (e.g. ownership or mode)?
> 
> 
> Ok, I think I understand what you're getting at now that I've had a cup
> of coffee ;)
> 
> I guess you're pointing out a problem with the overall model, given that
> the current implementation doesn't send anything in the RPC to denote
> the security context of the client's task?
> 
> It's a fair point, and not one I have a great answer for. I think that
> you're correct that for the most part that they won't change. But when
> they do, what's to be gained by ignoring that?
> 
> They'll never be permanent anyway...as soon as the inode gets tossed
> out of the cache or the client reboots then you'll see the change on
> the next access of it.
> 
> I’m not saying that we must ignore changes, I’m saying that nobody else, so far, has been willing to step up and propose a model for how these things are supposed to change.
> 
> All I’ve heard has been that “a polling model would be better because labels can change”. OK, but a polling model has a number of adjustable parameters; the frequency of polling being the most obvious. I want someone to explain to me, based on how we expect labels to change, how those parameters need to be set.
> If the expectation is that they will change once in a lifetime (just after file creation), then that would justify trying to poll once or twice, but it does not justify polling for the entire lifetime of the file. Perhaps a better solution would be to just have the server change the NFS filehandle (e.g. by bumping the generation counter)?
> 

Ugh...that sounds ugly and prone to cause ESTALE errors. 

I don't think we can make any assumption on how they will change, any
more than we do for something like the ownership or mode. It comes down
to an action by an administrator, and people are notoriously
unpredictable.

Maybe this is a naive question, but...why treat this differently from
any other inode attribute?

Granted, we do expect the client to enforce "permissions" based on this
label, so that does warrant extra caution. But, since we aren't
currently stuffing a "task context" into the RPC so that the server can
do the enforcement, all of this is inherently racy anyway.

As far as I'm concerned, this just comes down to the principle of least
surprise. As an administrator, if I change an attribute from a
different client, I don't expect to see that attribute "stuck" on all
of the other clients.

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH] nfs: set security label when revalidating inode

[Myklebust, Trond]

On Nov 3, 2013, at 12:01, Jeff Layton <jlayton@redhat.com> wrote:

> On Sun, 3 Nov 2013 16:40:12 +0000
> "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> 
>> 
>> On Nov 3, 2013, at 6:01, Jeff Layton <jlayton@redhat.com<mailto:jlayton@redhat.com>> wrote:
>> 
>> On Sun, 3 Nov 2013 05:14:38 -0500
>> Jeff Layton <jlayton@redhat.com<mailto:jlayton@redhat.com>> wrote:
>> 
>> On Sun, 3 Nov 2013 02:23:29 +0000
>> "Myklebust, Trond" <Trond.Myklebust@netapp.com<mailto:Trond.Myklebust@netapp.com>> wrote:
>> 
>> 
>> On Nov 2, 2013, at 6:57, Jeff Layton <jlayton@redhat.com<mailto:jlayton@redhat.com>> wrote:
>> 
>> Currently, we fetch the security label when revalidating an inode's
>> attributes, but don't apply it. This is in contrast to the readdir()
>> codepath where we do apply label changes.
>> 
>> OK. Why should we not just throw out the code that fetches the security label here?
>> 
>> IOW: what is the caching model that is being implemented in this patch; is it just “fetch label at random intervals” or is there real method to the madness?
>> 
>> Trond
>> 
>> I think that we should apply the new security label as soon as we
>> realize that it has changed. Why should we treat the security label
>> differently from any other inode attribute (e.g. ownership or mode)?
>> 
>> 
>> Ok, I think I understand what you're getting at now that I've had a cup
>> of coffee ;)
>> 
>> I guess you're pointing out a problem with the overall model, given that
>> the current implementation doesn't send anything in the RPC to denote
>> the security context of the client's task?
>> 
>> It's a fair point, and not one I have a great answer for. I think that
>> you're correct that for the most part that they won't change. But when
>> they do, what's to be gained by ignoring that?
>> 
>> They'll never be permanent anyway...as soon as the inode gets tossed
>> out of the cache or the client reboots then you'll see the change on
>> the next access of it.
>> 
>> I’m not saying that we must ignore changes, I’m saying that nobody else, so far, has been willing to step up and propose a model for how these things are supposed to change.
>> 
>> All I’ve heard has been that “a polling model would be better because labels can change”. OK, but a polling model has a number of adjustable parameters; the frequency of polling being the most obvious. I want someone to explain to me, based on how we expect labels to change, how those parameters need to be set.
>> If the expectation is that they will change once in a lifetime (just after file creation), then that would justify trying to poll once or twice, but it does not justify polling for the entire lifetime of the file. Perhaps a better solution would be to just have the server change the NFS filehandle (e.g. by bumping the generation counter)?
>> 
> 
> Ugh...that sounds ugly and prone to cause ESTALE errors. 
> 
> I don't think we can make any assumption on how they will change, any
> more than we do for something like the ownership or mode. It comes down
> to an action by an administrator, and people are notoriously
> unpredictable.

???? You’re saying that people choose security policies on a whim?

> Maybe this is a naive question, but...why treat this differently from
> any other inode attribute?

That’s another “What if we were to do X?” question. Please see above.

Trond

Re: [PATCH] nfs: set security label when revalidating inode

[Jeff Layton]

On Sun, 3 Nov 2013 18:41:43 +0000
"Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:

> 
> On Nov 3, 2013, at 12:01, Jeff Layton <jlayton@redhat.com> wrote:
> 
> > On Sun, 3 Nov 2013 16:40:12 +0000
> > "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:
> > 
> >> 
> >> On Nov 3, 2013, at 6:01, Jeff Layton <jlayton@redhat.com<mailto:jlayton@redhat.com>> wrote:
> >> 
> >> On Sun, 3 Nov 2013 05:14:38 -0500
> >> Jeff Layton <jlayton@redhat.com<mailto:jlayton@redhat.com>> wrote:
> >> 
> >> On Sun, 3 Nov 2013 02:23:29 +0000
> >> "Myklebust, Trond" <Trond.Myklebust@netapp.com<mailto:Trond.Myklebust@netapp.com>> wrote:
> >> 
> >> 
> >> On Nov 2, 2013, at 6:57, Jeff Layton <jlayton@redhat.com<mailto:jlayton@redhat.com>> wrote:
> >> 
> >> Currently, we fetch the security label when revalidating an inode's
> >> attributes, but don't apply it. This is in contrast to the readdir()
> >> codepath where we do apply label changes.
> >> 
> >> OK. Why should we not just throw out the code that fetches the security label here?
> >> 
> >> IOW: what is the caching model that is being implemented in this patch; is it just “fetch label at random intervals” or is there real method to the madness?
> >> 
> >> Trond
> >> 
> >> I think that we should apply the new security label as soon as we
> >> realize that it has changed. Why should we treat the security label
> >> differently from any other inode attribute (e.g. ownership or mode)?
> >> 
> >> 
> >> Ok, I think I understand what you're getting at now that I've had a cup
> >> of coffee ;)
> >> 
> >> I guess you're pointing out a problem with the overall model, given that
> >> the current implementation doesn't send anything in the RPC to denote
> >> the security context of the client's task?
> >> 
> >> It's a fair point, and not one I have a great answer for. I think that
> >> you're correct that for the most part that they won't change. But when
> >> they do, what's to be gained by ignoring that?
> >> 
> >> They'll never be permanent anyway...as soon as the inode gets tossed
> >> out of the cache or the client reboots then you'll see the change on
> >> the next access of it.
> >> 
> >> I’m not saying that we must ignore changes, I’m saying that nobody else, so far, has been willing to step up and propose a model for how these things are supposed to change.
> >> 
> >> All I’ve heard has been that “a polling model would be better because labels can change”. OK, but a polling model has a number of adjustable parameters; the frequency of polling being the most obvious. I want someone to explain to me, based on how we expect labels to change, how those parameters need to be set.
> >> If the expectation is that they will change once in a lifetime (just after file creation), then that would justify trying to poll once or twice, but it does not justify polling for the entire lifetime of the file. Perhaps a better solution would be to just have the server change the NFS filehandle (e.g. by bumping the generation counter)?
> >> 
> > 
> > Ugh...that sounds ugly and prone to cause ESTALE errors. 
> > 
> > I don't think we can make any assumption on how they will change, any
> > more than we do for something like the ownership or mode. It comes down
> > to an action by an administrator, and people are notoriously
> > unpredictable.
> 
> ???? You’re saying that people choose security policies on a whim?
> 
> > Maybe this is a naive question, but...why treat this differently from
> > any other inode attribute?
> 
> That’s another “What if we were to do X?” question. Please see above.
> 

I'm not sure I can answer that. Perhaps Dave or Steve D. can clarify
that, or direct us toward someone who can?

I had been operating under the assumption that the security label was
basically an inode attribute like any other, and therefore we should
treat it like we do any other attribute in the cache.

I'm a little unclear on what you've been suggesting however. Are you
saying that we should be setting it only on I_NEW inodes? IOW, rip out
all of the places where we set the label aside from nfs_fhget?

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH] nfs: set security label when revalidating inode

[Steve Dickson]

On 02/11/13 22:23, Myklebust, Trond wrote:
> 
> On Nov 2, 2013, at 6:57, Jeff Layton <jlayton@redhat.com> wrote:
> 
>> Currently, we fetch the security label when revalidating an inode's
>> attributes, but don't apply it. This is in contrast to the readdir()
>> codepath where we do apply label changes.
> 
> OK. Why should we not just throw out the code that fetches the security label here?
Looking back at the original code (aka David's tree), the label was being set 
in  nfs_refresh_inode() after the nfs_refresh_inode_locked() call:

int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr, struct nfs4_label *label)
{
    int status;

    if ((fattr->valid & NFS_ATTR_FATTR) == 0)
        return 0;
    spin_lock(&inode->i_lock);
    status = nfs_refresh_inode_locked(inode, fattr, label);
    spin_unlock(&inode->i_lock);
    if (nfs_server_capable(inode, NFS_CAP_SECURITY_LABEL)) {
        if (label && !status)
            nfs_setsecurity(inode, fattr, label);
    }

    return status; 
}

This code chunk got remove when I removed the setting of labels from 
all the original places they were being set (aka access, commits, etc).

There is an outstanding bug on how the client is not recognizing the
changing of a label.. So this patch will probably fix that bug...
 
> 
> IOW: what is the caching model that is being implemented in this patch; 
> is it just “fetch label at random intervals” or is there real method to the madness?
There is no caching model per say... I really don't think there needs to be
one... Labels are a client only thing meaning the server is not expect to
change the label and an application is expect to set them... So if there
is any caching to be done it should be done by the application, not the 
filesystem... IMHO...  

steved.

Re: [PATCH] nfs: set security label when revalidating inode

[Myklebust, Trond]

On Nov 4, 2013, at 10:19, Steve Dickson <SteveD@redhat.com> wrote:

> 
> 
> On 02/11/13 22:23, Myklebust, Trond wrote:
>> 
>> On Nov 2, 2013, at 6:57, Jeff Layton <jlayton@redhat.com> wrote:
>> 
>>> Currently, we fetch the security label when revalidating an inode's
>>> attributes, but don't apply it. This is in contrast to the readdir()
>>> codepath where we do apply label changes.
>> 
>> OK. Why should we not just throw out the code that fetches the security label here?
> Looking back at the original code (aka David's tree), the label was being set 
> in  nfs_refresh_inode() after the nfs_refresh_inode_locked() call:
> 
> int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr, struct nfs4_label *label)
> {
>    int status;
> 
>    if ((fattr->valid & NFS_ATTR_FATTR) == 0)
>        return 0;
>    spin_lock(&inode->i_lock);
>    status = nfs_refresh_inode_locked(inode, fattr, label);
>    spin_unlock(&inode->i_lock);
>    if (nfs_server_capable(inode, NFS_CAP_SECURITY_LABEL)) {
>        if (label && !status)
>            nfs_setsecurity(inode, fattr, label);
>    }
> 
>    return status; 
> }
> 
> This code chunk got remove when I removed the setting of labels from 
> all the original places they were being set (aka access, commits, etc).

> There is an outstanding bug on how the client is not recognizing the
> changing of a label.. So this patch will probably fix that bug…

I understood the question to be about why the client isn’t recognising changes that are made on the server. Are you saying that we’re failing to set the label correctly when the client itself changes it? That would be a bug under the existing caching rules.

>> 
>> IOW: what is the caching model that is being implemented in this patch; 
>> is it just “fetch label at random intervals” or is there real method to the madness?
> There is no caching model per say... I really don't think there needs to be
> one... Labels are a client only thing meaning the server is not expect to
> change the label and an application is expect to set them... So if there
> is any caching to be done it should be done by the application, not the 
> filesystem... IMHO...  

Right, but this argues against the need for polling.

Cheers,
  Trond


--
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH] nfs: set security label when revalidating inode

[Steve Dickson]

On 04/11/13 11:03, Myklebust, Trond wrote:
> 
> On Nov 4, 2013, at 10:19, Steve Dickson <SteveD@redhat.com> wrote:
> 
>>
>>
>> On 02/11/13 22:23, Myklebust, Trond wrote:
>>>
>>> On Nov 2, 2013, at 6:57, Jeff Layton <jlayton@redhat.com> wrote:
>>>
>>>> Currently, we fetch the security label when revalidating an inode's
>>>> attributes, but don't apply it. This is in contrast to the readdir()
>>>> codepath where we do apply label changes.
>>>
>>> OK. Why should we not just throw out the code that fetches the security label here?
>> Looking back at the original code (aka David's tree), the label was being set 
>> in  nfs_refresh_inode() after the nfs_refresh_inode_locked() call:
>>
>> int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr, struct nfs4_label *label)
>> {
>>    int status;
>>
>>    if ((fattr->valid & NFS_ATTR_FATTR) == 0)
>>        return 0;
>>    spin_lock(&inode->i_lock);
>>    status = nfs_refresh_inode_locked(inode, fattr, label);
>>    spin_unlock(&inode->i_lock);
>>    if (nfs_server_capable(inode, NFS_CAP_SECURITY_LABEL)) {
>>        if (label && !status)
>>            nfs_setsecurity(inode, fattr, label);
>>    }
>>
>>    return status; 
>> }
>>
>> This code chunk got remove when I removed the setting of labels from 
>> all the original places they were being set (aka access, commits, etc).
> 
>> There is an outstanding bug on how the client is not recognizing the
>> changing of a label.. So this patch will probably fix that bug…
> 
> I understood the question to be about why the client isn’t recognising changes 
> that are made on the server. Are you saying that we’re failing to set the label 
> correctly when the client itself changes it? That would be a bug under the 
> existing caching rules.
Yes... On app changes the label via nfs4_xattr_set_nfs4_label() 
but another app won't see the change since the label was not updated
by the getattr... Now would the label eventually get updated? 
Probably... through a lookup or open or something... 

Basically this is a bug in my forward port of Dave's code.  

Now I think you are questioning does the label even need
to be part of the getattr... As I just explained, I think 
so... How else will change be noticed?

steved.

> 
>>>
>>> IOW: what is the caching model that is being implemented in this patch; 
>>> is it just “fetch label at random intervals” or is there real method to the madness?
>> There is no caching model per say... I really don't think there needs to be
>> one... Labels are a client only thing meaning the server is not expect to
>> change the label and an application is expect to set them... So if there
>> is any caching to be done it should be done by the application, not the 
>> filesystem... IMHO...  
> 
> Right, but this argues against the need for polling.
> 
> Cheers,
>   Trond
> 
> 
> --
> Trond Myklebust
> Linux NFS client maintainer
> 
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com
> 

Labeled NFS: Is the value of FATTR4_WORD2_SECURITY_LABEL correct?

[Myklebust, Trond]

AFAICS from draft-ietf-nfsv4-minorversion2-20.txt, the ‘sec_label’ attribute has Id == 80.

Shouldn’t that mean that FATTR4_WORD2_SECURITY_LABEL should take the value (1 << (80-64))?

i.e.

#define FATTR4_WORD2_SECURITY_LABEL (1UL << 16)

instead of the current value of (1UL << 17)…

Trond

--
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: Labeled NFS: Is the value of FATTR4_WORD2_SECURITY_LABEL correct?

[Jeff Layton]

On Mon, 4 Nov 2013 19:20:09 +0000
"Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:

> AFAICS from draft-ietf-nfsv4-minorversion2-20.txt, the ‘sec_label’ attribute has Id == 80.
> 
> Shouldn’t that mean that FATTR4_WORD2_SECURITY_LABEL should take the value (1 << (80-64))?
> 
> i.e.
> 
> #define FATTR4_WORD2_SECURITY_LABEL (1UL << 16)
> 
> instead of the current value of (1UL << 17)…
> 
> Trond
> 

Yeah, that does look wrong. Well spotted!

Just to sanity check, the mdsthreshold bit is listed as bit 68 in
RFC5661:

    #define FATTR4_WORD2_MDSTHRESHOLD       (1UL << 4)

...so if we assume that that's correct, then
FATTR4_WORD2_SECURITY_LABEL is really set to the value for
change_sec_label...

-- 
Jeff Layton <jlayton@redhat.com>