Skip to content

Commit

Permalink
susedistribution: Run user virtio console on it's own socket
Browse files Browse the repository at this point in the history
This allows switching between root and non-root user.

NOTE: passing socked_path is possible only to updated os-autoinst
backend/qemu.pm (must have this path in virtio_console_names())
thus guarded with $OpenQA::Isotovideo::Interface::version ge 35
detection (changes in 20d0119b ("backend/qemu.pm: Add
virtio_console_user fifo name")).

Console names should be handled in library (why tests should care about
the internals?), but for now at least properly document that.

This also requires to add new getty (serial console) to
system_prepare.pm (will be working with newly generated images) and
shift numbers in wickedbase.pm. This will work also on older
backend/qemu.pm, getty just won't be used.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
  • Loading branch information
pevik committed Jan 23, 2023
1 parent cabcded commit 4a89bfb
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 6 deletions.
23 changes: 20 additions & 3 deletions lib/serial_terminal.pm
Expand Up @@ -59,20 +59,37 @@ sub add_serial_console {
prepare_serial_console();
Wrapper for add_serial_console.
Configure serial consoles for virtio support (root-virtio-terminal and
user-virtio-terminal).
NOTE: if test plans to use more consoles via VIRTIO_CONSOLE_NUM, it have to
call add_serial_console() with proper console name (beware different number
for ppc64le).
=cut

sub prepare_serial_console {
# Configure serial consoles for virtio support
# poo#18860 Enable console on hvc0 on SLES < 12-SP2
# poo#44699 Enable console on hvc1 to fix login issues on ppc64le
record_info('getty before', script_output('systemctl | grep serial-getty'));

if (!check_var('VIRTIO_CONSOLE', 0)) {
my $console = 'hvc1';

This comment has been minimized.

Copy link
@mloviska

mloviska Jan 26, 2023

Contributor

Hi Peto, this is causing failures in Xen tests.

https://openqa.suse.de/tests/10376352#step/journal_check/5

This comment has been minimized.

Copy link
@pevik

pevik Jan 26, 2023

Author Contributor

@mloviska thanks for a report, sorry for introducing a regression.
This is strange, because that console was properly added: https://openqa.suse.de/tests/10376352#step/system_prepare/11, and should be used for user-virtio-terminal.

Any hint why it's used in tests/console/journal_check.pm for root-virtio-terminal?

sub run {
my $self = shift;
my $bug_pattern = parse_bug_refs();
select_serial_terminal;
my @journal_output = split(/\n/, script_output("journalctl --no-pager --quiet -p ${\get_var('JOURNAL_LOG_LEVEL', 'err')} -o short-precise"));

For myself: 4a89bfb caused that.

This comment has been minimized.

Copy link
@mloviska

mloviska Jan 26, 2023

Contributor

the module runs on all our hypervisors, even those that do not support virtio console yet. So in this particular case that function selects tty.

This comment has been minimized.

Copy link
@pevik

pevik Jan 26, 2023

Author Contributor

BTW it's working on JeOS on Tumbleweed: https://openqa.suse.de/tests/10371740#step/journal_check/9
Testing with disabled adding that console on SLES: https://openqa.suse.de/tests/10379176#details

This comment has been minimized.

Copy link
@ggardet

ggardet Jan 26, 2023

Collaborator

This breaks on RPi3/4 (generalhw backend) as well: https://openqa.opensuse.org/tests/3065106#step/system_prepare/8

This comment has been minimized.

Copy link
@pevik

pevik Jan 26, 2023

Author Contributor

@ggardet also thanks for a report. Obviously add_serial_console($console); which adds user-virtio-terminal must be guarded under some if condition. But how to detect it? This code is intended for jobs which prepare qcow image for QEMU jobs which use it. Shouldn't there be a check in the beginning of prepare_serial_console() whether job is being run on BACKEND=qemu or BACKEND=svirt (svirt is used on s390x KVM based tests on osd)? Or are there other backends which need this function?
FYI also document the problems in https://progress.opensuse.org/issues/122935

This comment has been minimized.

Copy link
@ggardet

ggardet Jan 26, 2023

Collaborator

Not sure which if guard to add, but this can be tested with some VR on RPi2/3/4 machines.

This comment has been minimized.

Copy link
@pevik

pevik Jan 27, 2023

Author Contributor

Sure, I plan to do.

This comment has been minimized.

Copy link
@pevik

pevik Jan 27, 2023

Author Contributor

@ggardet @mloviska FYI #16305
@mloviska This will fix https://openqa.suse.de/tests/10376352#step/journal_check/5, because it will skip unless qemu backend.


# poo#18860 Enable console on hvc0 on SLES < 12-SP2 (root-virtio-terminal)
if (is_sle('<12-SP2') && !is_s390x) {
add_serial_console('hvc0');
}
# poo#44699 Enable console on hvc1 to fix login issues on ppc64le
# (root-virtio-terminal)
elsif (get_var('OFW')) {
add_serial_console('hvc1');
$console = 'hvc2';
}

# user-virtio-terminal
add_serial_console($console);
}

record_info('getty after', script_output('systemctl | grep serial-getty'));
}

=head2 get_login_message
Expand Down
7 changes: 6 additions & 1 deletion lib/susedistribution.pm
Expand Up @@ -21,6 +21,7 @@ use Utils::Backends;
use backend::svirt qw(SERIAL_TERMINAL_DEFAULT_DEVICE SERIAL_TERMINAL_DEFAULT_PORT);
use Cwd;
use autotest 'query_isotovideo';
use isotovideo;

=head1 SUSEDISTRIBUTION
Expand Down Expand Up @@ -428,7 +429,11 @@ sub init_consoles {

if (is_qemu) {
$self->add_console('root-virtio-terminal', 'virtio-terminal', {});
$self->add_console('user-virtio-terminal', 'virtio-terminal', {});

$self->add_console('user-virtio-terminal', 'virtio-terminal',
isotovideo::get_version() >= 35 ?
{socked_path => cwd() . '/virtio_console_user'} : {});

This comment has been minimized.

Copy link
@Vogtinator

Vogtinator Jan 27, 2023

Member

Not socket?

This comment has been minimized.

Copy link
@pevik

pevik Jan 27, 2023

Author Contributor

I'm sorry I don't follow you. What exactly do you mean on this code?
FYI the fix of the previous is: #16305

This comment has been minimized.

Copy link
@Vogtinator

Vogtinator Jan 27, 2023

Member

It says socked_path while I'd expect socket_path

This comment has been minimized.

Copy link
@pevik

pevik Jan 27, 2023

Author Contributor

Well, while this is really strange name (t => d change in f9c71d0e25c50ee9f9acf76560b80791ca3bf6c8) it's matching the code: https://github.com/os-autoinst/os-autoinst/blob/61f9e42e3d714cd59211107909c1d7879269eae2/consoles/virtio_terminal.pm#L46

The fix is really #16305

@cfconrad ^ :). But I'd like to remove this argument from console (replace is with user vs root and id), thus we don't have to bother to cleanup that.

This comment has been minimized.

Copy link
@Vogtinator

Vogtinator Jan 27, 2023

Member

Looks like it was socked_path already before that commit: $self->{socket_path} = $self->{args}->{socked_path} // cwd() . '/virtio_console';

This comment has been minimized.

Copy link
@pevik

pevik Jan 27, 2023

Author Contributor

Yes, I was wrong, thesocked_path appeared in os-autoinst/os-autoinst@6de2fec.

This comment has been minimized.

Copy link
@cfconrad

cfconrad Jan 30, 2023

Contributor

yes that was my typo, sorry!

Maybe replacing it with a simple suffix, might be good idea.

This comment has been minimized.

Copy link
@pevik

pevik Jan 30, 2023

Author Contributor

I'll prefer really move away the socket to os-autoinst. This is implementation detail, which should not be spread in both git repositories. But it will have to wait till I get some time for openQA.


for (my $num = 1; $num < get_var('VIRTIO_CONSOLE_NUM', 1); $num++) {
$self->add_console('root-virtio-terminal' . $num, 'virtio-terminal', {socked_path => cwd() . '/virtio_console' . $num});
}
Expand Down
2 changes: 1 addition & 1 deletion lib/wickedbase.pm
Expand Up @@ -993,7 +993,7 @@ sub pre_run_hook {
wait_serial($coninfo, undef, 0, no_regex => 1);
send_key 'ret';
if ($self->{name} eq 'before_test' && get_var('VIRTIO_CONSOLE_NUM', 1) > 1) {
my $serial_terminal = is_ppc64le ? 'hvc2' : 'hvc1';
my $serial_terminal = is_ppc64le ? 'hvc3' : 'hvc2';
add_serial_console($serial_terminal);
}
if ($self->{name} ne 'before_test' && get_var('WICKED_TCPDUMP')) {
Expand Down
2 changes: 1 addition & 1 deletion tests/console/system_prepare.pm
Expand Up @@ -5,7 +5,7 @@

# Summary: Execute SUT changes which should be permanent
# - Grant permissions on serial device
# - Add hvc0/hvc1 to /etc/securetty
# - Add hvc0/hvc1 and hvc1/hvc2 to /etc/securetty
# - Register modules if SCC_ADDONS, MEDIA_UPGRADE and in Regression flavor
# are defined
# - If system is vmware, set resolution to 1024x768 (and write to grub)
Expand Down

0 comments on commit 4a89bfb

Please sign in to comment.