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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Oct 24, 2011 at 16:51:49 -0600, Eric Blake wrote:
> On 10/19/2011 11:26 AM, 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
> 
> >
> > -    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) {
> 
> Same question about O_TRUNC vs. O_EXCL.
> 
> 
> > @@ -11121,6 +11134,10 @@ int virDomainSaveXML(const char *configDir,
> >    cleanup:
> >       VIR_FORCE_CLOSE(fd);
> >
> > +    if (newfile) {
> > +        unlink(newfile);
> 
> Same concern about blind unlink().

As mentioned by Daniel, these hunks shouldn't be here. In fact, this patch
shouldn't touch src/conf/domain_conf.c at all and the changes to that file
should be squashed into the next patch (which effectively deletes all the
changes). Sorry for the mess.

Jirka

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]