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

feat(core, uuid): add dm support #495

Merged
merged 15 commits into from
Nov 2, 2020

Conversation

akhilerm
Copy link
Contributor

@akhilerm akhilerm commented Oct 4, 2020

Why is this PR required? What issue does it fix?:
Adds DM support to NDM. With the changes in this PR: lvm, crypt, loop, dm devices will be natively supported in NDM.

What this PR does?:

  • adds DM fields to blockdevice core struct
  • add uuid generation algorithm for dm and loop devices
  • add dm device support to os disk filter

Does this PR require any upgrade changes?:
No

If the changes in this PR are manually verified, list down the scenarios covered::

Any additional information for your reviewer? :
udevadm info for dm devices (lvm and crypt)

k8s@worker-ak1:~$ udevadm info /dev/dm-0
P: /devices/virtual/block/dm-0
N: dm-0
S: disk/by-id/dm-name-vg0-lv0
S: disk/by-id/dm-uuid-LVM-j2xmqvbcVWBQK9Jdttte3CyeVTGgxtVV5VcCi3nxdwihZDxSquMOBaGL5eymBNvk
S: disk/by-uuid/2e5b68d5-134b-44c4-97bd-d857e4cb3fbc
S: mapper/vg0-lv0
S: vg0/lv0
E: DEVLINKS=/dev/disk/by-id/dm-name-vg0-lv0 /dev/disk/by-id/dm-uuid-LVM-j2xmqvbcVWBQK9Jdttte3CyeVTGgxtVV5VcCi3nxdwihZDxSquMOBaGL5eymBNvk /dev/mapper/vg0-lv0 /dev/vg0/lv0 /dev/disk/by-uuid/2e5b68d5-134b-44c4-97bd-d857e4cb3fbc
E: DEVNAME=/dev/dm-0
E: DEVPATH=/devices/virtual/block/dm-0
E: DEVTYPE=disk
E: DM_LV_NAME=lv0
E: DM_NAME=vg0-lv0
E: DM_SUSPENDED=0
E: DM_UDEV_DISABLE_LIBRARY_FALLBACK_FLAG=1
E: DM_UDEV_PRIMARY_SOURCE_FLAG=1
E: DM_UDEV_RULES=1
E: DM_UUID=LVM-j2xmqvbcVWBQK9Jdttte3CyeVTGgxtVV5VcCi3nxdwihZDxSquMOBaGL5eymBNvk
E: DM_VG_NAME=vg0
E: ID_FS_TYPE=ext4
E: ID_FS_USAGE=filesystem
E: ID_FS_UUID=2e5b68d5-134b-44c4-97bd-d857e4cb3fbc
E: ID_FS_UUID_ENC=2e5b68d5-134b-44c4-97bd-d857e4cb3fbc
E: ID_FS_VERSION=1.0
E: MAJOR=253
E: MINOR=0
E: SUBSYSTEM=block
E: TAGS=:systemd:
E: USEC_INITIALIZED=34095655487
k8s@worker-ak1:~$ udevadm info /dev/dm-2
P: /devices/virtual/block/dm-2
N: dm-2
S: disk/by-id/dm-name-backup
S: disk/by-id/dm-uuid-CRYPT-LUKS1-f4608c76343d4b5badaf6651d32f752b-backup
S: mapper/backup
E: DEVLINKS=/dev/mapper/backup /dev/disk/by-id/dm-name-backup /dev/disk/by-id/dm-uuid-CRYPT-LUKS1-f4608c76343d4b5badaf6651d32f752b-backup
E: DEVNAME=/dev/dm-2
E: DEVPATH=/devices/virtual/block/dm-2
E: DEVTYPE=disk
E: DM_NAME=backup
E: DM_SUSPENDED=0
E: DM_UDEV_PRIMARY_SOURCE_FLAG=1
E: DM_UDEV_RULES=1
E: DM_UUID=CRYPT-LUKS1-f4608c76343d4b5badaf6651d32f752b-backup
E: MAJOR=253
E: MINOR=2
E: SUBSYSTEM=block
E: SYSTEMD_READY=0
E: TAGS=:systemd:
E: USEC_INITIALIZED=662891813216

Mention if this PR is part of any design or a continuation of previous PRs

Checklist:

  • Fixes #
  • PR Title follows the convention of <type>(<scope>): <subject>
  • Has the change log section been updated?
  • Commit has unit tests
  • Commit has integration tests
  • (Optional) Are upgrade changes included in this PR? If not, mention the issue/PR to track:
  • (Optional) If documentation changes are required, which issue on https://github.com/openebs/openebs-docs is used to track them:

@akhilerm akhilerm marked this pull request as ready for review October 21, 2020 11:01
@akhilerm akhilerm requested a review from kmova October 21, 2020 11:01
cmd/ndm_daemonset/probe/udevprobe.go Show resolved Hide resolved
@@ -53,6 +54,14 @@ func generateUUID(bd blockdevice.BlockDevice) (string, bool) {
// the partition and the unique data will be stored in sectors where consumers do not have access.

switch {
/*case bd.DeviceAttributes.DeviceType == blockdevice.BlockDeviceTypeLoop:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this because we haven't added the ability to mark a block device as loop device in udev package yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its because we havent decided how we should generate UUID for loop devices. Since loop devices are only present virtually on the node, we may use the devicename + hostname method here.

cc: @kmova need your thoughts on this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will the loop names change on reboots @akhilerm.

Can we read a few chars from the loop device and then use them to generate an ID?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will the loop names change on reboots @akhilerm.

No @kmova , it wont.

Can we read a few chars from the loop device and then use them to generate an ID?

That means, the backing file right? Its content can change.

@RealHarshThakur
Copy link
Contributor

RealHarshThakur commented Oct 21, 2020

Two more questions in general :

  1. At this moment, we rely on end user to create LVM, right? If so, is there any plan to extend NDM to do that?
  2. How is NDM going to be bring LVM in context of block devices? All the holder devices and their partitions(if exist) which are usable would be considered as a single block device?

@akhilerm
Copy link
Contributor Author

Two more questions in general :

  1. At this moment, we rely on end user to create LVM, right? If so, is there any plan to extend NDM to do that?

Yes, there is. But how we implement that and will it be a separate component in NDM have to be thought through

  1. How is NDM going to be bring LVM in context of block devices? All the holder devices and their partitions(if exist) which are usable would be considered as a single block device?

Not as a single blockdevice. All devices, which does not have holders or partitions will be a blockdevice resource.
Consider the example:

NAME        MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
sdd           8:48   0   16G  0 disk 
└─sdd1        8:49   0   16G  0 part 
sdb           8:16   0   16G  0 disk 
└─sdb1        8:17   0   16G  0 part 
sr0          11:0    1 1024M  0 rom  
fd0           2:0    1    4K  0 disk 
sdc           8:32   0   50G  0 disk 
├─sdc2        8:34   0   10G  0 part 
├─sdc3        8:35   0   10G  0 part 
│ └─vg0-lv1 253:1    0   11G  0 lvm  
├─sdc1        8:33   0   10G  0 part 
│ ├─vg0-lv1 253:1    0   11G  0 lvm  
│ └─vg0-lv0 253:0    0    1G  0 lvm  
└─sdc4        8:36   0   10G  0 part 
sda           8:0    0  100G  0 disk 
├─sda2        8:2    0    1K  0 part 
├─sda5        8:5    0    8G  0 part 
└─sda1        8:1    0   92G  0 part /

The devices /dev/sdd1, /dev/sdb1, /dev/sdc2, /dev/sdc4, /dev/dm-0, /dev/dm-1 will be the blockdevices here.

@akhilerm akhilerm requested a review from pawanpraka1 October 22, 2020 06:40
Signed-off-by: Akhil Mohan <[email protected]>
Signed-off-by: Akhil Mohan <[email protected]>
Signed-off-by: Akhil Mohan <[email protected]>
Signed-off-by: Akhil Mohan <[email protected]>
Signed-off-by: Akhil Mohan <[email protected]>
cmd/ndm_daemonset/probe/uuid.go Show resolved Hide resolved
cmd/ndm_daemonset/probe/uuid.go Outdated Show resolved Hide resolved
ndm-operator.yaml Outdated Show resolved Hide resolved
cmd/ndm_daemonset/filter/osdiskexcludefilter.go Outdated Show resolved Hide resolved
- add dm support to os disk filter

Signed-off-by: Akhil Mohan <[email protected]>
Signed-off-by: Akhil Mohan <[email protected]>
Signed-off-by: Akhil Mohan <[email protected]>
Signed-off-by: Akhil Mohan <[email protected]>
Signed-off-by: Akhil Mohan <[email protected]>
Signed-off-by: Akhil Mohan <[email protected]>
@akhilerm akhilerm requested a review from pawanpraka1 October 30, 2020 11:23
kmova
kmova previously approved these changes Nov 1, 2020
@kmova
Copy link
Contributor

kmova commented Nov 1, 2020

The approach LGTM. Can be merged after @pawanpraka1 review.

RealHarshThakur
RealHarshThakur previously approved these changes Nov 1, 2020
@RealHarshThakur
Copy link
Contributor

LGTM! On a side note: Support for loop devices will be very important because we can use loop devices in integration tests and test for detection of dm and md(in future).

Signed-off-by: Akhil Mohan <[email protected]>
@akhilerm akhilerm dismissed stale reviews from RealHarshThakur and kmova via e89b6a3 November 2, 2020 04:50
@akhilerm
Copy link
Contributor Author

akhilerm commented Nov 2, 2020

@harshthakur9030 , I have added support for loop devices also into the PR.

Copy link
Contributor

@pawanpraka1 pawanpraka1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good.

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.

4 participants