Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for FreeBSD to ltfs_ordered_copy #380

Merged
merged 2 commits into from
Jun 23, 2023
Merged

Add support for FreeBSD to ltfs_ordered_copy #380

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66d2460
Fix ltfs_ordered_copy errno handling
micronn Dec 12, 2022
e2459a1
Add FreeBSD support to ltfs_ordered_copy
micronn Dec 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions src/utils/ltfs_ordered_copy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
# OO_Copyright_END

import sys
import errno
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This errno handling looks good to me.

import platform
import os.path
import argparse
Expand All @@ -44,6 +45,11 @@ import threading
from logging import getLogger, basicConfig, NOTSET, CRITICAL, ERROR, WARNING, INFO, DEBUG
from collections import deque

def is_errno(num, names):
if not num in errno.errorcode:
return False
return errno.errorcode[num] in names

class CopyItem:
""""""
def __init__(self, src, dst, vea_pre, cp_attr, cp_xattr, logger): #initialization
Expand All @@ -60,14 +66,14 @@ class CopyItem:

def eval(self): #access to the extended attributes present in some operating systems/filesystems by xattr
try:
self.vuuid = xattr.get(self.src, self.vea_pre + 'ltfs.volumeUUID')
self.vuuid = xattr.getxattr(self.src, self.vea_pre + 'ltfs.volumeUUID')
except Exception as e:
self.vuuid = ''
return (self.vuuid, self.part, self.start)

try:
self.part = xattr.get(self.src, self.vea_pre + 'ltfs.partition')
start_str = xattr.get(self.src, self.vea_pre + 'ltfs.startblock')
self.part = xattr.getxattr(self.src, self.vea_pre + 'ltfs.partition')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you using the pyxattr module? Now we are expecting the xattr module. And I think xattr.getxattr is not provided in the xattr package.

The reason why we assume the xattr module is it is provided by the pyhon-xattr on the official repository of RHEL7/8. As you see, the first target of this project is RHEL7 and RHEL8.

So I cannot accept this change. Please create another ltfs_ordered_copy for pyxattr.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In FreeBSD I use xattr, which provides xattr.getxattr and alike.

I don't have access to RHEL7/8 but I've seen some documents from redhat with a list of packages and their licenses, an I see one called pyxattr (in RHEL7) and one called python3-pyxattr (in RHEL8) with LGPLv2+ license. Based on that, I assume that RHEL is really using pyxattr module (because the other xattr is MIT licensed).

The pyxattr module (the one I assume is in RHEL) deprectared xattr.getxattr some time ago in favor of xattr.get. But the deprecated functions are still available (at least this is what I see in the main branch of pyxattr in github).

So, by now the use of xattr.xattrget works on both modules.

I understand you may have concerns with this PR because we don't know what the pyxattr folks will do regarding these deprecated functions. So, maybe the best way to deal with that is to only merge the errno handling code, and then I may ask the FreeBSD port maintainer to do the rest of the patch there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, sorry I was completely confused and I need to apologize to you.

The module we are using is pyxattr, so we can accept this. Let me have more time to confirm that this modification doesn't break the behavior on RHEL7/8.

The IBM docs says it is pyxattr.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I think it is a good idea to sepalate errno fix and getxattr fix. I can merge the errno fix ASAP as you mentioned.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, sorry for keeping as is so long time.

I decided to merge this on the master branch. We have a time to test it before releasing from this branch!

start_str = xattr.getxattr(self.src, self.vea_pre + 'ltfs.startblock')
self.start = int(start_str)
self.size = os.path.getsize(self.src)
except Exception as e:
Expand All @@ -89,12 +95,12 @@ class CopyItem:
if self.cp_xattr:
# Capture EAs of the source file
src_attributes = {}
for key in xattr.list(self.src):
src_attributes[key] = xattr.get(self.src, key)
for key in xattr.listxattr(self.src):
src_attributes[key] = xattr.getxattr(self.src, key)
# Set EAs of the destination file
(_, filename) = os.path.split(self.src)
for key in src_attributes:
xattr.set(self.dst, key, src_attributes[key])
xattr.setxattr(self.dst, key, src_attributes[key])
else: #Only copy data
shutil.copy(self.src, self.dst)
except Exception as e:
Expand Down Expand Up @@ -178,7 +184,7 @@ class CopyQueue:
self.logger.log(NOTSET + 1, 'Making a directory {}'.format(d))
os.mkdir(d)
except OSError as e:
if e.errno != 17: # Not EEXIST
if e.errno != errno.EEXIST:
self.logger.error(str(e) + "\n")
exit(1)

Expand Down Expand Up @@ -251,7 +257,7 @@ LTFS_SIG_VEA='ltfs.softwareProduct'
plat = platform.system()
if plat == 'Linux':
VEA_PREFIX='user.'
elif plat == 'Darwin':
elif plat == 'Darwin' or plat == 'FreeBSD':
VEA_PREFIX=''
else:
sys.stderr.write("unsupported platform '{0}'\n".format(plat))
Expand Down Expand Up @@ -362,18 +368,18 @@ if args.recursive == False and len(args.SOURCE) == 1:
logger.log(NOTSET + 1, 'Checking destination is LTFS or not')
direct_write_threads = 8
try:
sig = xattr.get(args.DEST, VEA_PREFIX + LTFS_SIG_VEA)
sig = xattr.getxattr(args.DEST, VEA_PREFIX + LTFS_SIG_VEA)

if sig.startswith(b"LTFS"):
logger.info("Destination {0} is LTFS".format(args.DEST))
direct_write_threads = 1
else:
logger.info("Destination {0} is not LTFS\n".format(args.DEST))
except IOError as e:
if e.errno != 61 and e.errno != 93 and e.errno != 95: # Not ENODATA, ENOATTR, EOPNOTSUPP
if not is_errno(e.errno, ['ENODATA', 'ENOATTR', 'ENOTSUP', 'EOPNOTSUPP']):
logger.error('Check destination (I/O):' + str(e))
exit(2)
if e.errno == 95 and args.p:
if is_errno(e.errno, ['ENOTSUP', 'EOPNOTSUPP']) and args.p:
logger.warning("{0} does not support xattr. Cannot use -p. Attributes will not be preserved during copy.".format(args.DEST))
args.p = False
logger.warning("Destination {0} is not LTFS".format(args.DEST))
Expand Down