newContainer: Not attach device if it is a CDROM

[teawater]

Got "docker: Error response from daemon: OCI runtime create failed:
QMP command failed: unknown." when "docker run --privileged" with kata.
In qemu part, it got:
"Could not open '/dev/sr0': Read-only file system"
or
"No medium found"
The cause is qemu need open block device to get its status.
But /dev/sr0 is a CDROM that cannot be opened.

This patch let newContainer doesn't attach device if it is a CDROM
to handle the issue.

Fixes #829

Signed-off-by: Hui Zhu [email protected]

[caoruidong]

/test

[jodh-intel]

Hi @teawater - thanks for raising. I'm a little confused though - we don't support --privileged:

• https://github.com/kata-containers/documentation/blob/master/Limitations.md#docker-run---privileged

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@d895cd0). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master     #828   +/-   ##
=========================================
  Coverage          ?   66.07%           
=========================================
  Files             ?       87           
  Lines             ?    10507           
  Branches          ?        0           
=========================================
  Hits              ?     6943           
  Misses            ?     2864           
  Partials          ?      700

[lifupan]

Hi @jodh-intel, can we give a container right to access the devices in the guest using --privileged ? Since
when add a block device to container using "-v /dev/sdbX:/dev/sdbX", kata only passthrough this
block device into container, but it cannot be mounted since container doesn't has the right.

[raravena80]

@teawater ping, any updates?

[teawater]

I didn't see any good reason that kata doesn't support "--privileged".
Maybe it is not a safe way. But "--privileged" is a very simple way to help the �user access block device inside the container at least. (BTW, we have a long way to go with access block device. I think we need a configurable way.)

[amshinde]

@teawater Kata does support some notion of privileged. The container has access to all devices inside the guest, we pass as many devices as we can from the host and the container should run with all the capabilities that are associated with privileged mode, basically all capabilities.
I am interested in understanding what is the exact error you got with qmp first, rather than ignoring the error altogether.
@lifupan You should be able to mount a device in privileged mode, please file a bug if you are seeing otherwise. I have tried this in the past simply by passing --cap-add SYS_ADMIN to the container.

[teawater]

@amshinde This is a qemu issue. qemu need open block device to get its status.
But /dev/sr0 is a cdrom.
With disk inside, qemu will get:
"Could not open '/dev/sr0': Read-only file system"
Without disk inside, qemu will get:
No medium found
So I think ignore the error is a suitable way to handle it.

[WeiZhang555]

Ignoring the "AttachError" may cover some real issue, so I think it's not safe.

@jodh-intel We do actually support --privileged, except the /dev/sr0 issue found by @teawater , I've met same issue before :-) , without cdrom, --privileged can work in "most" cases :-)

@lifupan

Hi @jodh-intel, can we give a container right to access the devices in the guest using --privileged ? Since when add a block device to container using "-v /dev/sdbX:/dev/sdbX", kata only passthrough this
block device into container, but it cannot be mounted since container doesn't has the right.

This is a good question, we have a feature request discussing this, see
#571

In a word, I think you should first consider --cap-add SYS_ADMIN as @amshinde suggested, --privileged may not be safe though it's convenient.

We'd better find a more robust way to handle /dev/sr0 error than ignoring the AttachError

[teawater]

I made a patch that not attach device if it is a CDROM.

[teawater]

@WeiZhang555 made a new version according to your comments.

[WeiZhang555]

LGTM

/test

[bergwolf]

Is CDROM the only device type we cannot handle right now? What about floppy disks? There are also other types of devices like /dev/kvm, /dev/autofs etc. that can be confusing to users if they represent the host ones. I wonder if we want to use a white list instead.

[jshachm]

vote for @bergwolf
Now that we begin to deal with the special device, white list is better.

[teawater]

I agree with a device list.
And what about let it can be config by the config file?

[amshinde]

@bergwolf Today we ignore all devices except block and vfio.

[amshinde]

lgtm

[bergwolf]

I see. Thanks @amshinde!
LGTM.

[bergwolf]

/retest

[sboeuf]

/test

[sboeuf]

@grahamwhaley is the failure on metrics-16-04 expected?
If that's the case, and if this is okay to ignore it, please go ahead and merge this ;)

[grahamwhaley]

Yep - metrics CI is coming back up today - just found one more 'cleanup' issue to fix now we are on baremetal build slaves. so, ignore metrics here right now, and i'll merge...