Open kissake opened 2 years ago
Update:
I found /var/public/cgi-bin/system.pl [https://github.com/very-undude/ultimatedeployment/blob/main/var/public/cgi-bin/system.pl] and found the code below which seems to search for volumes available to extend storage on line 182 in ExtendVolume:
local($command)="ls -1 /dev/sd* | grep -v \/dev\/sda";
local(@devices)=sudo_ $command
;
In this code, the issue becomes somewhat clear. In the first attempt, my device was /dev/vdb, and in the second, the device was /dev/sda. In neither case was it going to pass this filter (for context, the boot disk is /dev/vda).
I will try to work around this by ensuring neither disk is using virtio / both are using SCSI or SATA style interfaces (thus getting a /dev/sdX style device name), I expect this will address the issue in the short term / workaround this.
Ideas for addressing this / making this code a little more robust might be to use something like /proc/diskstats or /proc/scsi/scsi to get a more holistic view of available disks, and then filter out those that are clearly inappropriate (e.g. the device has a partition table on it, or it is the installed device (determined by the PV being associated with the udavg VG)) before proffering them to the user.
i.e. something like:
local($command)="cat /proc/diskstats | perl -an -e 'print \"$F[2]\n\" if $F[2] !~ /^dm/ and $F[2] !~ /a[0-9]*$/'"
The above would omit:
This would NOT address the issue that occurred in my second attempt, because at that time I had one device of a 'vd' type and another of a 'scsi' type, both of which were first, but it would help in the event where both original / boot / appliance device and the "new" device were on the same type bus.
Also apropos of this section of code, I don't think listing devices / volumes requires privileges (could be wrong?), so perhaps the 'sudo' call is unneeded for enumerating devices / here.
Should I submit a pull request / patch?
Good stuff! By all means submit a pull request. You would be the first!
I like your solution with proc/diskstats!
Some more comments/pointers:
I've only been testing on ESX and VirtualBox, both of which work with /dev/sdX., but having support for more platforms is always better!
In VMWare you can add a SCSI disk while the VM is online. The following line triggers a rescan of the SCSI bus, which is why we do not have to reboot the VM in ESX. (In Virtualbox we can only add a disk when the VM is off, so the rescan happens during reboot anyway...)
local(@result)=`echo "echo \'- - -\' \> /sys/class/scsi_host/host0/scan" | sudo su -`;
I'm currently working with patches, so your changes will end up in the main branch, but it will also be part of a patch, since releasing a patch is a lot easier than an entire new build... See: https://github.com/very-undude/ultimatedeployment/tree/dev/patches
In full fairness, I'm using QEMU/KVM to run the appliance, rather than the documented ESXi platform, so I'm not able to follow the process documented on YouTube precisely [https://www.youtube.com/watch?v=AuP7caO4-OU].
What happens for me is that I add a disk to the virtual machine (I tried VirtIO and SCSI attached) and reboot, and the device is not detected by the appliance. I also tried putting a logical volume manager (LVM) physical volume (PV) on it, no improvement, then I attached the PV to the volume group (VG) that holds the locallv logical volume (LV), and still nothing changed / improved.
My initial guess was that the system looked only for bare + new(?) /dev/sdX devices to add to the LVM setup, but the new device was /dev/sda, which put paid to that theory. My next guess is that it detects a change event, based on the fact that the VM wasn't shut down during the ESXi video (that I saw)
I'm guessing my creativity is unsupported, but if you wanted to point me at the rough area of the code where this is implemented, I'd welcome the chance to get my hands dirty to a) understand what is going on, and b) work to provide a patch if that makes sense.
If you don't have the time to point me in some direction, no worries; I may or may not jump on it later.
Best wishes.