Print this page
OS-2547 VM.js should protect writes to configuration files with a lockfile

@@ -65,10 +65,11 @@
 var assert = require('assert');
 var async = require('/usr/node/node_modules/async');
 var bunyan = require('/usr/node/node_modules/bunyan');
 var cp = require('child_process');
 var dladm = require('/usr/vm/node_modules/dladm');
+var lock = require('/usr/vm/node_modules/locker').lock;
 var EventEmitter = require('events').EventEmitter;
 var exec = cp.exec;
 var execFile = cp.execFile;
 var expat = require('/usr/node/node_modules/node-expat');
 var fs = require('fs');

@@ -4223,10 +4224,37 @@
             callback();
         }
     });
 }
 
+function writeAndRename(log, name, destfile, file_data, callback)
+{
+    var tempfile = destfile + '.new';
+
+    log.debug('writing ' + name + ' to ' + tempfile);
+
+    fs.writeFile(tempfile, file_data, function (err) {
+        if (err) {
+            callback(err);
+            return;
+        }
+
+        log.debug('wrote ' + name + ' to ' + tempfile);
+        log.debug('renaming from ' + tempfile + ' to ' + destfile);
+
+        fs.rename(tempfile, destfile, function (_err) {
+            if (_err) {
+                callback(_err);
+                return;
+            }
+
+            log.debug('renamed from ' + tempfile + ' to ' + destfile);
+            callback();
+        });
+    });
+}
+
 // writes a Zone's metadata JSON to /zones/<uuid>/config/metadata.json
 // and /zones/<uuid>/config/tags.json.
 function updateMetadata(vmobj, payload, log, callback)
 {
     var cmdata = {};

@@ -4312,29 +4340,21 @@
             tags[key] = payload.set_tags[key];
         }
     }
 
     mdata = {'customer_metadata': cmdata, 'internal_metadata': imdata};
-    fs.writeFile(mdata_filename, JSON.stringify(mdata, null, 2),
-        function (err) {
-            if (err) {
-                callback(err);
-            } else {
-                log.debug('wrote metadata to ' + mdata_filename);
-                fs.writeFile(tags_filename, JSON.stringify(tags, null, 2),
-                    function (e) {
-                        if (e) {
-                            callback(e);
-                        } else {
-                            log.debug('wrote tags to' + tags_filename);
-                            callback();
-                        }
-                    }
-                );
-            }
+
+    async.series([
+        function (next) {
+            writeAndRename(log, 'metadata', mdata_filename,
+                JSON.stringify(mdata, null, 2), next);
+        },
+        function (next) {
+            writeAndRename(log, 'tags', tags_filename,
+                JSON.stringify(tags, null, 2), next);
         }
-    );
+    ], callback);
 }
 
 function saveMetadata(payload, log, callback)
 {
     var protovm = {};

@@ -12003,10 +12023,12 @@
 exports.update = function (uuid, payload, options, callback)
 {
     var log;
     var new_vmobj;
     var vmobj;
+    var unlock;
+    var lockpath;
 
     // options parameter is optional
     if (arguments.length === 3) {
         callback = arguments[2];
         options = {};

@@ -12022,10 +12044,23 @@
     log.info('Updating VM ' + uuid + ' with initial payload:\n'
         + JSON.stringify(payload, null, 2));
 
     async.series([
         function (cb) {
+            lockpath = '/var/run/vm.' + uuid + '.config.lockfile';
+            log.debug('acquiring lock on ' + lockpath);
+            lock(lockpath, function (err, _unlock) {
+                log.debug('acquired lock on ' + lockpath);
+                if (err) {
+                    cb(err);
+                    return;
+                }
+                unlock = _unlock;
+                cb();
+            });
+        },
+        function (cb) {
             // for update we currently always load the whole vmobj since the
             // update functions may need to look at bits from the existing VM.
             VM.load(uuid, {log: log}, function (err, obj) {
                 if (err) {
                     cb(err);

@@ -12159,11 +12194,26 @@
             applyUpdates(vmobj, new_vmobj, payload, log, function () {
                 cb();
             });
         }
     ], function (e) {
+        // If we were able to hold the lockfile, and thus have an unlock
+        // callback, we must call it before returning, whether or not
+        // there was an error.
+        if (unlock) {
+            log.debug('releasing lock on ' + lockpath);
+            unlock(function (unlock_err) {
+                if (unlock_err) {
+                    log.error(err, 'unlock error! (path ' + lockpath + ')');
+                } else {
+                    log.debug('released lock on ' + lockpath);
+                }
+                callback(e);
+            });
+        } else {
         callback(e);
+        }
     });
 };
 
 function kill(uuid, log, callback)
 {