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

allow access to xen block devices other than the first #874

Merged
merged 1 commit into from Dec 17, 2017

Conversation

Projects
None yet
3 participants
@yomimono
Member

yomimono commented Dec 4, 2017

The logic for the Xen and Qubes cases is redundant with logic in mirage-block-xen's connect module: see https://github.com/mirage/mirage-block-xen/blob/master/lib/blkfront.ml#L488 .

@yomimono

This comment has been minimized.

Show comment
Hide comment
@yomimono

yomimono Dec 4, 2017

Member

/cc @cfcs , who discovered this.

Member

yomimono commented Dec 4, 2017

/cc @cfcs , who discovered this.

@cfcs

This comment has been minimized.

Show comment
Hide comment
@cfcs

cfcs Dec 5, 2017

Thank you so much!

cfcs commented Dec 5, 2017

Thank you so much!

@djs55

This comment has been minimized.

Show comment
Hide comment
@djs55

djs55 Dec 6, 2017

Member

+1 on removing the knowledge about Xenstore encoding of Linux /dev/xvd device names -- no-one should have to know about that scheme any more :)

Just checking I understand (I'm a bit rusty): the xl create foo.cfg write a backend key params=<filename> for each disk and then we give <filename> to the frontend? So we use <filename> as a unique id?

Member

djs55 commented Dec 6, 2017

+1 on removing the knowledge about Xenstore encoding of Linux /dev/xvd device names -- no-one should have to know about that scheme any more :)

Just checking I understand (I'm a bit rusty): the xl create foo.cfg write a backend key params=<filename> for each disk and then we give <filename> to the frontend? So we use <filename> as a unique id?

@yomimono

This comment has been minimized.

Show comment
Hide comment
@yomimono

yomimono Dec 8, 2017

Member

@djs55 Yeah, I'm a little rusty on this too :( but now is a great time to get familiar with it again, since there's so much exciting storage stuff going on.

For the -t xen case, mirage configure makes the foo.xl for unikernel foo, and there's some additional transformation done on the given strings before making the .xl file - see https://github.com/mirage/mirage/blob/master/lib/mirage.ml#L1559 for the exact logic.

Member

yomimono commented Dec 8, 2017

@djs55 Yeah, I'm a little rusty on this too :( but now is a great time to get familiar with it again, since there's so much exciting storage stuff going on.

For the -t xen case, mirage configure makes the foo.xl for unikernel foo, and there's some additional transformation done on the given strings before making the .xl file - see https://github.com/mirage/mirage/blob/master/lib/mirage.ml#L1559 for the exact logic.

@yomimono

This comment has been minimized.

Show comment
Hide comment
@yomimono

yomimono Dec 17, 2017

Member

@djs55 can you merge, comment, or close (or tag someone else in), please? I'd like to cut a point release with this bugfix.

Member

yomimono commented Dec 17, 2017

@djs55 can you merge, comment, or close (or tag someone else in), please? I'd like to cut a point release with this bugfix.

@djs55

This comment has been minimized.

Show comment
Hide comment
@djs55

djs55 Dec 17, 2017

Member

Sure, I think this is fine.

Member

djs55 commented Dec 17, 2017

Sure, I think this is fine.

@djs55 djs55 merged commit 0983925 into mirage:master Dec 17, 2017

1 of 3 checks passed

ci/dockercloud Your tests failed in Docker Cloud
Details
ci/dockercloud (/Dockerfile.doc) Your tests failed in Docker Cloud
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@yomimono

This comment has been minimized.

Show comment
Hide comment
@yomimono

yomimono Dec 17, 2017

Member

Thanks, @djs55 ! :)

Member

yomimono commented Dec 17, 2017

Thanks, @djs55 ! :)

cfcs pushed a commit to cfcs/mirage that referenced this pull request Jan 10, 2018

Your Name
TODO would be nice to upstream a proper patch, but the recent changes…
… broke my local patchset that I would like to work on today.

this patch makes me able to specify xen devices on qubes again (after the fix was rolled back since it conflicted with something else... if only we had a CI system).
relevant discussion:
- mirage#879
- mirage#878
- mirage#874
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment