Project

General

Profile

Bug #1527

lvremove fails to wait for input

Added by Florian Heigl over 6 years ago. Updated about 6 years ago.

Status:
Closed
Priority:
High
Assignee:
Category:
Aports
Target version:
Start date:
12/19/2012
Due date:
% Done:

100%

Estimated time:
Affected versions:
Security IDs:

Description

This is a real bug, not just a mis-display. But a non-critical one.

waxh0012:~# lvremove /dev/vgxen/lvveritas
Do you really want to remove active logical volume lvveritas? [y/n]:
Logical volume lvveritas not removed

Instead of waiting for a y/n it just takes nothing as "n" and cancels.
lvremove -f is working.

I'm unsure what's causing this, saw it on multiple installs.
It happens even in bash, so it might be ssh, so i'm pasting my env.

SSH_CLIENT=.......remove....22
USER=root
MAIL=/var/mail/root
HOME=/root
SSH_TTY=/dev/pts/0
PAGER=less
PS1=\h:\w\$
LOGNAME=root
TERM=xterm-256color
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
SHELL=/bin/ash
PWD=/root
CHARSET=UTF-8
SSH_CONNECTION=removed

Associated revisions

Revision eca60a85 (diff)
Added by Natanael Copa about 6 years ago

main/libc0.9.32: make getchar/putchar as inlines

fixes issue with lvm2 which modifies stdin.

Patch from Timo.

ref #1527

Revision 6429f9b3 (diff)
Added by Natanael Copa about 6 years ago

main/lvm2: rebuild against an uclibc with fixed getchar

fixes #1527

History

#1 Updated by Natanael Copa over 6 years ago

sounds like it fails to detect that you have an interactive terminal, as if you were doing lvremove ... </dev/null or similar. Have you tried from other terminal (one that is not xterm-256color)?

#2 Updated by Florian Heigl over 6 years ago

I tried setting TERM=vt100, no better.
I've seen this for quite some time but I'll retry with some more terminal types.
Maybe something is wrong with my termcap(ish, stuff, whatever it is)

#3 Updated by Florian Heigl over 6 years ago

I tried the following:
Add ncurses and related terminal stuff to support my OS X terminal's type.
Use a plain normal Xterm
Use a bash instead of default shell.

Both show same issue.

(Of course also used multiple servers to verify, just not multiple clients since I have less of those)

#4 Updated by Natanael Copa about 6 years ago

  • Target version set to Alpine 2.6.0

#5 Updated by Natanael Copa about 6 years ago

Finally looking at this. and it is trivial to reproduce.

the yes_no_prompt() function (defined in lib/display/display.c) does not work as expected. I have printf debugged it:

/*
 * Prompt for y or n from stdin.
 * Defaults to 'no' in silent mode.
 * All callers should support --yes and/or --force to override this.
 */
char yes_no_prompt(const char *prompt, ...)
{
        int c = 0, ret = 0;
        va_list ap;

        if (silent_mode()) {
                printf("DEBUG: in silent mode\n");
                return 'n';
        }
        printf("DEBUG: NOT in silent mode\n");

        sigint_allow();
        do {
                if (c == '\n' || !c) {
                        va_start(ap, prompt);
                        vfprintf(stderr, prompt, ap);
                        va_end(ap);
                        fflush(stderr);
                        ret = 0;
                }

                if ((c = getchar()) == EOF) {
                        printf("DEBUG: getchar gave us EOF\n");
                        ret = 'n';
                        break;
                }

                c = tolower(c);
                if ((c == 'y') || (c == 'n')) {
                        /* If both 'y' and 'n' given, begin again. */
                        if (ret && c != ret)
                                ret = -1;
                        else
                                ret = c;
                }
        } while (ret < 1 || c != '\n');

        sigint_restore();

        if (c != '\n')
                fprintf(stderr, "\n");

        return ret;
}

I get:

DEBUG: NOT in silent mode
Do you really want to remove active logical volume lv_test? [y/n]: DEBUG: getchar gave us EOF

So basically, getchar() does not work as expected.

I did find something suspicious in the initializing code, lib/commands/toolcontext.c: create_toolcontext()
It was me who added the printf DEBUG. we do get there...

#ifndef VALGRIND_POOL
        /* Set in/out stream buffering before glibc */
        if (set_buffering) {
                printf("DEBUG: we are setting buffering\n");
                /* Allocate 2 buffers */
                if (!(cmd->linebuffer = dm_malloc(2 * linebuffer_size))) {
                        log_error("Failed to allocate line buffer.");
                        goto out;
                }

                if (is_valid_fd(STDIN_FILENO)) {
                        if (!_reopen_stream(stdin, STDIN_FILENO, "r", "stdin", &new_stream))
                                goto_out;
                        stdin = new_stream;
                        if (setvbuf(stdin, cmd->linebuffer, _IOLBF, linebuffer_size)) {
                                log_sys_error("setvbuf", "");
                                goto out;
                        }
                }

                if (is_valid_fd(STDOUT_FILENO)) {
                        if (!_reopen_stream(stdout, STDOUT_FILENO, "w", "stdout", &new_stream))
                                goto_out;
                        stdout = new_stream;
                        if (setvbuf(stdout, cmd->linebuffer + linebuffer_size,
                                     _IOLBF, linebuffer_size)) {
                                log_sys_error("setvbuf", "");
                                goto out;
                        }
                }
                /* Buffers are used for lines without '\n' */
        } else
                /* Without buffering, must not use stdin/stdout */
                init_silent(1);
#endif


If i disable that #ifdef it works as expected. So something is buggy with uclibc's setvfbuf.

The uclibc doc1 says:

7) uClibc's setvbuf is more restrictive about when it can be called than glibc's
   is.  The standards specify that setvbuf must occur before any other operations
   take place on the stream.

1 https://www.kernel.org/pub/linux/libs/uclibc/Glibc_vs_uClibc_Differences.txt

#6 Updated by Timo Teräs about 6 years ago

setvbuf is not the culprit.

Problem is that LVM closes stdin, reopens the fd, and assigns stdin = new fd. glibc copes with this. However, in uClibc stdin (and stdout) have mirror variable __stdin that is used by the getchar() macro. Apparently the idea is to make sure getchar() does not get function local variable stdin.

I believe the usage of mirror variables __stdin and __stdout is wrong. They should be symbol aliases, or fix the getchar() macro to be an inline function that can safely refer to the stdin name resulting in using the right global symbol.

#7 Updated by Natanael Copa about 6 years ago

  • Priority changed from Low to High

#8 Updated by Natanael Copa about 6 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 100

#9 Updated by Natanael Copa about 6 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF