Finally, we seem to have all the bits to keep track of all used device
Ids and find a free device Id to use when creating a new device. Start
using it.
Ideally we should completely move away from retry logic when pool returns
-EEXISTS. For now I have retained that logic and I simply output a warning.
When things are stable, we should be able to get rid of it.
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
| ... | ... |
@@ -500,21 +500,41 @@ func (devices *DeviceSet) incNextDeviceId() {
|
| 500 | 500 |
devices.NextDeviceId = (devices.NextDeviceId + 1) & MaxDeviceId |
| 501 | 501 |
} |
| 502 | 502 |
|
| 503 |
-func (devices *DeviceSet) getNextDeviceId() int {
|
|
| 503 |
+func (devices *DeviceSet) getNextFreeDeviceId() (int, error) {
|
|
| 504 | 504 |
devices.incNextDeviceId() |
| 505 |
- return devices.NextDeviceId |
|
| 505 |
+ for i := 0; i <= MaxDeviceId; i++ {
|
|
| 506 |
+ if devices.isDeviceIdFree(devices.NextDeviceId) {
|
|
| 507 |
+ devices.markDeviceIdUsed(devices.NextDeviceId) |
|
| 508 |
+ return devices.NextDeviceId, nil |
|
| 509 |
+ } |
|
| 510 |
+ devices.incNextDeviceId() |
|
| 511 |
+ } |
|
| 512 |
+ |
|
| 513 |
+ return 0, fmt.Errorf("Unable to find a free device Id")
|
|
| 506 | 514 |
} |
| 507 | 515 |
|
| 508 | 516 |
func (devices *DeviceSet) createRegisterDevice(hash string) (*DevInfo, error) {
|
| 509 |
- deviceId := devices.getNextDeviceId() |
|
| 517 |
+ deviceId, err := devices.getNextFreeDeviceId() |
|
| 518 |
+ if err != nil {
|
|
| 519 |
+ return nil, err |
|
| 520 |
+ } |
|
| 521 |
+ |
|
| 510 | 522 |
for {
|
| 511 | 523 |
if err := devicemapper.CreateDevice(devices.getPoolDevName(), deviceId); err != nil {
|
| 512 | 524 |
if devicemapper.DeviceIdExists(err) {
|
| 513 |
- // Device Id already exists. Try a new one. |
|
| 514 |
- deviceId = devices.getNextDeviceId() |
|
| 525 |
+ // Device Id already exists. This should not |
|
| 526 |
+ // happen. Now we have a mechianism to find |
|
| 527 |
+ // a free device Id. So something is not right. |
|
| 528 |
+ // Give a warning and continue. |
|
| 529 |
+ log.Errorf("Warning: Device Id %d exists in pool but it is supposed to be unused", deviceId)
|
|
| 530 |
+ deviceId, err = devices.getNextFreeDeviceId() |
|
| 531 |
+ if err != nil {
|
|
| 532 |
+ return nil, err |
|
| 533 |
+ } |
|
| 515 | 534 |
continue |
| 516 | 535 |
} |
| 517 | 536 |
log.Debugf("Error creating device: %s", err)
|
| 537 |
+ devices.markDeviceIdFree(deviceId) |
|
| 518 | 538 |
return nil, err |
| 519 | 539 |
} |
| 520 | 540 |
break |
| ... | ... |
@@ -525,27 +545,41 @@ func (devices *DeviceSet) createRegisterDevice(hash string) (*DevInfo, error) {
|
| 525 | 525 |
info, err := devices.registerDevice(deviceId, hash, devices.baseFsSize, transactionId) |
| 526 | 526 |
if err != nil {
|
| 527 | 527 |
_ = devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId) |
| 528 |
+ devices.markDeviceIdFree(deviceId) |
|
| 528 | 529 |
return nil, err |
| 529 | 530 |
} |
| 530 | 531 |
|
| 531 | 532 |
if err := devices.updatePoolTransactionId(); err != nil {
|
| 532 | 533 |
devices.unregisterDevice(deviceId, hash) |
| 533 | 534 |
devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId) |
| 535 |
+ devices.markDeviceIdFree(deviceId) |
|
| 534 | 536 |
return nil, err |
| 535 | 537 |
} |
| 536 | 538 |
return info, nil |
| 537 | 539 |
} |
| 538 | 540 |
|
| 539 | 541 |
func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *DevInfo) error {
|
| 540 |
- deviceId := devices.getNextDeviceId() |
|
| 542 |
+ deviceId, err := devices.getNextFreeDeviceId() |
|
| 543 |
+ if err != nil {
|
|
| 544 |
+ return err |
|
| 545 |
+ } |
|
| 546 |
+ |
|
| 541 | 547 |
for {
|
| 542 | 548 |
if err := devicemapper.CreateSnapDevice(devices.getPoolDevName(), deviceId, baseInfo.Name(), baseInfo.DeviceId); err != nil {
|
| 543 | 549 |
if devicemapper.DeviceIdExists(err) {
|
| 544 |
- // Device Id already exists. Try a new one. |
|
| 545 |
- deviceId = devices.getNextDeviceId() |
|
| 550 |
+ // Device Id already exists. This should not |
|
| 551 |
+ // happen. Now we have a mechianism to find |
|
| 552 |
+ // a free device Id. So something is not right. |
|
| 553 |
+ // Give a warning and continue. |
|
| 554 |
+ log.Errorf("Warning: Device Id %d exists in pool but it is supposed to be unused", deviceId)
|
|
| 555 |
+ deviceId, err = devices.getNextFreeDeviceId() |
|
| 556 |
+ if err != nil {
|
|
| 557 |
+ return err |
|
| 558 |
+ } |
|
| 546 | 559 |
continue |
| 547 | 560 |
} |
| 548 | 561 |
log.Debugf("Error creating snap device: %s", err)
|
| 562 |
+ devices.markDeviceIdFree(deviceId) |
|
| 549 | 563 |
return err |
| 550 | 564 |
} |
| 551 | 565 |
break |
| ... | ... |
@@ -554,6 +588,7 @@ func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *DevInf |
| 554 | 554 |
transactionId := devices.allocateTransactionId() |
| 555 | 555 |
if _, err := devices.registerDevice(deviceId, hash, baseInfo.Size, transactionId); err != nil {
|
| 556 | 556 |
devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId) |
| 557 |
+ devices.markDeviceIdFree(deviceId) |
|
| 557 | 558 |
log.Debugf("Error registering device: %s", err)
|
| 558 | 559 |
return err |
| 559 | 560 |
} |
| ... | ... |
@@ -561,6 +596,7 @@ func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *DevInf |
| 561 | 561 |
if err := devices.updatePoolTransactionId(); err != nil {
|
| 562 | 562 |
devices.unregisterDevice(deviceId, hash) |
| 563 | 563 |
devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId) |
| 564 |
+ devices.markDeviceIdFree(deviceId) |
|
| 564 | 565 |
return err |
| 565 | 566 |
} |
| 566 | 567 |
return nil |
| ... | ... |
@@ -966,6 +1002,8 @@ func (devices *DeviceSet) deleteDevice(info *DevInfo) error {
|
| 966 | 966 |
return err |
| 967 | 967 |
} |
| 968 | 968 |
|
| 969 |
+ devices.markDeviceIdFree(info.DeviceId) |
|
| 970 |
+ |
|
| 969 | 971 |
return nil |
| 970 | 972 |
} |
| 971 | 973 |
|