Fix the creation of ZFS volumes

ZFS volumes were being created via absolute path rather than via ZFS volume name. Fix: #28
io7m · Jul 28, 2020 · 2330b5b · 2330b5b
1 parent cbb7353
commit 2330b5b
Show file tree

Hide file tree

Showing 9 changed files with 538 additions and 35 deletions.
diff --git a/README-CHANGES.xml b/README-CHANGES.xml
@@ -11,7 +11,7 @@
         <c:change date="2020-07-11T00:00:00+00:00" summary="Add the vm-kill command"/>
       </c:changes>
     </c:release>
-    <c:release date="2020-07-25T09:00:46+00:00" ticket-system="com.github.io7m.waxmill" version="0.0.3">
+    <c:release date="2020-07-25T00:00:00+00:00" ticket-system="com.github.io7m.waxmill" version="0.0.3">
       <c:changes>
         <c:change date="2020-07-25T00:00:00+00:00" summary="Add support for interface groups">
           <c:tickets>
@@ -28,13 +28,22 @@
             <c:ticket id="26"/>
           </c:tickets>
         </c:change>
-        <c:change date="2020-07-25T09:00:46+00:00" summary="Automatically create virtual machine filesystems">
+        <c:change date="2020-07-25T00:00:00+00:00" summary="Automatically create virtual machine filesystems">
           <c:tickets>
             <c:ticket id="23"/>
           </c:tickets>
         </c:change>
       </c:changes>
     </c:release>
+    <c:release date="2020-07-28T15:55:45+00:00" ticket-system="com.github.io7m.waxmill" version="0.0.4">
+      <c:changes>
+        <c:change date="2020-07-28T15:55:45+00:00" summary="Fix issue creating ZFS volumes">
+          <c:tickets>
+            <c:ticket id="28"/>
+          </c:tickets>
+        </c:change>
+      </c:changes>
+    </c:release>
   </c:releases>
   <c:ticket-systems>
     <c:ticket-system default="true" id="com.github.io7m.waxmill" url="https://www.github.com/io7m/waxmill/issues/"/>

diff --git a/com.io7m.waxmill.machines/src/main/java/com/io7m/waxmill/machines/WXMStorageBackends.java b/com.io7m.waxmill.machines/src/main/java/com/io7m/waxmill/machines/WXMStorageBackends.java
@@ -52,11 +52,25 @@ public static Path determineZFSVolumePath(
 
     return machineDirectory
       .resolve(machineId.toString())
-      .resolve(String.format(
-        "disk-%d_%d_%d",
-        Integer.valueOf(deviceID.busID()),
-        Integer.valueOf(deviceID.slotID()),
-        Integer.valueOf(deviceID.functionID())
-      ));
+      .resolve(determineZFSVolumeName(deviceID));
+  }
+
+  /**
+   * Derive the ZFS volume name for the given device ID.
+   *
+   * @param deviceID The device ID
+   *
+   * @return The volume name
+   */
+
+  public static String determineZFSVolumeName(
+    final WXMDeviceSlot deviceID)
+  {
+    return String.format(
+      "disk-%d_%d_%d",
+      Integer.valueOf(deviceID.busID()),
+      Integer.valueOf(deviceID.slotID()),
+      Integer.valueOf(deviceID.functionID())
+    );
   }
 }
diff --git a/com.io7m.waxmill.realize/src/main/java/com/io7m/waxmill/realize/WXMRealizationType.java b/com.io7m.waxmill.realize/src/main/java/com/io7m/waxmill/realize/WXMRealizationType.java
@@ -16,6 +16,8 @@
 
 package com.io7m.waxmill.realize;
 
+import com.io7m.waxmill.exceptions.WXMException;
+
 /**
  * A realization operation.
  */
@@ -29,5 +31,6 @@ public interface WXMRealizationType
    * @return A list of instructions
    */
 
-  WXMRealizationInstructions evaluate();
+  WXMRealizationInstructions evaluate()
+    throws WXMException;
 }
diff --git a/com.io7m.waxmill.realize/src/main/java/com/io7m/waxmill/realize/WXMRealizations.java b/com.io7m.waxmill.realize/src/main/java/com/io7m/waxmill/realize/WXMRealizations.java
@@ -18,6 +18,7 @@
 
 import com.io7m.junreachable.UnimplementedCodeException;
 import com.io7m.waxmill.client.api.WXMClientConfiguration;
+import com.io7m.waxmill.exceptions.WXMException;
 import com.io7m.waxmill.machines.WXMDeviceAHCIDisk;
 import com.io7m.waxmill.machines.WXMDeviceType;
 import com.io7m.waxmill.machines.WXMDeviceVirtioBlockStorage;
@@ -84,6 +85,7 @@ public static WXMRealizationType create(
 
   @Override
   public WXMRealizationInstructions evaluate()
+    throws WXMException
   {
     final var builder = WXMRealizationInstructions.builder();
 
@@ -129,6 +131,7 @@ public WXMRealizationInstructions evaluate()
   private void evaluateAHCIDisk(
     final WXMRealizationInstructions.Builder builder,
     final WXMDeviceAHCIDisk device)
+    throws WXMException
   {
     final var backend = device.backend();
     switch (backend.kind()) {
@@ -163,6 +166,7 @@ private void evaluateFile(
   private void evaluateVirtioBlock(
     final WXMRealizationInstructions.Builder builder,
     final WXMDeviceVirtioBlockStorage device)
+    throws WXMException
   {
     final var backend = device.backend();
     switch (backend.kind()) {
@@ -184,6 +188,7 @@ private void evaluateZFSVolume(
     final WXMRealizationInstructions.Builder builder,
     final WXMDeviceType device,
     final WXMStorageBackendZFSVolume zfs)
+    throws WXMException
   {
     builder.addSteps(
       new WXMZFSVolumeCheck(

diff --git a/...ll.realize/src/main/java/com/io7m/waxmill/realize/internal/WXMRuntimeDirectoryCreate.java b/...ll.realize/src/main/java/com/io7m/waxmill/realize/internal/WXMRuntimeDirectoryCreate.java
@@ -96,12 +96,11 @@ public void execute(
 
     LOG.info("checking if {} is a directory", path);
 
-    if (Files.isDirectory(path)) {
-      LOG.info("{} is a directory", path);
-      return;
-    }
-
     if (Files.exists(path)) {
+      if (Files.isDirectory(path)) {
+        LOG.info("{} is a directory", path);
+        return;
+      }
       throw new WXMException(this.notADirectory(path));
     }
 

diff --git a/...7m.waxmill.realize/src/main/java/com/io7m/waxmill/realize/internal/WXMZFSVolumeCheck.java b/...7m.waxmill.realize/src/main/java/com/io7m/waxmill/realize/internal/WXMZFSVolumeCheck.java
@@ -18,6 +18,7 @@
 
 import com.io7m.waxmill.client.api.WXMClientConfiguration;
 import com.io7m.waxmill.exceptions.WXMException;
+import com.io7m.waxmill.exceptions.WXMExceptionUnsatisfiedRequirement;
 import com.io7m.waxmill.machines.WXMDeviceSlot;
 import com.io7m.waxmill.machines.WXMDryRun;
 import com.io7m.waxmill.machines.WXMStorageBackendZFSVolume;
@@ -31,9 +32,11 @@
 
 import java.io.IOException;
 import java.math.BigInteger;
+import java.nio.file.FileStore;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
+import java.util.Locale;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.UUID;
@@ -52,6 +55,7 @@ public final class WXMZFSVolumeCheck implements WXMRealizationStepType
   private final WXMRealizeMessages messages;
   private final WXMStorageBackendZFSVolume zfsVolume;
   private final Path volumePath;
+  private final String volumeCreate;
   private List<WXMProcessDescription> processList;
 
   public WXMZFSVolumeCheck(
@@ -61,6 +65,7 @@ public WXMZFSVolumeCheck(
     final UUID inMachineId,
     final WXMDeviceSlot inSlot,
     final WXMStorageBackendZFSVolume inZFSVolume)
+    throws WXMException
   {
     this.clientConfiguration =
       Objects.requireNonNull(inClientConfiguration, "clientConfiguration");
@@ -75,6 +80,9 @@ public WXMZFSVolumeCheck(
     final var machineId =
       Objects.requireNonNull(inMachineId, "machineId");
 
+    this.volumeCreate =
+      this.determineVolumeCreatePath(machineId);
+
     this.volumePath =
       WXMStorageBackends.determineZFSVolumePath(
         this.clientConfiguration.virtualMachineRuntimeDirectory(),
@@ -86,6 +94,40 @@ public WXMZFSVolumeCheck(
     this.processList = this.processOpt.map(List::of).orElseGet(List::of);
   }
 
+  private String determineVolumeCreatePath(
+    final UUID machineId)
+    throws WXMException
+  {
+    final var base =
+      this.clientConfiguration.virtualMachineRuntimeDirectory()
+        .resolve(machineId.toString());
+
+    final FileStore store;
+    try {
+      store = Files.getFileStore(base);
+    } catch (final IOException e) {
+      throw new WXMException(e);
+    }
+
+    final var type = store.type();
+    if (!"ZFS".equals(type.toUpperCase(Locale.ROOT))) {
+      throw new WXMExceptionUnsatisfiedRequirement(
+        this.messages.format(
+          "errorNotZFS",
+          machineId,
+          base,
+          type
+        )
+      );
+    }
+
+    return String.format(
+      "%s/%s",
+      store.name(),
+      WXMStorageBackends.determineZFSVolumeName(this.slot)
+    );
+  }
+
   private Optional<WXMProcessDescription> makeProcesses()
   {
     return this.zfsVolume.expectedSize()
@@ -94,7 +136,7 @@ private Optional<WXMProcessDescription> makeProcesses()
         .addArguments("create")
         .addArguments("-V")
         .addArguments(size.toString())
-        .addArguments(this.volumePath.toString())
+        .addArguments(this.volumeCreate)
         .build()
       );
   }
@@ -105,7 +147,7 @@ public String description()
     final var expectedSize = this.zfsVolume.expectedSize();
     return this.messages.format(
       "zfsVolumeCheck",
-      this.volumePath,
+      this.volumeCreate,
       expectedSize.map(WXMZFSVolumeCheck::formatSize)
         .orElse("<unspecified>"),
       this.slot

diff --git a/com.io7m.waxmill.realize/src/main/resources/com/io7m/waxmill/realize/internal/Realize.xml b/com.io7m.waxmill.realize/src/main/resources/com/io7m/waxmill/realize/internal/Realize.xml
@@ -44,4 +44,11 @@
 ]]>
   </entry>
 
+  <entry key="errorNotZFS"><![CDATA[The specified directory is not a ZFS filesystem.
+  Machine:         {0}
+  Path:            {1}
+  Filesystem Type: {2}
+]]>
+  </entry>
+
 </properties>
diff --git a/...m.waxmill.tests/src/test/java/com/io7m/waxmill/tests/cmdline/WXMCommandVMRealizeTest.java b/...m.waxmill.tests/src/test/java/com/io7m/waxmill/tests/cmdline/WXMCommandVMRealizeTest.java
@@ -22,6 +22,7 @@
 import com.io7m.waxmill.machines.WXMStorageBackends;
 import com.io7m.waxmill.tests.WXMTestDirectories;
 import com.io7m.waxmill.xml.WXMClientConfigurationSerializers;
+import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -31,6 +32,7 @@
 import java.nio.file.Paths;
 import java.util.UUID;
 
+import static java.util.Locale.*;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public final class WXMCommandVMRealizeTest
@@ -215,6 +217,11 @@ public void realizeZFSVolumeVirtio()
     throws Exception
   {
     final var id = UUID.randomUUID();
+
+    final Path path = this.zfsDirectory.resolve(id.toString());
+    Files.createDirectories(path);
+    assumeZFSFilesystem(path);
+
     MainExitless.main(
       new String[]{
         "vm-define",
@@ -263,6 +270,11 @@ public void realizeZFSVolumeVirtioDryRun()
     throws Exception
   {
     final var id = UUID.randomUUID();
+
+    final Path path = this.zfsDirectory.resolve(id.toString());
+    Files.createDirectories(path);
+    assumeZFSFilesystem(path);
+
     MainExitless.main(
       new String[]{
         "vm-define",
@@ -357,6 +369,7 @@ public void realizeZFSVolumeVirtioExists()
       );
     Files.createDirectories(path.getParent());
     Files.write(path, new byte[128000]);
+    assumeZFSFilesystem(path);
 
     MainExitless.main(
       new String[]{
@@ -369,6 +382,15 @@ public void realizeZFSVolumeVirtioExists()
     );
   }
 
+  private static void assumeZFSFilesystem(final Path path)
+    throws IOException
+  {
+    Assumptions.assumeTrue(
+      "ZFS".equals(Files.getFileStore(path).type().toUpperCase(ROOT)),
+      String.format("%s is a ZFS filesystem", path)
+    );
+  }
+
   @Test
   public void realizeZFSVolumeVirtioExistsWrongSize()
     throws Exception
@@ -418,6 +440,7 @@ public void realizeZFSVolumeVirtioExistsWrongSize()
       );
     Files.createDirectories(path.getParent());
     Files.write(path, new byte[128001]);
+    assumeZFSFilesystem(path);
 
     MainExitless.main(
       new String[]{
@@ -435,6 +458,11 @@ public void realizeZFSVolumeAHCI()
     throws Exception
   {
     final var id = UUID.randomUUID();
+
+    final Path path = this.zfsDirectory.resolve(id.toString());
+    Files.createDirectories(path);
+    assumeZFSFilesystem(path);
+
     MainExitless.main(
       new String[]{
         "vm-define",
@@ -527,6 +555,7 @@ public void realizeZFSVolumeAHCIExists()
       );
     Files.createDirectories(path.getParent());
     Files.write(path, new byte[128000]);
+    assumeZFSFilesystem(path);
 
     MainExitless.main(
       new String[]{
@@ -588,6 +617,7 @@ public void realizeZFSVolumeAHCIExistsWrongSize()
       );
     Files.createDirectories(path.getParent());
     Files.write(path, new byte[128001]);
+    assumeZFSFilesystem(path);
 
     MainExitless.main(
       new String[]{
@@ -605,6 +635,11 @@ public void realizeZFSVolumeAHCIDryRun()
     throws Exception
   {
     final var id = UUID.randomUUID();
+
+    final Path path = this.zfsDirectory.resolve(id.toString());
+    Files.createDirectories(path);
+    assumeZFSFilesystem(path);
+
     MainExitless.main(
       new String[]{
         "vm-define",