From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <s.reiter@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id D54F16A47A
 for <pbs-devel@lists.proxmox.com>; Tue, 16 Feb 2021 18:07:40 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 2DB0119057
 for <pbs-devel@lists.proxmox.com>; Tue, 16 Feb 2021 18:07:39 +0100 (CET)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [212.186.127.180])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id B7E8018E20
 for <pbs-devel@lists.proxmox.com>; Tue, 16 Feb 2021 18:07:32 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 82B78461DB
 for <pbs-devel@lists.proxmox.com>; Tue, 16 Feb 2021 18:07:32 +0100 (CET)
From: Stefan Reiter <s.reiter@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Tue, 16 Feb 2021 18:07:07 +0100
Message-Id: <20210216170710.31767-20-s.reiter@proxmox.com>
X-Mailer: git-send-email 2.20.1
In-Reply-To: <20210216170710.31767-1-s.reiter@proxmox.com>
References: <20210216170710.31767-1-s.reiter@proxmox.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.029 Adjusted score from AWL reputation of From: address
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 RCVD_IN_DNSWL_MED        -2.3 Sender listed at https://www.dnswl.org/,
 medium trust
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: [pbs-devel] [PATCH proxmox-backup 19/22] file-restore: improve
 logging of VM with logrotate
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Tue, 16 Feb 2021 17:07:40 -0000

Keep the log files of the last 16 VM starts (log output generated by the
daemon binary via QEMU's serial-to-logfile interface). Also put them
into a seperate /var/log/proxmox-backup/file-restore directory.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/bin/proxmox_file_restore/block_driver.rs  | 32 ++++++++++++++-
 .../proxmox_file_restore/block_driver_qemu.rs | 39 +++++++++++++++----
 2 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/src/bin/proxmox_file_restore/block_driver.rs b/src/bin/proxmox_file_restore/block_driver.rs
index 0ba67f34..f2d5b00e 100644
--- a/src/bin/proxmox_file_restore/block_driver.rs
+++ b/src/bin/proxmox_file_restore/block_driver.rs
@@ -1,5 +1,5 @@
 //! Abstraction layer over different methods of accessing a block backup
-use anyhow::{bail, Error};
+use anyhow::{bail, format_err, Error};
 use serde::{Deserialize, Serialize};
 use serde_json::{json, Value};
 
@@ -8,10 +8,12 @@ use std::future::Future;
 use std::hash::BuildHasher;
 use std::pin::Pin;
 
-use proxmox_backup::backup::{BackupDir, BackupManifest};
+use proxmox_backup::backup::{backup_user, BackupDir, BackupManifest};
+use proxmox_backup::buildcfg;
 use proxmox_backup::client::BackupRepository;
 
 use proxmox::api::{api, cli::*};
+use proxmox::tools::fs::{create_path, CreateOptions};
 
 use super::block_driver_qemu::QemuBlockDriver;
 
@@ -155,3 +157,29 @@ pub fn complete_block_driver_ids<S: BuildHasher>(
         .flatten()
         .collect()
 }
+
+/// Create the /file-restore logging subdirectory with root ownership
+pub fn create_restore_log_dir() -> Result<String, Error> {
+    let logpath = format!("{}/file-restore", buildcfg::PROXMOX_BACKUP_LOG_DIR);
+
+    proxmox::try_block!({
+        let backup_user = backup_user()?;
+        let opts = CreateOptions::new()
+            .owner(backup_user.uid)
+            .group(backup_user.gid);
+
+        let opts_root = CreateOptions::new()
+            .owner(nix::unistd::ROOT)
+            .group(nix::unistd::Gid::from_raw(0));
+
+        create_path(buildcfg::PROXMOX_BACKUP_LOG_DIR, None, Some(opts))?;
+
+        // the QEMU logs may contain information from snapshots users should not have access to, so
+        // restrict to root (just like running the restore command itself)
+        create_path(&logpath, None, Some(opts_root))?;
+        Ok(())
+    })
+    .map_err(|err: Error| format_err!("unable to create file-restore log dir - {}", err))?;
+
+    Ok(logpath)
+}
diff --git a/src/bin/proxmox_file_restore/block_driver_qemu.rs b/src/bin/proxmox_file_restore/block_driver_qemu.rs
index 8bbea962..d406d523 100644
--- a/src/bin/proxmox_file_restore/block_driver_qemu.rs
+++ b/src/bin/proxmox_file_restore/block_driver_qemu.rs
@@ -211,16 +211,39 @@ async fn start_vm(
     nix::unistd::unlink(&pid_path)?;
     tools::fd_change_cloexec(pid_fd.0, false)?;
 
+    let logpath = create_restore_log_dir()?;
+    let logfile = &format!("{}/qemu.log", logpath);
+    let mut logrotate = tools::logrotate::LogRotate::new(logfile, false)
+        .ok_or_else(|| format_err!("could not get QEMU log file names"))?;
+
+    if let Err(err) = logrotate.do_rotate(CreateOptions::default(), Some(16)) {
+        eprintln!("warning: logrotate for QEMU log file failed - {}", err);
+    }
+
+    // preface log file with information about the VM
+    let mut logfd = OpenOptions::new()
+        .append(true)
+        .create_new(true)
+        .open(logfile)?;
+    writeln!(
+        logfd,
+        "[{}] file restore VM log for '{}'",
+        {
+            let now = proxmox::tools::time::epoch_i64();
+            proxmox::tools::time::epoch_to_rfc3339(now)?
+        },
+        tools::systemd::unescape_unit(name).unwrap_or_else(|_| "<invalid name>".to_owned())
+    )?;
+    tools::fd_change_cloexec(logfd.as_raw_fd(), false)?;
+
     let base_args = [
-        "-serial",
+        "-chardev",
         &format!(
-            "file:{}/file_restore_vm_{}.log",
-            buildcfg::PROXMOX_BACKUP_LOG_DIR,
-            {
-                let now = proxmox::tools::time::epoch_i64();
-                proxmox::tools::time::epoch_to_rfc3339(now)?
-            },
+            "file,id=log,path=/dev/null,logfile=/dev/fd/{},logappend=on",
+            logfd.as_raw_fd()
         ),
+        "-serial",
+        "chardev:log",
         "-vnc",
         "none",
         "-enable-kvm",
@@ -296,7 +319,7 @@ async fn start_vm(
                 cid += 1;
             } else {
                 eprint!("{}", out);
-                bail!("Starting VM failed. See QEMU output above for more information.");
+                bail!("Starting VM failed. See output above for more information.");
             }
         }
     }
-- 
2.20.1