Skip to content

RFC: Error proposal#166

Merged
yomimono merged 5 commits intomirage:masterfrom
hannesm:network-error
Oct 20, 2016
Merged

RFC: Error proposal#166
yomimono merged 5 commits intomirage:masterfrom
hannesm:network-error

Conversation

@hannesm
Copy link
Copy Markdown
Member

@hannesm hannesm commented Oct 6, 2016 •
edited
Loading

I started off with V1.NETWORK (see mirage/mirage#615)

  • It now uses result (in listen, write, and wrtev)
  • the error and stat types are provided externally
  • there is pretty printer module (M_pp) shipped as mirage-types.pp) providing pp_network_error atm (to be extended)

TCPIP is the only consumer of NETWORK and should certainly pass the result up the layer (and do other useful things (such as network interfaces set t.listen <- false to stop the listen loop), in order to do so, we need to adjust mirage-types further.

The error handling in tcpip/tests needs to be adjusted as well (pass it up the layer). I got it up to the point that the tests compile and pass :)

I fixed:

possibly missing some network devices?

@hannesm
Copy link
Copy Markdown
Member Author

hannesm commented Oct 6, 2016 •
edited
Loading

@samoht @avsm @talex5 @yomimono opinions? others?

travis (apart from xen) is happy https://travis-ci.org/mirage/mirage-dev/builds/165648953

@hannesm hannesm mentioned this pull request Oct 6, 2016
Closed
9 tasks
@samoht
Copy link
Copy Markdown
Member

samoht commented Oct 6, 2016

I cannot comment directly on the diffs as you didn't open a PR, but why did you create a subpackage for pretty-printing? It should just be available in the main package, right?

This was referenced Oct 6, 2016
Merged
Merged
Merged
Merged
Merged
Merged
@hannesm
Copy link
Copy Markdown
Member Author

hannesm commented Oct 6, 2016

@samoht lacking knowledge how to get it in mirage-type which doesn't have an mllib file. I opened PR and linked to them above

@samoht
Copy link
Copy Markdown
Member

samoht commented Oct 6, 2016

Yes, should probably be in an mllib file too, which propably depends on topkg-ing the repo first. Thanks for opening the PRs.

@hannesm
Copy link
Copy Markdown
Member Author

hannesm commented Oct 6, 2016

@samoht mirage (and mirage-types) is topkg'ed :D @avsm finished and merged your patches

@hannesm
Copy link
Copy Markdown
Member Author

hannesm commented Oct 6, 2016

I'm dissatisfied with the control flow duplication in mirage-net-unix and mirage-net-solo5...

@hannesm hannesm mentioned this pull request Oct 6, 2016
Merged
@hannesm
Copy link
Copy Markdown
Member Author

hannesm commented Oct 6, 2016

Stats from mirage-net-xen should likely be in mirage-types as well

@yomimono yomimono mentioned this pull request Oct 14, 2016
Closed
@yomimono
Copy link
Copy Markdown
Contributor

yomimono commented Oct 20, 2016

Everything's been merged now. I've updated this PR to remove the pointers to the network-error branches accordingly. There are still a couple of updates (removing mirage dependency on mirage-types-lwt, pointing mirage-vnetif at current master) which I'll merge when green.

@yomimono yomimono merged commit f2a5870 into mirage:master Oct 20, 2016
@hannesm hannesm deleted the network-error branch October 20, 2016 22:02
avsm pushed a commit to avsm/mirage-dev that referenced this pull request Jan 22, 2017
@talex5
Merge pull request mirage#166 from samoht/master
c7aa14b 
Improve caching on Dockerfile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hannesm @samoht @yomimono