8:49 a.m.
At Stefano's suggestion, this also adds a sentence pointing out that
SELinux must be disabled in order for passt support to work. I didn't
think to put this info in the NEWS file last month.
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
I've noticed that in some places, QEMU related points are marked with
"QEMU:" and in other places they are marked with "qemu:". In the
9.1.0
sections, the new features all use "qemu:" while the bugfixes all use
"QEMU:". I just went along with the flow in both cases, but we should
probably do a patch to standardize on one or the other (and then try
to stick to it). So which is more appropriate? using the
capitalization the way the QEMU project prefers it? Or the
capitalization the way the subdirectory in libvirt is named?
NEWS.rst | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/NEWS.rst b/NEWS.rst
index 1180d75310..df613abc69 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -38,6 +38,13 @@ v9.1.0 (unreleased)
A pvpanic device can be now defined as a PCI device (the original is an ISA
device) with ``<panic model='pvpanic'/>``.
+ * qemu: support automatic restart of inadvertantly terminated passt process
+
+ If the passt process that is serving as the backend of a -netdev
+ stream is terminated unexpectedly, libvirt now listens to QEMU's
+ notification of this, and starts up a new passt instance, thus
+ preserving network connectivity.
+
* **Improvements**
* RPM packaging changes
@@ -63,6 +70,15 @@ v9.1.0 (unreleased)
snapshot when it existed. In addition when external memory only snapshot
was created libvirt failed without producing any error.
+ * QEMU: properly report passt startup errors
+
+ Due to how the child passt process was started, the initial
+ support for passt (added in 9.0.0) would not see errors
+ encountered during startup, so libvirt would continue to setup and
+ start the guest; this led to a running guest with no network
+ connectivity. This issue has be corrected.
+
+ (NB: it is still necessary to disable SELinux to start passt.)
v9.0.0 (2023-01-16)
===================
--
2.39.2
10:11 a.m.
On Tue, Feb 28, 2023 at 09:49:26 -0500, Laine Stump wrote:
At Stefano's suggestion, this also adds a sentence pointing out
that
SELinux must be disabled in order for passt support to work. I didn't
think to put this info in the NEWS file last month.
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
I've noticed that in some places, QEMU related points are marked with
"QEMU:" and in other places they are marked with "qemu:". In the
9.1.0
sections, the new features all use "qemu:" while the bugfixes all use
"QEMU:". I just went along with the flow in both cases, but we should
probably do a patch to standardize on one or the other (and then try
to stick to it). So which is more appropriate? using the
capitalization the way the QEMU project prefers it? Or the
capitalization the way the subdirectory in libvirt is named?
[1]
NEWS.rst | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/NEWS.rst b/NEWS.rst
index 1180d75310..df613abc69 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -38,6 +38,13 @@ v9.1.0 (unreleased)
A pvpanic device can be now defined as a PCI device (the original is an ISA
device) with ``<panic model='pvpanic'/>``.
+ * qemu: support automatic restart of inadvertantly terminated passt process
+
Applying: NEWS: note new passt feature & bugfix for 9.1.0 release
.git/rebase-apply/patch:23: trailing whitespace.
* qemu: support automatic restart of inadvertantly terminated passt process
warning: 1 line adds whitespace errors.
Which also breaks syntax-check:
307/315 libvirt:syntax-check / trailing_blank
FAIL 0.30s exit status 2
>> MALLOC_PERTURB_=63 /bin/make -C
/home/pipo/build/libvirt/gcc/build-aux sc_trailing_blank
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
✀
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
stdout:
make: Entering directory '/home/pipo/build/libvirt/gcc/build-aux'
/home/pipo/libvirt/NEWS.rst:41: * qemu: support automatic restart of inadvertantly
terminated passt process
make: Leaving directory '/home/pipo/build/libvirt/gcc/build-aux'
+ If the passt process that is serving as the backend of a
-netdev
+ stream is terminated unexpectedly, libvirt now listens to QEMU's
+ notification of this, and starts up a new passt instance, thus
+ preserving network connectivity.
+
* **Improvements**
* RPM packaging changes
@@ -63,6 +70,15 @@ v9.1.0 (unreleased)
snapshot when it existed. In addition when external memory only snapshot
was created libvirt failed without producing any error.
+ * QEMU: properly report passt startup errors
[1] you can start by standardizing inside this patch ;)
+
+ Due to how the child passt process was started, the initial
+ support for passt (added in 9.0.0) would not see errors
+ encountered during startup, so libvirt would continue to setup and
+ start the guest; this led to a running guest with no network
+ connectivity. This issue has be corrected.
Drop the last sentence. It's obvious that we've fixed it when we are
mentioning it here.
+
+ (NB: it is still necessary to disable SELinux to start passt.)
With the build error fixed and tautological sentence dropped:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
12:52 p.m.
On 2/28/23 11:11 AM, Peter Krempa wrote:
On Tue, Feb 28, 2023 at 09:49:26 -0500, Laine Stump wrote:
> At Stefano's suggestion, this also adds a sentence pointing out that
> SELinux must be disabled in order for passt support to work. I didn't
> think to put this info in the NEWS file last month.
>
> Signed-off-by: Laine Stump <laine(a)redhat.com>
> ---
>
> I've noticed that in some places, QEMU related points are marked with
> "QEMU:" and in other places they are marked with "qemu:". In the
9.1.0
> sections, the new features all use "qemu:" while the bugfixes all use
> "QEMU:". I just went along with the flow in both cases, but we should
> probably do a patch to standardize on one or the other (and then try
> to stick to it). So which is more appropriate? using the
> capitalization the way the QEMU project prefers it? Or the
> capitalization the way the subdirectory in libvirt is named?
[1]
>
> NEWS.rst | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/NEWS.rst b/NEWS.rst
> index 1180d75310..df613abc69 100644
> --- a/NEWS.rst
> +++ b/NEWS.rst
> @@ -38,6 +38,13 @@ v9.1.0 (unreleased)
> A pvpanic device can be now defined as a PCI device (the original is an ISA
> device) with ``<panic model='pvpanic'/>``.
>
> + * qemu: support automatic restart of inadvertantly terminated passt process
> +
Applying: NEWS: note new passt feature & bugfix for 9.1.0 release
.git/rebase-apply/patch:23: trailing whitespace.
* qemu: support automatic restart of inadvertantly terminated passt process
warning: 1 line adds whitespace errors.
Which also breaks syntax-check:
307/315 libvirt:syntax-check / trailing_blank
FAIL 0.30s exit status 2
>>> MALLOC_PERTURB_=63 /bin/make -C /home/pipo/build/libvirt/gcc/build-aux
sc_trailing_blank
Sigh. The *one* time that I think "eh, what could go wrong with a docs
patch?" and send it without doing a build, emacs inserts a space on a
blank line :-/
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
✀
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
stdout:
make: Entering directory '/home/pipo/build/libvirt/gcc/build-aux'
/home/pipo/libvirt/NEWS.rst:41: * qemu: support automatic restart of inadvertantly
terminated passt process
make: Leaving directory '/home/pipo/build/libvirt/gcc/build-aux'
> + If the passt process that is serving as the backend of a -netdev
> + stream is terminated unexpectedly, libvirt now listens to QEMU's
> + notification of this, and starts up a new passt instance, thus
> + preserving network connectivity.
> +
> * **Improvements**
>
> * RPM packaging changes
> @@ -63,6 +70,15 @@ v9.1.0 (unreleased)
> snapshot when it existed. In addition when external memory only snapshot
> was created libvirt failed without producing any error.
>
> + * QEMU: properly report passt startup errors
[1] you can start by standardizing inside this patch ;)
But I did! Within each section, I maintained the the consistency of that
section.
> +
> + Due to how the child passt process was started, the initial
> + support for passt (added in 9.0.0) would not see errors
> + encountered during startup, so libvirt would continue to setup and
> + start the guest; this led to a running guest with no network
> + connectivity. This issue has be corrected.
Drop the last sentence. It's obvious that we've fixed it when we are
mentioning it here.
> +
> + (NB: it is still necessary to disable SELinux to start passt.)
With the build error fixed and tautological sentence dropped:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
Thanks!
12:06 p.m.
On Tue, Feb 28, 2023 at 09:49:26AM -0500, Laine Stump wrote:
+ * QEMU: properly report passt startup errors
+
+ Due to how the child passt process was started, the initial
+ support for passt (added in 9.0.0) would not see errors
+ encountered during startup, so libvirt would continue to setup and
+ start the guest; this led to a running guest with no network
+ connectivity. This issue has be corrected.
+
+ (NB: it is still necessary to disable SELinux to start passt.)
This is also true for AppArmor, so I would mention both.
Also, please make sure that there still are *two* empty lines between
the section for v9.1.0 and the one for v9.0.0. The current version of
the patch drops one of them.
With the tweaks mentioned above, plus the stuff already pointed out
by Peter,
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
12:53 p.m.
On Tue, 28 Feb 2023 10:06:18 -0800
Andrea Bolognani <abologna(a)redhat.com> wrote:
On Tue, Feb 28, 2023 at 09:49:26AM -0500, Laine Stump wrote:
> + * QEMU: properly report passt startup errors
> +
> + Due to how the child passt process was started, the initial
> + support for passt (added in 9.0.0) would not see errors
> + encountered during startup, so libvirt would continue to setup and
> + start the guest; this led to a running guest with no network
> + connectivity. This issue has be corrected.
> +
> + (NB: it is still necessary to disable SELinux to start passt.)
This is also true for AppArmor, so I would mention both.
Not in general -- thankfully, no pseudorandom label is forced by
libvirt 9.1.0 with AppArmor (because there are no labels), and libvirtd
simply runs passt unconfined (scrubbing the environment):
$ grep "/usr/bin" src/security/apparmor/usr.sbin.libvirtd.in
/usr/bin/* PUx,
Then yes, with any recent version of Debian and openSUSE packages of
passt, passt won't be able to create the socket or its PID file in the
path libvirt asks for, because of the profile shipping with passt
itself.
Still, that's not as bad as the deliberate breakage you have with
SELinux, and 'apparmor_parser -R /etc/apparmor.d/usr.bin.passt' will
do (checked on both Debian and openSUSE) -- no need for 'aa-teardown'.
Note that I'm *not* recommending to do this, just like I'm not
recommending to disable SELinux, and I don't think it's a good idea to
suggest in release notes that users do this, either.
--
Stefano
3:29 p.m.
On Tue, Feb 28, 2023 at 07:53:09PM +0100, Stefano Brivio wrote:
On Tue, 28 Feb 2023 10:06:18 -0800 Andrea Bolognani
<abologna(a)redhat.com> wrote:
> On Tue, Feb 28, 2023 at 09:49:26AM -0500, Laine Stump wrote:
> > + (NB: it is still necessary to disable SELinux to start passt.)
>
> This is also true for AppArmor, so I would mention both.
Not in general -- thankfully, no pseudorandom label is forced by
libvirt 9.1.0 with AppArmor (because there are no labels), and libvirtd
simply runs passt unconfined (scrubbing the environment):
$ grep "/usr/bin" src/security/apparmor/usr.sbin.libvirtd.in
/usr/bin/* PUx,
Then yes, with any recent version of Debian and openSUSE packages of
passt, passt won't be able to create the socket or its PID file in the
path libvirt asks for, because of the profile shipping with passt
itself.
From the user's point of view, what is the difference between
passt
not being able to start, or starting successfully but quitting
immediately afterwards because it can't create some files? I don't
think there's one. In both cases, you're going to see an error.
Note that I'm *not* recommending to do this, just like I'm
not
recommending to disable SELinux, and I don't think it's a good idea to
suggest in release notes that users do this, either.
This is a limitation of the current implementation of passt support
in libvirt. We're actively working on removing it, but in the
meantime it should be documented somewhere. Are the release notes the
best place for that? Unclear. I don't think it's a particularly bad
one. Anyone reading "you need to disable SELinux to use this feature"
will surely infer that they shouldn't put it into production yet :)
--
Andrea Bolognani / Red Hat / Virtualization
3:58 p.m.
On Tue, 28 Feb 2023 13:29:18 -0800
Andrea Bolognani <abologna(a)redhat.com> wrote:
On Tue, Feb 28, 2023 at 07:53:09PM +0100, Stefano Brivio wrote:
> On Tue, 28 Feb 2023 10:06:18 -0800 Andrea Bolognani <abologna(a)redhat.com>
wrote:
> > On Tue, Feb 28, 2023 at 09:49:26AM -0500, Laine Stump wrote:
> > > + (NB: it is still necessary to disable SELinux to start passt.)
> >
> > This is also true for AppArmor, so I would mention both.
>
> Not in general -- thankfully, no pseudorandom label is forced by
> libvirt 9.1.0 with AppArmor (because there are no labels), and libvirtd
> simply runs passt unconfined (scrubbing the environment):
>
> $ grep "/usr/bin" src/security/apparmor/usr.sbin.libvirtd.in
> /usr/bin/* PUx,
>
> Then yes, with any recent version of Debian and openSUSE packages of
> passt, passt won't be able to create the socket or its PID file in the
> path libvirt asks for, because of the profile shipping with passt
> itself.
From the user's point of view, what is the difference between passt
not being able to start, or starting successfully but quitting
immediately afterwards because it can't create some files? I don't
think there's one. In both cases, you're going to see an error.
Yes yes, that's what I meant, there's no difference -- *but "just" with
Debian or openSUSE packages*, because they ship AppArmor profiles for
passt.
If you don't use packages, or, let's say, the Arch Linux package
(https://aur.archlinux.org/packages/passt-git), this is not an issue,
no matter the LSM.
> Note that I'm *not* recommending to do this, just like
I'm not
> recommending to disable SELinux, and I don't think it's a good idea to
> suggest in release notes that users do this, either.
This is a limitation of the current implementation of passt support
in libvirt. We're actively working on removing it, but in the
meantime it should be documented somewhere. Are the release notes the
best place for that? Unclear. I don't think it's a particularly bad
one.
Me neither -- I actually suggested that if libvirt really needs to ship
a feature in this state, at least this should be added to the notes, so
that users don't think they're the ones doing something wrong, if
things don't work.
Anyone reading "you need to disable SELinux to use this
feature"
will surely infer that they shouldn't put it into production yet :)
I don't know, I guess you're most likely right, but I still see the
possible interpretation of a recommendation. At least as an argument in
the evaluation of vulnerability metrics. I'm really fine with either
Laine's version or your version, for what it's worth.
--
Stefano
663
days inactive
663
days old
6 comments
4 participants
participants (4)
-
Andrea Bolognani -
Laine Stump -
Peter Krempa -
Stefano Brivio