Re: [PATCH v2] scripts/qcow2-to-stdout.py: Add script to write qcow2 images to stdout

2024-07-29 Thread Alberto Garcia
On Sun 28 Jul 2024 01:01:07 AM +03, Nir Soffer wrote:
>> +def bitmap_set(bitmap, idx):
>> +bitmap[int(idx / 8)] |= 1 << (idx % 8)
>
> Should use floor division operator (//):
>
> bitmap[idx // 8] |= 1 << (idx % 8)
>
> Same for bitmap_test().

Right, also for all other cases of int(foo / bar). I'll update them.

>> +ret = subprocess.run(
>> +[
>> +QEMU_STORAGE_DAEMON,
>> +"--daemonize",
>
> Any reason to daemonize? managing the child process is easier if you
> don't daemonize.

--daemonize guarantees that when subprocess.run() returns the exported
raw_file is ready to use.

>> +if len(cluster) < cluster_size:
>> +cluster += bytes(cluster_size - len(cluster))
>
> This should be done only for the last cluster.

But that's what that condition is for, we'll only read less than
cluster_size bytes at the end of the file.

>> +if not bitmap_test(l1_bitmap, l1_idx):
>> +bitmap_set(l1_bitmap, l1_idx)
>> +allocated_l2_tables += 1
>
> This could be much more efficient using SEEK_DATA/SEEK_HOLE, avoiding
> reading the entire image twice. Or using "qemu-img map --output json"
> to simplify.

I prefer not to have external dependencies(*) so I would rather not use
qemu-img, but I certainly can use SEEK_DATA/SEEK_HOLE to avoid reading
data that is known to be zero in the first pass.

(*) there is of course qemu-storage-daemon but that one is optional and
I anyway cannot implement its functionality in this script.

>> +sys.stdout.buffer.write(cluster)
>> +else:
>> +skip += 1
>
> If would be easier to work with if you add a function iterating over
> the l2_entries, yielding the he cluster index to copy:
>
>   def iter_l2_entries(bitmap, clusters):
> for idx in range(clusters):
>   if bitmap_test(bitmap, idx):
> yield idx
>
> The copy loop can read using os.pread():
>
> for idx in iter_l2_entries(l2_bitmap, total_data_clusters):
> cluster = os.pread(fd, cluster_size, cluster_size * idx)
> sys.stdout.buffer.write(cluster)
>
> I'm not sure the offset is right in my example, it is hard to
> understand the semantics of skip in your code.

That part reads the input file sequentially from start to end, but
instead of reading empty clusters we use seek() to skip them. The 'skip'
variable keeps a counter of empty clusters since the last read.

Your proposal requires an additional function but I see that it can make
the code more readable, so I'll give it a try.

>> +if __name__ == "__main__":
>
> Usually code is easier to work with when __main__  calls main().

Good idea.

Thanks for the detailed review!

Berto



Re: [PATCH v2] scripts/qcow2-to-stdout.py: Add script to write qcow2 images to stdout

2024-07-27 Thread Nir Soffer
On Mon, Jul 1, 2024 at 6:13 PM Alberto Garcia  wrote:
>
> This tool converts a disk image to qcow2, writing the result directly
> to stdout. This can be used for example to send the generated file
> over the network.
>
> This is equivalent to using qemu-img to convert a file to qcow2 and
> then writing the result to stdout, with the difference that this tool
> does not need to create this temporary qcow2 file and therefore does
> not need any additional disk space.
>
> Implementing this directly in qemu-img is not really an option because
> it expects the output file to be seekable and it is also meant to be a
> generic tool that supports all combinations of file formats and image
> options. Instead, this tool can only produce qcow2 files with the
> basic options, without compression, encryption or other features.
>
> The input file is read twice. The first pass is used to determine
> which clusters contain non-zero data and that information is used to
> create the qcow2 header, refcount table and blocks, and L1 and L2
> tables. After all that metadata is created then the second pass is
> used to write the guest data.
> By default qcow2-to-stdout.py expects the input to be a raw file, but
> if qemu-storage-daemon is available then it can also be used to read
> images in other formats. Alternatively the user can also run qemu-ndb
> or qemu-storage-daemon manually instead.
>
> Signed-off-by: Alberto Garcia 
> Signed-off-by: Madeeha Javed 
> ---
>  scripts/qcow2-to-stdout.py | 377 +
>  1 file changed, 377 insertions(+)
>  create mode 100755 scripts/qcow2-to-stdout.py
>
> v2:
> - Define the QCOW2_V3_HDR_LENGTH and QCOW2_FEATURE_NAME_TABLE constants 
> [Manos]
> - Define the QEMU_STORAGE_DAEMON constant
> - Use isfile() instead of exists() for the input file
> - Refuse to write to stdout if it's a tty [Manos]
> - Move the bulk of the code to a function called from __main__ [Manos]
> - Remove the qcow2_ prefix from qcow2_cluster_size and qcow2_refcount_bits
> - Formatting fixes suggested by the Python black formatter [Manos]
> - On error pass the string directly to sys.exit()
> - Capture the output of qemu-storage-daemon [Manos]
> - Use a contextmanager to run qemu-storage-daemon [Manos]
> - Update patch description to mention why this cannot be implemeted directly 
> in qemu-img [Manos]
>
> v1: https://lists.gnu.org/archive/html/qemu-block/2024-06/msg00073.html
>
> diff --git a/scripts/qcow2-to-stdout.py b/scripts/qcow2-to-stdout.py
> new file mode 100755
> index 00..d486a80e86
> --- /dev/null
> +++ b/scripts/qcow2-to-stdout.py
> @@ -0,0 +1,377 @@
> +#!/usr/bin/env python3
> +
> +# This tool reads a disk image in any format and converts it to qcow2,
> +# writing the result directly to stdout.
> +#
> +# Copyright (C) 2024 Igalia, S.L.
> +#
> +# Authors: Alberto Garcia 
> +#  Madeeha Javed 
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +#
> +# qcow2 files produced by this script are always arranged like this:
> +#
> +# - qcow2 header
> +# - refcount table
> +# - refcount blocks
> +# - L1 table
> +# - L2 tables
> +# - Data clusters
> +#
> +# A note about variable names: in qcow2 there is one refcount table
> +# and one (active) L1 table, although each can occupy several
> +# clusters. For the sake of simplicity the code sometimes talks about
> +# refcount tables and L1 tables when referring to those clusters.
> +
> +import argparse
> +import atexit

This is unused now

> +import math
> +import os
> +import signal
> +import struct
> +import subprocess
> +import sys
> +import tempfile
> +import time
> +from contextlib import contextmanager
> +
> +QCOW2_DEFAULT_CLUSTER_SIZE = 65536
> +QCOW2_DEFAULT_REFCOUNT_BITS = 16
> +QCOW2_DEFAULT_VERSION = 3
> +QCOW2_FEATURE_NAME_TABLE = 0x6803F857
> +QCOW2_V3_HEADER_LENGTH = 112  # Header length in QEMU 9.0. Must be a 
> multiple of 8
> +QCOW_OFLAG_COPIED = 1 << 63
> +QEMU_STORAGE_DAEMON = "qemu-storage-daemon"
> +
> +
> +def bitmap_set(bitmap, idx):
> +bitmap[int(idx / 8)] |= 1 << (idx % 8)

Should use floor division operator (//):

bitmap[idx // 8] |= 1 << (idx % 8)

Same for bitmap_test().

> +
> +
> +def bitmap_test(bitmap, idx):

bitmap_is_set() can be more clear. For example it is obvious that it
returns True if the bit is set

> +return (bitmap[int(idx / 8)] & (1 << (idx % 8))) != 0
> +
> +
> +# create_qcow2_file() expects a raw input file. If we have a different
> +# format we can use qemu-storage-daemon to make it appear as raw.
> +@contextmanager
> +def get_input_as_raw_file(input_file, input_format):
> +if input_format == "raw":
> +yield input_file
> +return
> +try:
> +temp_dir = tempfile.mkdtemp()
> +pid_file = temp_dir + "/pid"
> +raw_file = temp_dir + "/raw"

This is fragile, better to use os.path.join()

> +open(raw_file, "wb").close()
> +ret = subprocess.run(
> +[
> +QEMU_STORAGE_DAEMON,
> +   

Re: [PATCH v2] scripts/qcow2-to-stdout.py: Add script to write qcow2 images to stdout

2024-07-26 Thread Manos Pitsidianakis

On Mon, 01 Jul 2024 18:11, Alberto Garcia  wrote:

This tool converts a disk image to qcow2, writing the result directly
to stdout. This can be used for example to send the generated file
over the network.

This is equivalent to using qemu-img to convert a file to qcow2 and
then writing the result to stdout, with the difference that this tool
does not need to create this temporary qcow2 file and therefore does
not need any additional disk space.

Implementing this directly in qemu-img is not really an option because
it expects the output file to be seekable and it is also meant to be a
generic tool that supports all combinations of file formats and image
options. Instead, this tool can only produce qcow2 files with the
basic options, without compression, encryption or other features.

The input file is read twice. The first pass is used to determine
which clusters contain non-zero data and that information is used to
create the qcow2 header, refcount table and blocks, and L1 and L2
tables. After all that metadata is created then the second pass is
used to write the guest data.

By default qcow2-to-stdout.py expects the input to be a raw file, but
if qemu-storage-daemon is available then it can also be used to read
images in other formats. Alternatively the user can also run qemu-ndb
or qemu-storage-daemon manually instead.

Signed-off-by: Alberto Garcia 
Signed-off-by: Madeeha Javed 
---
scripts/qcow2-to-stdout.py | 377 +
1 file changed, 377 insertions(+)
create mode 100755 scripts/qcow2-to-stdout.py

v2:
- Define the QCOW2_V3_HDR_LENGTH and QCOW2_FEATURE_NAME_TABLE constants [Manos]
- Define the QEMU_STORAGE_DAEMON constant
- Use isfile() instead of exists() for the input file
- Refuse to write to stdout if it's a tty [Manos]
- Move the bulk of the code to a function called from __main__ [Manos]
- Remove the qcow2_ prefix from qcow2_cluster_size and qcow2_refcount_bits
- Formatting fixes suggested by the Python black formatter [Manos]
- On error pass the string directly to sys.exit()
- Capture the output of qemu-storage-daemon [Manos]
- Use a contextmanager to run qemu-storage-daemon [Manos]
- Update patch description to mention why this cannot be implemeted directly in 
qemu-img [Manos]

v1: https://lists.gnu.org/archive/html/qemu-block/2024-06/msg00073.html

diff --git a/scripts/qcow2-to-stdout.py b/scripts/qcow2-to-stdout.py
new file mode 100755
index 00..d486a80e86
--- /dev/null
+++ b/scripts/qcow2-to-stdout.py
@@ -0,0 +1,377 @@
+#!/usr/bin/env python3
+
+# This tool reads a disk image in any format and converts it to qcow2,
+# writing the result directly to stdout.
+#
+# Copyright (C) 2024 Igalia, S.L.
+#
+# Authors: Alberto Garcia 
+#  Madeeha Javed 
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# qcow2 files produced by this script are always arranged like this:
+#
+# - qcow2 header
+# - refcount table
+# - refcount blocks
+# - L1 table
+# - L2 tables
+# - Data clusters
+#
+# A note about variable names: in qcow2 there is one refcount table
+# and one (active) L1 table, although each can occupy several
+# clusters. For the sake of simplicity the code sometimes talks about
+# refcount tables and L1 tables when referring to those clusters.
+
+import argparse
+import atexit
+import math
+import os
+import signal
+import struct
+import subprocess
+import sys
+import tempfile
+import time
+from contextlib import contextmanager
+
+QCOW2_DEFAULT_CLUSTER_SIZE = 65536
+QCOW2_DEFAULT_REFCOUNT_BITS = 16
+QCOW2_DEFAULT_VERSION = 3
+QCOW2_FEATURE_NAME_TABLE = 0x6803F857
+QCOW2_V3_HEADER_LENGTH = 112  # Header length in QEMU 9.0. Must be a multiple 
of 8
+QCOW_OFLAG_COPIED = 1 << 63
+QEMU_STORAGE_DAEMON = "qemu-storage-daemon"
+
+
+def bitmap_set(bitmap, idx):
+bitmap[int(idx / 8)] |= 1 << (idx % 8)
+
+
+def bitmap_test(bitmap, idx):
+return (bitmap[int(idx / 8)] & (1 << (idx % 8))) != 0
+
+
+# create_qcow2_file() expects a raw input file. If we have a different
+# format we can use qemu-storage-daemon to make it appear as raw.
+@contextmanager
+def get_input_as_raw_file(input_file, input_format):
+if input_format == "raw":
+yield input_file
+return
+try:
+temp_dir = tempfile.mkdtemp()
+pid_file = temp_dir + "/pid"
+raw_file = temp_dir + "/raw"
+open(raw_file, "wb").close()
+ret = subprocess.run(
+[
+QEMU_STORAGE_DAEMON,
+"--daemonize",
+"--pidfile", pid_file,
+"--blockdev", 
f"driver=file,node-name=file0,driver=file,filename={input_file},read-only=on",
+"--blockdev", 
f"driver={input_format},node-name=disk0,file=file0,read-only=on",
+"--export", 
f"type=fuse,id=export0,node-name=disk0,mountpoint={raw_file},writable=off",
+],
+capture_output=True,
+)
+if ret.returncode != 0:
+sys.exit("[Error] Could not start the qemu-storage-

Re: [PATCH v2] scripts/qcow2-to-stdout.py: Add script to write qcow2 images to stdout

2024-07-26 Thread Alberto Garcia
ping

On Mon, Jul 01, 2024 at 05:11:40PM +0200, Alberto Garcia wrote:
> This tool converts a disk image to qcow2, writing the result directly
> to stdout. This can be used for example to send the generated file
> over the network.



[PATCH v2] scripts/qcow2-to-stdout.py: Add script to write qcow2 images to stdout

2024-07-01 Thread Alberto Garcia
This tool converts a disk image to qcow2, writing the result directly
to stdout. This can be used for example to send the generated file
over the network.

This is equivalent to using qemu-img to convert a file to qcow2 and
then writing the result to stdout, with the difference that this tool
does not need to create this temporary qcow2 file and therefore does
not need any additional disk space.

Implementing this directly in qemu-img is not really an option because
it expects the output file to be seekable and it is also meant to be a
generic tool that supports all combinations of file formats and image
options. Instead, this tool can only produce qcow2 files with the
basic options, without compression, encryption or other features.

The input file is read twice. The first pass is used to determine
which clusters contain non-zero data and that information is used to
create the qcow2 header, refcount table and blocks, and L1 and L2
tables. After all that metadata is created then the second pass is
used to write the guest data.

By default qcow2-to-stdout.py expects the input to be a raw file, but
if qemu-storage-daemon is available then it can also be used to read
images in other formats. Alternatively the user can also run qemu-ndb
or qemu-storage-daemon manually instead.

Signed-off-by: Alberto Garcia 
Signed-off-by: Madeeha Javed 
---
 scripts/qcow2-to-stdout.py | 377 +
 1 file changed, 377 insertions(+)
 create mode 100755 scripts/qcow2-to-stdout.py

v2:
- Define the QCOW2_V3_HDR_LENGTH and QCOW2_FEATURE_NAME_TABLE constants [Manos]
- Define the QEMU_STORAGE_DAEMON constant
- Use isfile() instead of exists() for the input file
- Refuse to write to stdout if it's a tty [Manos]
- Move the bulk of the code to a function called from __main__ [Manos]
- Remove the qcow2_ prefix from qcow2_cluster_size and qcow2_refcount_bits
- Formatting fixes suggested by the Python black formatter [Manos]
- On error pass the string directly to sys.exit()
- Capture the output of qemu-storage-daemon [Manos]
- Use a contextmanager to run qemu-storage-daemon [Manos]
- Update patch description to mention why this cannot be implemeted directly in 
qemu-img [Manos]

v1: https://lists.gnu.org/archive/html/qemu-block/2024-06/msg00073.html

diff --git a/scripts/qcow2-to-stdout.py b/scripts/qcow2-to-stdout.py
new file mode 100755
index 00..d486a80e86
--- /dev/null
+++ b/scripts/qcow2-to-stdout.py
@@ -0,0 +1,377 @@
+#!/usr/bin/env python3
+
+# This tool reads a disk image in any format and converts it to qcow2,
+# writing the result directly to stdout.
+#
+# Copyright (C) 2024 Igalia, S.L.
+#
+# Authors: Alberto Garcia 
+#  Madeeha Javed 
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# qcow2 files produced by this script are always arranged like this:
+#
+# - qcow2 header
+# - refcount table
+# - refcount blocks
+# - L1 table
+# - L2 tables
+# - Data clusters
+#
+# A note about variable names: in qcow2 there is one refcount table
+# and one (active) L1 table, although each can occupy several
+# clusters. For the sake of simplicity the code sometimes talks about
+# refcount tables and L1 tables when referring to those clusters.
+
+import argparse
+import atexit
+import math
+import os
+import signal
+import struct
+import subprocess
+import sys
+import tempfile
+import time
+from contextlib import contextmanager
+
+QCOW2_DEFAULT_CLUSTER_SIZE = 65536
+QCOW2_DEFAULT_REFCOUNT_BITS = 16
+QCOW2_DEFAULT_VERSION = 3
+QCOW2_FEATURE_NAME_TABLE = 0x6803F857
+QCOW2_V3_HEADER_LENGTH = 112  # Header length in QEMU 9.0. Must be a multiple 
of 8
+QCOW_OFLAG_COPIED = 1 << 63
+QEMU_STORAGE_DAEMON = "qemu-storage-daemon"
+
+
+def bitmap_set(bitmap, idx):
+bitmap[int(idx / 8)] |= 1 << (idx % 8)
+
+
+def bitmap_test(bitmap, idx):
+return (bitmap[int(idx / 8)] & (1 << (idx % 8))) != 0
+
+
+# create_qcow2_file() expects a raw input file. If we have a different
+# format we can use qemu-storage-daemon to make it appear as raw.
+@contextmanager
+def get_input_as_raw_file(input_file, input_format):
+if input_format == "raw":
+yield input_file
+return
+try:
+temp_dir = tempfile.mkdtemp()
+pid_file = temp_dir + "/pid"
+raw_file = temp_dir + "/raw"
+open(raw_file, "wb").close()
+ret = subprocess.run(
+[
+QEMU_STORAGE_DAEMON,
+"--daemonize",
+"--pidfile", pid_file,
+"--blockdev", 
f"driver=file,node-name=file0,driver=file,filename={input_file},read-only=on",
+"--blockdev", 
f"driver={input_format},node-name=disk0,file=file0,read-only=on",
+"--export", 
f"type=fuse,id=export0,node-name=disk0,mountpoint={raw_file},writable=off",
+],
+capture_output=True,
+)
+if ret.returncode != 0:
+sys.exit("[Error] Could not start the qemu-storage-daemon:\n" +
+ ret.stderr.dec