From 2d9f26d758c7761be816924ca8b3421dd9e8fb45 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Wed, 14 Apr 2021 11:43:48 +0200 Subject: [vm/DiskImage] Remove File constructor, make sure open file doesn't leak --- src/main/java/org/openslx/vm/disk/DiskImage.java | 70 +++++++++++----------- .../java/org/openslx/vm/disk/DiskImageQcow2.java | 18 +----- .../java/org/openslx/vm/disk/DiskImageVdi.java | 18 +----- .../java/org/openslx/vm/disk/DiskImageVmdk.java | 21 +------ 4 files changed, 38 insertions(+), 89 deletions(-) (limited to 'src') diff --git a/src/main/java/org/openslx/vm/disk/DiskImage.java b/src/main/java/org/openslx/vm/disk/DiskImage.java index 38964f4..5706db3 100644 --- a/src/main/java/org/openslx/vm/disk/DiskImage.java +++ b/src/main/java/org/openslx/vm/disk/DiskImage.java @@ -1,5 +1,6 @@ package org.openslx.vm.disk; +import java.io.Closeable; import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; @@ -9,6 +10,7 @@ import java.util.function.Predicate; import org.openslx.bwlp.thrift.iface.Virtualizer; import org.openslx.thrifthelper.TConst; +import org.openslx.util.Util; /** * Disk image for virtual machines. @@ -19,31 +21,13 @@ import org.openslx.thrifthelper.TConst; * @author Manuel Bentele * @version 1.0 */ -public abstract class DiskImage +public abstract class DiskImage implements Closeable { /** * Stores the image file of the disk. */ private RandomAccessFile diskImage = null; - /** - * Creates a new disk image from an existing image file with a known disk image format. - * - * @param diskImage file to a disk storing the image content. - * - * @throws FileNotFoundException cannot find specified disk image file. - * @throws IOException cannot access the content of the disk image file. - * - * @implNote Do not use this constructor to create a new disk image from an image file with - * unknown disk image format. Instead, use the factory method - * {@link #newInstance(File)} to probe unknown disk image files before creation. - */ - public DiskImage( File diskImage ) throws FileNotFoundException, IOException - { - final RandomAccessFile diskFile = new RandomAccessFile( diskImage, "r" ); - this.diskImage = diskFile; - } - /** * Creates a new disk image from an existing image file with a known disk image format. * @@ -53,7 +37,7 @@ public abstract class DiskImage * unknown disk image format. Instead, use the factory method * {@link #newInstance(File)} to probe unknown disk image files before creation. */ - public DiskImage( RandomAccessFile diskImage ) + protected DiskImage( RandomAccessFile diskImage ) { this.diskImage = diskImage; } @@ -124,30 +108,46 @@ public abstract class DiskImage /** * Creates a new disk image from an existing image file with an unknown disk image format. * - * @param diskImage file to a disk storing the image content. + * @param diskImagePath file to a disk storing the image content. * @return concrete disk image instance. * * @throws FileNotFoundException cannot find specified disk image file. * @throws IOException cannot access the content of the disk image file. * @throws DiskImageException disk image file has an invalid and unknown disk image format. */ - public static DiskImage newInstance( File diskImage ) throws FileNotFoundException, IOException, DiskImageException + public static DiskImage newInstance( File diskImagePath ) throws FileNotFoundException, IOException, DiskImageException { - final RandomAccessFile diskFile = new RandomAccessFile( diskImage, "r" ); - final DiskImage diskImageInstance; - - if ( DiskImageQcow2.probe( diskFile ) ) { - diskImageInstance = new DiskImageQcow2( diskFile ); - } else if ( DiskImageVdi.probe( diskFile ) ) { - diskImageInstance = new DiskImageVdi( diskFile ); - } else if ( DiskImageVmdk.probe( diskFile ) ) { - diskImageInstance = new DiskImageVmdk( diskFile ); - } else { - final String errorMsg = new String( "File '" + diskImage.getAbsolutePath() + "' is not a valid disk image!" ); - throw new DiskImageException( errorMsg ); + // Make sure this doesn't escape the scope, in case instantiation fails - we can't know when the GC + // would come along and close this file, which is problematic on Windows (blocking rename/delete) + final RandomAccessFile fileHandle = new RandomAccessFile( diskImagePath, "r" ); + + try { + if ( DiskImageQcow2.probe( fileHandle ) ) { + return new DiskImageQcow2( fileHandle ); + } else if ( DiskImageVdi.probe( fileHandle ) ) { + return new DiskImageVdi( fileHandle ); + } else if ( DiskImageVmdk.probe( fileHandle ) ) { + return new DiskImageVmdk( fileHandle ); + } + } catch ( Exception e ) { + Util.safeClose( fileHandle ); + throw e; } + Util.safeClose( fileHandle ); + final String errorMsg = new String( "File '" + diskImagePath.getAbsolutePath() + "' is not a valid disk image!" ); + throw new DiskImageException( errorMsg ); + } - return diskImageInstance; + @Override + public void close() throws IOException + { + Util.safeClose( diskImage ); + } + + @Override + protected void finalize() throws Throwable + { + close(); } /** diff --git a/src/main/java/org/openslx/vm/disk/DiskImageQcow2.java b/src/main/java/org/openslx/vm/disk/DiskImageQcow2.java index 5f72a00..a9826e4 100644 --- a/src/main/java/org/openslx/vm/disk/DiskImageQcow2.java +++ b/src/main/java/org/openslx/vm/disk/DiskImageQcow2.java @@ -1,8 +1,5 @@ package org.openslx.vm.disk; -import java.io.File; -import java.io.FileNotFoundException; -import java.io.IOException; import java.io.RandomAccessFile; /** @@ -46,25 +43,12 @@ public class DiskImageQcow2 extends DiskImage */ private static final int QCOW2_MAGIC = 0x514649fb; - /** - * Creates a new QCOW2 disk image from an existing QCOW2 image file. - * - * @param diskImage file to a QCOW2 disk storing the image content. - * - * @throws FileNotFoundException cannot find specified QCOW2 disk image file. - * @throws IOException cannot access the content of the QCOW2 disk image file. - */ - public DiskImageQcow2( File disk ) throws FileNotFoundException, IOException - { - super( disk ); - } - /** * Creates a new QCOW2 disk image from an existing QCOW2 image file. * * @param diskImage file to a QCOW2 disk storing the image content. */ - public DiskImageQcow2( RandomAccessFile disk ) + DiskImageQcow2( RandomAccessFile disk ) { super( disk ); } diff --git a/src/main/java/org/openslx/vm/disk/DiskImageVdi.java b/src/main/java/org/openslx/vm/disk/DiskImageVdi.java index 1c34c1d..9be49d8 100644 --- a/src/main/java/org/openslx/vm/disk/DiskImageVdi.java +++ b/src/main/java/org/openslx/vm/disk/DiskImageVdi.java @@ -1,8 +1,5 @@ package org.openslx.vm.disk; -import java.io.File; -import java.io.FileNotFoundException; -import java.io.IOException; import java.io.RandomAccessFile; /** @@ -18,25 +15,12 @@ public class DiskImageVdi extends DiskImage */ private static final int VDI_MAGIC = 0x7f10dabe; - /** - * Creates a new VDI disk image from an existing VDI image file. - * - * @param diskImage file to a VDI disk storing the image content. - * - * @throws FileNotFoundException cannot find specified VDI disk image file. - * @throws IOException cannot access the content of the VDI disk image file. - */ - public DiskImageVdi( File diskImage ) throws FileNotFoundException, IOException - { - super( diskImage ); - } - /** * Creates a new VDI disk image from an existing VDI image file. * * @param diskImage file to a VDI disk storing the image content. */ - public DiskImageVdi( RandomAccessFile diskImage ) + DiskImageVdi( RandomAccessFile diskImage ) { super( diskImage ); } diff --git a/src/main/java/org/openslx/vm/disk/DiskImageVmdk.java b/src/main/java/org/openslx/vm/disk/DiskImageVmdk.java index c9bfdbf..58314fc 100644 --- a/src/main/java/org/openslx/vm/disk/DiskImageVmdk.java +++ b/src/main/java/org/openslx/vm/disk/DiskImageVmdk.java @@ -1,8 +1,5 @@ package org.openslx.vm.disk; -import java.io.File; -import java.io.FileNotFoundException; -import java.io.IOException; import java.io.RandomAccessFile; import org.openslx.util.Util; @@ -43,24 +40,8 @@ public class DiskImageVmdk extends DiskImage * @param diskImage file to a VMDK disk storing the image content. * * @throws DiskImageException parsing of the VMDK's embedded descriptor file failed. - * @throws FileNotFoundException cannot find specified VMDK disk image file. - * @throws IOException cannot access the content of the VMDK disk image file. */ - public DiskImageVmdk( File diskImage ) throws DiskImageException, FileNotFoundException, IOException - { - super( diskImage ); - - this.vmdkConfig = this.parseVmdkConfig(); - } - - /** - * Creates a new VMDK disk image from an existing VMDK image file. - * - * @param diskImage file to a VMDK disk storing the image content. - * - * @throws DiskImageException parsing of the VMDK's embedded descriptor file failed. - */ - public DiskImageVmdk( RandomAccessFile diskImage ) throws DiskImageException + DiskImageVmdk( RandomAccessFile diskImage ) throws DiskImageException { super( diskImage ); -- cgit v1.2.3-55-g7522