diff --git a/docs/images.md b/docs/images.md index 74ec19c1..af91de11 100644 --- a/docs/images.md +++ b/docs/images.md @@ -84,6 +84,25 @@ When using multiple writers, each writer should modify a distinct byte range. If two writers modify the same byte range concurrently they will overwrite each other data. +### transfer_format + +The transfer data format. Always available when using the `nbd` and +`memory` backends, not available when using the `file` backend. When +using the `http` backend `transfer_format` is available only if the +remote server reports it. + +Since 2.4.6. + +### virtual_size + +The underlying image virtual size. Available only when the backend is +using `raw` transfer format. Always available when using the `nbd` and +`memory` backends, not available when using the `file` backend. When +using the `http` backend the `virtual_size` is available only if the +remote server reports it. + +Since 2.4.6. + ### Errors Specific errors for OPTIONS request: @@ -112,9 +131,9 @@ Response: Content-Length: 122 {"unix_socket": "\u0000/org/ovirt/imageio", "features": ["extents", "zero", "flush"], - "max_readers": 8, "max_writers": 8} + "max_readers": 8, "max_writers": 8, "transfer_format": "raw", "virtual_size": 53687091200} -Get options for ticket-id with read-write access using nbd backend: +Get options for ticket-id with read-write access using `nbd` backend: $ curl -k -X OPTIONS https://server:54322/images/{ticket-id} | jq { @@ -125,11 +144,13 @@ Get options for ticket-id with read-write access using nbd backend: "flush" ], "max_readers": 8, - "max_writers": 8 + "max_writers": 8, + "transfer_format": "raw", + "virtual_size": 53687091200 } -The nbd backend is used when specifying the "raw" transfer format when -creating an image transfer in oVirt API. +The nbd backend is used when creating an image transfer with +format="raw" in oVirt API. Get options for ticket-id with read-only access using file backend: @@ -143,6 +164,9 @@ Get options for ticket-id with read-only access using file backend: "max_writers": 1 } +Note that `virtual_size` and `transfer_format` are not reported, and this +backend does not support multiple writers. + Get all available options for the special `*` ticket: $ curl -sk -X OPTIONS 'https://server:54322/images/*' | jq @@ -268,7 +292,7 @@ Since 2.0 Request zero extents: - GET /images/{ticket-id}/extent + GET /images/{ticket-id}/extents Response: @@ -280,7 +304,7 @@ Response: Request dirty extents: - GET /images/{ticket-id}/extent?context=dirty + GET /images/{ticket-id}/extents?context=dirty Response: diff --git a/ovirt_imageio/_internal/backends/file.py b/ovirt_imageio/_internal/backends/file.py index 03626c6e..66edaf24 100644 --- a/ovirt_imageio/_internal/backends/file.py +++ b/ovirt_imageio/_internal/backends/file.py @@ -74,6 +74,13 @@ def __init__(self, fio, sparse=False, max_connections=8): def max_readers(self): return self._max_connections + @property + def format(self): + """ + Currently we don't have access to the underlying disk format. + """ + return None + # io.FileIO interface def readinto(self, buf): diff --git a/ovirt_imageio/_internal/backends/http.py b/ovirt_imageio/_internal/backends/http.py index 93335a0d..3e6d109d 100644 --- a/ovirt_imageio/_internal/backends/http.py +++ b/ovirt_imageio/_internal/backends/http.py @@ -63,7 +63,6 @@ def __init__(self, url, cafile=None, secure=True, connect_timeout=10, self._connect_timeout = connect_timeout self._read_timeout = read_timeout self._position = 0 - self._size = None self._extents = {} # Initlized during connection. @@ -74,6 +73,8 @@ def __init__(self, url, cafile=None, secure=True, connect_timeout=10, self._can_flush = False self._max_readers = 1 self._max_writers = 1 + self._format = None + self._size = None if connect: self._connect() @@ -104,8 +105,10 @@ def clone(self): backend._can_flush = self._can_flush backend._max_readers = self._max_readers backend._max_writers = self._max_writers + backend._format = self._format - # Copy size and extents to save expensive EXTENTS calls. + # Copy extents and size to save expensive EXTENTS calls with old + # imageio servers. backend._size = self._size for ctx in list(self._extents): backend._extents[ctx] = self._extents[ctx].copy() @@ -133,6 +136,10 @@ def _connect(self): # max_writers does not support multiple writers. self._max_writers = options.get("max_writers", 1) + # Newer server reports also transfer_format and virtual_size. + self._format = options.get("transfer_format") + self._size = options.get("virtual_size") + self._optimize_connection(options.get("unix_socket")) except Exception: self._con.close() @@ -155,6 +162,14 @@ def max_readers(self): def max_writers(self): return self._max_writers + @property + def format(self): + """ + Will be None for old server (imageio < 2.4.6) or when server is using + file backend. + """ + return self._format + # Preferred interface. def read_from(self, reader, length, buf): @@ -325,10 +340,17 @@ def seek(self, n, how=os.SEEK_SET): return self._position def size(self): - # We have 2 bad options: - # - Get last extent, may be slow, and may not be neded otherwise. - # - Emulate HEAD request, logging tracebacks in the remote server. - # Getting extents is more polite, so lets use it if we can. + """ + Return backend size in bytes. + + With newer server (imageio >= 2.4.6) reporting the virtual size the + size is intialized in connect(). + + Otherwise we have 2 bad options: + - Get the last extent, may be slow, and may not be neded otherwise. + - Emulate HEAD request, logging tracebacks in the remote server. + Getting extents is more polite, so lets use it if we can. + """ if self._size is None: if self._can_extents: last = list(self.extents())[-1] @@ -454,7 +476,7 @@ def _put_header(self, length): self._con.putheader("content-length", length) self._con.putheader("content-type", "application/octet-stream") self._con.putheader("content-range", "bytes {}-{}/*".format( - self._position, self._position + length - 1)) + self._position, self._position + length - 1)) self._con.endheaders() diff --git a/ovirt_imageio/_internal/backends/memory.py b/ovirt_imageio/_internal/backends/memory.py index 9f54259d..e15e52ea 100644 --- a/ovirt_imageio/_internal/backends/memory.py +++ b/ovirt_imageio/_internal/backends/memory.py @@ -72,6 +72,10 @@ def max_writers(self): # support more than one writer concurrently. return 1 + @property + def format(self): + return "raw" + # io.BaseIO interface def readinto(self, buf): diff --git a/ovirt_imageio/_internal/backends/nbd.py b/ovirt_imageio/_internal/backends/nbd.py index 3ee683af..5fc156ca 100644 --- a/ovirt_imageio/_internal/backends/nbd.py +++ b/ovirt_imageio/_internal/backends/nbd.py @@ -95,6 +95,10 @@ def max_readers(self): def max_writers(self): return self._max_connections + @property + def format(self): + return "raw" + # Backend interface def readinto(self, buf): diff --git a/ovirt_imageio/_internal/handlers/images.py b/ovirt_imageio/_internal/handlers/images.py index cade4e5e..2f4f68ef 100644 --- a/ovirt_imageio/_internal/handlers/images.py +++ b/ovirt_imageio/_internal/handlers/images.py @@ -268,5 +268,11 @@ def options(self, req, resp, ticket_id): options["max_readers"] = ctx.backend.max_readers options["max_writers"] = ctx.backend.max_writers + # Optional backend options. + if ctx.backend.format is not None: + options["transfer_format"] = ctx.backend.format + if ctx.backend.format == "raw": + options["virtual_size"] = ctx.backend.size() + resp.headers["allow"] = ",".join(allow) resp.send_json(options) diff --git a/test/handlers/images_test.py b/test/handlers/images_test.py index f00875a5..d38325bc 100644 --- a/test/handlers/images_test.py +++ b/test/handlers/images_test.py @@ -14,7 +14,9 @@ import pytest from ovirt_imageio._internal import config +from ovirt_imageio._internal import qemu_img from ovirt_imageio._internal import server +from ovirt_imageio._internal.units import KiB, MiB, GiB from .. import testutil from .. import http @@ -236,7 +238,7 @@ def test_upload_invalid_range(tmpdir, srv, client, content_range): def test_upload_close_connection(tmpdir, srv, client): - image_size = 4096 + image_size = 4 * KiB image = testutil.create_tempfile(tmpdir, "image", b"a" * image_size) ticket = testutil.create_ticket( url="file://" + str(image), size=image_size) @@ -296,24 +298,24 @@ def test_download(tmpdir, srv, client, rng, start, end): def test_download_image_size_gt_ticket_size(tmpdir, srv, client): image = testutil.create_tempfile(tmpdir, "image", size=8192) - ticket = testutil.create_ticket(url="file://" + str(image), size=4096) + ticket = testutil.create_ticket(url="file://" + str(image), size=4 * KiB) srv.auth.add(ticket) res = client.get("/images/" + ticket["uuid"]) assert res.status == 200 - assert len(res.read()) == 4096 + assert len(res.read()) == 4 * KiB def test_download_ticket_size_gt_image_size(tmpdir, srv, client): - image = testutil.create_tempfile(tmpdir, "image", size=4096) + image = testutil.create_tempfile(tmpdir, "image", size=4 * KiB) ticket = testutil.create_ticket(url="file://" + str(image), size=8192) srv.auth.add(ticket) res = client.get("/images/" + ticket["uuid"]) assert res.status == 200 - assert len(res.read()) == 4096 + assert len(res.read()) == 4 * KiB def test_download_range_forbidden(tmpdir, srv, client): - image = testutil.create_tempfile(tmpdir, "image", size=4096) + image = testutil.create_tempfile(tmpdir, "image", size=4 * KiB) ticket = testutil.create_ticket(url="file://" + str(image), size=8192) srv.auth.add(ticket) res = client.get("/images/" + ticket["uuid"], @@ -324,7 +326,7 @@ def test_download_range_forbidden(tmpdir, srv, client): def test_download_range_unavailable(tmpdir, srv, client): image = testutil.create_tempfile(tmpdir, "image", size=8192) - ticket = testutil.create_ticket(url="file://" + str(image), size=4096) + ticket = testutil.create_ticket(url="file://" + str(image), size=4 * KiB) srv.auth.add(ticket) res = client.get("/images/" + ticket["uuid"], headers={"Range": "bytes=0-4096"}) @@ -333,7 +335,7 @@ def test_download_range_unavailable(tmpdir, srv, client): def test_download_no_range(tmpdir, srv, client): - size = 1024 + size = KiB image = testutil.create_tempfile(tmpdir, "image", size=size) ticket = testutil.create_ticket(url="file://" + str(image), size=size) srv.auth.add(ticket) @@ -344,7 +346,7 @@ def test_download_no_range(tmpdir, srv, client): def test_download_extends_ticket(tmpdir, srv, client, fake_time): - size = 1024 + size = KiB image = testutil.create_tempfile(tmpdir, "image", size=size) ticket = testutil.create_ticket(url="file://" + str(image), size=size) srv.auth.add(ticket) @@ -381,7 +383,7 @@ def test_download_partial_not_satistieble(tmpdir, srv, client): # actual image size reported by vdsm - one byte difference is enough to # cause a failure. # See https://bugzilla.redhat.com/1512315. - size = 1024 + size = KiB image = testutil.create_tempfile(tmpdir, "image", size=size) ticket = testutil.create_ticket(url="file://" + str(image), size=size + 1) srv.auth.add(ticket) @@ -397,7 +399,7 @@ def test_download_partial_no_range(tmpdir, srv, client): # is only an upper limit. Or maybe we should treat the ticket size as the # expected size? # This is another variant of https://bugzilla.redhat.com/1512315. - size = 1024 + size = KiB image = testutil.create_tempfile(tmpdir, "image", size=size) ticket = testutil.create_ticket(url="file://" + str(image), size=size + 1) srv.auth.add(ticket) @@ -405,7 +407,7 @@ def test_download_partial_no_range(tmpdir, srv, client): assert res.status == http_client.OK # Should return the available image data, not the ticket size. Reading # this response will fail with IncompleteRead. - assert res.length == 1024 + assert res.length == KiB def test_download_partial_no_range_empty(tmpdir, srv, client): @@ -413,7 +415,7 @@ def test_download_partial_no_range_empty(tmpdir, srv, client): # http response that fail on the client side with BadStatusLine: ''. # See https://bugzilla.redhat.com/1512312 image = testutil.create_tempfile(tmpdir, "image") # Empty image - ticket = testutil.create_ticket(url="file://" + str(image), size=1024) + ticket = testutil.create_ticket(url="file://" + str(image), size=KiB) srv.auth.add(ticket) res = client.get("/images/" + ticket["uuid"]) assert res.status == http_client.OK @@ -421,7 +423,7 @@ def test_download_partial_no_range_empty(tmpdir, srv, client): def test_download_no_range_end(tmpdir, srv, client): - size = 1024 + size = KiB image = testutil.create_tempfile(tmpdir, "image", size=size) ticket = testutil.create_ticket(url="file://" + str(image), size=size) srv.auth.add(ticket) @@ -433,7 +435,7 @@ def test_download_no_range_end(tmpdir, srv, client): def test_download_holes(tmpdir, srv, client): - size = 1024 + size = KiB image = testutil.create_tempfile(tmpdir, "image", size=size) ticket = testutil.create_ticket(url="file://" + str(image), size=size) srv.auth.add(ticket) @@ -445,7 +447,7 @@ def test_download_holes(tmpdir, srv, client): def test_download_filename_in_ticket(tmpdir, srv, client): - size = 1024 + size = KiB filename = "\u05d0.raw" # hebrew aleph image = testutil.create_tempfile(tmpdir, "image", size=size) ticket = testutil.create_ticket(url="file://" + str(image), size=size, @@ -472,7 +474,7 @@ def test_download_out_of_range(tmpdir, srv, client, rng, end): def test_download_progress(tmpdir, srv, client, monkeypatch): # We need to read at least one buffer to update the transfered value. - monkeypatch.setattr(srv.config.backend_file, "buffer_size", 1024**2) + monkeypatch.setattr(srv.config.backend_file, "buffer_size", MiB) # And we need to request enough data so the server does not complete before # the client read all the data. @@ -511,7 +513,7 @@ def test_download_progress(tmpdir, srv, client, monkeypatch): def test_download_close_connection(tmpdir, srv, client): - image_size = 4096 + image_size = 4 * KiB image = testutil.create_tempfile(tmpdir, "image", b"a" * image_size) ticket = testutil.create_ticket( url="file://" + str(image), size=image_size) @@ -704,8 +706,8 @@ def test_options_all(srv, client): assert "max_writers" not in options -def test_options_read_write(srv, client, tmpdir): - size = 128 * 1024 +def test_options_file_read_write(srv, client, tmpdir): + size = 128 * KiB image = testutil.create_tempfile(tmpdir, "image", size=size) ticket = testutil.create_ticket( url="file://" + str(image), size=size, ops=["read", "write"]) @@ -716,12 +718,16 @@ def test_options_read_write(srv, client, tmpdir): assert set(res.getheader("allow").split(',')) == allows options = json.loads(res.read()) assert set(options["features"]) == ALL_FEATURES + + # File backend specific options. assert options["max_readers"] == srv.config.daemon.max_connections - assert options["max_writers"] == 1 # Using file backend. + assert options["max_writers"] == 1 + assert "transfer_format" not in options + assert "virtual_size" not in options -def test_options_read(srv, client, tmpdir): - size = 128 * 1024 +def test_options_file_read(srv, client, tmpdir): + size = 128 * KiB image = testutil.create_tempfile(tmpdir, "image", size=size) ticket = testutil.create_ticket( url="file://" + str(image), size=size, ops=["read"]) @@ -732,12 +738,16 @@ def test_options_read(srv, client, tmpdir): assert set(res.getheader("allow").split(',')) == allows options = json.loads(res.read()) assert set(options["features"]) == BASE_FEATURES + + # File backend specific options. assert options["max_readers"] == srv.config.daemon.max_connections - assert options["max_writers"] == 1 # Using file backend. + assert options["max_writers"] == 1 + assert "transfer_format" not in options + assert "virtual_size" not in options -def test_options_write(srv, client, tmpdir): - size = 128 * 1024 +def test_options_file_write(srv, client, tmpdir): + size = 128 * KiB image = testutil.create_tempfile(tmpdir, "image", size=size) ticket = testutil.create_ticket( url="file://" + str(image), size=size, ops=["write"]) @@ -749,12 +759,81 @@ def test_options_write(srv, client, tmpdir): assert set(res.getheader("allow").split(',')) == allows options = json.loads(res.read()) assert set(options["features"]) == ALL_FEATURES + + # File backend specific options. + assert options["max_readers"] == srv.config.daemon.max_connections + assert options["max_writers"] == 1 + assert "transfer_format" not in options + assert "virtual_size" not in options + + +@pytest.mark.parametrize("fmt", ["raw", "qcow2"]) +def test_options_nbd_read(srv, client, tmpdir, nbd_server, fmt): + # Create disk. + size = GiB + disk = str(tmpdir.join(f"disk.{fmt}")) + qemu_img.create(disk, fmt, size=size) + + # Start nbd server exporting the disk. + nbd_server.image = disk + nbd_server.fmt = fmt + nbd_server.read_only = True + nbd_server.start() + + # Add ticket using nbd server url. + ticket = testutil.create_ticket( + url=nbd_server.sock.url(), size=size, ops=["read"]) + srv.auth.add(ticket) + + # Get OPTIONS. + res = client.options("/images/" + ticket["uuid"]) + allows = {"OPTIONS", "GET"} + assert res.status == 200 + assert set(res.getheader("allow").split(',')) == allows + options = json.loads(res.read()) + assert set(options["features"]) == BASE_FEATURES + + # NBD backend specific options. + assert options["max_readers"] == srv.config.daemon.max_connections + assert options["max_writers"] == srv.config.daemon.max_connections + assert options["transfer_format"] == "raw" + assert options["virtual_size"] == size + + +@pytest.mark.parametrize("fmt", ["raw", "qcow2"]) +def test_options_nbd_write(srv, client, tmpdir, nbd_server, fmt): + # Create disk. + size = GiB + disk = str(tmpdir.join(f"disk.{fmt}")) + qemu_img.create(disk, fmt, size=size) + + # Start nbd server exporting the disk. + nbd_server.image = disk + nbd_server.fmt = fmt + nbd_server.start() + + # Add ticket using nbd server url. + ticket = testutil.create_ticket( + url=nbd_server.sock.url(), size=size, ops=["write"]) + srv.auth.add(ticket) + + # Get OPTIONS. + res = client.options("/images/" + ticket["uuid"]) + allows = {"OPTIONS", "GET", "PUT", "PATCH"} + assert res.status == 200 + assert set(res.getheader("allow").split(',')) == allows + options = json.loads(res.read()) + assert set(options["features"]) == ALL_FEATURES + + # NBD backend specific options. assert options["max_readers"] == srv.config.daemon.max_connections - assert options["max_writers"] == 1 # Using file backend. + assert options["max_writers"] == srv.config.daemon.max_connections + assert options["transfer_format"] == "raw" + assert options["virtual_size"] == size def test_options_extends_ticket(srv, client, tmpdir, fake_time): - size = 128 * 1024 + size = 128 * KiB image = testutil.create_tempfile(tmpdir, "image", size=size) ticket = testutil.create_ticket(url="file://" + str(image), size=size) srv.auth.add(ticket) @@ -786,7 +865,7 @@ def test_options_for_nonexistent_ticket(srv, client): def test_options_ticket_expired(srv, client, tmpdir, fake_time): - size = 128 * 1024 + size = 128 * KiB image = testutil.create_tempfile(tmpdir, "image", size=size) ticket = testutil.create_ticket( url="file://" + str(image), size=size, timeout=300) @@ -828,9 +907,9 @@ def test_response_version_error(tmpdir, srv, client): ]) def test_keep_alive_connection_on_success(tmpdir, srv, client, method, body): # After successful request the connection should remain open. - image = testutil.create_tempfile(tmpdir, "image", size=1024) + image = testutil.create_tempfile(tmpdir, "image", size=KiB) ticket = testutil.create_ticket(url="file://" + str(image), - size=1024) + size=KiB) srv.auth.add(ticket) uri = "/images/%(uuid)s" % ticket # Disabling auto_open so we can test if a connection was closed. @@ -915,7 +994,7 @@ def test_cors_options_all(srv, client): def test_cors_get_ok(tmpdir, srv, client): - size = 8192 + size = 8 * KiB image = testutil.create_tempfile(tmpdir, "image", size=size) ticket = testutil.create_ticket(url="file://" + str(image), size=size) srv.auth.add(ticket) @@ -944,7 +1023,7 @@ def test_cors_get_error(srv, client): def test_cors_put_ok(tmpdir, srv, client): - size = 8192 + size = 8 * KiB image = testutil.create_tempfile(tmpdir, "image", size=size) ticket = testutil.create_ticket(url="file://" + str(image), size=size) srv.auth.add(ticket)