[libvirt] [PATCH] Make saving domain XML more robust

When saving domain XML (either config or status) we just overwrite old content of the file. In case something fails during that process (e.g. disk gets full) we lose both old and new content. This patch makes the process more robust by writing the new content into a separate file and only if that succeeds the original file is atomically replaced with the new one. --- src/conf/domain_conf.c | 25 +++++++++++++++++++++---- 1 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f9007ce..04c2b1c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11025,6 +11025,7 @@ int virDomainSaveXML(const char *configDir, const char *xml) { char *configFile = NULL; + char *newfile = NULL; int fd = -1, ret = -1; size_t towrite; @@ -11038,12 +11039,17 @@ int virDomainSaveXML(const char *configDir, goto cleanup; } - if ((fd = open(configFile, + if (virAsprintf(&newfile, "%s.new", configFile) < 0) { + virReportOOMError(); + goto cleanup; + } + + if ((fd = open(newfile, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR )) < 0) { virReportSystemError(errno, _("cannot create config file '%s'"), - configFile); + newfile); goto cleanup; } @@ -11053,14 +11059,21 @@ int virDomainSaveXML(const char *configDir, if (safewrite(fd, xml, towrite) < 0) { virReportSystemError(errno, _("cannot write config file '%s'"), - configFile); + newfile); goto cleanup; } if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("cannot save config file '%s'"), - configFile); + newfile); + goto cleanup; + } + + if (rename(newfile, configFile) < 0) { + virReportSystemError(errno, + _("cannot rename config file '%s' as '%s'"), + newfile, configFile); goto cleanup; } @@ -11068,6 +11081,10 @@ int virDomainSaveXML(const char *configDir, cleanup: VIR_FORCE_CLOSE(fd); + if (newfile) { + unlink(newfile); + VIR_FREE(newfile); + } VIR_FREE(configFile); return ret; } -- 1.7.7

On Thu, Oct 06, 2011 at 12:22:01PM +0200, Jiri Denemark wrote:
When saving domain XML (either config or status) we just overwrite old content of the file. In case something fails during that process (e.g. disk gets full) we lose both old and new content. This patch makes the process more robust by writing the new content into a separate file and only if that succeeds the original file is atomically replaced with the new one. --- src/conf/domain_conf.c | 25 +++++++++++++++++++++---- 1 files changed, 21 insertions(+), 4 deletions(-)
By doing this you've almost addressed this BZ... https://bugzilla.redhat.com/show_bug.cgi?id=489946 ...almost... Checkout slides 78 -> 119 of the presentation linked there, paying note to the horrible tales of woe about OS-X and fsync() :-( We probably want to add some kind of virFileSync(int fd) API call that does the neccessary black magic.
if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("cannot save config file '%s'"), - configFile); + newfile); + goto cleanup; + } + + if (rename(newfile, configFile) < 0) { + virReportSystemError(errno, + _("cannot rename config file '%s' as '%s'"), + newfile, configFile); goto cleanup; }
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange
-
Jiri Denemark