Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

virtio-blk: implement DISCARD #272

Merged
merged 2 commits into from
Mar 9, 2020
Merged

Conversation

djs55
Copy link
Collaborator

@djs55 djs55 commented Feb 21, 2020

The protocol defines DISCARD in v1.1:

https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.pdf

In this simple implementation we advertise a maximum of 1 segment per request, which fits the backend API block_delete(_, offset, length). Experimentally Linux 5.4.9 seems to send one request at a time (tested by deleting files and then executing fstrim)

Note we only advertise the feature if the backing file itself supports it.

Switching from AHCI virtio-blk works around the known deadlock in the AHCI implementation #94

Signed-off-by: David Scott [email protected]

@djs55 djs55 force-pushed the virtio-blk-upstream branch from f0b688c to 6d0f4d1 Compare February 23, 2020 10:34
@@ -340,8 +396,16 @@ pci_vtblk_init(struct pci_devinst *pi, char *opts)

pthread_mutex_init(&sc->vsc_mtx, NULL);

/* Customise the capabilities per-device */
vi_consts = (struct virtio_consts*)malloc(sizeof(struct virtio_consts));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not testing for malloc failure here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh yes good point. I've added and force-pushed a NULL check on the next line ⬇️

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. There is a calloc a few lines above also unchecked. But that’s unrelated to this PR

@djs55 djs55 force-pushed the virtio-blk-upstream branch from 11dae8f to e90eb74 Compare March 6, 2020 11:43
The protocol defines DISCARD in v1.1:

https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.pdf

In this simple implementation we advertise a maximum of 1 segment per request,
which fits the backend API `block_delete(_, offset, length)`. Experimentally
Linux 5.4.9 seems to send one request at a time (tested by deleting files and
then executing `fstrim`)

Note we only advertise the feature if the backing file itself supports it.

Signed-off-by: David Scott <[email protected]>
@djs55 djs55 force-pushed the virtio-blk-upstream branch from e90eb74 to de9ab9a Compare March 6, 2020 12:00
@djs55 djs55 force-pushed the virtio-blk-upstream branch from 40d3953 to 63de6b5 Compare March 7, 2020 14:04
@djs55
Copy link
Collaborator Author

djs55 commented Mar 9, 2020

Thanks!

@djs55 djs55 merged commit d7ee35d into moby:master Mar 9, 2020
@djs55 djs55 deleted the virtio-blk-upstream branch March 9, 2020 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants