hyperhq / runv

Hypervisor-based Runtime for OCI
Apache License 2.0
828 stars 129 forks source link

add build os for file driver.go #607

Open allencloud opened 7 years ago

allencloud commented 7 years ago

First, let me illustrate without // +build linux, what will code report?

In Line https://github.com/hyperhq/runv/blob/master/hypervisor/driver.go#L67, we have code

var VsockCidManager vsock.VsockCidAllocator

And we can find VsockCidAllocator is an interface ONLY on Linux, since it is described in file https://github.com/hyperhq/runv/blob/master/lib/vsock/vsock.go#L1

// +build linux

package vsock

import (
    "fmt"
    "sync"

    "github.com/RoaringBitmap/roaring"
)

const hyperDefaultVsockCid = 1024
const hyperDefaultVsockBitmapSize = 16384

type VsockCidAllocator interface {
    sync.Locker
    GetCid() (uint32, error)
    MarkCidInuse(uint32) bool
    ReleaseCid(uint32)
}

So if the file is compiled on other OS types, it will throw a compile error.

So I add a build os for file driver.go.

Signed-off-by: Allen Sun shlallen1990@gmail.com

allencloud commented 7 years ago

Or we can remove the // +build linux from https://github.com/hyperhq/runv/blob/master/lib/vsock/vsock.go#L1. And this action will work as well.

bergwolf commented 7 years ago

The right fix is to add VsockCidAllocator to lib/vsock/vsock_unsupported.go. Would you like to update and fix it there?

allencloud commented 7 years ago

Yes, It is another solution to this. I will carry that in this PR. But maybe in a couple of days. Thanks for your advice and feedback. @bergwolf

allencloud commented 7 years ago

I just add the same interface definition into vsock_unsupported.go. And this will make darwin os report no compile error. PTAL @bergwolf

bergwolf commented 7 years ago

@allencloud Thanks for the quick update. I'm afraid the fix is not completed yet, e.g, NewDefaultVsockCidAllocator() is called in driverloader/driverloader_linux.go.

Please add a dummy implementation of the interface in vsock_unsupported.go. We'd like to get proper errors other than runtime failures when vsock is not supported. The dummy implementation will include NewDefaultVsockCidAllocator() and all the methods declared in VsockCidAllocator.