[libvirt] [PATCH 3/4] Introduce virXMLSaveFile as a wrapper for virFileRewrite

[Daniel P. Berrange]

On Wed, Oct 19, 2011 at 07:26:26PM +0200, Jiri Denemark wrote:
> Every time we write XML into a file we call virEmitXMLWarning to write a
> warning that the file is automatically generated. virXMLSaveFile
> simplifies this into a single step and makes rewriting existing XML file
> safe by using virFileRewrite internally.
> ---
>  src/conf/domain_conf.c   |   25 +++++++++++++++++++++----
>  src/libvirt_private.syms |    1 +
>  src/util/util.c          |    4 +++-
>  src/util/xml.c           |   36 ++++++++++++++++++++++++++++++++++++
>  src/util/xml.h           |    5 +++++
>  5 files changed, 66 insertions(+), 5 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 5959593..6656e8b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -11078,6 +11078,7 @@ int virDomainSaveXML(const char *configDir,
>                       const char *xml)
>  {
>      char *configFile = NULL;
> +    char *newfile = NULL;
>      int fd = -1, ret = -1;
>      size_t towrite;
>  
> @@ -11091,12 +11092,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;
>      }
>  
> @@ -11106,14 +11112,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;
>      }
>  
> @@ -11121,6 +11134,10 @@ int virDomainSaveXML(const char *configDir,
>   cleanup:
>      VIR_FORCE_CLOSE(fd);
>  
> +    if (newfile) {
> +        unlink(newfile);
> +        VIR_FREE(newfile);
> +    }
>      VIR_FREE(configFile);
>      return ret;
>  }

Since this hunk is obliterated by the next patch, I'm thinking you
perhaps forgot to squash this into patch 4 before posting ?


> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 1c7910b..b05bf61 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1292,6 +1292,7 @@ virKeycodeValueTranslate;
>  # xml.h
>  virXMLParseHelper;
>  virXMLPropString;
> +virXMLSaveFile;
>  virXPathBoolean;
>  virXPathInt;
>  virXPathLong;

> diff --git a/src/util/xml.c b/src/util/xml.c
> index b0942da..1ff728e 100644
> --- a/src/util/xml.c
> +++ b/src/util/xml.c
> @@ -16,12 +16,14 @@
>  #include <stdarg.h>
>  #include <limits.h>
>  #include <math.h>               /* for isnan() */
> +#include <sys/stat.h>
>  
>  #include "virterror_internal.h"
>  #include "xml.h"
>  #include "buf.h"
>  #include "util.h"
>  #include "memory.h"
> +#include "virfile.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_XML
>  
> @@ -797,3 +799,37 @@ error:
>      }
>      goto cleanup;
>  }
> +
> +
> +struct rewrite_data {

s/rewrite_data/virXMLRewritFileData/

> +    const char *warnName;
> +    const char *warnCommand;
> +    const char *xml;
> +};
> +
> +static int
> +virXMLRewriteFile(int fd, void *opaque)
> +{
> +    struct rewrite_data *data = opaque;
> +
> +    if (data->warnName && data->warnCommand) {
> +        if (virEmitXMLWarning(fd, data->warnName, data->warnCommand) < 0)
> +            return -1;
> +    }
> +
> +    if (safewrite(fd, data->xml, strlen(data->xml)) < 0)
> +        return -1;
> +
> +    return 0;
> +}
> +
> +int
> +virXMLSaveFile(const char *path,
> +               const char *warnName,
> +               const char *warnCommand,
> +               const char *xml)
> +{
> +    struct rewrite_data data = { warnName, warnCommand, xml };
> +
> +    return virFileRewrite(path, S_IRUSR | S_IWUSR, virXMLRewriteFile, &data);
> +}
> diff --git a/src/util/xml.h b/src/util/xml.h
> index d30e066..c492063 100644
> --- a/src/util/xml.h
> +++ b/src/util/xml.h
> @@ -138,4 +138,9 @@ xmlDocPtr      virXMLParseHelper(int domcode,
>  # define virXMLParseFileCtxt(filename, pctxt)                           \
>      virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, pctxt)
>  
> +int virXMLSaveFile(const char *path,
> +                   const char *warnName,
> +                   const char *warnCommand,
> +                   const char *xml);
> +
>  #endif                          /* __VIR_XML_H__ */

ACK with those small changes


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 :|