Skip to content

Commit

Permalink
options: Report image format and virtual size
Browse files Browse the repository at this point in the history
Report image virtual size in OPTIONS so clients can get the image size
without a possibly slow extent call.

Report the format since OPTIONS can report the virtual size only when
the backend provides raw format. This is used when using the http
backend to report OPTIONS to the client.

Reporting virtual size is easy with the nbd and memory backends since
they always use raw format.

When using file backend and qcow2 image, we don't have access to the
image virtual size, and this size is not helpful to the user uploading
or downloading data. Currently we don't know about the image format
since engine does not report it in the ticket.

The http backend reports the info from the remote server, so it depends
on the backend used by the remote server, and on having new server
reporting the format and size.

To keep code and the API simple, we report virtual size only when using
the nbd and memory backends. When engine will report the image format
for the file backend, we can also report the size for raw images access
via the file backend.

Change-Id: I89118301c98dc2d11c25a4d1e7ef83df26336f01
Related: #67
Bug-Url: https://bugzilla.redhat.com/1924945
Signed-off-by: Nir Soffer <[email protected]>
  • Loading branch information
nirs authored and aesteve-rh committed Aug 5, 2022
1 parent 42ba935 commit d5df688
Show file tree
Hide file tree
Showing 7 changed files with 164 additions and 19 deletions.
34 changes: 29 additions & 5 deletions docs/images.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
{
Expand All @@ -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:

Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions ovirt_imageio/_internal/backends/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
36 changes: 29 additions & 7 deletions ovirt_imageio/_internal/backends/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand All @@ -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):
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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()

Expand Down
4 changes: 4 additions & 0 deletions ovirt_imageio/_internal/backends/memory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
4 changes: 4 additions & 0 deletions ovirt_imageio/_internal/backends/nbd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
6 changes: 6 additions & 0 deletions ovirt_imageio/_internal/handlers/images.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
92 changes: 85 additions & 7 deletions test/handlers/images_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +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
from ovirt_imageio._internal.units import KiB, MiB, GiB

from .. import testutil
from .. import http
Expand Down Expand Up @@ -705,7 +706,7 @@ def test_options_all(srv, client):
assert "max_writers" not in options


def test_options_read_write(srv, client, tmpdir):
def test_options_file_read_write(srv, client, tmpdir):
size = 128 * KiB
image = testutil.create_tempfile(tmpdir, "image", size=size)
ticket = testutil.create_ticket(
Expand All @@ -717,11 +718,15 @@ 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):
def test_options_file_read(srv, client, tmpdir):
size = 128 * KiB
image = testutil.create_tempfile(tmpdir, "image", size=size)
ticket = testutil.create_ticket(
Expand All @@ -733,11 +738,15 @@ 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):
def test_options_file_write(srv, client, tmpdir):
size = 128 * KiB
image = testutil.create_tempfile(tmpdir, "image", size=size)
ticket = testutil.create_ticket(
Expand All @@ -750,8 +759,77 @@ 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):
Expand Down

0 comments on commit d5df688

Please sign in to comment.