Browse code

devmapper: Use device id as specified by caller

Currently devicemapper CreateDevice and CreateSnapDevice keep on retrying
device creation till a suitable device id is found.

With new transaction mechanism we need to store device id in transaction
before it has been created.

So change the logic in such a way that caller decides the devices Id to
use. If that device Id is not available, caller bumps up the device Id
and retries.

That way caller can update transaciton too when it tries a new Id. Transaction
related patches will come later in the series.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>

Vivek Goyal authored on 2014/12/04 03:06:43
Showing 2 changed files
... ...
@@ -394,6 +394,50 @@ func (devices *DeviceSet) initMetaData() error {
394 394
 	return nil
395 395
 }
396 396
 
397
+func (devices *DeviceSet) incNextDeviceId() {
398
+	// Ids are 24bit, so wrap around
399
+	devices.NextDeviceId = (devices.NextDeviceId + 1) & 0xffffff
400
+}
401
+
402
+func (devices *DeviceSet) createDevice(deviceId *int) error {
403
+	for {
404
+		if err := devicemapper.CreateDevice(devices.getPoolDevName(), *deviceId); err != nil {
405
+			if devicemapper.DeviceIdExists(err) {
406
+				// Device Id already exists. Try a new one.
407
+				devices.incNextDeviceId()
408
+				*deviceId = devices.NextDeviceId
409
+				continue
410
+			}
411
+			log.Debugf("Error creating device: %s", err)
412
+			return err
413
+		}
414
+		break
415
+	}
416
+	devices.incNextDeviceId()
417
+	return nil
418
+}
419
+
420
+func (devices *DeviceSet) createSnapDevice(baseInfo *DevInfo, deviceId *int) error {
421
+	log.Debugf("[deviceset] createSnapDevice() DeviceId=%d", *deviceId)
422
+	defer log.Debugf("[deviceset] createSnapDevice() END DeviceId=%d", *deviceId)
423
+
424
+	for {
425
+		if err := devicemapper.CreateSnapDevice(devices.getPoolDevName(), *deviceId, baseInfo.Name(), baseInfo.DeviceId); err != nil {
426
+			if devicemapper.DeviceIdExists(err) {
427
+				// Device Id already exists. Try a new one.
428
+				devices.incNextDeviceId()
429
+				*deviceId = devices.NextDeviceId
430
+				continue
431
+			}
432
+			log.Debugf("Error creating snap device: %s", err)
433
+			return err
434
+		}
435
+		break
436
+	}
437
+	devices.incNextDeviceId()
438
+	return nil
439
+}
440
+
397 441
 func (devices *DeviceSet) loadMetadata(hash string) *DevInfo {
398 442
 	info := &DevInfo{Hash: hash, devices: devices}
399 443
 
... ...
@@ -439,20 +483,16 @@ func (devices *DeviceSet) setupBaseImage() error {
439 439
 
440 440
 	log.Debugf("Initializing base device-mapper thin volume")
441 441
 
442
-	id := devices.NextDeviceId
443
-
444 442
 	// Create initial device
445
-	if err := devicemapper.CreateDevice(devices.getPoolDevName(), &id); err != nil {
443
+	deviceId := devices.NextDeviceId
444
+	if err := devices.createDevice(&deviceId); err != nil {
446 445
 		return err
447 446
 	}
448 447
 
449
-	// Ids are 24bit, so wrap around
450
-	devices.NextDeviceId = (id + 1) & 0xffffff
451
-
452
-	log.Debugf("Registering base device (id %v) with FS size %v", id, devices.baseFsSize)
453
-	info, err := devices.registerDevice(id, "", devices.baseFsSize)
448
+	log.Debugf("Registering base device (id %v) with FS size %v", deviceId, devices.baseFsSize)
449
+	info, err := devices.registerDevice(deviceId, "", devices.baseFsSize)
454 450
 	if err != nil {
455
-		_ = devicemapper.DeleteDevice(devices.getPoolDevName(), id)
451
+		_ = devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
456 452
 		return err
457 453
 	}
458 454
 
... ...
@@ -751,6 +791,9 @@ func (devices *DeviceSet) initDevmapper(doInit bool) error {
751 751
 }
752 752
 
753 753
 func (devices *DeviceSet) AddDevice(hash, baseHash string) error {
754
+	log.Debugf("[deviceset] AddDevice() hash=%s basehash=%s", hash, baseHash)
755
+	defer log.Debugf("[deviceset] AddDevice END")
756
+
754 757
 	baseInfo, err := devices.lookupDevice(baseHash)
755 758
 	if err != nil {
756 759
 		return err
... ...
@@ -767,15 +810,11 @@ func (devices *DeviceSet) AddDevice(hash, baseHash string) error {
767 767
 	}
768 768
 
769 769
 	deviceId := devices.NextDeviceId
770
-
771
-	if err := devicemapper.CreateSnapDevice(devices.getPoolDevName(), &deviceId, baseInfo.Name(), baseInfo.DeviceId); err != nil {
770
+	if err := devices.createSnapDevice(baseInfo, &deviceId); err != nil {
772 771
 		log.Debugf("Error creating snap device: %s", err)
773 772
 		return err
774 773
 	}
775 774
 
776
-	// Ids are 24bit, so wrap around
777
-	devices.NextDeviceId = (deviceId + 1) & 0xffffff
778
-
779 775
 	if _, err := devices.registerDevice(deviceId, hash, baseInfo.Size); err != nil {
780 776
 		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
781 777
 		log.Debugf("Error registering device: %s", err)
... ...
@@ -67,6 +67,7 @@ var (
67 67
 	ErrGetLoopbackBackingFile = errors.New("Unable to get loopback backing file")
68 68
 	ErrLoopbackSetCapacity    = errors.New("Unable set loopback capacity")
69 69
 	ErrBusy                   = errors.New("Device is Busy")
70
+	ErrDeviceIdExists         = errors.New("Device Id Exists")
70 71
 
71 72
 	dmSawBusy  bool
72 73
 	dmSawExist bool
... ...
@@ -97,6 +98,16 @@ type (
97 97
 	AddNodeType int
98 98
 )
99 99
 
100
+// Returns whether error conveys the information about device Id already
101
+// exist or not. This will be true if device creation or snap creation
102
+// operation fails if device or snap device already exists in pool.
103
+// Current implementation is little crude as it scans the error string
104
+// for exact pattern match. Replacing it with more robust implementation
105
+// is desirable.
106
+func DeviceIdExists(err error) bool {
107
+	return fmt.Sprint(err) == fmt.Sprint(ErrDeviceIdExists)
108
+}
109
+
100 110
 func (t *Task) destroy() {
101 111
 	if t != nil {
102 112
 		DmTaskDestroy(t.unmanaged)
... ...
@@ -528,33 +539,29 @@ func ResumeDevice(name string) error {
528 528
 	return nil
529 529
 }
530 530
 
531
-func CreateDevice(poolName string, deviceId *int) error {
532
-	log.Debugf("[devmapper] CreateDevice(poolName=%v, deviceId=%v)", poolName, *deviceId)
533
-
534
-	for {
535
-		task, err := TaskCreateNamed(DeviceTargetMsg, poolName)
536
-		if task == nil {
537
-			return err
538
-		}
531
+func CreateDevice(poolName string, deviceId int) error {
532
+	log.Debugf("[devmapper] CreateDevice(poolName=%v, deviceId=%v)", poolName, deviceId)
533
+	task, err := TaskCreateNamed(DeviceTargetMsg, poolName)
534
+	if task == nil {
535
+		return err
536
+	}
539 537
 
540
-		if err := task.SetSector(0); err != nil {
541
-			return fmt.Errorf("Can't set sector %s", err)
542
-		}
538
+	if err := task.SetSector(0); err != nil {
539
+		return fmt.Errorf("Can't set sector %s", err)
540
+	}
543 541
 
544
-		if err := task.SetMessage(fmt.Sprintf("create_thin %d", *deviceId)); err != nil {
545
-			return fmt.Errorf("Can't set message %s", err)
546
-		}
542
+	if err := task.SetMessage(fmt.Sprintf("create_thin %d", deviceId)); err != nil {
543
+		return fmt.Errorf("Can't set message %s", err)
544
+	}
547 545
 
548
-		dmSawExist = false // reset before the task is run
549
-		if err := task.Run(); err != nil {
550
-			if dmSawExist {
551
-				// Already exists, try next id
552
-				*deviceId++
553
-				continue
554
-			}
546
+	dmSawExist = false // reset before the task is run
547
+	if err := task.Run(); err != nil {
548
+		// Caller wants to know about ErrDeviceIdExists so that it can try with a different device id.
549
+		if dmSawExist {
550
+			return ErrDeviceIdExists
551
+		} else {
555 552
 			return fmt.Errorf("Error running CreateDevice %s", err)
556 553
 		}
557
-		break
558 554
 	}
559 555
 	return nil
560 556
 }
... ...
@@ -607,7 +614,7 @@ func ActivateDevice(poolName string, name string, deviceId int, size uint64) err
607 607
 	return nil
608 608
 }
609 609
 
610
-func CreateSnapDevice(poolName string, deviceId *int, baseName string, baseDeviceId int) error {
610
+func CreateSnapDevice(poolName string, deviceId int, baseName string, baseDeviceId int) error {
611 611
 	devinfo, _ := GetInfo(baseName)
612 612
 	doSuspend := devinfo != nil && devinfo.Exists != 0
613 613
 
... ...
@@ -617,44 +624,39 @@ func CreateSnapDevice(poolName string, deviceId *int, baseName string, baseDevic
617 617
 		}
618 618
 	}
619 619
 
620
-	for {
621
-		task, err := TaskCreateNamed(DeviceTargetMsg, poolName)
622
-		if task == nil {
623
-			if doSuspend {
624
-				ResumeDevice(baseName)
625
-			}
626
-			return err
620
+	task, err := TaskCreateNamed(DeviceTargetMsg, poolName)
621
+	if task == nil {
622
+		if doSuspend {
623
+			ResumeDevice(baseName)
627 624
 		}
625
+		return err
626
+	}
628 627
 
629
-		if err := task.SetSector(0); err != nil {
630
-			if doSuspend {
631
-				ResumeDevice(baseName)
632
-			}
633
-			return fmt.Errorf("Can't set sector %s", err)
628
+	if err := task.SetSector(0); err != nil {
629
+		if doSuspend {
630
+			ResumeDevice(baseName)
634 631
 		}
632
+		return fmt.Errorf("Can't set sector %s", err)
633
+	}
635 634
 
636
-		if err := task.SetMessage(fmt.Sprintf("create_snap %d %d", *deviceId, baseDeviceId)); err != nil {
637
-			if doSuspend {
638
-				ResumeDevice(baseName)
639
-			}
640
-			return fmt.Errorf("Can't set message %s", err)
635
+	if err := task.SetMessage(fmt.Sprintf("create_snap %d %d", deviceId, baseDeviceId)); err != nil {
636
+		if doSuspend {
637
+			ResumeDevice(baseName)
641 638
 		}
639
+		return fmt.Errorf("Can't set message %s", err)
640
+	}
642 641
 
643
-		dmSawExist = false // reset before the task is run
644
-		if err := task.Run(); err != nil {
645
-			if dmSawExist {
646
-				// Already exists, try next id
647
-				*deviceId++
648
-				continue
649
-			}
650
-
651
-			if doSuspend {
652
-				ResumeDevice(baseName)
653
-			}
642
+	dmSawExist = false // reset before the task is run
643
+	if err := task.Run(); err != nil {
644
+		if doSuspend {
645
+			ResumeDevice(baseName)
646
+		}
647
+		// Caller wants to know about ErrDeviceIdExists so that it can try with a different device id.
648
+		if dmSawExist {
649
+			return ErrDeviceIdExists
650
+		} else {
654 651
 			return fmt.Errorf("Error running DeviceCreate (createSnapDevice) %s", err)
655 652
 		}
656
-
657
-		break
658 653
 	}
659 654
 
660 655
 	if doSuspend {