 |
 |
@abh3 requested changes on this pull request.
New code new set of comments! However, we are quickly converging.
In src/XrdPosix/XrdPosixConfig.cc:
> @@ -87,6 +87,7 @@ extern bool oidsOK; extern bool p2lSRC; extern bool p2lSGI; extern bool autoPGRD; +extern int usingEC;
This really should be a bool
> @@ -1367,7 +1368,11 @@ int XrdHttpReq::ProcessHTTPReq() {
l = resourceplusopaque.length() + 1;
xrdreq.open.dlen = htonl(l);
xrdreq.open.mode = htons(kXR_ur | kXR_uw | kXR_gw | kXR_gr | kXR_or);
- xrdreq.open.options = htons(kXR_mkpath | kXR_open_wrto | kXR_delete);
+ if (usingEC == -1) usingEC = getenv("XRDCL_EC")? 1 : 0;
Well, that's one way of doing it. However, given that http plugin is loaded after xrootd, the envar will have been set by the time it initializes. So, I would suggest changing usingEC to a bool and add a one-time test/set in XrdHttpProtocol.cc after line 1037 and be done with it.
In src/XrdFfs/XrdFfsXrootdfs.cc:
> @@ -89,6 +89,8 @@ static struct fuse_opt xrootdfs_opts[14];
enum { OPT_KEY_HELP, OPT_KEY_SECSSS, };
+int usingEC = 0;
Suggest making this a bool.
In src/XrdFfs/XrdFfsXrootdfs.cc:
> @@ -127,6 +129,7 @@ static void* xrootdfs_init(struct fuse_conn_info *conn)
free(pwbuf);
/* put Xrootd related initialization calls here, after fuse daemonize itself. */
+ if (getenv("XRDCL_EC")) usingEC = 1;
If you change usingEC to a bool then please use true.
In src/XrdPosix/XrdPosixConfig.cc:
> @@ -177,6 +178,11 @@ void XrdPosixConfig::EnvInfo(XrdOucEnv &theEnv)
// method as it picks it up during init time. We leave the code for historical
// reasons but we really should have gotten rid of EnvInfo()!
// if (XrdPosixGlobals::myCache2) XrdPosixGlobals::myCache2->EnvInfo(theEnv);
+
+// Test if XRDCL_EC is set. That env var. is set at XrdCl::PlugInManager::LoadFactory
+// in XrdClPlugInManager.cc, which is called (by XrdOssGetSS while loading
+// libXrdPss.so) before this function.
+ XrdPosixGlobals::usingEC = getenv("XRDCL_EC")? 1 : 0;
once you make usingEC a bool, please use true and false. Also, see my comment in the corresponding test/set in the constructor.
In src/XrdPosix/XrdPosixXrootd.cc:
> @@ -94,6 +94,7 @@ bool oidsOK = false; bool p2lSRC = false; bool p2lSGI = false; bool autoPGRD = false; +int usingEC = -1;
This should be a bool set to false.
In src/XrdPosix/XrdPosixXrootd.cc:
> @@ -180,6 +181,12 @@ XrdPosixXrootd::XrdPosixXrootd(int fdnum, int dirnum, int thrnum)
static XrdSysMutex myMutex;
char *cfn;
+// Test if XRDCL_EC is set. That env var. is set at XrdCl::PlugInManager::LoadFactory
+// in XrdClPlugInManager.cc, which is called (by XrdOssGetSS while loading
+// libXrdPss.so) before this function.
+// Note: some standalone programs will call this constructor directly.
+ if (XrdPosixGlobals::usingEC == -1) XrdPosixGlobals::usingEC = getenv("XRDCL_EC")? 1 : 0;
Once it's a bool use true and false. Note that the test is immaterial as this is a one-time constructor. In fact, now that I think about it, you have to call the constructor anyway to use XrdPosix. So, this is really the only place you need the test and the one in XrdPosixConfig::EnvInfo need not be there. However, that only works if someone doesn't declare XrdPosixXroot as a static object at file level. So, I guess we can keep both.
In src/XrdPosix/XrdPosixXrootd.cc:
> @@ -1040,6 +1047,7 @@ int XrdPosixXrootd::Readdir64_r(DIR *dirp, struct dirent64 *entry, /* R e n a m e */ /******************************************************************************/ +int ec_rename(const char*, const char*, XrdPosixAdmin*);
OK, how come you decided not to make these private member methods of this class? It would seem like a preferable thing to do. Then you can avoid pre-declaring these. Even then if the non-namespace functions (they technically are functions not methods) were added at the top of the file declarations would not be necessary at all.
In src/XrdPosix/XrdPosixXrootd.cc:
> @@ -1133,6 +1144,7 @@ void XrdPosixXrootd::Seekdir(DIR *dirp, long loc) /* S t a t */ /******************************************************************************/ +int ec_stat(const char*, struct stat*, XrdPosixAdmin*);
Same comment as above.
In src/XrdPosix/XrdPosixXrootd.cc:
> @@ -1316,6 +1331,7 @@ int XrdPosixXrootd::Truncate(const char *path, off_t Size) /* U n l i n k */ /******************************************************************************/ +int ec_unlink(const char*, XrdPosixAdmin*);
Same comment as above.
In src/XrdPosix/XrdPosixXrootd.cc:
> @@ -1464,3 +1483,213 @@ int XrdPosixXrootd::Fault(XrdPosixFile *fp, int ecode)
errno = ecode;
return -1;
}
+
See my longer comment about this under rename.
> @@ -696,6 +696,7 @@ int TPCHandler::ProcessPushReq(const std::string & resource, XrdHttpExtReq &req) #endif } +int usingEC = -1;
This should be a bool.
> @@ -729,7 +730,8 @@ int TPCHandler::ProcessPullReq(const std::string &resource, XrdHttpExtReq &req)
XrdSfsFileOpenMode mode = SFS_O_CREAT;
auto overwrite_header = req.headers.find("Overwrite");
if ((overwrite_header == req.headers.end()) || (overwrite_header->second == "T")) {
- mode = SFS_O_TRUNC;
+ if (usingEC == -1) usingEC = getenv("XRDCL_EC")? 1 : 0;
Since XrdTpc is loaded by XrdHttp the envar will have been set by the time XrdTpc initializes. So, the usingEC test/set can be moved to XrdTpcConfigure.cc:22 with, of course, a comment why it is there. Then you can also use true/false.
> @@ -237,8 +237,14 @@ int main(int argc, char *argv[])
printf("Error_accessing: %s\n", argv[1]);
return 1;
}
- while ( (len = XrdPosixXrootd::Read(fd, buf, N)) > 0 )
- adler = adler32(adler, (const Bytef*)buf, len);
+ off_t totbytes = 0;
Actually, in all the places you have this code could prefix it with the comment:
//!!! TO DO: Remove code once XrdClEC read regression is fixed (comment appears in all such places).
Just an added reminder so that we will not forget to do this and can easily find the places
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Use REPLY-ALL to reply to list
To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1