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

usb privileged version #11

Merged
merged 5 commits into from
Apr 4, 2016

Conversation

MichalCz
Copy link
Contributor

Hey,

Perhaps you would be interested in a version that allows usb debugging and connecting an actual device to the docker container.

There's a security drawback - the container must run in privileged mode, which mean it has actual root access and it can't be that trusted. That's why I'm proposing this PR separately to another one I'm pushing simultaneously. If you have an idea on some kind of a switch I could add it later.

Best,
Mike

@MichalCz
Copy link
Contributor Author

MichalCz commented Apr 1, 2016

Hey, I'll be developing a little bit here - it seems not running fine any more with screen.

@stucki
Copy link
Owner

stucki commented Apr 1, 2016

Did you know that it's possible to run adb over the network?

# On the host, run adb in daemon mode:
sudo adb -a fork-server server

# Inside the container, set an alias for adb (adb is used from the build path)
alias adb='/home/cmbuild/android/out/host/linux-x86/bin/adb -H <ip of your host>'

# Test:
adb devices

Works fine for me without any modifications to the Docker image.

However, we could still let users run Docker in privileged mode and forward the USB device. Ok with me, however this should be configurable right from the start. Can you update the PR please?

@stucki
Copy link
Owner

stucki commented Apr 1, 2016

Besides this: The tests in front of mkdir don't belong to this change, do they?

@MichalCz
Copy link
Contributor Author

MichalCz commented Apr 4, 2016

Hey, yes I know you can run adb over network, but that means you need to have adb somehow installed on the host - the beauty of your docker image is that you needn't do that.

Anyway I added a -p / --privileged switch for this. I must check what makes screen not working, so this is not merge-able yet.

@stucki
Copy link
Owner

stucki commented Apr 4, 2016

How about calling the flag --enable-usb? As a user, I don't need to know the relation with Dockers privileged mode, so this is otherwise not self-explaining.

Regarding the screen check: How about you're just keeping the line like it was before? No matter what, it's not related to this change, so it should be move into a different PR anyway. And then, this one could be merged right away...

@MichalCz
Copy link
Contributor Author

MichalCz commented Apr 4, 2016

How about this? :)

@stucki stucki merged commit cab01da into stucki:master Apr 4, 2016
@stucki
Copy link
Owner

stucki commented Apr 4, 2016

Thank you!

@MichalCz
Copy link
Contributor Author

MichalCz commented Apr 4, 2016

Ur welcome. :)

I have a branch that fixes docker #8755, but I don't think this should be merged to master - docker or ubuntu would fix that sooner or later. For now if anybody needs it, just cherry-pick my commit 5b769a7c.

@stucki
Copy link
Owner

stucki commented Apr 5, 2016

Can you please take a look at 6b6aba0 + cfd6174
I remember that I fixed a thing like this in the past, however I thought it was fixed, couldn't reproduce this with my Ubuntu 14.04.
In any case it would be good to create a new ticket if this is still needed. Therefore, I'm closing here.

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