Skip to contentSkip to conversation

xend: open qemu-dm log files in append mode

PATCH In review Yosuke Iwamatsu 3 participants 10 messages Jun 26, 2008 – Jul 1, 2008

lore
Share & tools
lore

Yosuke Iwamatsu

author
#0 lore raw

PATCH 1/1 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.

Signed-off-by Yosuke Iwamatsu <[email protected]>

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

xend: Open the qemu-dm log file in append mode.

Signed-off-by Yosuke Iwamatsu <[email protected]>
tools/python/xen/xend/image.py +1 -5
diff -r 926a366ca82f tools/python/xen/xend/image.py
@@ -378,13 +378,9 @@ class ImageHandler:
         # keep track of pid and spawned options to kill it later
 
         self.logfile = "/var/log/xen/qemu-dm-%s.log" %  str(self.vm.info['name_label'])
-        if os.path.exists(self.logfile):
-            if os.path.exists(self.logfile + ".1"):
-                os.unlink(self.logfile + ".1")
-            os.rename(self.logfile, self.logfile + ".1")
 
         null = os.open("/dev/null", os.O_RDONLY)
-        logfd = os.open(self.logfile, os.O_WRONLY|os.O_CREAT|os.O_TRUNC|os.O_APPEND)
+        logfd = os.open(self.logfile, os.O_WRONLY|os.O_CREAT|os.O_APPEND)
         
         sys.stderr.flush()
         pid = os.fork()

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#1 lore raw

Yosuke Iwamatsu writes ("[Xen-devel] [PATCH] xend: open qemu-dm log files in append mode"):

Quoted 3 lines

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.

#2 lore raw
Quoted 8 lines · Ian Jackson

Yosuke Iwamatsu writes ("[Xen-devel] [PATCH] xend: open qemu-dm log files in append mode"):

Quoted 3 lines

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

Signature · 4 lines
|: 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 :|
#3 lore raw

Daniel P. Berrange writes ("Re: [Xen-devel] [PATCH] xend: open qemu-dm log files in append mode"):

Quoted 3 lines

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.

author
#4 lore raw
Quoted 8 lines · Ian Jackson

Daniel P. Berrange writes ("Re: [Xen-devel] [PATCH] xend: open qemu-dm log files in append mode"):

Quoted 2 lines

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

xend: improve the rotation of qemu-dm logfiles.

Signed-off-by Yosuke Iwamatsu <[email protected]>
tools/python/xen/xend/image.py +9 -2
diff -r 926a366ca82f tools/python/xen/xend/image.py
@@ -378,9 +378,16 @@ class ImageHandler:
         # keep track of pid and spawned options to kill it later
 
         self.logfile = "/var/log/xen/qemu-dm-%s.log" %  str(self.vm.info['name_label'])
+
+        # rotate log
+        nr_max_log_rotation = 20
+        if os.path.exists(self.logfile + ".%d" % nr_max_log_rotation):
+            os.unlink(self.logfile + ".%d" % nr_max_log_rotation)
+        for n in range(nr_max_log_rotation - 1, 0, -1):
+            if os.path.exists(self.logfile + ".%d" % n):
+                os.rename(self.logfile + ".%d" % n,
+                          self.logfile + ".%d" % (n + 1))
         if os.path.exists(self.logfile):
-            if os.path.exists(self.logfile + ".1"):
-                os.unlink(self.logfile + ".1")
             os.rename(self.logfile, self.logfile + ".1")
 
         null = os.open("/dev/null", os.O_RDONLY)

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#5 lore raw

Yosuke Iwamatsu writes ("Re: [Xen-devel] [PATCH] xend: open qemu-dm log files in append mode"):

Quoted 5 lines

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.

author
#6 lore raw
Quoted 11 lines · Ian Jackson

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

xend: open qemu-dm logfile in append mode if log rotation is disabled by config.

Signed-off-by Yosuke Iwamatsu <[email protected]>
tools/python/xen/xend/image.py +3 -1
diff -r 7e4f23538380 tools/python/xen/xend/image.py
@@ -380,8 +380,10 @@ class ImageHandler:
         self.logfile = "/var/log/xen/qemu-dm-%s.log" %  str(self.vm.info['name_label'])
 
         # rotate log
+        logfile_mode = os.O_WRONLY|os.O_CREAT|os.O_APPEND
         logrotate_count = XendOptions.instance().get_qemu_dm_logrotate_count()
         if logrotate_count > 0:
+            logfile_mode |= os.O_TRUNC
             if os.path.exists("%s.%d" % (self.logfile, logrotate_count)):
                 os.unlink("%s.%d" % (self.logfile, logrotate_count))
             for n in range(logrotate_count - 1, 0, -1):

@@ -392,7 +394,7 @@ class ImageHandler:
                 os.rename(self.logfile, self.logfile + ".1")
 
         null = os.open("/dev/null", os.O_RDONLY)
-        logfd = os.open(self.logfile, os.O_WRONLY|os.O_CREAT|os.O_TRUNC|os.O_APPEND)
+        logfd = os.open(self.logfile, logfile_mode)
         
         sys.stderr.flush()
         pid = os.fork()

xend: improve the rotation of qemu-dm logfiles.

Signed-off-by Yosuke Iwamatsu <[email protected]>
tools/examples/xend-config.sxp +3 -0
diff -r 10e79ad54c91 tools/examples/xend-config.sxp
@@ -242,3 +242,6 @@
 
 # Script to run when the label of a resource has changed.
 #(resource-label-change-script '')
+
+# Rotation count of qemu-dm log file.
+#(qemu-dm-logrotate-count 10)
tools/python/xen/xend/XendOptions.py +7 -0
diff -r 10e79ad54c91 tools/python/xen/xend/XendOptions.py
@@ -132,6 +132,9 @@ class XendOptions:
     """Default script to configure a backend network interface"""
     vif_script = osdep.vif_script
 
+    """Default rotation count of qemu-dm log file."""
+    qemu_dm_logrotate_count = 10
+
     def __init__(self):
         self.configure()
 

@@ -350,6 +353,10 @@ class XendOptions:
 
     def get_vnc_x509_verify(self):
         return self.get_config_string('vnc-x509-verify', self.xend_vnc_x509_verify)
+
+    def get_qemu_dm_logrotate_count(self):
+        return self.get_config_int("qemu-dm-logrotate-count",
+                                   self.qemu_dm_logrotate_count)
 
 
 class XendOptionsFile(XendOptions):
tools/python/xen/xend/image.py +12 -4
diff -r 10e79ad54c91 tools/python/xen/xend/image.py
@@ -378,10 +378,18 @@ class ImageHandler:
         # keep track of pid and spawned options to kill it later
 
         self.logfile = "/var/log/xen/qemu-dm-%s.log" %  str(self.vm.info['name_label'])
-        if os.path.exists(self.logfile):
-            if os.path.exists(self.logfile + ".1"):
-                os.unlink(self.logfile + ".1")
-            os.rename(self.logfile, self.logfile + ".1")
+
+        # rotate log
+        logrotate_count = XendOptions.instance().get_qemu_dm_logrotate_count()
+        if logrotate_count > 0:
+            if os.path.exists("%s.%d" % (self.logfile, logrotate_count)):
+                os.unlink("%s.%d" % (self.logfile, logrotate_count))
+            for n in range(logrotate_count - 1, 0, -1):
+                if os.path.exists("%s.%d" % (self.logfile, n)):
+                    os.rename("%s.%d" % (self.logfile, n),
+                              "%s.%d" % (self.logfile, (n + 1)))
+            if os.path.exists(self.logfile):
+                os.rename(self.logfile, self.logfile + ".1")
 
         null = os.open("/dev/null", os.O_RDONLY)
         logfd = os.open(self.logfile, os.O_WRONLY|os.O_CREAT|os.O_TRUNC|os.O_APPEND)

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#7 lore raw

Yosuke Iwamatsu writes ("Re: [Xen-devel] [PATCH] xend: open qemu-dm log files in append mode"):

Quoted 7 lines

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.

author
#8 lore raw
Quoted 11 lines · Ian Jackson

Yosuke Iwamatsu writes ("Re: [Xen-devel] [PATCH] xend: open qemu-dm log files in append mode"):

Quoted 7 lines

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.

Quoted 1 line

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 <[email protected]>

--Yosuke