Skip to content

Questions about the listAttachedFcdsForVM function #3678

@akutz

Description

@akutz

👋 Howdy,

I was reading the CSI driver code and had a few issues about the listAttachedFcdsForVM function:

  • On line 359 there is a check for the device's type using a helper function for getting the type name:

    if vmDevices.TypeName(device) == "VirtualDisk" {

    However, on the very next line, 360, the actual correct way to assert the device's type is used:

    if virtualDisk, ok := device.(*vimtypes.VirtualDisk); ok

    Why check the type twice? Especially when the idiomatic approach is already present in the second attempt?

  • Should this function assume that all FCDs attached to a VM must be PVCs?

    As I understand it, all PVCs --> CNS volumes --> FCDs --> VMDKs, but the reverse is not true. A VMDK is not always an FCD, an FCD is not always a CNS volume, and a CNS volume is not always a PVC. Why is CSI assuming that an FCD must have a corresponding PVC?

    Should the function be renamed to listAttachedFCDsWithPVCsForVM?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions