Logfiles should always be opened for append.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
diff -r a9c67c2daf4b tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Mon Nov 28 11:57:23 2011 +0000
+++ b/tools/libxl/libxl_dm.c Mon Nov 28 12:25:52 2011 +0000
@@ -830,7 +830,7 @@ int libxl__create_device_model(libxl__gc
libxl__xs_write(gc, XBT_NULL, libxl__sprintf(gc, "%s/disable_pf",
path), "%d", !info->xen_platform_pci);
libxl_create_logfile(ctx, libxl__sprintf(gc, "qemu-dm-%s",
info->dom_name), &logfile);
- logfile_w = open(logfile, O_WRONLY|O_CREAT, 0644);
+ logfile_w = open(logfile, O_WRONLY|O_CREAT|O_APPEND, 0644);
free(logfile);
null = open("/dev/null", O_RDONLY);
diff -r a9c67c2daf4b tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Mon Nov 28 11:57:23 2011 +0000
+++ b/tools/libxl/xl_cmdimpl.c Mon Nov 28 12:25:52 2011 +0000
@@ -1597,7 +1597,8 @@ start:
exit(-1);
}
- CHK_ERRNO(( logfile = open(fullname, O_WRONLY|O_CREAT, 0644) )<0);
+ CHK_ERRNO(( logfile = open(fullname, O_WRONLY|O_CREAT|O_APPEND,
+ 0644) )<0);
free(fullname);
free(name);
Ian Jackson
2011-Nov-28 12:30 UTC
[PATCH 2/2] qemu-dm: open char devices "file:..." with O_APPEND
The "file:..." character open method is used by serial and parallel
ports, to divert the output to a file (and these devices never produce
any input). This is like a logfile, and so should be opened for
append.
In qemu-xen-unstable, this is used only for the qemu stderr by libxl.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
diff --git a/qemu-char.c b/qemu-char.c
index 35e428d..324ed16 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -588,7 +588,7 @@ static CharDriverState *qemu_chr_open_file_out(const char
*file_out)
{
int fd_out;
- TFR(fd_out = open(file_out, O_WRONLY | O_TRUNC | O_CREAT | O_BINARY,
0666));
+ TFR(fd_out = open(file_out, O_WRONLY | O_TRUNC | O_CREAT | O_BINARY |
O_APPEND, 0666));
if (fd_out < 0)
return NULL;
return qemu_chr_open_fd(-1, fd_out);
On Mon, 2011-11-28 at 12:28 +0000, Ian Jackson wrote:> Logfiles should always be opened for append. > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>Both this and the 2/2 patch look sensible to me and are: Acked-by: Ian Campbell <ian.campbell@citrix.com>> diff -r a9c67c2daf4b tools/libxl/libxl_dm.c > --- a/tools/libxl/libxl_dm.c Mon Nov 28 11:57:23 2011 +0000 > +++ b/tools/libxl/libxl_dm.c Mon Nov 28 12:25:52 2011 +0000 > @@ -830,7 +830,7 @@ int libxl__create_device_model(libxl__gc > libxl__xs_write(gc, XBT_NULL, libxl__sprintf(gc, "%s/disable_pf", path), "%d", !info->xen_platform_pci); > > libxl_create_logfile(ctx, libxl__sprintf(gc, "qemu-dm-%s", info->dom_name), &logfile); > - logfile_w = open(logfile, O_WRONLY|O_CREAT, 0644); > + logfile_w = open(logfile, O_WRONLY|O_CREAT|O_APPEND, 0644); > free(logfile); > null = open("/dev/null", O_RDONLY); > > diff -r a9c67c2daf4b tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Mon Nov 28 11:57:23 2011 +0000 > +++ b/tools/libxl/xl_cmdimpl.c Mon Nov 28 12:25:52 2011 +0000 > @@ -1597,7 +1597,8 @@ start: > exit(-1); > } > > - CHK_ERRNO(( logfile = open(fullname, O_WRONLY|O_CREAT, 0644) )<0); > + CHK_ERRNO(( logfile = open(fullname, O_WRONLY|O_CREAT|O_APPEND, > + 0644) )<0);fullname here came from libxl_create_logfile so has been rotated such that the file will be empty. I don''t think O_APPEND is harmful or wrong given this though. Do we need to arrange for libxl_create_logfile to be called for some of these others though? Ian.> free(fullname); > free(name); > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel
Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/2] libxl: open logs with
O_APPEND"):> On Mon, 2011-11-28 at 12:28 +0000, Ian Jackson wrote:
> > Logfiles should always be opened for append.
> >
> > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
>
> Both this and the 2/2 patch look sensible to me and are:
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Thanks. I will apply then.
> > - CHK_ERRNO(( logfile = open(fullname, O_WRONLY|O_CREAT, 0644)
)<0);
> > + CHK_ERRNO(( logfile = open(fullname,
O_WRONLY|O_CREAT|O_APPEND,
> > + 0644) )<0);
>
> fullname here came from libxl_create_logfile so has been rotated such
> that the file will be empty. I don''t think O_APPEND is harmful or
wrong
> given this though.
O_APPEND sets a flag which applies throughout the future life of the
open-file, so that even if the file later grows for some reason,
writes will never overwrite existing data.
> Do we need to arrange for libxl_create_logfile to be called for some of
> these others though?
The one in libxl_create_device_model came from libxl_create_logfile
too.
Obviously the one in qemu can''t call libxl_create_logfile but in our
setup it''s fed /dev/stderr anyway.
Ian.
Ian Campbell
2011-Nov-28 17:57 UTC
Re: [PATCH 2/2] qemu-dm: open char devices "file:..." with O_APPEND
On Mon, 2011-11-28 at 12:30 +0000, Ian Jackson wrote:> The "file:..." character open method is used by serial and parallel > ports, to divert the output to a file (and these devices never produce > any input). This is like a logfile, and so should be opened for > append. > > In qemu-xen-unstable, this is used only for the qemu stderr by libxl.Do we need a similar patch for qemu upstream?> > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> > > diff --git a/qemu-char.c b/qemu-char.c > index 35e428d..324ed16 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -588,7 +588,7 @@ static CharDriverState *qemu_chr_open_file_out(const char *file_out) > { > int fd_out; > > - TFR(fd_out = open(file_out, O_WRONLY | O_TRUNC | O_CREAT | O_BINARY, 0666)); > + TFR(fd_out = open(file_out, O_WRONLY | O_TRUNC | O_CREAT | O_BINARY | O_APPEND, 0666)); > if (fd_out < 0) > return NULL; > return qemu_chr_open_fd(-1, fd_out); > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel