From 1a2bf23ef7e78d86f5bbad9af89c7b3004c1b145 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Mon, 20 Sep 2021 14:51:00 +0200 Subject: vpdupdate: Use locking to prevent update race. There are udev rules that may run vpdupdate multiple times in parallel. With a number of processes reading/updating/archiving the database the readers may see an inconsistent state. Lock the database file during archival. Signed-off-by: Michal Suchanek --- src/internal/updater.cpp | 52 +++++++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/src/internal/updater.cpp b/src/internal/updater.cpp index 391a6e6..d509cce 100644 --- a/src/internal/updater.cpp +++ b/src/internal/updater.cpp @@ -62,9 +62,9 @@ int storeComponents( System* root, VpdDbEnv& db ); int storeComponents( Component* root, VpdDbEnv& db ); void printUsage( ); void printVersion( ); -int ensureEnv( const string& env ); +int ensureEnv( const string& env, const string& file ); void archiveDB( const string& fullPath ); -int __lsvpdInit(string env, string file); +int __lsvpdInit( VpdDbEnv::UpdateLock *lock ); void __lsvpdFini(void); void lsvpdSighandler(int sig); @@ -74,6 +74,7 @@ const string BASE( "/sys/bus" ); bool isRoot(void); VpdDbEnv *db; +VpdDbEnv::UpdateLock *dblock; string env = DB_DIR, file = DB_FILENAME; @@ -83,6 +84,7 @@ int main( int argc, char** argv ) bool done = false; int index = 0, rc = 1; bool limitSCSISize = false; + VpdDbEnv::UpdateLock *lock; string platform = PlatformCollector::get_platform_name(); switch (PlatformCollector::platform_type) { @@ -127,7 +129,9 @@ int main( int argc, char** argv ) return 0; case 'a': + lock = new VpdDbEnv::UpdateLock(env, file, false); archiveDB( env + '/' + file ); + delete lock; return 0; case -1: @@ -151,12 +155,9 @@ int main( int argc, char** argv ) l.log( "vpdupdate: Constructing full devices database", LOG_NOTICE ); rc = initializeDB( limitSCSISize ); - if (rc) { - __lsvpdFini(); - return rc; - } __lsvpdFini(); + return rc; } bool isRoot() @@ -325,23 +326,28 @@ ZDONE:; */ int initializeDB( bool limitSCSI ) { + VpdDbEnv::UpdateLock *lock; System * root; int ret; - if( ensureEnv( env ) != 0 ) + if( ensureEnv( env, file ) != 0 ) return -1; string fullPath = env + "/" + file; + lock = new VpdDbEnv::UpdateLock(env, file, false); removeOldArchiveDB( ); archiveDB( fullPath ); + /* The db is now archived so when signal handler runs it should remove + * any db it finds */ + dblock = lock; Gatherer info( limitSCSI ); - ret = __lsvpdInit(env, file); + ret = __lsvpdInit(lock); if ( ret != 0 ) { Logger l; - l.log( " Could not allocate memory for the VPD database.", LOG_ERR); + l.log( "Could not allocate memory for the VPD database.", LOG_ERR); return ret; } @@ -358,11 +364,10 @@ int initializeDB( bool limitSCSI ) { Logger l; l.log( "Saving components to database failed.", LOG_ERR ); - return ret; } delete root; - return 0; + return ret; } /** @@ -409,7 +414,7 @@ int storeComponents( System* root, VpdDbEnv& db ) return 0; } -int ensureEnv( const string& env ) +int ensureEnv( const string& env, const string& file ) { struct stat info; int ret = -1; @@ -418,14 +423,14 @@ int ensureEnv( const string& env ) if( stat( env.c_str( ), &info ) == 0 ) { if (!S_ISDIR(info.st_mode & S_IFMT)) { - logger.log("/var/lib/lsvpd is not a directory\n", LOG_ERR); + logger.log(env + " is not a directory\n", LOG_ERR); return ret; } if ( ((info.st_mode & S_IRWXU) != S_IRWXU) || ((info.st_mode & S_IRGRP) != S_IRGRP) || ((info.st_mode & S_IROTH) != S_IROTH) ) { - logger.log("Failed to create vpd.db, no valid " + logger.log("Failed to create " + file + ", no valid " "permission\n", LOG_ERR); return ret; } @@ -438,7 +443,7 @@ int ensureEnv( const string& env ) return ret; } - if( ( ret = ensureEnv( env.substr( 0, idx ) ) ) != 0 ) + if( ( ret = ensureEnv( env.substr( 0, idx ) , env.substr( idx + 1 ) ) ) != 0 ) { return ret; } @@ -448,7 +453,7 @@ int ensureEnv( const string& env ) S_IRGRP | S_IWGRP | S_IXGRP | S_IROTH | S_IXOTH ) != 0 ) { - logger.log( "Failed to create directory for vpd db.", LOG_ERR ); + logger.log( "Failed to create directory " + env + " for vpd db.", LOG_ERR ); return -1; } return ret; @@ -458,7 +463,7 @@ int ensureEnv( const string& env ) * @brief initializes data base access, sets up signal handling * to ensure proper cleanup if process if prematurely aborted */ -int __lsvpdInit(string env, string file) +int __lsvpdInit( VpdDbEnv::UpdateLock *lock ) { struct sigaction sigact; @@ -475,7 +480,7 @@ int __lsvpdInit(string env, string file) sigaction(SIGQUIT, &sigact, NULL); sigaction(SIGTERM, &sigact, NULL); - db = new VpdDbEnv( env, file, false ); + db = new VpdDbEnv( *lock ); if ( db == NULL ) return -1; else @@ -491,6 +496,7 @@ void __lsvpdFini() if (db != NULL) { try { delete db; + dblock = NULL; } catch (VpdException & ve) { } } /* if */ @@ -499,7 +505,6 @@ void __lsvpdFini() void lsvpdSighandler(int sig) { - int fp; struct sigaction sigact; switch (sig) { @@ -516,12 +521,9 @@ void lsvpdSighandler(int sig) sigemptyset(&sigact.sa_mask); sigaction(sig, &sigact, NULL); - /* Remove temporary file */ - unlink((env + "/" + file).c_str()); - fp = open(env.c_str(), O_RDWR); - if (fp >= 0) { - fsync(fp); - close(fp); + if (dblock != NULL) { + /* Remove temporary file */ + unlink((env + "/" + file).c_str()); } __lsvpdFini(); -- cgit v1.2.3 From 25e11c079194f410240c4d4dc7aae492d5b65130 Mon Sep 17 00:00:00 2001 From: Michal Suchanek Date: Wed, 8 Dec 2021 16:50:04 +0100 Subject: Check for new enough libvpd2 that provides locking API. Signed-off-by: Michal Suchanek --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index e9fc871..dc80e1e 100644 --- a/configure.ac +++ b/configure.ac @@ -56,8 +56,8 @@ AC_CHECK_LIB(sgutils2,sg_lib_version, [SGUTILS_LIB="sgutils2"],[]) # exit 1 ]) AM_CONDITIONAL([SGUTIL1], [ test x$SGUTILS_LIB == xsgutils ]) AM_CONDITIONAL([SGUTIL2], [ test x$SGUTILS_LIB == xsgutils2 ]) -AC_CHECK_LIB(vpd,unpack_system,[],[ - echo "VPD library(libvpd) version 2 is required for lsvpd" +PKG_CHECK_MODULES([LIBVPD2], [libvpd_cxx-2 >= 2.2.9],[],[ + echo "VPD library(libvpd) version 2.2.9 is required for lsvpd" exit 1]) AC_FUNC_CLOSEDIR_VOID -- cgit v1.2.3 From bcb370092a78d4995f1aad3c9612ea20e59de8a1 Mon Sep 17 00:00:00 2001 From: Mahesh Salgaonkar Date: Fri, 25 Feb 2022 21:12:03 +0530 Subject: lsvpd v1.7.14 release notes Major changes since v1.7.13 - Prevent corruption of database file when running vpdupdate Signed-off-by: Mahesh Salgaonkar --- ChangeLog | 3 +++ configure.ac | 2 +- lsvpd.spec.in | 5 ++++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index ff0d869..4a237e2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,6 @@ +1.7.14: Mahesh Salgaonkar - Fri Feb 25 2022 +- Prevent corruption of database file when running vpdupdate + 1.7.13: Vasant Hegde - Thu Sep 9 2021 - Add support for SCSI loc code diff --git a/configure.ac b/configure.ac index dc80e1e..034f9aa 100644 --- a/configure.ac +++ b/configure.ac @@ -3,7 +3,7 @@ AC_PREREQ([2.69]) #base -AC_INIT([lsvpd],[1.7.13],[hegdevasant@linux.vnet.ibm.com, kamalesh@linux.vnet.ibm.com]) +AC_INIT([lsvpd],[1.7.14],[mahesh@linux.ibm.com,sv@linux.ibm.com]) AC_CONFIG_HEADER([config/config.h]) AC_SUBST(DATE, [`date`]) AC_CONFIG_MACRO_DIR([m4]) diff --git a/lsvpd.spec.in b/lsvpd.spec.in index 63b0247..dcbb7ce 100644 --- a/lsvpd.spec.in +++ b/lsvpd.spec.in @@ -20,7 +20,7 @@ Requires: iprutils >= 2.3.12 Requires(pre): iprutils >= 2.3.12 Requires(postun): iprutils >= 2.3.12 -BuildRequires: libvpd-devel >= 2.2.5 +BuildRequires: libvpd-devel >= 2.2.9 BuildRequires: librtas-devel BuildRequires: zlib-devel BuildRequires: sg3_utils-devel @@ -79,6 +79,9 @@ exit 0 %changelog +* Fri Feb 25 2022 - Mahesh Salgaonkar - 1.7.14 +- Prevent corruption of database file when running vpdupdate + * Thu Sep 9 2021 - Vasant Hegde - 1.7.13 - Add support for SCSI loc code -- cgit v1.2.3