Skip to content
This repository has been archived by the owner on Feb 8, 2021. It is now read-only.

add build os for file driver.go #607

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

allencloud
Copy link
Contributor

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 [email protected]

@allencloud
Copy link
Contributor Author

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
Copy link
Member

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

@allencloud
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@bergwolf
Copy link
Member

@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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants