Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation: New disk schema #778

Closed
Tinyblargon opened this issue May 25, 2023 · 7 comments
Closed

Implementation: New disk schema #778

Tinyblargon opened this issue May 25, 2023 · 7 comments

Comments

@Tinyblargon
Copy link
Collaborator

@mleone87 I have started on making the Terraform provider leverage the new disk implementation referenced Telmate/proxmox-api-go#255

I was wonder if the new schema structure referenced in vm_qemu.md and pxe_example.tf is satisfactory? As this would introduce a breaking change upgrade the provider to v3.0.0

@mleone87
Copy link
Collaborator

mleone87 commented Jun 1, 2023

Hey @Tinyblargon !
why not something similar??

disks {
        scsi {
            scsi0 {
                   ...
                    size               = 32
                    storage            = "local-lvm"
            }
        }
        sata {
            sata0 {
                   ...
                    size               = 32
                    storage            = "local-lvm"
            }
        }
    }

Other strucures would fit also

@Tinyblargon
Copy link
Collaborator Author

@mleone87

Changing disk_0 to ide0, sata0, scsi0, virtio0 is a good idea as it makes it more clear with which kind of disks the user is working with. Especially when scsi can have 30+ drives.

Personally, I'd prefer to keep the sub schema of cdrom, disk, passthrough as it defines more clearly which settings belong to which kind of disk. But i also see the argument for merging them as the vast majority of the users would only use the disk option. I'm gonna look into making the configuration constraints work with a structure where cdrom, disk, passthrough share the same schema.

@mleone87
Copy link
Collaborator

mleone87 commented Jun 1, 2023

oh I see what you mean with disk/cdrmo/passthrough sub-objects, I forgot about that!

I would avoid too much recursion in disk object since it would be difficult to understand and, also, I would keep the cdrom/disk/passthrough objects separates as they are in proxmox API

@Tinyblargon
Copy link
Collaborator Author

@mleone87 am i interpenetrating your last comment correctly with this schema?

disks {
  ide {
    //<arguments omitted for brevity...>
  }
  sata {
    //<arguments omitted for brevity...>
  }
  scsi {
    //<arguments omitted for brevity...>
  }
  virtio {
    virtio0 {
      cdrom {
        iso = "local:iso/proxmox-ve_7.4-1.iso"
      }
    }
    virtio1 {
      disk {
        size    = 32
        storage = "local-lvm"
        //<arguments omitted for brevity...>
      }
    }
    virtio2 {
      passthrough {
        file = "/dev/disk/by-id/nvme-Seagate_FireCuda_SSD"
        //<arguments omitted for brevity...>
      }
    }
    //<arguments omitted for brevity...>
  }
}

@mleone87
Copy link
Collaborator

mleone87 commented Jun 5, 2023

@Tinyblargon yeah!

@github-actions
Copy link

github-actions bot commented Aug 5, 2023

This issue is stale because it has been open for 60 days with no activity. Please update the provider to the latest version and, in the issue persist, provide full configuration and debug logs

@github-actions
Copy link

This issue was closed because it has been inactive for 5 days since being marked as stale.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants