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 shlallen1990@gmail.com

@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

Signed-off-by: Allen Sun <shlallen1990@gmail.com>
@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.

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

Successfully merging this pull request may close these issues.

2 participants