Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ceph-disk: get Nonetype when ceph-disk list with --format plain on single device. #6410

Merged
2 commits merged into from Nov 3, 2015

Conversation

Projects
None yet
2 participants
@Vicente-Cheng
Copy link
Contributor

Vicente-Cheng commented Oct 28, 2015

with dmcrypt device, ceph-disk list will get Nonetype with the real device
i.e. /dev/sdb1 for /dev/dm-1, `ceph-disk --list /dev/sdb1` will get Nonetype

that because the holder we query is `dm-x`, the devices includ the path
is `sdX`, it cannot match.

if we list format plain and meet dmcrypt device, query again with the
holder (dm-x).

Also change the list_devices() parameter to let other function to use it
easily.

Signed-off-by: Vicente Cheng freeze.bilsted@gmail.com

@ghost ghost added bug fix core labels Oct 28, 2015

@ghost ghost self-assigned this Oct 28, 2015

@Vicente-Cheng Vicente-Cheng force-pushed the Vicente-Cheng:ceph-disk-list-plain-single-failure branch from bd13c48 to 25b0360 Oct 28, 2015

@@ -2851,8 +2855,8 @@ def list_dev(dev, uuid_map, journal_map):

return info

def list_devices(args):
partmap = list_all_partitions(args.path)
def list_devices(path=[]):

This comment has been minimized.

@ghost

ghost Oct 28, 2015

there is no need for =[]

This comment has been minimized.

@Vicente-Cheng

Vicente-Cheng Oct 28, 2015

Author Contributor

OH, I just add it for convince when other function could use easily.
It looks like not make sense, I will remove the =[]

thanks!!
vicente

@ghost

This comment has been minimized.

Copy link

ghost commented Oct 28, 2015

Nice catch ! Good to merge with unit tests covering the modified lines ( see src/test/python/ceph-disk/tests/test_ceph_disk.py ).

holder = '/dev/' + dmcrypt['holders'][0]
# refind with the dm-x path
req.append(holder)
devices = list_devices(req)

This comment has been minimized.

@ghost

ghost Oct 28, 2015

devices = list_devices([holder]) would also work here

This comment has been minimized.

@Vicente-Cheng

Vicente-Cheng Oct 28, 2015

Author Contributor

that's great!! 👍

thanks for reminder : )

ceph-disk: get Nonetype when ceph-disk list with --format plain on si…
…ngle device.

    with dmcrypt device, ceph-disk list will get Nonetype with the real device
    i.e. /dev/sdb1 for /dev/dm-1, `ceph-disk --list /dev/sdb1` will get Nonetype

    that because the holder we query is `dm-x`, the devices includ the path
    is `sdX`, it cannot match.

    if we list format plain and meet dmcrypt device, query again with the
    holder (dm-x).

    Also change the list_devices() parameter to let other function to use it
    easily.

Signed-off-by: Vicente Cheng <freeze.bilsted@gmail.com>

@Vicente-Cheng Vicente-Cheng force-pushed the Vicente-Cheng:ceph-disk-list-plain-single-failure branch from 25b0360 to 570285b Oct 28, 2015

@Vicente-Cheng

This comment has been minimized.

Copy link
Contributor Author

Vicente-Cheng commented Oct 28, 2015

Hi @dachary,
I completed the unit tests covering this change.

thanks!!!
vicente

}]
with patch.multiple(
ceph_disk,
list_devices=lambda path: dev_latest,

This comment has been minimized.

@ghost

ghost Oct 28, 2015

list_devices=lambda path: devices,

should work here, don't you think ? Or is there a need for dev_latest ?

This comment has been minimized.

@Vicente-Cheng

Vicente-Cheng Oct 29, 2015

Author Contributor

Yes, that could work normal.
I return a new value due to the code path. (It could use the same return we ever)
I will change it : ), thanks for reminder!!

test: ceph-disk: coverage list_format_dev_plain() new behavior.
    modify the unittest to coverage the list_format_dev_plain()
    new behavior.

Signed-off-by: Vicente Cheng <freeze.bilsted@gmail.com>

@Vicente-Cheng Vicente-Cheng force-pushed the Vicente-Cheng:ceph-disk-list-plain-single-failure branch from c8f9827 to 83afe15 Oct 29, 2015

ghost pushed a commit that referenced this pull request Nov 3, 2015

Loic Dachary
Merge pull request #6410 from Vicente-Cheng/ceph-disk-list-plain-sing…
…le-failure

ceph-disk: get Nonetype when ceph-disk list with --format plain on single device.

Reviewed-by: Loic Dachary <ldachary@redhat.com>

@ghost ghost merged commit cab3d3f into ceph:master Nov 3, 2015

@Vicente-Cheng Vicente-Cheng deleted the Vicente-Cheng:ceph-disk-list-plain-single-failure branch Nov 4, 2015

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.