[PATCH] NEWS: note new passt feature & bugfix for 9.1.0 release
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@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
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@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@redhat.com>
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@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@redhat.com>
Thanks!
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@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
On Tue, 28 Feb 2023 10:06:18 -0800 Andrea Bolognani <abologna@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
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@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
On Tue, 28 Feb 2023 13:29:18 -0800 Andrea Bolognani <abologna@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@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
participants (4)
-
Andrea Bolognani -
Laine Stump -
Peter Krempa -
Stefano Brivio