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

spec: make sure we always have the target dir #271

Merged
merged 1 commit into from Jul 6, 2023
Merged

Conversation

lxbsz
Copy link
Member

@lxbsz lxbsz commented Jun 29, 2023

rtslib now requires /etc/target or /var/target to be setup (it depends on the version). The issue is that rtslib does not make those dirs and instead relies on targetcli or ceph-iscs to make them.

For container cases we do not install targetcli, so ceph-iscsi's daemons will crash or fail to setup disks, when we make rtslib calls that try to access the etc/var target dirs.

When rtslib is fixed drop this and update the dep. Note that we add both etc and var, because the kernel and userspace developers keep switching the dir they want to use.

@lxbsz lxbsz requested a review from idryomov June 29, 2023 06:40
@idryomov idryomov changed the title spec: make sure we always have the target dir is spec: make sure we always have the target dir Jun 29, 2023
ceph-iscsi.spec Outdated Show resolved Hide resolved
@idryomov
Copy link
Contributor

idryomov commented Jun 29, 2023

The issue is that rtslib does not make those dirs and instead relies on targetcli or ceph-iscs to make them.

Typo: ceph-iscsi

For container cases we do not install targetcli, so ceph-iscsi's daemons will crash or fail to setup disks, when we make rtslib calls that try to access the etc/var target dirs.

ceph-container doesn't mount /etc/target and/or /var/target from the host either, so I suspect the only reason it happens to work when these directories are created during package installation is that ceph-iscsi doesn't (need to) communicate with the kernel via either of them. This get us past the checks in rtslib but no meaningful ceph-iscsi <-> kernel communication can occur because they would be reading from/writing to different locations (container vs host).

@lxbsz
Copy link
Member Author

lxbsz commented Jun 29, 2023

The issue is that rtslib does not make those dirs and instead relies on targetcli or ceph-iscs to make them.

Typo: ceph-iscsi

For container cases we do not install targetcli, so ceph-iscsi's daemons will crash or fail to setup disks, when we make rtslib calls that try to access the etc/var target dirs.

ceph-container doesn't mount /etc/target and/or /var/target from the host either, so I suspect the only reason it happens to work when there directories are created during package installation is that ceph-iscsi doesn't (need to) communicate with the kernel via either of them. This get us past the checks in rtslib but no meaningful ceph-iscsi <-> kernel communication can occur because they would be reading from/writing to different locations (container vs host).

Yeah, sounds reasonble.

ceph-iscsi.spec Outdated Show resolved Hide resolved
rtslib now requires /etc/target or /var/target to be setup (it
depends on the version). The issue is that rtslib does not make
those dirs and instead relies on targetcli or ceph-iscsi to make
them.

For container cases we do not install targetcli, so ceph-iscsi's
daemons will crash or fail to setup disks, when we make rtslib
calls that try to access the etc/var target dirs.

When rtslib is fixed drop this and update the dep. Note that we
add both etc and var, because the kernel and userspace developers
keep switching the dir they want to use.

Signed-off-by: Xiubo Li <xiubli@redhat.com>
@idryomov
Copy link
Contributor

idryomov commented Jul 6, 2023

Additional note: since open-iscsi/rtslib-fb@99072f0 rtslib is more aggressive about changing dbroot to what it likes better (upstream it's the "preferred" /etc/target which aligns with LIO in the kernel; however in Fedora, CentOS and RHEL it's the "default" /var/target). This can cause trouble on live upgrades:

  1. rtslib attempts to change dbroot upon restart of ceph-iscsi
  2. kernel rejects the change because dbroot is in use (i.e. there are registered devices from before the upgrade)
  3. rtslib checks whether the directory it attempted to change dbroot to exists, sees that it doesn't exist, assumes that to be reason for the rejection and throws an exception

https://github.com/open-iscsi/rtslib-fb/blob/745d51a46e6718e34c59b728fd96ec08bbf906dc/rtslib/root.py#L169-L182

@idryomov idryomov merged commit 97f5b02 into ceph:main Jul 6, 2023
1 check passed
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.

None yet

2 participants