Skip to content

Commit 84723b3

Browse files
committed
review comments: 1
Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
1 parent efd7149 commit 84723b3

3 files changed

Lines changed: 34 additions & 14 deletions

File tree

internal/controller/device/vpci/vpci.go

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,28 +65,48 @@ func (m *Manager) AddToVM(ctx context.Context, opts *AddOptions) (err error) {
6565
m.mu.Lock()
6666
defer m.mu.Unlock()
6767

68-
// Check for an existing assignment with the same device key.
69-
if existingGUID, ok := m.keyToGUID[key]; ok {
70-
dev := m.devices[existingGUID]
68+
// Check if the caller-provided GUID is already tracked.
69+
if existingDev, ok := m.devices[opts.VMBusGUID]; ok {
70+
// The GUID exists — verify the device settings match what was originally assigned.
71+
// A mismatch means the caller is trying to reuse a GUID for a different device,
72+
// which is a configuration error.
73+
if existingDev.key != key {
74+
return fmt.Errorf(
75+
"vmBusGUID %s is already assigned to device (instanceID=%s, vfIndex=%d), but caller provided different settings (instanceID=%s, vfIndex=%d)",
76+
opts.VMBusGUID,
77+
existingDev.key.deviceInstanceID, existingDev.key.virtualFunctionIndex,
78+
key.deviceInstanceID, key.virtualFunctionIndex,
79+
)
80+
}
7181

72-
// If a previous assignment left the device in an invalid state
82+
// If a previous assignment left the device in an invalid state,
7383
// reject new callers until the existing assignment is cleaned up.
74-
if dev.invalid {
75-
return fmt.Errorf("vpci device with vmBusGUID %s is in an invalid state", existingGUID)
84+
if existingDev.invalid {
85+
return fmt.Errorf("vpci device with vmBusGUID %s is in an invalid state", opts.VMBusGUID)
7686
}
7787

78-
// Increase the refCount and return the existing device.
79-
dev.refCount++
88+
// Same GUID, same device — reuse the existing assignment.
89+
existingDev.refCount++
8090

8191
log.G(ctx).WithFields(logrus.Fields{
8292
"deviceInstanceID": key.deviceInstanceID,
8393
"virtualFunctionIndex": key.virtualFunctionIndex,
84-
"refCount": dev.refCount,
94+
"refCount": existingDev.refCount,
8595
}).Debug("vPCI device already assigned, reusing existing assignment")
8696

8797
return nil
8898
}
8999

100+
// The GUID is new — check whether the same device key is already assigned
101+
// under a different GUID. This means the caller provided an inconsistent GUID.
102+
if existingGUID, ok := m.keyToGUID[key]; ok {
103+
return fmt.Errorf(
104+
"vpci device (instanceID=%s, vfIndex=%d) is already assigned with vmBusGUID %s, but caller provided %s",
105+
key.deviceInstanceID, key.virtualFunctionIndex,
106+
existingGUID, opts.VMBusGUID,
107+
)
108+
}
109+
90110
// Device not attached to VM.
91111
// Build the VirtualPciDevice settings for HCS call.
92112

@@ -125,7 +145,7 @@ func (m *Manager) AddToVM(ctx context.Context, opts *AddOptions) (err error) {
125145
m.keyToGUID[key] = opts.VMBusGUID
126146

127147
// Guest-side: device attach notification.
128-
if err := m.addGuestVPCIDevice(ctx, opts.VMBusGUID); err != nil {
148+
if err := m.waitGuestDeviceReady(ctx, opts.VMBusGUID); err != nil {
129149
// Mark the device as invalid so the caller can call RemoveFromVM
130150
// to clean up the host-side assignment.
131151
dev.invalid = true

internal/controller/device/vpci/vpci_lcow.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ import (
88
"github.com/Microsoft/hcsshim/internal/protocol/guestresource"
99
)
1010

11-
// addGuestVPCIDevice notifies the guest about the new device and blocks until
11+
// waitGuestDeviceReady notifies the guest about the new device and blocks until
1212
// the required sysfs/device paths are available before workloads use them.
13-
func (m *Manager) addGuestVPCIDevice(ctx context.Context, vmBusGUID string) error {
13+
func (m *Manager) waitGuestDeviceReady(ctx context.Context, vmBusGUID string) error {
1414
return m.linuxGuestVPCI.AddVPCIDevice(ctx, guestresource.LCOWMappedVPCIDevice{
1515
VMBusGUID: vmBusGUID,
1616
})

internal/controller/device/vpci/vpci_wcow.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ package vpci
44

55
import "context"
66

7-
// addGuestVPCIDevice is a no-op for Windows guests. WCOW does not require a
7+
// waitGuestDeviceReady is a no-op for Windows guests. WCOW does not require a
88
// guest-side notification as part of vPCI device assignment.
9-
func (m *Manager) addGuestVPCIDevice(_ context.Context, _ string) error {
9+
func (m *Manager) waitGuestDeviceReady(_ context.Context, _ string) error {
1010
return nil
1111
}

0 commit comments

Comments
 (0)