diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java index 106fe31a0f18..23025bf95bbe 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java @@ -212,19 +212,51 @@ private String validateBackupArgs(TakeBackupCommand command) { * Sum the per-disk size lines emitted by nasbackup.sh. Single-volume mode emits one * line containing just the byte count; multi-volume mode emits one line per disk * whose first whitespace-separated token is the byte count. + * + * Lines that do not start with a byte count are ignored rather than parsed. The + * script is not the only thing that can write to this stream: mount helpers and + * storage clients emit warnings of their own, and a single one of them used to + * abort the whole backup with a NumberFormatException. That failure is unusually + * expensive - the backup record stays in BackingUp with no timeout and no + * transition to Failed, and blocks every later restore of that instance - and it + * only ever hit the multi-volume branch, which is taken for a STOPPED instance, + * so it went unnoticed while running instances backed up normally. */ private long parseBackupSize(String stdout, List diskPaths) { long backupSize = 0L; if (CollectionUtils.isNullOrEmpty(diskPaths)) { - List outputLines = Arrays.asList(stdout.split("\n")); - if (!outputLines.isEmpty()) { - backupSize = Long.parseLong(outputLines.get(outputLines.size() - 1).trim()); + String[] outputLines = stdout.split("\n"); + for (int i = outputLines.length - 1; i >= 0; i--) { + Long size = parseSizeLine(outputLines[i]); + if (size != null) { + backupSize = size; + break; + } } } else { for (String line : stdout.split("\n")) { - backupSize = backupSize + Long.parseLong(line.split(" ")[0].trim()); + Long size = parseSizeLine(line); + if (size != null) { + backupSize = backupSize + size; + } } } return backupSize; } + + /** + * The byte count a size line starts with, or null when the line is not one. + */ + private Long parseSizeLine(String line) { + String token = line.trim().split("\\s+")[0]; + if (token.isEmpty()) { + return null; + } + try { + return Long.parseLong(token); + } catch (NumberFormatException e) { + logger.debug("Ignoring non-numeric line in backup script output: {}", line.trim()); + return null; + } + } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java new file mode 100644 index 000000000000..a564deaa5b77 --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java @@ -0,0 +1,86 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.hypervisor.kvm.resource.wrapper; + +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; + +public class LibvirtTakeBackupCommandWrapperTest { + + private final LibvirtTakeBackupCommandWrapper wrapper = new LibvirtTakeBackupCommandWrapper(); + + private long parseBackupSize(String stdout, List diskPaths) throws Exception { + Method method = LibvirtTakeBackupCommandWrapper.class + .getDeclaredMethod("parseBackupSize", String.class, List.class); + method.setAccessible(true); + return (long) method.invoke(wrapper, stdout, diskPaths); + } + + private static final List TWO_DISKS = Arrays.asList("/disk/a", "/disk/b"); + + @Test + public void testMultiVolumeSumsEveryDisk() throws Exception { + Assert.assertEquals(3145728L, parseBackupSize("1048576\n2097152", TWO_DISKS)); + } + + @Test + public void testSingleVolumeTakesTheLastLine() throws Exception { + Assert.assertEquals(2097152L, parseBackupSize("1048576\n2097152", null)); + } + + /** + * A mount helper or storage client warning on the same stream must not abort the + * backup. Before this was tolerated, one such line threw NumberFormatException and + * left the backup in BackingUp for ever, blocking every later restore of the + * instance. Only the multi-volume branch was affected, which is the branch taken + * for a STOPPED instance. + */ + @Test + public void testMultiVolumeIgnoresNonNumericLines() throws Exception { + String stdout = "2026-08-25T17:38:38.403+0000 -1 auth: unable to find a keyring on /etc/ceph/ceph.keyring\n" + + "1048576\n" + + "2097152"; + Assert.assertEquals(3145728L, parseBackupSize(stdout, TWO_DISKS)); + } + + @Test + public void testSingleVolumeSkipsTrailingNoise() throws Exception { + String stdout = "1048576\nmount: warning: something happened"; + Assert.assertEquals(1048576L, parseBackupSize(stdout, null)); + } + + @Test + public void testMultiVolumeKeepsTrailingTokensOnASizeLine() throws Exception { + // Size lines may carry the path after the byte count; only the first token counts. + Assert.assertEquals(3145728L, parseBackupSize("1048576 /disk/a\n2097152 /disk/b", TWO_DISKS)); + } + + @Test + public void testBlankLinesAreIgnored() throws Exception { + Assert.assertEquals(1048576L, parseBackupSize("\n1048576\n\n", TWO_DISKS)); + } + + @Test + public void testNoNumericLineYieldsZeroRatherThanThrowing() throws Exception { + Assert.assertEquals(0L, parseBackupSize("only warnings here\nand here", TWO_DISKS)); + Assert.assertEquals(0L, parseBackupSize("only warnings here", null)); + } +}