winterhazel commented on PR #9433: URL: https://github.com/apache/cloudstack/pull/9433#issuecomment-2311217747
> As the controllers are already created during the VM creation process, it's better to synchronize these controller details during VM creation. Synchronizing in the attach process would still set the same controller details (if you mean dynamic reconfiguration of disk controllers in VM, maybe the existing controller reconfiguration based on the data disk controller can be done as a separate improvement later). The changes here would fix the attach issue when data disk controller is not set in the VM's template and VM detail. @sureshanaparti I recognize that your changes fix the attachment for this case. However, they are not fixing the root of the problem, just working-around one specific situation that can lead to it. This problem does not happen because the two settings have different values. It happens due to an inconsistent behavior of the code. I think you may have misunderstood what I'm proposing. Just to ensure that we have the same thing in mind: my idea is execute on the volume attachment process the same logic that the VM creation/start workflows use when adding the disk controllers to the VM's spec. Context: - Suppose we have a template that has `rootDiskController` set to `pvscsi` and `dataDiskController` set to `lsilogic`. Important info: - VMware does allow us to create a VM using a PVSCSI for the root disk and LSI Logic controllers (two different SCSI subtypes) for the data disks. What happens on VM creation/start: - When a VM is created/started in CloudStack, we verify the controllers defined for the root and data disk ( `rootDiskController` and `dataDiskController` details). - We see that both are using SCSI subtypes and, even though we could create one PVSCSI for the root disk and LSI Logic controllers for data disks, we intentionally ignore the `dataDiskController` setting defined by the operator and choose to use only PVSCSI controllers for both the root disk and the data disks. What is happening on volume attachment: - Then, when a user tries to attach a new data disk, the process fails because we only consider `dataDiskController` and try to attach with LSI Logic instead. However, there are no LSI Logic controllers because we ignored the data disk controller chosen by the operator and opted to create PVSCI controllers for both root and data disks when creating/starting the VM. My proposal: - Implement the same logic we did on VM creation/start (choosing PVSCSI instead of LSI Logic for the data disks) on volume attachment. > @winterhazel The data disk controller is matched with the SCSI root disk controller only when the data disk controller is not set in the VM's template and VM detail. If template has the root and data disk controllers set to _pvscsi_ and _osdefault_ respectively, then the controllers are created with _pvscsi_ controller to ensure root disk is configured to proper controller to create the VM. Later, any unsupported data disk controller (say, _lsilogic_) set would fail the attach disk, so the operator should ensure to set the root and data disk controller appropriately. I don't think we have to place the responsibility on operators in this case. The attachment fails because **we** ignore the `dataDiskController` defined by the operator and **we** create/start the VM using another controller instead. > @winterhazel are you content with @sureshanaparti 's reply? @DaanHoogland I'm fine with this being merged, because I do acknowledge that the changes here and my proposal are not exclusive, and that it did fix one situation. However, I still think that #9400 should not be considered solved with this. The changes here do not touch the root of the problem, and only solve a specific case that would be covered by modifying the volume attachment workflow. I'll address it on another PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
