From 014411b0984fb85588123912a12f3f1d39215c27 Mon Sep 17 00:00:00 2001

From: Adam Williamson <awilliam@redhat.com>

Date: Tue, 23 Jan 2024 11:24:00 -0800

Subject: [PATCH 21/21] Allow image size specification using any SI or IEC unit

Currently, oz only allows image sizes to be specified as integer

amounts of gibibytes or tebibytes (that's IEC power-of-two units).

This is unfortunately inflexible. Consider that storage media are

typically specified in SI power-of-ten sizes, so a USB stick may

be 16 gigabytes (SI power-of-ten GB) in size - that's

16,000,000,000 bytes. Let's say we want to build an image that

will fit on such a USB stick. oz will only allow us to build an

image of 15,032,385,536 bytes (14 gibibytes) or 16,106,127,360

bytes (15 gibibytes). So we're either slightly too big, or leaving

nearly a gigabyte on the table.

This allows the image size to be specified in the TDL with most

any IEC or SI unit suffix, from B (bytes) all the way up to YiB

(yobibytes). A size with no suffix or the suffix "G" is taken as

gibibytes and a size with the suffix "T" is taken as tebibytes,

as before, but other ambiguous suffixes are not accepted. All

casing is accepted. Behind the scenes, we convert the size to

bytes and specify it that way in the libvirt XML when creating

the image in _internal_generate_diskimage.

This does change the interface of generate_diskimage(), by making

the unit for the size argument bytes instead of gibibytes. I can't

see a clean way to avoid this while allowing flexibility. I have

checked, and AFAICT, among active projects, only oz itself and

the ImageFactory TinMan plugin call this function. The TinMan

plugin will need a trivial change to its fallback default value.

Signed-off-by: Adam Williamson <awilliam@redhat.com>

---

 oz/Fedora.py                                  |  6 +--

 oz/Guest.py                                   | 16 +++----

 oz/TDL.py                                     | 47 +++++++++++++------

 oz/Windows.py                                 |  8 ++--

 oz/ozutil.py                                  | 17 +++++++

 oz/tdl.rng                                    |  2 +-

 tests/guest/test_guest.py                     |  3 +-

 ... => test-58-disk-size-tebibyte-compat.tdl} |  0

 tests/tdl/test-63-disk-size-exbibyte.tdl      | 15 ++++++

 tests/tdl/test-64-disk-size-zettabyte.tdl     | 15 ++++++

 tests/tdl/test-65-disk-size-byte.tdl          | 15 ++++++

 tests/tdl/test_tdl.py                         |  5 +-

 12 files changed, 117 insertions(+), 32 deletions(-)

 rename tests/tdl/{test-58-disk-size-terabyte.tdl => test-58-disk-size-tebibyte-compat.tdl} (100%)

 create mode 100644 tests/tdl/test-63-disk-size-exbibyte.tdl

 create mode 100644 tests/tdl/test-64-disk-size-zettabyte.tdl

 create mode 100644 tests/tdl/test-65-disk-size-byte.tdl

diff --git a/oz/Fedora.py b/oz/Fedora.py

index 89ecca8..c6152f1 100644

--- a/oz/Fedora.py

+++ b/oz/Fedora.py

@@ -298,11 +298,11 @@ class FedoraGuest(oz.RedHat.RedHatLinuxCDYumGuest):

         initrdline += " inst.nosave=output_ks"

         self._modify_isolinux(initrdline)

-    def generate_diskimage(self, size=10, force=False):

+    def generate_diskimage(self, size=10*1024*1024*1024, force=False):

         """

         Method to generate a diskimage.  By default, a blank diskimage of

-        10GB will be created; the caller can override this with the size

-        parameter, specified in GB.  If force is False (the default), then

+        10 GiB will be created; the caller can override this with the size

+        parameter, specified in bytes.  If force is False (the default), then

         a diskimage will not be created if a cached JEOS is found.  If

         force is True, a diskimage will be created regardless of whether a

         cached JEOS exists.  See the oz-install man page for more

diff --git a/oz/Guest.py b/oz/Guest.py

index 249cce0..8a4e485 100644

--- a/oz/Guest.py

+++ b/oz/Guest.py

@@ -571,7 +571,7 @@ class Guest(object):

         return xml

-    def _internal_generate_diskimage(self, size=10, force=False,

+    def _internal_generate_diskimage(self, size=10*1024*1024*1024, force=False,

                                      create_partition=False,

                                      image_filename=None,

                                      backing_filename=None):

@@ -587,7 +587,7 @@ class Guest(object):

             # we'll copy the JEOS itself later on

             return

-        self.log.info("Generating %dGB diskimage for %s", size, self.tdl.name)

+        self.log.info("Generating %s diskimage for %s", oz.ozutil.sizeof_fmt(int(size)), self.tdl.name)

         diskimage = self.diskimage

         if image_filename:

@@ -628,17 +628,17 @@ class Guest(object):

             # allows creation without an explicit capacity element.

             qcow_size = oz.ozutil.check_qcow_size(backing_filename)

             if qcow_size:

-                capacity = qcow_size / 1024 / 1024 / 1024

+                capacity = qcow_size

                 backing_format = 'qcow2'

             else:

-                capacity = os.path.getsize(backing_filename) / 1024 / 1024 / 1024

+                capacity = os.path.getsize(backing_filename)

                 backing_format = 'raw'

             backing = oz.ozutil.lxml_subelement(vol, "backingStore")

             oz.ozutil.lxml_subelement(backing, "path", backing_filename)

             oz.ozutil.lxml_subelement(backing, "format", None,

                                       {"type": backing_format})

-        oz.ozutil.lxml_subelement(vol, "capacity", str(int(capacity)), {'unit': 'G'})

+        oz.ozutil.lxml_subelement(vol, "capacity", str(int(capacity)), {'unit': 'B'})

         vol_xml = lxml.etree.tostring(vol, pretty_print=True, encoding="unicode")

         # sigh.  Yes, this is racy; if a pool is defined during this loop, we

@@ -718,11 +718,11 @@ class Guest(object):

                 g_handle.create_msdos_partition_table()

                 g_handle.cleanup()

-    def generate_diskimage(self, size=10, force=False):

+    def generate_diskimage(self, size=10*1024*1024*1024, force=False):

         """

         Method to generate a diskimage.  By default, a blank diskimage of

-        10GB will be created; the caller can override this with the size

-        parameter, specified in GB.  If force is False (the default), then

+        10 GiB will be created; the caller can override this with the size

+        parameter, specified in bytes.  If force is False (the default), then

         a diskimage will not be created if a cached JEOS is found.  If

         force is True, a diskimage will be created regardless of whether a

         cached JEOS exists.  See the oz-install man page for more

diff --git a/oz/TDL.py b/oz/TDL.py

index cb373cb..aee74a3 100644

--- a/oz/TDL.py

+++ b/oz/TDL.py

@@ -327,20 +327,39 @@ class TDL(object):

             # a sensible default

             return None

-        match = re.match(r'([0-9]*) *([GT]?)$', size)

-        if not match or len(match.groups()) != 2:

-            raise oz.OzException.OzException("Invalid disk size; it must be specified as a size in gigabytes, optionally suffixed with 'G' or 'T'")

-

-        number = match.group(1)

-        suffix = match.group(2)

-

-        if not suffix or suffix == 'G':

-            # for backwards compatibility, we assume G when there is no suffix

-            size = number

-        elif suffix == 'T':

-            size = str(int(number) * 1024)

-

-        return size

+        # drop spaces and downcase

+        size = size.replace(" ", "").lower()

+        # isolate digits

+        number = ""

+        suffix = ""

+        for (idx, char) in enumerate(size):

+            if char.isdigit():

+                number += char

+            else:

+                suffix = size[idx:]

+                break

+

+        if not suffix:

+            # for backwards compatibility, we assume GiB when there is no suffix

+            suffix = "gib"

+

+        # also for backwards compatibility with an earlier attempt to support

+        # suffixes, treat "T" and "G" as "TiB" and "GiB"

+        units = {"b": 1, "g": 2**30, "t": 2**40}

+        tenscale = 3

+        twoscale = 10

+        for (i, pref) in enumerate(("k", "m", "g", "t", "p", "e", "z", "y"), start=1):

+            # this is giving us {"gib": 2 ** 30, "gb": 10 ** 9}, etc

+            units[pref + "b"] = (10 ** (i*tenscale))

+            units[pref + "ib"] = (2 ** (i*twoscale))

+

+        factor = units.get(suffix)

+        if not number or not factor:

+            raise oz.OzException.OzException(

+                "Invalid disk size; it must be specified as an integer size with an optional SI or IEC unit suffix, e.g. '10TB' or '16GiB'"

+            )

+

+        return str(int(number) * factor)

     def _parse_commands(self, xpath):

         """

diff --git a/oz/Windows.py b/oz/Windows.py

index 78948ac..777a306 100644

--- a/oz/Windows.py

+++ b/oz/Windows.py

@@ -78,12 +78,12 @@ class Windows_v5(Windows):

                                            self.iso_contents],

                                           printfn=self.log.debug)

-    def generate_diskimage(self, size=10, force=False):

+    def generate_diskimage(self, size=10*1024*1024*1024, force=False):

         """

         Method to generate a diskimage.  By default, a blank diskimage of

-        10GB will be created; the caller can override this with the size

-        parameter, specified in GB.  If force is False (the default), then

-        a diskimage will not be created if a cached JEOS is found.  If

+        10 GiB will be created; the caller can override this with the size

+        parameter, specified in bytes.  If force is False (the default),

+        then a diskimage will not be created if a cached JEOS is found.  If

         force is True, a diskimage will be created regardless of whether a

         cached JEOS exists.  See the oz-install man page for more

         information about JEOS caching.

diff --git a/oz/ozutil.py b/oz/ozutil.py

index 689559f..be27d3f 100644

--- a/oz/ozutil.py

+++ b/oz/ozutil.py

@@ -1178,3 +1178,20 @@ def get_free_port():

     sock.close()

     return listen_port

+

+

+def sizeof_fmt(num, suffix="B"):

+    """

+    Give a convenient human-readable representation of a large size in

+    bytes. Initially by Fred Cirera:

+    https://web.archive.org/web/20111010015624/http://blogmag.net/blog/read/38/Print_human_readable_file_size

+    edited by multiple contributors at:

+    https://stackoverflow.com/questions/1094841

+    Per Richard Fontana this is too trivial to be copyrightable, so

+    there are no licensing concerns

+    """

+    for unit in ("", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei", "Zi"):

+        if abs(num) < 1024.0:

+            return "%3.1f%s%s" % (num, unit, suffix)

+        num /= 1024.0

+    return "%.1f%s%s" % (num, 'Yi', suffix)

diff --git a/oz/tdl.rng b/oz/tdl.rng

index dc29c9a..9ca94cc 100644

--- a/oz/tdl.rng

+++ b/oz/tdl.rng

@@ -325,7 +325,7 @@

   <define name='disk_size'>

     <data type="string">

-      <param name="pattern">([0-9]*) *([GT]?)</param>

+      <param name="pattern">([0-9]*) *([kKmMgGtTpPeEzZyY]?[iI]?[bB]?)</param>

     </data>

   </define>

diff --git a/tests/guest/test_guest.py b/tests/guest/test_guest.py

index 625eb67..85f6b03 100644

--- a/tests/guest/test_guest.py

+++ b/tests/guest/test_guest.py

@@ -352,7 +352,8 @@ def test_geteltorito_bogus_bootp(tmpdir):

 def test_init_guest():

     guest = setup_guest(tdlxml2)

-    assert guest.disksize == 20

+    # size without units is taken to be GiB

+    assert guest.disksize == 20*(2**30)

     assert guest.image_name() == 'tester'

     assert guest.output_image_path() in (

         # user's image storage

diff --git a/tests/tdl/test-58-disk-size-terabyte.tdl b/tests/tdl/test-58-disk-size-tebibyte-compat.tdl

similarity index 100%

rename from tests/tdl/test-58-disk-size-terabyte.tdl

rename to tests/tdl/test-58-disk-size-tebibyte-compat.tdl

diff --git a/tests/tdl/test-63-disk-size-exbibyte.tdl b/tests/tdl/test-63-disk-size-exbibyte.tdl

new file mode 100644

index 0000000..e3f6b12

--- /dev/null

+++ b/tests/tdl/test-63-disk-size-exbibyte.tdl

@@ -0,0 +1,15 @@

+<template>

+  <name>help</name>

+  <os>

+    <name>Fedora</name>

+    <version>12</version>

+    <arch>i386</arch>

+    <install type='url'>

+      <url>http://download.fedoraproject.org/pub/fedora/linux/releases/12/Fedora/x86_64/os/</url>

+    </install>

+  </os>

+  <description>My Fedora 12 JEOS image</description>

+  <disk>

+    <size>2EiB</size>

+  </disk>

+</template>

diff --git a/tests/tdl/test-64-disk-size-zettabyte.tdl b/tests/tdl/test-64-disk-size-zettabyte.tdl

new file mode 100644

index 0000000..26b7105

--- /dev/null

+++ b/tests/tdl/test-64-disk-size-zettabyte.tdl

@@ -0,0 +1,15 @@

+<template>

+  <name>help</name>

+  <os>

+    <name>Fedora</name>

+    <version>12</version>

+    <arch>i386</arch>

+    <install type='url'>

+      <url>http://download.fedoraproject.org/pub/fedora/linux/releases/12/Fedora/x86_64/os/</url>

+    </install>

+  </os>

+  <description>My Fedora 12 JEOS image</description>

+  <disk>

+    <size>10ZB</size>

+  </disk>

+</template>

diff --git a/tests/tdl/test-65-disk-size-byte.tdl b/tests/tdl/test-65-disk-size-byte.tdl

new file mode 100644

index 0000000..b6e0774

--- /dev/null

+++ b/tests/tdl/test-65-disk-size-byte.tdl

@@ -0,0 +1,15 @@

+<template>

+  <name>help</name>

+  <os>

+    <name>Fedora</name>

+    <version>12</version>

+    <arch>i386</arch>

+    <install type='url'>

+      <url>http://download.fedoraproject.org/pub/fedora/linux/releases/12/Fedora/x86_64/os/</url>

+    </install>

+  </os>

+  <description>My Fedora 12 JEOS image</description>

+  <disk>

+    <size>10000000 B</size>

+  </disk>

+</template>

diff --git a/tests/tdl/test_tdl.py b/tests/tdl/test_tdl.py

index a94b2c1..366f1bc 100644

--- a/tests/tdl/test_tdl.py

+++ b/tests/tdl/test_tdl.py

@@ -92,8 +92,11 @@ tests = {

     "test-55-files-http-url.tdl": True,

     "test-56-invalid-disk-size.tdl": False,

     "test-57-invalid-disk-size.tdl": False,

-    "test-58-disk-size-terabyte.tdl": True,

+    "test-58-disk-size-tebibyte-compat.tdl": True,

     "test-59-command-sorting.tdl": True,

+    "test-63-disk-size-exbibyte.tdl": True,

+    "test-64-disk-size-zettabyte.tdl": True,

+    "test-65-disk-size-byte.tdl": True,

 }

 # Test that iterates over all .tdl files

-- 

2.43.0