From 2d29afa6ea43501ecb328a2abe0dee401b25ab30 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Wed, 10 May 2023 14:09:16 +0200 Subject: [vbox] Implement .transformPrivacy(), remove placeholder code The placeholders aren't being used anymore by the client-side scripts, so we might as well just not add them. The privacy transformation was entirely missing, so add those. We also now filter unused HDDs from the XML's MediaRegistry. --- .../VirtualizationConfigurationVirtualBox.java | 54 ++++---- ...alizationConfigurationVirtualboxFileFormat.java | 153 ++++++++++----------- 2 files changed, 105 insertions(+), 102 deletions(-) diff --git a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualBox.java b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualBox.java index 39fbec6..4566e92 100644 --- a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualBox.java +++ b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualBox.java @@ -16,12 +16,12 @@ import org.openslx.bwlp.thrift.iface.OperatingSystem; import org.openslx.thrifthelper.TConst; import org.openslx.util.Util; import org.openslx.virtualization.Version; -import org.openslx.virtualization.configuration.VirtualizationConfigurationVirtualboxFileFormat.PlaceHolder; -import org.openslx.virtualization.hardware.VirtOptionValue; +import org.openslx.virtualization.configuration.VirtualizationConfigurationVirtualboxFileFormat.MatchMode; import org.openslx.virtualization.hardware.ConfigurationGroups; import org.openslx.virtualization.hardware.Ethernet; import org.openslx.virtualization.hardware.SoundCard; import org.openslx.virtualization.hardware.Usb; +import org.openslx.virtualization.hardware.VirtOptionValue; import org.openslx.virtualization.virtualizer.VirtualizerVirtualBox; import org.w3c.dom.Attr; import org.w3c.dom.Element; @@ -110,7 +110,7 @@ public class VirtualizationConfigurationVirtualBox extends VirtualizationConfigu @Override public void transformPrivacy() throws VirtualizationConfigurationException { - + config.addPlaceHolders(); } @Override @@ -122,15 +122,16 @@ public class VirtualizationConfigurationVirtualBox extends VirtualizationConfigu @Override public boolean addEmptyHddTemplate() { - return this.addHddTemplate( "%VM_DISK_PATH%", "%VM_DISK_MODE%", "%VM_DISK_REDOLOGDIR%" ); + return this.addHddTemplate( VirtualizationConfigurationVirtualboxFileFormat.DUMMY_VALUE, + VirtualizationConfigurationVirtualboxFileFormat.DUMMY_VALUE, + VirtualizationConfigurationVirtualboxFileFormat.DUMMY_VALUE ); } @Override public boolean addHddTemplate( String diskImage, String hddMode, String redoDir ) { - config.changeAttribute( "/VirtualBox/Machine/MediaRegistry/HardDisks/HardDisk[@location='" - + PlaceHolder.HDDLOCATION.toString() + "']", "location", diskImage ); - config.changeAttribute( "/VirtualBox/Machine", "snapshotFolder", redoDir ); + config.changeAttribute( "/VirtualBox/Machine/MediaRegistry/HardDisks/HardDisk", "location", diskImage, MatchMode.FIRST_ONLY ); + config.changeAttribute( "/VirtualBox/Machine", "snapshotFolder", redoDir, MatchMode.FIRST_ONLY ); return true; } @@ -138,15 +139,15 @@ public class VirtualizationConfigurationVirtualBox extends VirtualizationConfigu public boolean addHddTemplate( File diskImage, String hddMode, String redoDir ) { String diskImagePath = diskImage.getName(); - config.changeAttribute( "/VirtualBox/Machine/MediaRegistry/HardDisks/HardDisk", "location", diskImagePath ); + config.changeAttribute( "/VirtualBox/Machine/MediaRegistry/HardDisks/HardDisk", "location", diskImagePath, MatchMode.FIRST_ONLY ); UUID newhdduuid = UUID.randomUUID(); // patching the new uuid in the vbox config file here String vboxUUid = "{" + newhdduuid.toString() + "}"; - config.changeAttribute( "/VirtualBox/Machine/MediaRegistry/HardDisks/HardDisk", "uuid", vboxUUid ); + config.changeAttribute( "/VirtualBox/Machine/MediaRegistry/HardDisks/HardDisk", "uuid", vboxUUid, MatchMode.FIRST_ONLY ); config.changeAttribute( "/VirtualBox/Machine/StorageControllers/StorageController/AttachedDevice/Image", "uuid", - vboxUUid ); + vboxUUid, MatchMode.FIRST_ONLY ); // the order of the UUID is BIG_ENDIAN but we need to change the order of the first 8 Bytes // to be able to write them to the vdi file... the PROBLEM here is that the first 8 @@ -179,7 +180,7 @@ public class VirtualizationConfigurationVirtualBox extends VirtualizationConfigu newMachineUuid = UUID.randomUUID(); } String machineUUid = "{" + newMachineUuid.toString() + "}"; - return config.changeAttribute( "/VirtualBox/Machine", "uuid", machineUUid ); + return config.changeAttribute( "/VirtualBox/Machine", "uuid", machineUUid, MatchMode.EXACTLY_ONE ); } @Override @@ -200,7 +201,8 @@ public class VirtualizationConfigurationVirtualBox extends VirtualizationConfigu LOGGER.error( "Failed to set network adapter to NAT." ); status = false; } else { - status = config.changeAttribute( "/VirtualBox/Machine/Hardware/Network/Adapter[@slot='0']", "MACAddress", "080027B86D12" ); + status = config.changeAttribute( "/VirtualBox/Machine/Hardware/Network/Adapter[@slot='0']", "MACAddress", + "080027B86D12", MatchMode.EXACTLY_ONE ); } } else { status = false; @@ -212,7 +214,7 @@ public class VirtualizationConfigurationVirtualBox extends VirtualizationConfigu @Override public void setOs( String vendorOsId ) { - config.changeAttribute( "/VirtualBox/Machine", "OSType", vendorOsId ); + config.changeAttribute( "/VirtualBox/Machine", "OSType", vendorOsId, MatchMode.EXACTLY_ONE ); final OperatingSystem os = VirtualizationConfigurationUtils.getOsOfVirtualizerFromList( this.osList, TConst.VIRT_VIRTUALBOX, vendorOsId ); @@ -222,13 +224,14 @@ public class VirtualizationConfigurationVirtualBox extends VirtualizationConfigu @Override public boolean addDisplayName( String name ) { - return config.changeAttribute( "/VirtualBox/Machine", "name", name ); + return config.changeAttribute( "/VirtualBox/Machine", "name", name, MatchMode.EXACTLY_ONE ); } @Override public boolean addRam( int mem ) { - return config.changeAttribute( "/VirtualBox/Machine/Hardware/Memory", "RAMSize", Integer.toString( mem ) ); + return config.changeAttribute( "/VirtualBox/Machine/Hardware/Memory", "RAMSize", + Integer.toString( mem ), MatchMode.EXACTLY_ONE ); } @Override @@ -277,7 +280,7 @@ public class VirtualizationConfigurationVirtualBox extends VirtualizationConfigu return; } floppyImage.setAttribute( "uuid", - VirtualizationConfigurationVirtualboxFileFormat.PlaceHolder.FLOPPYUUID.toString() ); + VirtualizationConfigurationVirtualboxFileFormat.DUMMY_VALUE ); // register the image in the media registry Element floppyImages = (Element)config.addNewNode( "/VirtualBox/Machine/MediaRegistry", "FloppyImages" ); if ( floppyImages == null ) { @@ -291,9 +294,9 @@ public class VirtualizationConfigurationVirtualBox extends VirtualizationConfigu return; } floppyImageReg.setAttribute( "uuid", - VirtualizationConfigurationVirtualboxFileFormat.PlaceHolder.FLOPPYUUID.toString() ); + VirtualizationConfigurationVirtualboxFileFormat.DUMMY_VALUE ); floppyImageReg.setAttribute( "location", - VirtualizationConfigurationVirtualboxFileFormat.PlaceHolder.FLOPPYLOCATION.toString() ); + VirtualizationConfigurationVirtualboxFileFormat.DUMMY_VALUE ); } } @@ -307,7 +310,8 @@ public class VirtualizationConfigurationVirtualBox extends VirtualizationConfigu @Override public boolean addCpuCoreCount( int nrOfCores ) { - return config.changeAttribute( "/VirtualBox/Machine/Hardware/CPU", "count", Integer.toString( nrOfCores ) ); + return config.changeAttribute( "/VirtualBox/Machine/Hardware/CPU", "count", + Integer.toString( nrOfCores ), MatchMode.EXACTLY_ONE ); } class VBoxSoundCardModel extends VirtOptionValue @@ -323,11 +327,11 @@ public class VirtualizationConfigurationVirtualBox extends VirtualizationConfigu { // XXX I guess this "present" hack will be nicer with enum too if ( Util.isEmptyString( this.id ) ) { - config.changeAttribute( "/VirtualBox/Machine/Hardware/AudioAdapter", "enabled", "false" ); + config.changeAttribute( "/VirtualBox/Machine/Hardware/AudioAdapter", "enabled", "false", MatchMode.MULTIPLE ); return; } - config.changeAttribute( "/VirtualBox/Machine/Hardware/AudioAdapter", "enabled", "true" ); - config.changeAttribute( "/VirtualBox/Machine/Hardware/AudioAdapter", "controller", this.id ); + config.changeAttribute( "/VirtualBox/Machine/Hardware/AudioAdapter", "enabled", "true", MatchMode.FIRST_ONLY ); + config.changeAttribute( "/VirtualBox/Machine/Hardware/AudioAdapter", "controller", this.id, MatchMode.FIRST_ONLY ); } @Override @@ -358,7 +362,7 @@ public class VirtualizationConfigurationVirtualBox extends VirtualizationConfigu @Override public void apply() { - config.changeAttribute( "/VirtualBox/Machine/Hardware/Display", "accelerate3D", this.id ); + config.changeAttribute( "/VirtualBox/Machine/Hardware/Display", "accelerate3D", this.id, MatchMode.EXACTLY_ONE ); } @Override @@ -419,9 +423,9 @@ public class VirtualizationConfigurationVirtualBox extends VirtualizationConfigu present = false; } config.changeAttribute( "/VirtualBox/Machine/Hardware/Network/Adapter[@slot='" + index + "']", "enabled", - Boolean.toString( present ) ); + Boolean.toString( present ), MatchMode.EXACTLY_ONE ); config.changeAttribute( "/VirtualBox/Machine/Hardware/Network/Adapter[@slot='" + index + "']", "type", - dev ); + dev, MatchMode.EXACTLY_ONE ); } @Override diff --git a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualboxFileFormat.java b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualboxFileFormat.java index 691d483..c0fe62b 100644 --- a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualboxFileFormat.java +++ b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualboxFileFormat.java @@ -7,7 +7,9 @@ import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -76,8 +78,10 @@ public class VirtualizationConfigurationVirtualboxFileFormat private final static String FILE_FORMAT_SCHEMA_PREFIX_PATH = Resources.PATH_SEPARATOR + "virtualbox" + Resources.PATH_SEPARATOR + "xsd"; + public static final String DUMMY_VALUE = "[dummy]"; + // list of nodes to automatically remove when reading the vbox file - private static String[] blacklist = { + private static final String[] BLACKLIST = { "/VirtualBox/Machine/Hardware/GuestProperties", "/VirtualBox/Machine/Hardware/VideoCapture", "/VirtualBox/Machine/Hardware/HID", @@ -89,31 +93,13 @@ public class VirtualizationConfigurationVirtualboxFileFormat "/VirtualBox/Machine/Hardware/StorageControllers/StorageController/AttachedDevice[not(@type='HardDisk')]", "/VirtualBox/Machine/MediaRegistry/FloppyImages", "/VirtualBox/Machine/MediaRegistry/DVDImages" }; - - public static enum PlaceHolder + + public static enum MatchMode { - FLOPPYUUID( "%VM_FLOPPY_UUID%" ), - FLOPPYLOCATION( "%VM_FLOPPY_LOCATION%" ), - CPU( "%VM_CPU_CORES%" ), - MEMORY( "%VM_RAM%" ), - MACHINEUUID( "%VM_MACHINE_UUID%" ), - NETWORKMAC( "%VM_NIC_MAC%" ), - HDDLOCATION( "%VM_HDD_LOCATION%" ), - HDDUUID( "%VM_HDD_UUID_" ); - - private final String holderName; - - private PlaceHolder( String name ) - { - this.holderName = name; - } - - @Override - public String toString() - { - return holderName; - } - } + EXACTLY_ONE, + MULTIPLE, + FIRST_ONLY, + }; /** * Creates a vbox configuration by constructing a DOM from the given VirtualBox machine @@ -213,7 +199,7 @@ public class VirtualizationConfigurationVirtualboxFileFormat this.validate(); } catch ( VirtualizationConfigurationException e ) { // do not print output of failed validation if placeholders are available - // since thoses placeholder values violate the defined UUID pattern + // since those placeholder values violate the defined UUID pattern if ( !this.checkForPlaceholders() ) { final String errorMsg = "XML configuration is not a valid VirtualBox v" + version.toString() + " configuration: " + e.getLocalizedMessage(); @@ -228,6 +214,7 @@ public class VirtualizationConfigurationVirtualboxFileFormat ensureHardwareUuid(); setOsType(); fixUsb(); // Since we now support selecting specific speed + removeUnusedHdds(); if ( checkForPlaceholders() ) { return; } @@ -293,6 +280,42 @@ public class VirtualizationConfigurationVirtualboxFileFormat controller.setAttribute( "name", "EHCI" ); controller.setAttribute( "type", "EHCI" ); } + + /** + * Remove any HDDs from MediaRegistry that aren't being used + */ + private void removeUnusedHdds() + { + Set existing = new HashSet<>(); + NodeList list = findNodes( "/VirtualBox/Machine/Hardware/StorageControllers/StorageController/AttachedDevice/Image" ); + if ( list != null && list.getLength() != 0 ) { + for ( int i = 0; i < list.getLength(); ++i ) { + Node item = list.item( i ); + if ( ! ( item instanceof Element ) ) + continue; + Element e = (Element)item; + String uuid = e.getAttribute( "uuid" ); + if ( Util.isEmptyString( uuid ) ) + continue; + existing.add( uuid ); + } + } + // Now check registry + list = findNodes( "/VirtualBox/Machine/MediaRegistry/HardDisks/HardDisk" ); + if ( list != null && list.getLength() != 0 ) { + for ( int i = 0; i < list.getLength(); ++i ) { + Node item = list.item( i ); + if ( ! ( item instanceof Element ) ) + continue; + Element e = (Element)item; + String uuid = e.getAttribute( "uuid" ); + if ( !existing.contains( uuid ) ) { + LOGGER.info( "Removing unused HDD " + uuid + " from MediaRegistry" ); + e.getParentNode().removeChild( e ); + } + } + } + } /** * Saves the machine's uuid as hardware uuid to prevent VMs from @@ -339,46 +362,11 @@ public class VirtualizationConfigurationVirtualboxFileFormat */ public void addPlaceHolders() { - // placeholder for the machine uuid - changeAttribute( "/VirtualBox/Machine", "uuid", PlaceHolder.MACHINEUUID.toString() ); - // placeholder for the location of the virtual hdd - changeAttribute( "/VirtualBox/Machine/MediaRegistry/HardDisks/HardDisk", "location", PlaceHolder.HDDLOCATION.toString() ); - - // placeholder for the memory - changeAttribute( "/VirtualBox/Machine/Hardware/Memory", "RAMSize", PlaceHolder.MEMORY.toString() ); - - // placeholder for the CPU - changeAttribute( "/VirtualBox/Machine/Hardware/CPU", "count", PlaceHolder.CPU.toString() ); - - // placeholder for the MACAddress - changeAttribute( "/VirtualBox/Machine/Hardware/Network/Adapter[@slot='0']", "MACAddress", PlaceHolder.NETWORKMAC.toString() ); - - NodeList hdds = findNodes( "/VirtualBox/Machine/MediaRegistry/HardDisks/HardDisk" ); - for ( int i = 0; i < hdds.getLength(); i++ ) { - Element hdd = (Element)hdds.item( i ); - if ( hdd == null ) - continue; - String hddUuid = hdd.getAttribute( "uuid" ); - hdd.setAttribute( "uuid", PlaceHolder.HDDUUID.toString() + i + "%" ); - final NodeList images; - if ( this.getVersion().isSmallerThan( Version.valueOf( "1.17" ) ) ) { - images = findNodes( "/VirtualBox/Machine/StorageControllers/StorageController/AttachedDevice/Image" ); - } else { - images = findNodes( - "/VirtualBox/Machine/Hardware/StorageControllers/StorageController/AttachedDevice/Image" ); - } - - for ( int j = 0; j < images.getLength(); j++ ) { - Element image = (Element)images.item( j ); - if ( image == null ) - continue; - if ( hddUuid.equals( image.getAttribute( "uuid" ) ) ) { - image.setAttribute( "uuid", PlaceHolder.HDDUUID.toString() + i + "%" ); - break; - } - } - } + changeAttribute( "/VirtualBox/Machine/MediaRegistry/HardDisks/HardDisk", "location", DUMMY_VALUE, MatchMode.MULTIPLE ); + // in case it already has a snapshot + changeAttribute( "/VirtualBox/Machine/MediaRegistry/HardDisks/HardDisk/HardDisk", "location", DUMMY_VALUE, MatchMode.MULTIPLE ); + changeAttribute( "/VirtualBox/Machine", "snapshotFolder", DUMMY_VALUE, MatchMode.FIRST_ONLY ); } /** @@ -394,7 +382,7 @@ public class VirtualizationConfigurationVirtualboxFileFormat Element hdd = (Element)hdds.item( i ); if ( hdd == null ) continue; - if ( hdd.getAttribute( "location" ).equals( PlaceHolder.HDDLOCATION.toString() ) ) { + if ( hdd.getAttribute( "location" ).equals( DUMMY_VALUE ) ) { return true; } } @@ -410,7 +398,7 @@ public class VirtualizationConfigurationVirtualboxFileFormat private void removeBlacklistedElements() throws XPathExpressionException { // iterate over the blackList - for ( String blackedTag : blacklist ) { + for ( String blackedTag : BLACKLIST ) { XPathExpression blackedExpr = XmlHelper.compileXPath( blackedTag ); NodeList blackedNodes = (NodeList)blackedExpr.evaluate( this.doc, XPathConstants.NODESET ); for ( int i = 0; i < blackedNodes.getLength(); i++ ) { @@ -576,25 +564,36 @@ public class VirtualizationConfigurationVirtualboxFileFormat } /** - * Function used to change the value of an attribute of given element. - * The given xpath to the element needs to find a single node, or this function will return - * false. If only one element was found, it will return the result of calling addAttributeToNode. - * Note that due to the way setAttribute() works, this function to create the attribute if it - * doesn't exists. + * Function used to change the value of an attribute of given element(s). * * @param elementXPath given as an xpath expression * @param attribute attribute to change * @param value to set the attribute to - * @return state of the change operation whether the attribute was changed successful or not. + * @param mode what to do if multiple nodes match XPath + * @return state of the change operation whether the attribute was changed successfully or not. */ - public boolean changeAttribute( String elementXPath, String attribute, String value ) + public boolean changeAttribute( String elementXPath, String attribute, String value, MatchMode mode ) { NodeList nodes = findNodes( elementXPath ); - if ( nodes == null || nodes.getLength() != 1 ) { - LOGGER.error( "No unique node could be found for: " + elementXPath ); + if ( nodes == null || nodes.getLength() == 0 ) { + if ( mode != MatchMode.MULTIPLE ) { + LOGGER.error( "No node could be found for: " + elementXPath ); + } + return false; + } + if ( nodes.getLength() != 1 && ( mode == MatchMode.EXACTLY_ONE ) ) { + LOGGER.error( "Multiple nodes found for: " + elementXPath ); return false; } - return addAttributeToNode( nodes.item( 0 ), attribute, value ); + boolean ret = true; + for ( int i = 0; i < nodes.getLength(); ++i ) { + if ( !addAttributeToNode( nodes.item( i ), attribute, value ) ) { + ret = false; + } + if ( mode == MatchMode.FIRST_ONLY ) + break; + } + return ret; } /** -- cgit v1.2.3-55-g7522