Skip to content

Conversation

@rhvgoyal
Copy link
Collaborator

@rhvgoyal rhvgoyal commented Jun 8, 2015

Fixes #39

It can happen that we created a metadata volume and later there was not
enough space to create data volume and script exited, leaving behind
metadata volume. Later if user adds more space to volume group and re-runs
the script, it fails saying metadata volume already exists.

For now fix this by checking if metadata volume exists or not. If it exists
use it (instead of erroring out).

I think ideally we should have cleanup logic and cleanup any logical
volumes created if error happens later. That probably also means
getting rid of "set -e".

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

@rhvgoyal
Copy link
Collaborator Author

rhvgoyal commented Jun 9, 2015

I think handling of subshells is little different. if checks for exit code. Here is the sample code.
#!/bin/bash

if i=$(exit 1); then
echo "subshell exited with zero status"
exit 0
fi

echo "subshell exited with non-zero status"

@rhvgoyal
Copy link
Collaborator Author

rhvgoyal commented Jun 9, 2015

My understanding is that one can not create a zero sized logical volume. So the very fact command exited with success status means, lv exists, that also means that size is greater than 0.

@rhvgoyal
Copy link
Collaborator Author

rhvgoyal commented Jun 9, 2015

@cgwalters does above answer your queries?

@cgwalters
Copy link
Member

Ok, right. Your patch will likely work, but I find it confusing that we're retrieving the size and not doing anything with it. If we're just trying to verify existence, how about:

if lvs "${VG}/${META_LV_NAME}" &>/dev/null; then
  return 0
fi

?

@rhvgoyal
Copy link
Collaborator Author

rhvgoyal commented Jun 9, 2015

Sounds good. I will modify my patch.

…e new one

It can happen that we created a metadata volume and later there was not
enough space to create data volume and script exited, leaving behind
metadata volume. Later if user adds more space to volume group and re-runs
the script, it fails saying metadata volume already exists.

For now fix this by checking if metadata volume exists or not. If it exists
use it (instead of erroring out).

I think ideally we should have cleanup logic and cleanup any logical
volumes created if error happens later. That probably also means
getting rid of "set -e".

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
@rhvgoyal
Copy link
Collaborator Author

rhvgoyal commented Jun 9, 2015

@cgwalters

Refereshed the patch as per your suggestion. Tested and it works. PTAL.

@cgwalters
Copy link
Member

👍

cgwalters added a commit that referenced this pull request Jun 9, 2015
docker-storage-setup: If metadata volume already exists, do not create new one
@cgwalters cgwalters merged commit d4bbe4b into projectatomic:master Jun 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

if metadata lv already exists do not try to create a new one

2 participants