1. 10 Aug, 2013 10 commits
  2. 03 Jul, 2013 15 commits
    • Yan, Zheng's avatar
      ceph: fix race between cap issue and revoke · 6ee6b953
      Yan, Zheng authored
      
      If we receive new caps from the auth MDS and the non-auth MDS is
      revoking the newly issued caps, we should release the caps from
      the non-auth MDS. The scenario is filelock's state changes from
      SYNC to LOCK. Non-auth MDS revokes Fc cap, the client gets Fc cap
      from the auth MDS at the same time.
      Signed-off-by: default avatarYan, Zheng <zheng.z.yan@intel.com>
      Reviewed-by: default avatarSage Weil <sage@inktank.com>
      6ee6b953
    • Yan, Zheng's avatar
      ceph: fix cap revoke race · b1530f57
      Yan, Zheng authored
      
      If caps are been revoking by the auth MDS, don't consider them as
      issued even they are still issued by non-auth MDS. The non-auth
      MDS should also be revoking/exporting these caps, the client just
      hasn't received the cap revoke/export message.
      
      The race I encountered is: When caps are exporting to new MDS, the
      client receives cap import message and cap revoke message from the
      new MDS, then receives cap export message from the old MDS. When
      the client receives cap revoke message from the new MDS, the revoking
      caps are still issued by the old MDS, so the client does nothing.
      Later when the cap export message is received, the client removes
      the caps issued by the old MDS. (Another way to fix the race is
      calling ceph_check_caps() in handle_cap_export())
      Signed-off-by: default avatarYan, Zheng <zheng.z.yan@intel.com>
      Reviewed-by: default avatarSage Weil <sage@inktank.com>
      b1530f57
    • Yan, Zheng's avatar
      ceph: fix pending vmtruncate race · b415bf4f
      Yan, Zheng authored
      The locking order for pending vmtruncate is wrong, it can lead to
      following race:
      
              write                  wmtruncate work
      ------------------------    ----------------------
      lock i_mutex
      check i_truncate_pending   check i_truncate_pending
      truncate_inode_pages()     lock i_mutex (blocked)
      copy data to page cache
      unlock i_mutex
                                 truncate_inode_pages()
      
      The fix is take i_mutex before calling __ceph_do_pending_vmtruncate()
      
      Fixes: http://tracker.ceph.com/issues/5453
      
      Signed-off-by: default avatarYan, Zheng <zheng.z.yan@intel.com>
      Reviewed-by: default avatarSage Weil <sage@inktank.com>
      b415bf4f
    • Sasha Levin's avatar
      ceph: avoid accessing invalid memory · 54464296
      Sasha Levin authored
      
      when mounting ceph with a dev name that starts with a slash, ceph
      would attempt to access the character before that slash. Since we
      don't actually own that byte of memory, we would trigger an
      invalid access:
      
      [   43.499934] BUG: unable to handle kernel paging request at ffff880fa3a97fff
      [   43.500984] IP: [<ffffffff818f3884>] parse_mount_options+0x1a4/0x300
      [   43.501491] PGD 743b067 PUD 10283c4067 PMD 10282a6067 PTE 8000000fa3a97060
      [   43.502301] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
      [   43.503006] Dumping ftrace buffer:
      [   43.503596]    (ftrace buffer empty)
      [   43.504046] CPU: 0 PID: 10879 Comm: mount Tainted: G        W    3.10.0-sasha #1129
      [   43.504851] task: ffff880fa625b000 ti: ffff880fa3412000 task.ti: ffff880fa3412000
      [   43.505608] RIP: 0010:[<ffffffff818f3884>]  [<ffffffff818f3884>] parse_mount_options$
      [   43.506552] RSP: 0018:ffff880fa3413d08  EFLAGS: 00010286
      [   43.507133] RAX: ffff880fa3a98000 RBX: ffff880fa3a98000 RCX: 0000000000000000
      [   43.507893] RDX: ffff880fa3a98001 RSI: 000000000000002f RDI: ffff880fa3a98000
      [   43.508610] RBP: ffff880fa3413d58 R08: 0000000000001f99 R09: ffff880fa3fe64c0
      [   43.509426] R10: ffff880fa3413d98 R11: ffff880fa38710d8 R12: ffff880fa3413da0
      [   43.509792] R13: ffff880fa3a97fff R14: 0000000000000000 R15: ffff880fa3413d90
      [   43.509792] FS:  00007fa9c48757e0(0000) GS:ffff880fd2600000(0000) knlGS:000000000000$
      [   43.509792] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
      [   43.509792] CR2: ffff880fa3a97fff CR3: 0000000fa3bb9000 CR4: 00000000000006b0
      [   43.509792] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
      [   43.509792] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
      [   43.509792] Stack:
      [   43.509792]  0000e5180000000e ffffffff85ca1900 ffff880fa38710d8 ffff880fa3413d98
      [   43.509792]  0000000000000120 0000000000000000 ffff880fa3a98000 0000000000000000
      [   43.509792]  ffffffff85cf32a0 0000000000000000 ffff880fa3413dc8 ffffffff818f3c72
      [   43.509792] Call Trace:
      [   43.509792]  [<ffffffff818f3c72>] ceph_mount+0xa2/0x390
      [   43.509792]  [<ffffffff81226314>] ? pcpu_alloc+0x334/0x3c0
      [   43.509792]  [<ffffffff81282f8d>] mount_fs+0x8d/0x1a0
      [   43.509792]  [<ffffffff812263d0>] ? __alloc_percpu+0x10/0x20
      [   43.509792]  [<ffffffff8129f799>] vfs_kern_mount+0x79/0x100
      [   43.509792]  [<ffffffff812a224d>] do_new_mount+0xcd/0x1c0
      [   43.509792]  [<ffffffff812a2e8d>] do_mount+0x15d/0x210
      [   43.509792]  [<ffffffff81220e55>] ? strndup_user+0x45/0x60
      [   43.509792]  [<ffffffff812a2fdd>] SyS_mount+0x9d/0xe0
      [   43.509792]  [<ffffffff83fd816c>] tracesys+0xdd/0xe2
      [   43.509792] Code: 4c 8b 5d c0 74 0a 48 8d 50 01 49 89 14 24 eb 17 31 c0 48 83 c9 ff $
      [   43.509792] RIP  [<ffffffff818f3884>] parse_mount_options+0x1a4/0x300
      [   43.509792]  RSP <ffff880fa3413d08>
      [   43.509792] CR2: ffff880fa3a97fff
      [   43.509792] ---[ end trace 22469cd81e93af51 ]---
      Signed-off-by: default avatarSasha Levin <sasha.levin@oracle.com>
      Reviewed-by: default avatarSage Weil <sage@inktan.com>
      54464296
    • majianpeng's avatar
      ceph: Reconstruct the func ceph_reserve_caps. · 93faca6e
      majianpeng authored
      
      Drop ignored return value.  Fix allocation failure case to not leak.
      Signed-off-by: default avatarJianpeng Ma <majianpeng@gmail.com>
      Reviewed-by: default avatarSage Weil <sage@inktank.com>
      93faca6e
    • majianpeng's avatar
      fb3101b6
    • Jianpeng Ma's avatar
      ceph: remove sb_start/end_write in ceph_aio_write. · 0405a149
      Jianpeng Ma authored
      
      Either in vfs_write or io_submit,it call file_start/end_write.
      The different between file_start/end_write and sb_start/end_write is
      file_ only handle regular file.But i think in ceph_aio_write,it only
      for regular file.
      Signed-off-by: default avatarJianpeng Ma <majianpeng@gmail.com>
      Acked-by: default avatarYan, Zheng <zheng.z.yan@intel.com>
      0405a149
    • majianpeng's avatar
    • majianpeng's avatar
      ceph: fix sleeping function called from invalid context. · a1dc1937
      majianpeng authored
      
      [ 1121.231883] BUG: sleeping function called from invalid context at kernel/rwsem.c:20
      [ 1121.231935] in_atomic(): 1, irqs_disabled(): 0, pid: 9831, name: mv
      [ 1121.231971] 1 lock held by mv/9831:
      [ 1121.231973]  #0:  (&(&ci->i_ceph_lock)->rlock){+.+...},at:[<ffffffffa02bbd38>] ceph_getxattr+0x58/0x1d0 [ceph]
      [ 1121.231998] CPU: 3 PID: 9831 Comm: mv Not tainted 3.10.0-rc6+ #215
      [ 1121.232000] Hardware name: To Be Filled By O.E.M. To Be Filled By
      O.E.M./To be filled by O.E.M., BIOS 080015  11/09/2011
      [ 1121.232027]  ffff88006d355a80 ffff880092f69ce0 ffffffff8168348c ffff880092f69cf8
      [ 1121.232045]  ffffffff81070435 ffff88006d355a20 ffff880092f69d20 ffffffff816899ba
      [ 1121.232052]  0000000300000004 ffff8800b76911d0 ffff88006d355a20 ffff880092f69d68
      [ 1121.232056] Call Trace:
      [ 1121.232062]  [<ffffffff8168348c>] dump_stack+0x19/0x1b
      [ 1121.232067]  [<ffffffff81070435>] __might_sleep+0xe5/0x110
      [ 1121.232071]  [<ffffffff816899ba>] down_read+0x2a/0x98
      [ 1121.232080]  [<ffffffffa02baf70>] ceph_vxattrcb_layout+0x60/0xf0 [ceph]
      [ 1121.232088]  [<ffffffffa02bbd7f>] ceph_getxattr+0x9f/0x1d0 [ceph]
      [ 1121.232093]  [<ffffffff81188d28>] vfs_getxattr+0xa8/0xd0
      [ 1121.232097]  [<ffffffff8118900b>] getxattr+0xab/0x1c0
      [ 1121.232100]  [<ffffffff811704f2>] ? final_putname+0x22/0x50
      [ 1121.232104]  [<ffffffff81155f80>] ? kmem_cache_free+0xb0/0x260
      [ 1121.232107]  [<ffffffff811704f2>] ? final_putname+0x22/0x50
      [ 1121.232110]  [<ffffffff8109e63d>] ? trace_hardirqs_on+0xd/0x10
      [ 1121.232114]  [<ffffffff816957a7>] ? sysret_check+0x1b/0x56
      [ 1121.232120]  [<ffffffff81189c9c>] SyS_fgetxattr+0x6c/0xc0
      [ 1121.232125]  [<ffffffff81695782>] system_call_fastpath+0x16/0x1b
      [ 1121.232129] BUG: scheduling while atomic: mv/9831/0x10000002
      [ 1121.232154] 1 lock held by mv/9831:
      [ 1121.232156]  #0:  (&(&ci->i_ceph_lock)->rlock){+.+...}, at:
      [<ffffffffa02bbd38>] ceph_getxattr+0x58/0x1d0 [ceph]
      
      I think move the ci->i_ceph_lock down is safe because we can't free
      ceph_inode_info at there.
      
      CC: stable@vger.kernel.org  # 3.8+
      Signed-off-by: default avatarJianpeng Ma <majianpeng@gmail.com>
      Reviewed-by: default avatarSage Weil <sage@inktank.com>
      a1dc1937
    • Yan, Zheng's avatar
    • Yan, Zheng's avatar
      ceph: clear migrate seq when MDS restarts · 667ca05c
      Yan, Zheng authored
      Signed-off-by: default avatarYan, Zheng <zheng.z.yan@intel.com>
      Reviewed-by: default avatarSage Weil <sage@inktank.com>
      667ca05c
    • Yan, Zheng's avatar
      ceph: check migrate seq before changing auth cap · b8c2f3ae
      Yan, Zheng authored
      
      We may receive old request reply from the exporter MDS after receiving
      the importer MDS' cap import message.
      Signed-off-by: default avatarYan, Zheng <zheng.z.yan@intel.com>
      Reviewed-by: default avatarSage Weil <sage@inktank.com>
      b8c2f3ae
    • Yan, Zheng's avatar
      ceph: fix race between page writeback and truncate · fc2744aa
      Yan, Zheng authored
      
      The client can receive truncate request from MDS at any time.
      So the page writeback code need to get i_size, truncate_seq and
      truncate_size atomically
      Signed-off-by: default avatarYan, Zheng <zheng.z.yan@intel.com>
      Reviewed-by: default avatarSage Weil <sage@inktank.com>
      fc2744aa
    • Yan, Zheng's avatar
      3803da49
    • Yan, Zheng's avatar
      ceph: fix cap release race · bb137f84
      Yan, Zheng authored
      
      ceph_encode_inode_release() can race with ceph_open() and release
      caps wanted by open files. So it should call __ceph_caps_wanted()
      to get the wanted caps.
      Signed-off-by: default avatarYan, Zheng <zheng.z.yan@intel.com>
      Reviewed-by: default avatarSage Weil <sage@inktank.com>
      bb137f84
  3. 01 Jul, 2013 3 commits
  4. 29 Jun, 2013 2 commits
    • Artem Bityutskiy's avatar
      UBIFS: fix a horrid bug · 605c912b
      Artem Bityutskiy authored
      
      Al Viro pointed me to the fact that '->readdir()' and '->llseek()' have no
      mutual exclusion, which means the 'ubifs_dir_llseek()' can be run while we are
      in the middle of 'ubifs_readdir()'.
      
      This means that 'file->private_data' can be freed while 'ubifs_readdir()' uses
      it, and this is a very bad bug: not only 'ubifs_readdir()' can return garbage,
      but this may corrupt memory and lead to all kinds of problems like crashes an
      security holes.
      
      This patch fixes the problem by using the 'file->f_version' field, which
      '->llseek()' always unconditionally sets to zero. We set it to 1 in
      'ubifs_readdir()' and whenever we detect that it became 0, we know there was a
      seek and it is time to clear the state saved in 'file->private_data'.
      
      I tested this patch by writing a user-space program which runds readdir and
      seek in parallell. I could easily crash the kernel without these patches, but
      could not crash it with these patches.
      
      Cc: stable@vger.kernel.org
      Reported-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
      Tested-by: default avatarArtem Bityutskiy <artem.bityutskiy@linux.intel.com>
      Signed-off-by: default avatarArtem Bityutskiy <artem.bityutskiy@linux.intel.com>
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
      605c912b
    • Artem Bityutskiy's avatar
      UBIFS: prepare to fix a horrid bug · 33f1a63a
      Artem Bityutskiy authored
      
      Al Viro pointed me to the fact that '->readdir()' and '->llseek()' have no
      mutual exclusion, which means the 'ubifs_dir_llseek()' can be run while we are
      in the middle of 'ubifs_readdir()'.
      
      First of all, this means that 'file->private_data' can be freed while
      'ubifs_readdir()' uses it.  But this particular patch does not fix the problem.
      This patch is only a preparation, and the fix will follow next.
      
      In this patch we make 'ubifs_readdir()' stop using 'file->f_pos' directly,
      because 'file->f_pos' can be changed by '->llseek()' at any point. This may
      lead 'ubifs_readdir()' to returning inconsistent data: directory entry names
      may correspond to incorrect file positions.
      
      So here we introduce a local variable 'pos', read 'file->f_pose' once at very
      the beginning, and then stick to 'pos'. The result of this is that when
      'ubifs_dir_llseek()' changes 'file->f_pos' while we are in the middle of
      'ubifs_readdir()', the latter "wins".
      
      Cc: stable@vger.kernel.org
      Reported-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
      Tested-by: default avatarArtem Bityutskiy <artem.bityutskiy@linux.intel.com>
      Signed-off-by: default avatarArtem Bityutskiy <artem.bityutskiy@linux.intel.com>
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
      33f1a63a
  5. 26 Jun, 2013 1 commit
  6. 24 Jun, 2013 1 commit
  7. 20 Jun, 2013 1 commit
  8. 17 Jun, 2013 1 commit
    • Maxim Patlasov's avatar
      fuse: hold i_mutex in fuse_file_fallocate() · 14c14414
      Maxim Patlasov authored
      
      Changing size of a file on server and local update (fuse_write_update_size)
      should be always protected by inode->i_mutex. Otherwise a race like this is
      possible:
      
      1. Process 'A' calls fallocate(2) to extend file (~FALLOC_FL_KEEP_SIZE).
      fuse_file_fallocate() sends FUSE_FALLOCATE request to the server.
      2. Process 'B' calls ftruncate(2) shrinking the file. fuse_do_setattr()
      sends shrinking FUSE_SETATTR request to the server and updates local i_size
      by i_size_write(inode, outarg.attr.size).
      3. Process 'A' resumes execution of fuse_file_fallocate() and calls
      fuse_write_update_size(inode, offset + length). But 'offset + length' was
      obsoleted by ftruncate from previous step.
      
      Changed in v2 (thanks Brian and Anand for suggestions):
       - made relation between mutex_lock() and fuse_set_nowrite(inode) more
         explicit and clear.
       - updated patch description to use ftruncate(2) in example
      Signed-off-by: default avatarMaxim V. Patlasov <MPatlasov@parallels.com>
      Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
      Signed-off-by: default avatarMiklos Szeredi <mszeredi@suse.cz>
      14c14414
  9. 15 Jun, 2013 2 commits
  10. 14 Jun, 2013 4 commits
    • Dave Chinner's avatar
      xfs: don't shutdown log recovery on validation errors · d302cf1d
      Dave Chinner authored
      
      Unfortunately, we cannot guarantee that items logged multiple times
      and replayed by log recovery do not take objects back in time. When
      they are taken back in time, the go into an intermediate state which
      is corrupt, and hence verification that occurs on this intermediate
      state causes log recovery to abort with a corruption shutdown.
      
      Instead of causing a shutdown and unmountable filesystem, don't
      verify post-recovery items before they are written to disk. This is
      less than optimal, but there is no way to detect this issue for
      non-CRC filesystems If log recovery successfully completes, this
      will be undone and the object will be consistent by subsequent
      transactions that are replayed, so in most cases we don't need to
      take drastic action.
      
      For CRC enabled filesystems, leave the verifiers in place - we need
      to call them to recalculate the CRCs on the objects anyway. This
      recovery problem can be solved for such filesystems - we have a LSN
      stamped in all metadata at writeback time that we can to determine
      whether the item should be replayed or not. This is a separate piece
      of work, so is not addressed by this patch.
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit 9222a9cf)
      d302cf1d
    • Dave Chinner's avatar
      xfs: ensure btree root split sets blkno correctly · 088c9f67
      Dave Chinner authored
      
      For CRC enabled filesystems, the BMBT is rooted in an inode, so it
      passes through a different code path on root splits than the
      freespace and inode btrees. This is much less traversed by xfstests
      than the other trees. When testing on a 1k block size filesystem,
      I've been seeing ASSERT failures in generic/234 like:
      
      XFS: Assertion failed: cur->bc_btnum != XFS_BTNUM_BMAP || cur->bc_private.b.allocated == 0, file: fs/xfs/xfs_btree.c, line: 317
      
      which are generally preceded by a lblock check failure. I noticed
      this in the bmbt stats:
      
      $ pminfo -f xfs.btree.block_map
      
      xfs.btree.block_map.lookup
          value 39135
      
      xfs.btree.block_map.compare
          value 268432
      
      xfs.btree.block_map.insrec
          value 15786
      
      xfs.btree.block_map.delrec
          value 13884
      
      xfs.btree.block_map.newroot
          value 2
      
      xfs.btree.block_map.killroot
          value 0
      .....
      
      Very little coverage of root splits and merges. Indeed, on a 4k
      filesystem, block_map.newroot and block_map.killroot are both zero.
      i.e. the code is not exercised at all, and it's the only generic
      btree infrastructure operation that is not exercised by a default run
      of xfstests.
      
      Turns out that on a 1k filesystem, generic/234 accounts for one of
      those two root splits, and that is somewhat of a smoking gun. In
      fact, it's the same problem we saw in the directory/attr code where
      headers are memcpy()d from one block to another without updating the
      self describing metadata.
      
      Simple fix - when copying the header out of the root block, make
      sure the block number is updated correctly.
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit ade1335a)
      088c9f67
    • Dave Chinner's avatar
      xfs: fix implicit padding in directory and attr CRC formats · 5170711d
      Dave Chinner authored
      
      Michael L. Semon has been testing CRC patches on a 32 bit system and
      been seeing assert failures in the directory code from xfs/080.
      Thanks to Michael's heroic efforts with printk debugging, we found
      that the problem was that the last free space being left in the
      directory structure was too small to fit a unused tag structure and
      it was being corrupted and attempting to log a region out of bounds.
      Hence the assert failure looked something like:
      
      .....
      #5 calling xfs_dir2_data_log_unused() 36 32
      #1 4092 4095 4096
      #2 8182 8183 4096
      XFS: Assertion failed: first <= last && last < BBTOB(bp->b_length), file: fs/xfs/xfs_trans_buf.c, line: 568
      
      Where #1 showed the first region of the dup being logged (i.e. the
      last 4 bytes of a directory buffer) and #2 shows the corrupt values
      being calculated from the length of the dup entry which overflowed
      the size of the buffer.
      
      It turns out that the problem was not in the logging code, nor in
      the freespace handling code. It is an initial condition bug that
      only shows up on 32 bit systems. When a new buffer is initialised,
      where's the freespace that is set up:
      
      [  172.316249] calling xfs_dir2_leaf_addname() from xfs_dir_createname()
      [  172.316346] #9 calling xfs_dir2_data_log_unused()
      [  172.316351] #1 calling xfs_trans_log_buf() 60 63 4096
      [  172.316353] #2 calling xfs_trans_log_buf() 4094 4095 4096
      
      Note the offset of the first region being logged? It's 60 bytes into
      the buffer. Once I saw that, I pretty much knew that the bug was
      going to be caused by this.
      
      Essentially, all direct entries are rounded to 8 bytes in length,
      and all entries start with an 8 byte alignment. This means that we
      can decode inplace as variables are naturally aligned. With the
      directory data supposedly starting on a 8 byte boundary, and all
      entries padded to 8 bytes, the minimum freespace in a directory
      block is supposed to be 8 bytes, which is large enough to fit a
      unused data entry structure (6 bytes in size). The fact we only have
      4 bytes of free space indicates a directory data block alignment
      problem.
      
      And what do you know - there's an implicit hole in the directory
      data block header for the CRC format, which means the header is 60
      byte on 32 bit intel systems and 64 bytes on 64 bit systems. Needs
      padding. And while looking at the structures, I found the same
      problem in the attr leaf header. Fix them both.
      
      Note that this only affects 32 bit systems with CRCs enabled.
      Everything else is just fine. Note that CRC enabled filesystems created
      before this fix on such systems will not be readable with this fix
      applied.
      Reported-by: default avatarMichael L. Semon <mlsemon35@gmail.com>
      Debugged-by: default avatarMichael L. Semon <mlsemon35@gmail.com>
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit 8a1fd295)
      5170711d
    • Dave Chinner's avatar
      xfs: don't emit v5 superblock warnings on write · 47ad2fcb
      Dave Chinner authored
      
      We write the superblock every 30s or so which results in the
      verifier being called. Right now that results in this output
      every 30s:
      
      XFS (vda): Version 5 superblock detected. This kernel has EXPERIMENTAL support enabled!
      Use of these features in this kernel is at your own risk!
      
      And spamming the logs.
      
      We don't need to check for whether we support v5 superblocks or
      whether there are feature bits we don't support set as these are
      only relevant when we first mount the filesytem. i.e. on superblock
      read. Hence for the write verification we can just skip all the
      checks (and hence verbose output) altogether.
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit 34510185)
      47ad2fcb