Beginnings of a loop provider. #4

Merged
merged 1 commit into from Jan 29, 2015

Conversation

Projects
None yet
3 participants

kat-co commented Jan 27, 2015

  • Loop provider not yet hooked up to plumbing, but meat of creating loop devices is present.
  • Nascent testing.
+
+ blockDevices := make([]storage.BlockDevice, len(volIds))
+ for idx, volId := range volIds {
+ device, ok := lvs.volIdToBlockDevice[volId]
@axw

axw Jan 27, 2015

Collaborator

We can't assume that we'll have created the device in the same session. Doesn't matter for now, since this code isn't called yet.

@kat-co

kat-co Jan 27, 2015

I've left it this way for now since I'd have to write some code to inspect the loop devices to transform them back into the BlockDevice struct. I'll take care of this later.

storage/provider/loop.go
+ if err == NoLoopDeviceErr {
+ nextDevNum, err := findAvailableDeviceNumber(lvs.runCmd)
+ if err == nil {
+ createLoopDevice(lvs.runCmd, nextDevNum)
@axw

axw Jan 27, 2015

Collaborator
  • should be checking error here.
storage/provider/loop.go
+}
+
+func providerId(arg storage.VolumeParams) string {
+ uuid, err := utils.NewUUID()
@axw

axw Jan 27, 2015

Collaborator

This needs to be something we can link back to the loop device. Perhaps just compose it from the machine ID and the loop device name. We'd need to pass the machine ID when creating the provider.

Collaborator

axw commented Jan 27, 2015

Probably good enough for now, but it won't work if the agent bounces.

kat-co commented Jan 27, 2015

I think I see a potential issue: I think we want the block file to be correlated with the provider ID so we can re-attach them across restarts. If we include the loop device in the provider ID, this may not make sense over time: e.g. we could run into a situation where we have machine1-loop0, machine1-loop9.

Collaborator

axw commented Jan 28, 2015

@katco- Good point. In that model refactor that I spoke about in the standup, I've changed the storage provider so that CreateVolume returns a Volume and AttachVolume returns a VolumeAttachment. The former contains information specific to the volume (e.g. size, persistence), whereas the latter contains information specific to the attachment (e.g. the device name).

So... have the volume ID encode the machine ID and the name of the file. You can then map that to the device path using losetup -j if we find it's necessary.

storage/provider/init.go
storage.RegisterDefaultPool("local", storage.StorageKindBlock, LoopPool)
}
+
+func RunCmdFn() RunCommandFn {
@wallyworld

wallyworld Jan 29, 2015

Owner

hardly needs it, but a doc comment would be kosher

storage/provider/loop.go
+ panic(err)
+ }
+ return uuid.String()
+ return fmt.Sprintf("%s-%s", machineId, loopDeviceName)
@wallyworld

wallyworld Jan 29, 2015

Owner

comment this out for clarity

Owner

wallyworld commented Jan 29, 2015

LGTM for demo purposes once tests pass

Beginnings of a loop provider.
- Loop provider not yet hooked up to plumbing, but meat of creating loop devices is present.
- Nascent testing.
- Modified the LXC config-file to allow LXC containers to mount loop devices within the container.

wallyworld added a commit that referenced this pull request Jan 29, 2015

Merge pull request #4 from katco-/storage-feature
Beginnings of a loop provider.

@wallyworld wallyworld merged commit ef637d5 into wallyworld:storage-feature Jan 29, 2015

wallyworld pushed a commit that referenced this pull request Oct 22, 2015

Merge branch 'master' into networking-user-docs
* master: (259 commits)
  bzr: switch to utils/bzr
  worker/uniter/remotestate: fix Actions test
  worker/uniter/remotestate: fix data race
  worker/uniter/remotestate: fix data race in test
  juju/series: new package
  Minimal fix for lp:1493850.
  Further refactoring
  Tweak func sig
  Code review fixes
  FIx data race in tests
  Correctly handle spaces in a test.
  Fix up the merge from PR#3217.
  Add comments identifying some of the steps in status test #12.
  Add comments identifying some of the steps in status test #4.
  Add comments identifying the status tests.
  Clean up a few status tests.
  Sync up the status-related API params.
  Ensure the status tests run.
  apiserver/common: fix tests
  Add package docs
  ...

wallyworld pushed a commit that referenced this pull request Mar 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment