[libvirt PATCH] news: Document that we build with musl
A bit of effort by me and Michal helped make this the case, and it helped us uncover some potential issues. I am not documenting it as supported or adding an Alpine container into the CI, but since there were some distribution bugs mentioning libvirt issues I thing it would be nice of us to notify those distribution maintainers that read our release news. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- NEWS.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index a5b6106bc2c2..f0270b9bb159 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -21,6 +21,9 @@ v8.2.0 (unreleased) * **Bug fixes** + * Both build and tests should now pass on Alpine Linux or any other + distribution with musl libc. + v8.1.0 (2022-03-01) =================== -- 2.35.1
On 3/9/22 11:27, Martin Kletzander wrote:
A bit of effort by me and Michal helped make this the case, and it helped us uncover some potential issues. I am not documenting it as supported or adding an Alpine container into the CI, but since there were some distribution bugs mentioning libvirt issues I thing it would be nice of us to notify those distribution maintainers that read our release news.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal
On Wed, Mar 09, 2022 at 11:27:22AM +0100, Martin Kletzander wrote:
A bit of effort by me and Michal helped make this the case, and it helped us uncover some potential issues. I am not documenting it as supported or adding an Alpine container into the CI, but since there were some distribution bugs mentioning libvirt issues I thing it would be nice of us to notify those distribution maintainers that read our release news.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- NEWS.rst | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/NEWS.rst b/NEWS.rst index a5b6106bc2c2..f0270b9bb159 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -21,6 +21,9 @@ v8.2.0 (unreleased)
* **Bug fixes**
+ * Both build and tests should now pass on Alpine Linux or any other + distribution with musl libc.
IMHO even though you don't say it explicitly, anyone reading this will interpret it at meaning libvirt is expected to work with musl, which is misleading Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Wed, Mar 09, 2022 at 10:36:26AM +0000, Daniel P. Berrangé wrote:
On Wed, Mar 09, 2022 at 11:27:22AM +0100, Martin Kletzander wrote:
A bit of effort by me and Michal helped make this the case, and it helped us uncover some potential issues. I am not documenting it as supported or adding an Alpine container into the CI, but since there were some distribution bugs mentioning libvirt issues I thing it would be nice of us to notify those distribution maintainers that read our release news.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- NEWS.rst | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/NEWS.rst b/NEWS.rst index a5b6106bc2c2..f0270b9bb159 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -21,6 +21,9 @@ v8.2.0 (unreleased)
* **Bug fixes**
+ * Both build and tests should now pass on Alpine Linux or any other + distribution with musl libc.
IMHO even though you don't say it explicitly, anyone reading this will interpret it at meaning libvirt is expected to work with musl, which is misleading
I read your message right after pushing, not seeing that Michal's reply is just a start of a thread. Should I add "Note however that such scenarios are not treated as supported" ? Or feel free to add whatever wording seems appropriate.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 3/9/22 11:36, Daniel P. Berrangé wrote:
On Wed, Mar 09, 2022 at 11:27:22AM +0100, Martin Kletzander wrote:
A bit of effort by me and Michal helped make this the case, and it helped us uncover some potential issues. I am not documenting it as supported or adding an Alpine container into the CI, but since there were some distribution bugs mentioning libvirt issues I thing it would be nice of us to notify those distribution maintainers that read our release news.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- NEWS.rst | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/NEWS.rst b/NEWS.rst index a5b6106bc2c2..f0270b9bb159 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -21,6 +21,9 @@ v8.2.0 (unreleased)
* **Bug fixes**
+ * Both build and tests should now pass on Alpine Linux or any other + distribution with musl libc.
IMHO even though you don't say it explicitly, anyone reading this will interpret it at meaning libvirt is expected to work with musl, which is misleading
With everything turning into a container nowadays, I believe it's time to revisit our decision of supported platforms. Or even if we don't want to support Alpine, that's fine, but perhaps have it CI, allow it to fail (just like we're doing for rawhide/sid) and see if we still compile? The benefit would be more portable code (like Martin's patches from past week demonstrate). Michal
On Wed, Mar 09, 2022 at 01:14:39PM +0100, Michal Prívozník wrote:
On 3/9/22 11:36, Daniel P. Berrangé wrote:
On Wed, Mar 09, 2022 at 11:27:22AM +0100, Martin Kletzander wrote:
A bit of effort by me and Michal helped make this the case, and it helped us uncover some potential issues. I am not documenting it as supported or adding an Alpine container into the CI, but since there were some distribution bugs mentioning libvirt issues I thing it would be nice of us to notify those distribution maintainers that read our release news.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- NEWS.rst | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/NEWS.rst b/NEWS.rst index a5b6106bc2c2..f0270b9bb159 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -21,6 +21,9 @@ v8.2.0 (unreleased)
* **Bug fixes**
+ * Both build and tests should now pass on Alpine Linux or any other + distribution with musl libc.
IMHO even though you don't say it explicitly, anyone reading this will interpret it at meaning libvirt is expected to work with musl, which is misleading
With everything turning into a container nowadays, I believe it's time to revisit our decision of supported platforms. Or even if we don't want to support Alpine, that's fine, but perhaps have it CI, allow it to fail (just like we're doing for rawhide/sid) and see if we still compile? The benefit would be more portable code (like Martin's patches from past week demonstrate).
I'm not saying we shouldn't support Alpine, just that I don't believe it works today, and making it work is not an quick & easy change at all. Unlike the other C libraries we deal with, musl doesn't allow you to use malloc after fork in a threaded application, and will deadlock. This is a difficult problem for us to address given what we expect to be able todo in the fork/exec scenario in libvirt. The behaviour we expect is not required by POSIX, but Linux(glibc)/macOS/FreeBSD platforms have allowed it anyway, because without it, writing threaded apps which fork is much more difficult. It will require alot of work to virCommand, most likely involving a switch to use of posix_spawn, while also removing the ability to run callback hooks, and anything else we do inbetween fork/exec, that is not covered by posix_spawn features. This hook removal would in turn require significant changes to the QEMU driver, probably requiring us to create a helper program. We would exec libvirt_qemu, which does what we used todo in the hook, and then libvirt_qemu would have to exec the real QEMU in place of itself. This is all likely useful long term, but not quick or easy and is quite likely to introduce regressions as we work through it. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 3/9/22 13:25, Daniel P. Berrangé wrote:
On Wed, Mar 09, 2022 at 01:14:39PM +0100, Michal Prívozník wrote:
On 3/9/22 11:36, Daniel P. Berrangé wrote:
On Wed, Mar 09, 2022 at 11:27:22AM +0100, Martin Kletzander wrote:
A bit of effort by me and Michal helped make this the case, and it helped us uncover some potential issues. I am not documenting it as supported or adding an Alpine container into the CI, but since there were some distribution bugs mentioning libvirt issues I thing it would be nice of us to notify those distribution maintainers that read our release news.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- NEWS.rst | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/NEWS.rst b/NEWS.rst index a5b6106bc2c2..f0270b9bb159 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -21,6 +21,9 @@ v8.2.0 (unreleased)
* **Bug fixes**
+ * Both build and tests should now pass on Alpine Linux or any other + distribution with musl libc.
IMHO even though you don't say it explicitly, anyone reading this will interpret it at meaning libvirt is expected to work with musl, which is misleading
With everything turning into a container nowadays, I believe it's time to revisit our decision of supported platforms. Or even if we don't want to support Alpine, that's fine, but perhaps have it CI, allow it to fail (just like we're doing for rawhide/sid) and see if we still compile? The benefit would be more portable code (like Martin's patches from past week demonstrate).
I'm not saying we shouldn't support Alpine, just that I don't believe it works today, and making it work is not an quick & easy change at all.
Unlike the other C libraries we deal with, musl doesn't allow you to use malloc after fork in a threaded application, and will deadlock.
While that used to be true, it's not like that anymore: https://git.musl-libc.org/cgit/musl/commit/?id=167390f05564e0a4d3fcb4329377f... In fact, I'm running a VM with musl-1.2.2-r7 and running QEMU and LXC guests just fine. Michal
On Wed, Mar 09, 2022 at 01:53:49PM +0100, Michal Prívozník wrote:
On 3/9/22 13:25, Daniel P. Berrangé wrote:
On Wed, Mar 09, 2022 at 01:14:39PM +0100, Michal Prívozník wrote:
On 3/9/22 11:36, Daniel P. Berrangé wrote:
On Wed, Mar 09, 2022 at 11:27:22AM +0100, Martin Kletzander wrote:
A bit of effort by me and Michal helped make this the case, and it helped us uncover some potential issues. I am not documenting it as supported or adding an Alpine container into the CI, but since there were some distribution bugs mentioning libvirt issues I thing it would be nice of us to notify those distribution maintainers that read our release news.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- NEWS.rst | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/NEWS.rst b/NEWS.rst index a5b6106bc2c2..f0270b9bb159 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -21,6 +21,9 @@ v8.2.0 (unreleased)
* **Bug fixes**
+ * Both build and tests should now pass on Alpine Linux or any other + distribution with musl libc.
IMHO even though you don't say it explicitly, anyone reading this will interpret it at meaning libvirt is expected to work with musl, which is misleading
With everything turning into a container nowadays, I believe it's time to revisit our decision of supported platforms. Or even if we don't want to support Alpine, that's fine, but perhaps have it CI, allow it to fail (just like we're doing for rawhide/sid) and see if we still compile? The benefit would be more portable code (like Martin's patches from past week demonstrate).
I'm not saying we shouldn't support Alpine, just that I don't believe it works today, and making it work is not an quick & easy change at all.
Unlike the other C libraries we deal with, musl doesn't allow you to use malloc after fork in a threaded application, and will deadlock.
While that used to be true, it's not like that anymore:
https://git.musl-libc.org/cgit/musl/commit/?id=167390f05564e0a4d3fcb4329377f...
Ah well that's excellent, glad to see sanity prevailed on allowing malloc across fork and unlocking mutexes in child that were locked in the parent. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (3)
-
Daniel P. Berrangé -
Martin Kletzander -
Michal Prívozník