[libvirt] Move esxVMX_IndexToDiskName to util.c
I needed the inverse function to virDiskNameToIndex() for the ESX driver and added it to esx_vmx.c. The pending VirtualBox 3.1 patch needs disk index to disk name mapping too. So I want to move esxVMX_IndexToDiskName() to util.c. esxVMX_IndexToDiskName() handles indices up to 701. This limit comes from a gap in the disk name to disk index mapping of virDiskNameToIndex(): sdzy -> 700 sdzz -> 701 sdaaa -> 728 sdaab -> 729 This line in virDiskNameToIndex() causes this gap: idx = (idx + i) * 26; It can be fixed by altering this line to: idx = (idx + (i < 1 ? 0 : 1)) * 26; But this change breaks compatibility for indices > 701. So, I made two patches for either option and ask you for comments. Matthias
I needed the inverse function to virDiskNameToIndex() for the ESX driver and added it to esx_vmx.c. The pending VirtualBox 3.1 patch needs disk index to disk name mapping too. So I want to move esxVMX_IndexToDiskName() to util.c.
esxVMX_IndexToDiskName() handles indices up to 701. This limit comes from a gap in the disk name to disk index mapping of virDiskNameToIndex():
sdzy -> 700 sdzz -> 701 sdaaa -> 728 sdaab -> 729
This line in virDiskNameToIndex() causes this gap:
idx = (idx + i) * 26;
It can be fixed by altering this line to:
idx = (idx + (i < 1 ? 0 : 1)) * 26;
But this change breaks compatibility for indices > 701.
ACK to patch A. I am not sure if we should go with patch B as it is good to squash bugs and not make them seem like a feature, I guess it is ok to break compatibility if it is real bug like this instead on inventing ways around it. Regards, Pritesh
On 11/20/2009 02:16 PM, Matthias Bolte wrote:
I needed the inverse function to virDiskNameToIndex() for the ESX driver and added it to esx_vmx.c. The pending VirtualBox 3.1 patch needs disk index to disk name mapping too. So I want to move esxVMX_IndexToDiskName() to util.c.
esxVMX_IndexToDiskName() handles indices up to 701. This limit comes from a gap in the disk name to disk index mapping of virDiskNameToIndex():
sdzy -> 700 sdzz -> 701 sdaaa -> 728 sdaab -> 729
This line in virDiskNameToIndex() causes this gap:
idx = (idx + i) * 26;
It can be fixed by altering this line to:
idx = (idx + (i < 1 ? 0 : 1)) * 26;
But this change breaks compatibility for indices > 701.
So, I made two patches for either option and ask you for comments.
Matthias
I agree with Pritesh, so ACK to patch A. I highly doubt anyone is depending on the broken behavior anyways, so let's just fix the bug. - Cole
2009/11/30 Cole Robinson <crobinso@redhat.com>:
On 11/20/2009 02:16 PM, Matthias Bolte wrote:
I needed the inverse function to virDiskNameToIndex() for the ESX driver and added it to esx_vmx.c. The pending VirtualBox 3.1 patch needs disk index to disk name mapping too. So I want to move esxVMX_IndexToDiskName() to util.c.
esxVMX_IndexToDiskName() handles indices up to 701. This limit comes from a gap in the disk name to disk index mapping of virDiskNameToIndex():
sdzy -> 700 sdzz -> 701 sdaaa -> 728 sdaab -> 729
This line in virDiskNameToIndex() causes this gap:
idx = (idx + i) * 26;
It can be fixed by altering this line to:
idx = (idx + (i < 1 ? 0 : 1)) * 26;
But this change breaks compatibility for indices > 701.
So, I made two patches for either option and ask you for comments.
Matthias
I agree with Pritesh, so ACK to patch A. I highly doubt anyone is depending on the broken behavior anyways, so let's just fix the bug.
- Cole
Okay. I'm going to apply and push patch A then. Matthias
participants (3)
-
Cole Robinson -
Matthias Bolte -
pritesh