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

Fix targetclid daemon infinite stuck #165

Merged
merged 1 commit into from
Mar 30, 2020

Conversation

pkalever
Copy link
Contributor

We need to open a byte IO stream because we are actually dealing with
binary data in memory.

Signed-off-by: Prasanna Kumar Kalever prasanna.kalever@redhat.com

We need to open a byte IO stream because we are actually dealing with
binary data in memory.

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
@maurizio-lombardi maurizio-lombardi merged commit 7ecf883 into open-iscsi:master Mar 30, 2020
pkalever pushed a commit to pkalever/gluster-block that referenced this pull request Mar 31, 2020
targetcli is hanging with
* open-iscsi/targetcli-fb#156

This is fixed now with
* open-iscsi/targetcli-fb#165

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
pkalever pushed a commit to pkalever/gluster-block that referenced this pull request Mar 31, 2020
targetcli is hanging with
* open-iscsi/targetcli-fb#156

This is fixed now with
* open-iscsi/targetcli-fb#165

Reviewed-by: Xiubo Li <xiubli@redhat.com>
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
pkalever pushed a commit to pkalever/gluster-block that referenced this pull request Mar 31, 2020
targetcli is hanging with
* open-iscsi/targetcli-fb#156

This is fixed now with
* open-iscsi/targetcli-fb#165

Reviewed-by: Xiubo Li <xiubli@redhat.com>
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
pkalever pushed a commit to gluster/gluster-block that referenced this pull request Mar 31, 2020
targetcli is hanging with
* open-iscsi/targetcli-fb#156

This is fixed now with
* open-iscsi/targetcli-fb#165

Reviewed-by: Xiubo Li <xiubli@redhat.com>
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
@iammattcoleman
Copy link
Contributor

I somehow totally botched my test of this pull request the other day. targetcli freezes on my system with this change.

@iammattcoleman
Copy link
Contributor

iammattcoleman commented Apr 2, 2020

@pkalever I didn't actually get a chance to do the debugging I mentioned in #164 yet. I'll check it out more when I wake up.

In case you want to look into it before then, my IDE is warning me about...

  • line 172: print(str(e), file=f) # push error to stream has file=f highlighted with the error "Expected type 'Optional[_Writer]', got 'BytesIO' instead"
  • line 182: connection.sendall(output.encode()) # actual string has encode highlighted because the bytes object doesn't have an encode() method

We need to figure out why our systems are behaving so differently. My main test system has Python 3.5, but I've also tested on 3.7 and was unable to reproduce the hangs you had with StringIO. When I changed to BytesIO, it started hanging and seems like you were describing your system was doing with StringIO.

@pkalever
Copy link
Contributor Author

pkalever commented Apr 2, 2020

@iammattcoleman is this some kind of upstream setup? which can be accessed?
Just curious to debug this environmental issue.

@iammattcoleman
Copy link
Contributor

@pkalever You won't be able to reproduce my exact setup: I'm developing for Ubuntu and am using custom-built targetcli-fb/configshell-fb/rtslib-fb packages. The packages are built using Fedora's spec files as a base, so they're almost identical to the latest Fedora packages.

Differences include:

  • all three packages have been updated to use Debian's path, /etc/rtslib-fb-target
  • targetcli-fb has been updated to version 2.1.51
  • targetcli-fb has my "Use StringIO as a buffer instead of a file" commit applied as a patch
  • configshell-fb has been updated to version 1.1.27
  • configshell-fb has my "Ensure that all output reaches the client when daemonized" commit applied as a patch
  • rtslib-fb has been updated to version 2.1.71

Can you describe your setup with Python 2? I'd like to set up a VM with a similar configuration.

@pkalever
Copy link
Contributor Author

pkalever commented Apr 3, 2020

@iammattcoleman mine is a simple centos7 vagrant box, running targetcli/rtslib/configshell master source installed.

If you are interested in bringup the vagrant boxes or get to the same environment then follow: https://github.com/gluster/gluster-block#how-to-quickly-bringup-gluster-block-environment-locally-

or there is a Vagrantfile in the repo which gives context about OS env and the ansible playbooks will guide you through detailed steps.

Thanks!

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

3 participants