Skip to content

Commit

Permalink
refactor(block): block update methods return results
Browse files Browse the repository at this point in the history
Now `update_rate_limiter` returns an error if
it is incorrectly called on the vhost-user-block.
Additionally error formatting has been updated to
use `Display`.

Signed-off-by: Egor Lazarchuk <[email protected]>
  • Loading branch information
ShadowCurse committed Feb 9, 2024
1 parent 3b9bd9f commit 684930f
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 19 deletions.
21 changes: 13 additions & 8 deletions src/vmm/src/devices/virtio/block/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,24 @@ impl Block {
}
}

Check warning on line 59 in src/vmm/src/devices/virtio/block/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/device.rs#L59

Added line #L59 was not covered by tests

pub fn update_rate_limiter(&mut self, bytes: BucketUpdate, ops: BucketUpdate) {
pub fn update_rate_limiter(
&mut self,
bytes: BucketUpdate,
ops: BucketUpdate,
) -> Result<(), BlockError> {
match self {
Self::Virtio(b) => b.update_rate_limiter(bytes, ops),
Self::VhostUser(_) => {}
Self::Virtio(b) => {
b.update_rate_limiter(bytes, ops);
Ok(())

Check warning on line 69 in src/vmm/src/devices/virtio/block/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/device.rs#L61-L69

Added lines #L61 - L69 were not covered by tests
}
Self::VhostUser(_) => Err(BlockError::InvalidBlockBackend),

Check warning on line 71 in src/vmm/src/devices/virtio/block/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/device.rs#L71

Added line #L71 was not covered by tests
}
}

Check warning on line 73 in src/vmm/src/devices/virtio/block/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/device.rs#L73

Added line #L73 was not covered by tests

pub fn update_config(&mut self) -> Result<(), BlockError> {
match self.backend {
BlockBackend::Virtio(_) => Err(BlockError::InvalidBlockBackend),
BlockBackend::VhostUser(ref mut b) => {
b.config_update().map_err(BlockError::VhostUserBackend)
}
match self {
Self::Virtio(_) => Err(BlockError::InvalidBlockBackend),
Self::VhostUser(b) => b.config_update().map_err(BlockError::VhostUserBackend),

Check warning on line 78 in src/vmm/src/devices/virtio/block/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/device.rs#L75-L78

Added lines #L75 - L78 were not covered by tests
}
}

Check warning on line 80 in src/vmm/src/devices/virtio/block/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/device.rs#L80

Added line #L80 was not covered by tests

Expand Down
9 changes: 5 additions & 4 deletions src/vmm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ impl Vmm {
.with_virtio_device_with_id(TYPE_BLOCK, drive_id, |block: &mut Block| {

Check warning on line 622 in src/vmm/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/lib.rs#L622

Added line #L622 was not covered by tests
block
.update_disk_image(path_on_host)
.map_err(|err| format!("{:?}", err))
.map_err(|err| err.to_string())

Check warning on line 625 in src/vmm/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/lib.rs#L625

Added line #L625 was not covered by tests
})
.map_err(VmmError::DeviceManager)
}
Expand All @@ -636,8 +636,9 @@ impl Vmm {
) -> Result<(), VmmError> {
self.mmio_device_manager
.with_virtio_device_with_id(TYPE_BLOCK, drive_id, |block: &mut Block| {
block.update_rate_limiter(rl_bytes, rl_ops);
Ok(())
block
.update_rate_limiter(rl_bytes, rl_ops)
.map_err(|err| err.to_string())

Check warning on line 641 in src/vmm/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/lib.rs#L638-L641

Added lines #L638 - L641 were not covered by tests
})
.map_err(VmmError::DeviceManager)
}
Expand All @@ -646,7 +647,7 @@ impl Vmm {
pub fn update_vhost_user_block_config(&mut self, drive_id: &str) -> Result<(), VmmError> {
self.mmio_device_manager
.with_virtio_device_with_id(TYPE_BLOCK, drive_id, |block: &mut Block| {
block.update_config().map_err(|err| format!("{:?}", err))
block.update_config().map_err(|err| err.to_string())

Check warning on line 650 in src/vmm/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/lib.rs#L649-L650

Added lines #L649 - L650 were not covered by tests
})
.map_err(VmmError::DeviceManager)
}
Expand Down
9 changes: 2 additions & 7 deletions tests/integration_tests/functional/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -804,20 +804,18 @@ def test_send_ctrl_alt_del(test_microvm_with_api):
def _drive_patch(test_microvm):
"""Exercise drive patch test scenarios."""
# Patches without mandatory fields for virtio block are not allowed.
expected_msg = "Invalid device type found on the MMIO bus. Please verify the request arguments."
expected_msg = "Unable to patch the block device: Device manager error: Running method expected different backend. Please verify the request arguments"
with pytest.raises(RuntimeError, match=expected_msg):
test_microvm.api.drive.patch(drive_id="scratch")

# Patches with any fields for vhost-user block are not allowed.
expected_msg = "Invalid device type found on the MMIO bus. Please verify the request arguments."
with pytest.raises(RuntimeError, match=expected_msg):
test_microvm.api.drive.patch(
drive_id="scratch_vub",
path_on_host="some_path",
)

# Patches with any fields for vhost-user block are not allowed.
expected_msg = "Invalid device type found on the MMIO bus. Please verify the request arguments."
with pytest.raises(RuntimeError, match=expected_msg):
test_microvm.api.drive.patch(
drive_id="scratch_vub",
Expand Down Expand Up @@ -848,10 +846,7 @@ def _drive_patch(test_microvm):
)

# Updates to `path_on_host` with an invalid path are not allowed.
expected_msg = (
"Unable to patch the block device: Device manager error: BackingFile(Os { code: 2, "
f'kind: NotFound, message: "No such file or directory" }}, "{drive_path}") Please verify the request arguments.'
)
expected_msg = f"Unable to patch the block device: Device manager error: Virtio backend error: Error manipulating the backing file: No such file or directory (os error 2) {drive_path} Please verify the request arguments"
with pytest.raises(RuntimeError, match=re.escape(expected_msg)):
test_microvm.api.drive.patch(drive_id="scratch", path_on_host=drive_path)

Expand Down

0 comments on commit 684930f

Please sign in to comment.