Adam Williamson
4 years ago
parent
92814e9c94
commit
6489429663
@ -1,36 +0,0 @@
|
|||||||
From 442f79e8b018ef4fed91c1256d5f4796cc42a36e Mon Sep 17 00:00:00 2001
|
|
||||||
From: Adam Williamson <awilliam@redhat.com>
|
|
||||||
Date: Wed, 14 Apr 2021 21:43:27 -0700
|
|
||||||
Subject: [PATCH] Fix return value of _dbus_do_call
|
|
||||||
|
|
||||||
This was relying on perl's default return behaviour before
|
|
||||||
(return the result of the final line of the function, if it's an
|
|
||||||
expression). But now that's wrong, as the last line is something
|
|
||||||
else. We need to be explicit about what we're returning (which
|
|
||||||
would generally be a good thing to do anyway).
|
|
||||||
|
|
||||||
Related progress issue: https://progress.opensuse.org/issues/91163
|
|
||||||
|
|
||||||
Signed-off-by: Adam Williamson <awilliam@redhat.com>
|
|
||||||
---
|
|
||||||
backend/qemu.pm | 3 ++-
|
|
||||||
1 file changed, 2 insertions(+), 1 deletion(-)
|
|
||||||
|
|
||||||
diff --git a/backend/qemu.pm b/backend/qemu.pm
|
|
||||||
index 735e349c..04bb05fb 100644
|
|
||||||
--- a/backend/qemu.pm
|
|
||||||
+++ b/backend/qemu.pm
|
|
||||||
@@ -141,8 +141,9 @@ sub _dbus_do_call {
|
|
||||||
my $bus = Net::DBus->system(private => 1);
|
|
||||||
my $bus_service = $bus->get_service("org.opensuse.os_autoinst.switch");
|
|
||||||
my $bus_object = $bus_service->get_object("/switch", "org.opensuse.os_autoinst.switch");
|
|
||||||
- $bus_object->$fn(@args);
|
|
||||||
+ my @result = $bus_object->$fn(@args);
|
|
||||||
$bus->get_connection->disconnect;
|
|
||||||
+ return @result;
|
|
||||||
}
|
|
||||||
|
|
||||||
sub _dbus_call {
|
|
||||||
--
|
|
||||||
2.31.1
|
|
||||||
|
|
||||||
@ -1,49 +0,0 @@
|
|||||||
From 8776da7055d572fc6223eb6ce9ad4c3a5b2f925b Mon Sep 17 00:00:00 2001
|
|
||||||
From: Adam Williamson <awilliam@redhat.com>
|
|
||||||
Date: Tue, 13 Apr 2021 09:02:55 -0700
|
|
||||||
Subject: [PATCH] Re-connect to dbus for each call
|
|
||||||
|
|
||||||
By using the system bus and leaving our connection to it open
|
|
||||||
for the entire lifetime of the isotovideo process, we're setting
|
|
||||||
ourselves up to receive a lot of signals we don't deal with, and
|
|
||||||
just letting them accumulate. Since all we want to do is send a
|
|
||||||
signal and then go away, let's use a private socket, and
|
|
||||||
re-initialize the connection for each signal. The overhead of
|
|
||||||
doing this is not large, and we're sending small numbers of
|
|
||||||
signals per isotovideo process anyway (one signal per network on
|
|
||||||
startup, one signal at startup if a debug variable is set, and
|
|
||||||
one signal per network at end).
|
|
||||||
|
|
||||||
Thanks to @dvdhrm and @berrange for their help with this.
|
|
||||||
|
|
||||||
Related progress issue: https://progress.opensuse.org/issues/90872
|
|
||||||
---
|
|
||||||
backend/qemu.pm | 12 ++++++++----
|
|
||||||
1 file changed, 8 insertions(+), 4 deletions(-)
|
|
||||||
|
|
||||||
diff --git a/backend/qemu.pm b/backend/qemu.pm
|
|
||||||
index 6c64f497..735e349c 100644
|
|
||||||
--- a/backend/qemu.pm
|
|
||||||
+++ b/backend/qemu.pm
|
|
||||||
@@ -135,10 +135,14 @@ sub stop_qemu {
|
|
||||||
|
|
||||||
sub _dbus_do_call {
|
|
||||||
my ($self, $fn, @args) = @_;
|
|
||||||
- $self->{dbus} ||= Net::DBus->system;
|
|
||||||
- $self->{dbus_service} ||= $self->{dbus}->get_service("org.opensuse.os_autoinst.switch");
|
|
||||||
- $self->{dbus_object} ||= $self->{dbus_service}->get_object("/switch", "org.opensuse.os_autoinst.switch");
|
|
||||||
- $self->{dbus_object}->$fn(@args);
|
|
||||||
+ # we intentionally do not persist the dbus connection to avoid
|
|
||||||
+ # queueing up signals we are not interested in handling:
|
|
||||||
+ # https://progress.opensuse.org/issues/90872
|
|
||||||
+ my $bus = Net::DBus->system(private => 1);
|
|
||||||
+ my $bus_service = $bus->get_service("org.opensuse.os_autoinst.switch");
|
|
||||||
+ my $bus_object = $bus_service->get_object("/switch", "org.opensuse.os_autoinst.switch");
|
|
||||||
+ $bus_object->$fn(@args);
|
|
||||||
+ $bus->get_connection->disconnect;
|
|
||||||
}
|
|
||||||
|
|
||||||
sub _dbus_call {
|
|
||||||
--
|
|
||||||
2.31.1
|
|
||||||
|
|
||||||
@ -1,34 +0,0 @@
|
|||||||
From b1ea2d3376d8d337bb84230eead0759a57905f8c Mon Sep 17 00:00:00 2001
|
|
||||||
From: Adam Williamson <awilliam@redhat.com>
|
|
||||||
Date: Thu, 15 Apr 2021 16:43:24 -0700
|
|
||||||
Subject: [PATCH] Update qemu argument used to disable floppy drive for qemu
|
|
||||||
6.0
|
|
||||||
|
|
||||||
The way we were doing this before no longer works with qemu 6.0.
|
|
||||||
Markus Armbruster suggests using -nodefaults, but that's a bigger
|
|
||||||
gun and we'd have to check we aren't relying on the default
|
|
||||||
devices in any way before using it. This was his second choice
|
|
||||||
suggestion "If you'd prefer not to". It does seem to work (with
|
|
||||||
both older and newer qemu). See:
|
|
||||||
|
|
||||||
https://bugs.launchpad.net/bugs/1923663
|
|
||||||
---
|
|
||||||
backend/qemu.pm | 2 +-
|
|
||||||
1 file changed, 1 insertion(+), 1 deletion(-)
|
|
||||||
|
|
||||||
diff --git a/backend/qemu.pm b/backend/qemu.pm
|
|
||||||
index 04bb05fb..28a6f88a 100644
|
|
||||||
--- a/backend/qemu.pm
|
|
||||||
+++ b/backend/qemu.pm
|
|
||||||
@@ -857,7 +857,7 @@ sub start_qemu {
|
|
||||||
{
|
|
||||||
# Remove floppy drive device on architectures
|
|
||||||
unless ($arch eq 'aarch64' || $arch eq 'arm' || $vars->{QEMU_NO_FDC_SET}) {
|
|
||||||
- sp('global', 'isa-fdc.driveA=');
|
|
||||||
+ sp('global', 'isa-fdc.fdtypeA=none');
|
|
||||||
}
|
|
||||||
|
|
||||||
sp('m', $vars->{QEMURAM}) if $vars->{QEMURAM};
|
|
||||||
--
|
|
||||||
2.31.1
|
|
||||||
|
|
||||||
@ -1,108 +0,0 @@
|
|||||||
From 61b1834c5366c78f0e4d3b21a142d7dd6d6322c1 Mon Sep 17 00:00:00 2001
|
|
||||||
From: Fabian Vogt <fvogt@suse.de>
|
|
||||||
Date: Tue, 13 Apr 2021 15:41:00 +0200
|
|
||||||
Subject: [PATCH] signalblocker: Also block SIGCHLD
|
|
||||||
|
|
||||||
The perl-unaware threads created by OpenCV also crash when they encounter a
|
|
||||||
SIGCHLD signal, which happens by using perl's "system" function for instance.
|
|
||||||
---
|
|
||||||
signalblocker.pm | 22 +++++++++++-----------
|
|
||||||
t/28-signalblocker.t | 14 ++++++++++++--
|
|
||||||
2 files changed, 23 insertions(+), 13 deletions(-)
|
|
||||||
|
|
||||||
diff --git a/signalblocker.pm b/signalblocker.pm
|
|
||||||
index 1ff3652c..dea56e12 100644
|
|
||||||
--- a/signalblocker.pm
|
|
||||||
+++ b/signalblocker.pm
|
|
||||||
@@ -1,4 +1,4 @@
|
|
||||||
-# Copyright © 2020 SUSE LLC
|
|
||||||
+# Copyright © 2021 SUSE LLC
|
|
||||||
#
|
|
||||||
# This program is free software; you can redistribute it and/or modify
|
|
||||||
# it under the terms of the GNU General Public License as published by
|
|
||||||
@@ -19,23 +19,23 @@ use Mojo::Base -base;
|
|
||||||
use bmwqemu;
|
|
||||||
use POSIX ':signal_h';
|
|
||||||
|
|
||||||
-# OpenCV forks a lot of threads and the TERM signal we may get from the
|
|
||||||
-# parent process would be delivered to an undefined thread. But as those
|
|
||||||
-# threads do not have a perl interpreter, the perl signal handler (we set
|
|
||||||
-# later) would crash. So we need to block the TERM signal in the forked
|
|
||||||
-# processes before we set the signal handler of our choice.
|
|
||||||
+# OpenCV forks a lot of threads and the signals we may get (TERM from the
|
|
||||||
+# parent, CHLD from children) would be delivered to an undefined thread.
|
|
||||||
+# But as those threads do not have a perl interpreter, the perl signal
|
|
||||||
+# handler would crash. We need to block those signals in those threads, so
|
|
||||||
+# that they get delivered only to those threads which can handle it.
|
|
||||||
|
|
||||||
sub new {
|
|
||||||
my ($class, @args) = @_;
|
|
||||||
|
|
||||||
# block signals
|
|
||||||
- bmwqemu::diag('Blocking SIGTERM');
|
|
||||||
+ bmwqemu::diag('Blocking SIGCHLD and SIGCHLD');
|
|
||||||
my %old_sig = %SIG;
|
|
||||||
$SIG{TERM} = 'IGNORE';
|
|
||||||
$SIG{INT} = 'IGNORE';
|
|
||||||
$SIG{HUP} = 'IGNORE';
|
|
||||||
- my $sigset = POSIX::SigSet->new(SIGTERM);
|
|
||||||
- die "Could not block SIGTERM\n" unless defined sigprocmask(SIG_BLOCK, $sigset, undef);
|
|
||||||
+ my $sigset = POSIX::SigSet->new(SIGCHLD, SIGTERM);
|
|
||||||
+ die "Could not block SIGCHLD and SIGTERM\n" unless defined sigprocmask(SIG_BLOCK, $sigset, undef);
|
|
||||||
|
|
||||||
# create the actual object holding the information to restore the previous state
|
|
||||||
my $self = $class->SUPER::new(@args);
|
|
||||||
@@ -48,8 +48,8 @@ sub DESTROY {
|
|
||||||
my ($self) = @_;
|
|
||||||
|
|
||||||
# set back signal handling to default to be able to terminate properly
|
|
||||||
- bmwqemu::diag('Unblocking SIGTERM');
|
|
||||||
- die "Could not unblock SIGTERM\n" unless defined sigprocmask(SIG_UNBLOCK, $self->{_sigset}, undef);
|
|
||||||
+ bmwqemu::diag('Unblocking SIGCHLD and SIGTERM');
|
|
||||||
+ die "Could not unblock SIGCHLD and SIGTERM\n" unless defined sigprocmask(SIG_UNBLOCK, $self->{_sigset}, undef);
|
|
||||||
%SIG = %{$self->{_old_sig}};
|
|
||||||
}
|
|
||||||
|
|
||||||
diff --git a/t/28-signalblocker.t b/t/28-signalblocker.t
|
|
||||||
index 31ddb6f6..9e1a99a5 100755
|
|
||||||
--- a/t/28-signalblocker.t
|
|
||||||
+++ b/t/28-signalblocker.t
|
|
||||||
@@ -1,6 +1,6 @@
|
|
||||||
#!/usr/bin/perl
|
|
||||||
#
|
|
||||||
-# Copyright (c) 2020 SUSE LLC
|
|
||||||
+# Copyright (c) 2021 SUSE LLC
|
|
||||||
#
|
|
||||||
# This program is free software; you can redistribute it and/or modify
|
|
||||||
# it under the terms of the GNU General Public License as published by
|
|
||||||
@@ -44,9 +44,13 @@ my $no_signal_blocker = $ENV{OS_AUTOINST_TEST_NO_SIGNAL_BLOCKER};
|
|
||||||
sub thread_count { scalar split("\n", `ps huH p $$`) }
|
|
||||||
is(my $last_thread_count = thread_count, 1, 'initially one thread');
|
|
||||||
|
|
||||||
-# count SIGTERMs we receive; this handler should work after creating/destroying the signal blocker
|
|
||||||
+# count SIGTERMs we receive; those handlers should work after creating/destroying the signal blocker
|
|
||||||
+# Note that without these handlers, there won't be any crash in Perl's signal handler as it's never
|
|
||||||
+# registered for those signals.
|
|
||||||
my $received_sigterm = 0;
|
|
||||||
$SIG{TERM} = sub { $received_sigterm += 1; note "received SIGTERM $received_sigterm"; };
|
|
||||||
+my $received_sigchld = 0;
|
|
||||||
+$SIG{CHLD} = sub { $received_sigchld += 1; note "received SIGCHLD $received_sigchld"; };
|
|
||||||
|
|
||||||
# initialize OpenCV via signalblocker and create_threads
|
|
||||||
{
|
|
||||||
@@ -75,6 +79,12 @@ waitpid $fork, 0;
|
|
||||||
note 'waiting for at least one signal to be handled' and sleep .2 until $received_sigterm >= 1 || ($timeout -= .2) < 0;
|
|
||||||
note "handled $received_sigterm TERM signals";
|
|
||||||
ok($received_sigterm > 0, "received SIGTERM $received_sigterm times; no crashes after at least 200 ms idling time");
|
|
||||||
+
|
|
||||||
+$received_sigchld = 0;
|
|
||||||
+# 0 here means WIFEXITED and WEXITSTATUS == 0
|
|
||||||
+cmp_ok(system("true"), '==', 0, 'system returns exit status');
|
|
||||||
+is($received_sigchld, 1, 'got SIGCHLD after system');
|
|
||||||
+
|
|
||||||
cmp_ok(thread_count, '<=', $last_thread_count, 'still no new threads after sending signals');
|
|
||||||
|
|
||||||
done_testing;
|
|
||||||
--
|
|
||||||
2.31.1
|
|
||||||
|
|
||||||
@ -1 +1 @@
|
|||||||
SHA512 (os-autoinst-24ec8f98d9ce9d3223231c16ff21237f73391310.tar.gz) = 3dcd6aff0e26733aa3bde84d525b63e79d5cc1749cfaa4d394abcbc68b4d50d1d44339c6dbfed311d4636827cadd12fd26ecdd55c6118a48e98ec35657029c06
|
SHA512 (os-autoinst-f21226c3b2c8987e5b5840dde04229e3298a07e0.tar.gz) = 8ba2d66de1061ee7d268c161b37007c2f50b6b434874437f1d17000b311f9e8c2c9998052b4c3e8d2169f5819dbd6f7987e21bdef6dd7bdaf11847a18a935e39
|
||||||
|
|||||||
Loading…
Reference in new issue