[PATCH] xend: open qemu-dm log files in append mode

[PATCH] xend: open qemu-dm log files in append mode

[Yosuke Iwamatsu]

[-- Attachment #1: Type: text/plain, Size: 333 bytes --]

qemu-dm log files would be easily overwritten if domains reboot several
times. With this patch, a single qemu-dm log file is used per domain and
log messages will be continuously appended to it.

Signed-off-by: Yosuke Iwamatsu <y-iwamatsu@ab.jp.nec.com>

Regards,
-----------------------
Yosuke Iwamatsu
        NEC Corporation






[-- Attachment #2: qemu-dm-log-append.patch --]
[-- Type: all/allfiles, Size: 974 bytes --]

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] xend: open qemu-dm log files in append mode

[Ian Jackson]

Yosuke Iwamatsu writes ("[Xen-devel] [PATCH] xend: open qemu-dm log files in append mode"):
> qemu-dm log files would be easily overwritten if domains reboot several
> times. With this patch, a single qemu-dm log file is used per domain and
> log messages will be continuously appended to it.

Surely it would be better to rotate the logs rather than just
appending.  Having each run in a separate file makes finding the
relevant log entries much easier.

Ian.

Re: [PATCH] xend: open qemu-dm log files in append mode

[Daniel P. Berrange]

On Thu, Jun 26, 2008 at 03:47:43PM +0100, Ian Jackson wrote:
> Yosuke Iwamatsu writes ("[Xen-devel] [PATCH] xend: open qemu-dm log files in append mode"):
> > qemu-dm log files would be easily overwritten if domains reboot several
> > times. With this patch, a single qemu-dm log file is used per domain and
> > log messages will be continuously appended to it.
> 
> Surely it would be better to rotate the logs rather than just
> appending.  Having each run in a separate file makes finding the
> relevant log entries much easier.

XenD already does rotate the qemu logs unless the code I added todo 
that has since been removed, so if the domain reboots we should keep
at least one historic log

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [PATCH] xend: open qemu-dm log files in append mode

[Ian Jackson]

Daniel P. Berrange writes ("Re: [Xen-devel] [PATCH] xend: open qemu-dm log files in append mode"):
> XenD already does rotate the qemu logs unless the code I added todo 
> that has since been removed, so if the domain reboots we should keep
> at least one historic log

It only keeps one.  I think Yuji Shimada would like more than one but
that would be a straightforward change to make to the code.

Ian.

Re: [PATCH] xend: open qemu-dm log files in append mode

[Yosuke Iwamatsu]

[-- Attachment #1: Type: text/plain, Size: 804 bytes --]

Ian Jackson wrote:
> Daniel P. Berrange writes ("Re: [Xen-devel] [PATCH] xend: open qemu-dm log files in append mode"):
>> XenD already does rotate the qemu logs unless the code I added todo 
>> that has since been removed, so if the domain reboots we should keep
>> at least one historic log
> 
> It only keeps one.  I think Yuji Shimada would like more than one but
> that would be a straightforward change to make to the code.
> 
> Ian.

So the attached patch improves the qemu-log rotation in xend to keep
maximum of 20 backup logfiles per domain. I still prefer the previous
patch that will avoid information loss when, for example, a guest
repeats cursh and reboot. But this patch might be at least a little bit
better than the current one-historic-backup solution. Any comment?

Thanks,
   Yosuke

[-- Attachment #2: qemu-log-rotate.patch --]
[-- Type: all/allfiles, Size: 1171 bytes --]

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] xend: open qemu-dm log files in append mode

[Ian Jackson]

Yosuke Iwamatsu writes ("Re: [Xen-devel] [PATCH] xend: open qemu-dm log files in append mode"):
> So the attached patch improves the qemu-log rotation in xend to keep
> maximum of 20 backup logfiles per domain. I still prefer the previous
> patch that will avoid information loss when, for example, a guest
> repeats cursh and reboot. But this patch might be at least a little bit
> better than the current one-historic-backup solution. Any comment?

I think this patch is much better.

It would be good if the maximum logfiles to keep would be
configurable; making it part of the ordinary xend configuration would
do.

Also, one minor stylistic point.  This idiom
    if os.path.exists(self.logfile + ".%d" % nr_max_log_rotation):
is rather unusual, although not actually wrong.  It combines string
concatenation and %-formatting.  Surely one of
    "%s.%d" % (self.logfile, nr_max_log_rotation)
    self.logfile + '.' + `nr_max_log_rotation`
would be better ?  (I would have used the former.)

Ian.

Re: [PATCH] xend: open qemu-dm log files in append mode

[Yosuke Iwamatsu]

[-- Attachment #1: Type: text/plain, Size: 944 bytes --]

Ian Jackson wrote:
> It would be good if the maximum logfiles to keep would be
> configurable; making it part of the ordinary xend configuration would
> do.
> 
> Also, one minor stylistic point.  This idiom
>     if os.path.exists(self.logfile + ".%d" % nr_max_log_rotation):
> is rather unusual, although not actually wrong.  It combines string
> concatenation and %-formatting.  Surely one of
>     "%s.%d" % (self.logfile, nr_max_log_rotation)
>     self.logfile + '.' + `nr_max_log_rotation`
> would be better ?  (I would have used the former.)

Done in qemu-log-rotate-config.patch.
I've set the logrotate count default to 10. It is small enough not to
annoy users by filling xen log directory with many files, I think.

I also created a patch (qemu-log-append-config.patch) which opens
qemu-dm logfiles in append mode if the logrotate count is 0. If this
makes sense, please apply it after qemu-log-rotate-config.patch.

Thanks,
--Yosuke

[-- Attachment #2: qemu-log-append-config.patch --]
[-- Type: all/allfiles, Size: 1253 bytes --]

[-- Attachment #3: qemu-log-rotate-config.patch --]
[-- Type: all/allfiles, Size: 2721 bytes --]

[-- Attachment #4: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] xend: open qemu-dm log files in append mode

[Ian Jackson]

Yosuke Iwamatsu writes ("Re: [Xen-devel] [PATCH] xend: open qemu-dm log files in append mode"):
> Done in qemu-log-rotate-config.patch.
> I've set the logrotate count default to 10. It is small enough not to
> annoy users by filling xen log directory with many files, I think.
> 
> I also created a patch (qemu-log-append-config.patch) which opens
> qemu-dm logfiles in append mode if the logrotate count is 0. If this
> makes sense, please apply it after qemu-log-rotate-config.patch.

That's great.  And, after a quick review, your patch looks sane to me.
I assume you've tested both cases.

You forgot to add a Signed-Off-By.

Ian.

Re: [PATCH] xend: open qemu-dm log files in append mode

[Yosuke Iwamatsu]

Ian Jackson wrote:
> Yosuke Iwamatsu writes ("Re: [Xen-devel] [PATCH] xend: open qemu-dm log files in append mode"):
>> Done in qemu-log-rotate-config.patch.
>> I've set the logrotate count default to 10. It is small enough not to
>> annoy users by filling xen log directory with many files, I think.
>>
>> I also created a patch (qemu-log-append-config.patch) which opens
>> qemu-dm logfiles in append mode if the logrotate count is 0. If this
>> makes sense, please apply it after qemu-log-rotate-config.patch.
> 
> That's great.  And, after a quick review, your patch looks sane to me.
> I assume you've tested both cases.

Year, tested and it looked working well.

> You forgot to add a Signed-Off-By.

Signed-off-bys are written inside the attachments themselves, but anyway
here is confirmation:

Signed-off-by: Yosuke Iwamatsu <y-iwamatsu@ab.jp.nec.com>


--Yosuke

Re: [PATCH] xend: open qemu-dm log files in append mode

[Ian Jackson]

Yosuke Iwamatsu writes ("Re: [Xen-devel] [PATCH] xend: open qemu-dm log files in append mode"):
> Signed-off-by: Yosuke Iwamatsu <y-iwamatsu@ab.jp.nec.com>

Acked-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Ian.