[libvirt PATCH 1/9] src: remove After=local-fs.target from systemd units

Daniel P. Berrangé posted 9 patches 2 years, 7 months ago
[libvirt PATCH 1/9] src: remove After=local-fs.target from systemd units
Posted by Daniel P. Berrangé 2 years, 7 months ago
All services are ordered after local-fs.target unless they have set
DefaultDependencies=no, which we do not do.

https://gitlab.com/libvirt/libvirt/-/issues/489
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/ch/virtchd.service.in               | 1 -
 src/interface/virtinterfaced.service.in | 1 -
 src/libxl/virtxend.service.in           | 1 -
 src/lxc/virtlxcd.service.in             | 1 -
 src/network/virtnetworkd.service.in     | 1 -
 src/node_device/virtnodedevd.service.in | 1 -
 src/nwfilter/virtnwfilterd.service.in   | 1 -
 src/qemu/virtqemud.service.in           | 1 -
 src/remote/libvirtd.service.in          | 1 -
 src/remote/virtproxyd.service.in        | 1 -
 src/secret/virtsecretd.service.in       | 1 -
 src/storage/virtstoraged.service.in     | 1 -
 src/util/virstring.c                    | 6 ++++++
 src/vbox/virtvboxd.service.in           | 1 -
 src/vz/virtvzd.service.in               | 1 -
 15 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/src/ch/virtchd.service.in b/src/ch/virtchd.service.in
index a07c04a845..6e3b13446f 100644
--- a/src/ch/virtchd.service.in
+++ b/src/ch/virtchd.service.in
@@ -8,7 +8,6 @@ Wants=systemd-machined.service
 After=network.target
 After=dbus.service
 After=apparmor.service
-After=local-fs.target
 After=remote-fs.target
 After=systemd-logind.service
 After=systemd-machined.service
diff --git a/src/interface/virtinterfaced.service.in b/src/interface/virtinterfaced.service.in
index 1be3ab32dc..5cb2cd19dc 100644
--- a/src/interface/virtinterfaced.service.in
+++ b/src/interface/virtinterfaced.service.in
@@ -7,7 +7,6 @@ Requires=virtinterfaced-admin.socket
 After=network.target
 After=dbus.service
 After=apparmor.service
-After=local-fs.target
 Documentation=man:virtinterfaced(8)
 Documentation=https://libvirt.org
 
diff --git a/src/libxl/virtxend.service.in b/src/libxl/virtxend.service.in
index abb1972777..c6a88f7fe9 100644
--- a/src/libxl/virtxend.service.in
+++ b/src/libxl/virtxend.service.in
@@ -8,7 +8,6 @@ Wants=virtlockd.socket
 After=network.target
 After=dbus.service
 After=apparmor.service
-After=local-fs.target
 After=remote-fs.target
 After=xencommons.service
 Conflicts=xendomains.service
diff --git a/src/lxc/virtlxcd.service.in b/src/lxc/virtlxcd.service.in
index 2623f7375a..06c70ccde2 100644
--- a/src/lxc/virtlxcd.service.in
+++ b/src/lxc/virtlxcd.service.in
@@ -8,7 +8,6 @@ Wants=systemd-machined.service
 After=network.target
 After=dbus.service
 After=apparmor.service
-After=local-fs.target
 After=remote-fs.target
 After=systemd-logind.service
 After=systemd-machined.service
diff --git a/src/network/virtnetworkd.service.in b/src/network/virtnetworkd.service.in
index 48423e777d..f35cccb8f7 100644
--- a/src/network/virtnetworkd.service.in
+++ b/src/network/virtnetworkd.service.in
@@ -10,7 +10,6 @@ After=iptables.service
 After=ip6tables.service
 After=dbus.service
 After=apparmor.service
-After=local-fs.target
 Documentation=man:virtnetworkd(8)
 Documentation=https://libvirt.org
 
diff --git a/src/node_device/virtnodedevd.service.in b/src/node_device/virtnodedevd.service.in
index 3ceed30f29..2ac41db32e 100644
--- a/src/node_device/virtnodedevd.service.in
+++ b/src/node_device/virtnodedevd.service.in
@@ -7,7 +7,6 @@ Requires=virtnodedevd-admin.socket
 After=network.target
 After=dbus.service
 After=apparmor.service
-After=local-fs.target
 Documentation=man:virtnodedevd(8)
 Documentation=https://libvirt.org
 
diff --git a/src/nwfilter/virtnwfilterd.service.in b/src/nwfilter/virtnwfilterd.service.in
index 37fa54d684..d6e98240a8 100644
--- a/src/nwfilter/virtnwfilterd.service.in
+++ b/src/nwfilter/virtnwfilterd.service.in
@@ -7,7 +7,6 @@ Requires=virtnwfilterd-admin.socket
 After=network.target
 After=dbus.service
 After=apparmor.service
-After=local-fs.target
 Documentation=man:virtnwfilterd(8)
 Documentation=https://libvirt.org
 
diff --git a/src/qemu/virtqemud.service.in b/src/qemu/virtqemud.service.in
index 032cbcbbf0..46917b746d 100644
--- a/src/qemu/virtqemud.service.in
+++ b/src/qemu/virtqemud.service.in
@@ -10,7 +10,6 @@ Wants=systemd-machined.service
 After=network.target
 After=dbus.service
 After=apparmor.service
-After=local-fs.target
 After=remote-fs.target
 After=systemd-logind.service
 After=systemd-machined.service
diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in
index 11507207a1..b691d35938 100644
--- a/src/remote/libvirtd.service.in
+++ b/src/remote/libvirtd.service.in
@@ -16,7 +16,6 @@ After=ip6tables.service
 After=dbus.service
 After=iscsid.service
 After=apparmor.service
-After=local-fs.target
 After=remote-fs.target
 After=systemd-logind.service
 After=systemd-machined.service
diff --git a/src/remote/virtproxyd.service.in b/src/remote/virtproxyd.service.in
index dd3bdf3429..9b829641f7 100644
--- a/src/remote/virtproxyd.service.in
+++ b/src/remote/virtproxyd.service.in
@@ -7,7 +7,6 @@ Requires=virtproxyd-admin.socket
 After=network.target
 After=dbus.service
 After=apparmor.service
-After=local-fs.target
 Documentation=man:virtproxyd(8)
 Documentation=https://libvirt.org
 
diff --git a/src/secret/virtsecretd.service.in b/src/secret/virtsecretd.service.in
index 774cfc3ecd..3804fe553b 100644
--- a/src/secret/virtsecretd.service.in
+++ b/src/secret/virtsecretd.service.in
@@ -7,7 +7,6 @@ Requires=virtsecretd-admin.socket
 After=network.target
 After=dbus.service
 After=apparmor.service
-After=local-fs.target
 Documentation=man:virtsecretd(8)
 Documentation=https://libvirt.org
 
diff --git a/src/storage/virtstoraged.service.in b/src/storage/virtstoraged.service.in
index e1a1ea6820..235fbc6798 100644
--- a/src/storage/virtstoraged.service.in
+++ b/src/storage/virtstoraged.service.in
@@ -8,7 +8,6 @@ After=network.target
 After=dbus.service
 After=iscsid.service
 After=apparmor.service
-After=local-fs.target
 After=remote-fs.target
 Documentation=man:virtstoraged(8)
 Documentation=https://libvirt.org
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 6b728ff047..e189b9de31 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -50,6 +50,7 @@ virStrToLong_i(char const *s, char **end_ptr, int base, int *result)
 
     errno = 0;
     val = g_ascii_strtoll(s, &p, base);
+    g_assert(errno != EAGAIN);
     err = (errno || (!end_ptr && *p) || p == s || (int) val != val);
     if (end_ptr)
         *end_ptr = p;
@@ -71,6 +72,7 @@ virStrToLong_ui(char const *s, char **end_ptr, int base, unsigned int *result)
 
     errno = 0;
     val = g_ascii_strtoull(s, &p, base);
+    g_assert(errno != EAGAIN);
 
     /* This one's tricky.  We _want_ to allow "-1" as shorthand for
      * UINT_MAX regardless of whether long is 32-bit or 64-bit.  But
@@ -103,6 +105,7 @@ virStrToLong_uip(char const *s, char **end_ptr, int base, unsigned int *result)
 
     errno = 0;
     val = g_ascii_strtoull(s, &p, base);
+    g_assert(errno != EAGAIN);
     err = (memchr(s, '-', p - s) ||
            errno || (!end_ptr && *p) || p == s || (unsigned int) val != val);
     if (end_ptr)
@@ -160,6 +163,7 @@ virStrToLong_ulp(char const *s, char **end_ptr, int base,
 
     errno = 0;
     val = g_ascii_strtoull(s, &p, base);
+    g_assert(errno != EAGAIN);
     err = (memchr(s, '-', p - s) ||
            errno || (!end_ptr && *p) || p == s || (unsigned long) val != val);
     if (end_ptr)
@@ -202,6 +206,7 @@ virStrToLong_ull(char const *s, char **end_ptr, int base,
 
     errno = 0;
     val = g_ascii_strtoull(s, &p, base);
+    g_assert(errno != EAGAIN);
     err = (errno || (!end_ptr && *p) || p == s);
     if (end_ptr)
         *end_ptr = p;
@@ -223,6 +228,7 @@ virStrToLong_ullp(char const *s, char **end_ptr, int base,
 
     errno = 0;
     val = g_ascii_strtoull(s, &p, base);
+    g_assert(errno != EAGAIN);
     err = (memchr(s, '-', p - s) ||
            errno || (!end_ptr && *p) || p == s);
     if (end_ptr)
diff --git a/src/vbox/virtvboxd.service.in b/src/vbox/virtvboxd.service.in
index e73206591a..a567ed2443 100644
--- a/src/vbox/virtvboxd.service.in
+++ b/src/vbox/virtvboxd.service.in
@@ -7,7 +7,6 @@ Requires=virtvboxd-admin.socket
 After=network.target
 After=dbus.service
 After=apparmor.service
-After=local-fs.target
 After=remote-fs.target
 Documentation=man:virtvboxd(8)
 Documentation=https://libvirt.org
diff --git a/src/vz/virtvzd.service.in b/src/vz/virtvzd.service.in
index bd98d96262..5521e89e10 100644
--- a/src/vz/virtvzd.service.in
+++ b/src/vz/virtvzd.service.in
@@ -7,7 +7,6 @@ Requires=virtvzd-admin.socket
 After=network.target
 After=dbus.service
 After=apparmor.service
-After=local-fs.target
 After=remote-fs.target
 Documentation=man:virtvzd(8)
 Documentation=https://libvirt.org
-- 
2.40.1

Re: [libvirt PATCH 1/9] src: remove After=local-fs.target from systemd units
Posted by Peter Krempa 2 years, 7 months ago
On Wed, Jun 21, 2023 at 14:32:24 +0100, Daniel P. Berrangé wrote:
> All services are ordered after local-fs.target unless they have set
> DefaultDependencies=no, which we do not do.
> 
> https://gitlab.com/libvirt/libvirt/-/issues/489
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/ch/virtchd.service.in               | 1 -
>  src/interface/virtinterfaced.service.in | 1 -
>  src/libxl/virtxend.service.in           | 1 -
>  src/lxc/virtlxcd.service.in             | 1 -
>  src/network/virtnetworkd.service.in     | 1 -
>  src/node_device/virtnodedevd.service.in | 1 -
>  src/nwfilter/virtnwfilterd.service.in   | 1 -
>  src/qemu/virtqemud.service.in           | 1 -
>  src/remote/libvirtd.service.in          | 1 -
>  src/remote/virtproxyd.service.in        | 1 -
>  src/secret/virtsecretd.service.in       | 1 -
>  src/storage/virtstoraged.service.in     | 1 -
>  src/util/virstring.c                    | 6 ++++++
>  src/vbox/virtvboxd.service.in           | 1 -
>  src/vz/virtvzd.service.in               | 1 -
>  15 files changed, 6 insertions(+), 14 deletions(-)


> diff --git a/src/util/virstring.c b/src/util/virstring.c
> index 6b728ff047..e189b9de31 100644
> --- a/src/util/virstring.c
> +++ b/src/util/virstring.c
> @@ -50,6 +50,7 @@ virStrToLong_i(char const *s, char **end_ptr, int base, int *result)
>  
>      errno = 0;
>      val = g_ascii_strtoll(s, &p, base);
> +    g_assert(errno != EAGAIN);
>      err = (errno || (!end_ptr && *p) || p == s || (int) val != val);
>      if (end_ptr)
>          *end_ptr = p;
> @@ -71,6 +72,7 @@ virStrToLong_ui(char const *s, char **end_ptr, int base, unsigned int *result)
>  
>      errno = 0;
>      val = g_ascii_strtoull(s, &p, base);
> +    g_assert(errno != EAGAIN);
>  
>      /* This one's tricky.  We _want_ to allow "-1" as shorthand for
>       * UINT_MAX regardless of whether long is 32-bit or 64-bit.  But
> @@ -103,6 +105,7 @@ virStrToLong_uip(char const *s, char **end_ptr, int base, unsigned int *result)
>  
>      errno = 0;
>      val = g_ascii_strtoull(s, &p, base);
> +    g_assert(errno != EAGAIN);
>      err = (memchr(s, '-', p - s) ||
>             errno || (!end_ptr && *p) || p == s || (unsigned int) val != val);
>      if (end_ptr)
> @@ -160,6 +163,7 @@ virStrToLong_ulp(char const *s, char **end_ptr, int base,
>  
>      errno = 0;
>      val = g_ascii_strtoull(s, &p, base);
> +    g_assert(errno != EAGAIN);
>      err = (memchr(s, '-', p - s) ||
>             errno || (!end_ptr && *p) || p == s || (unsigned long) val != val);
>      if (end_ptr)
> @@ -202,6 +206,7 @@ virStrToLong_ull(char const *s, char **end_ptr, int base,
>  
>      errno = 0;
>      val = g_ascii_strtoull(s, &p, base);
> +    g_assert(errno != EAGAIN);
>      err = (errno || (!end_ptr && *p) || p == s);
>      if (end_ptr)
>          *end_ptr = p;
> @@ -223,6 +228,7 @@ virStrToLong_ullp(char const *s, char **end_ptr, int base,
>  
>      errno = 0;
>      val = g_ascii_strtoull(s, &p, base);
> +    g_assert(errno != EAGAIN);
>      err = (memchr(s, '-', p - s) ||
>             errno || (!end_ptr && *p) || p == s);
>      if (end_ptr)

You've accidentally added some debugging code to this patch.

For the systemd stuff:

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Re: [libvirt PATCH 1/9] src: remove After=local-fs.target from systemd units
Posted by Daniel P. Berrangé 2 years, 7 months ago
On Thu, Jun 22, 2023 at 09:27:48AM +0200, Peter Krempa wrote:
> On Wed, Jun 21, 2023 at 14:32:24 +0100, Daniel P. Berrangé wrote:
> > All services are ordered after local-fs.target unless they have set
> > DefaultDependencies=no, which we do not do.
> > 
> > https://gitlab.com/libvirt/libvirt/-/issues/489
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  src/ch/virtchd.service.in               | 1 -
> >  src/interface/virtinterfaced.service.in | 1 -
> >  src/libxl/virtxend.service.in           | 1 -
> >  src/lxc/virtlxcd.service.in             | 1 -
> >  src/network/virtnetworkd.service.in     | 1 -
> >  src/node_device/virtnodedevd.service.in | 1 -
> >  src/nwfilter/virtnwfilterd.service.in   | 1 -
> >  src/qemu/virtqemud.service.in           | 1 -
> >  src/remote/libvirtd.service.in          | 1 -
> >  src/remote/virtproxyd.service.in        | 1 -
> >  src/secret/virtsecretd.service.in       | 1 -
> >  src/storage/virtstoraged.service.in     | 1 -
> >  src/util/virstring.c                    | 6 ++++++
> >  src/vbox/virtvboxd.service.in           | 1 -
> >  src/vz/virtvzd.service.in               | 1 -
> >  15 files changed, 6 insertions(+), 14 deletions(-)
> 
> 
> > diff --git a/src/util/virstring.c b/src/util/virstring.c
> > index 6b728ff047..e189b9de31 100644
> > --- a/src/util/virstring.c
> > +++ b/src/util/virstring.c
> > @@ -50,6 +50,7 @@ virStrToLong_i(char const *s, char **end_ptr, int base, int *result)
> >  
> >      errno = 0;
> >      val = g_ascii_strtoll(s, &p, base);
> > +    g_assert(errno != EAGAIN);
> >      err = (errno || (!end_ptr && *p) || p == s || (int) val != val);
> >      if (end_ptr)
> >          *end_ptr = p;
> > @@ -71,6 +72,7 @@ virStrToLong_ui(char const *s, char **end_ptr, int base, unsigned int *result)
> >  
> >      errno = 0;
> >      val = g_ascii_strtoull(s, &p, base);
> > +    g_assert(errno != EAGAIN);
> >  
> >      /* This one's tricky.  We _want_ to allow "-1" as shorthand for
> >       * UINT_MAX regardless of whether long is 32-bit or 64-bit.  But
> > @@ -103,6 +105,7 @@ virStrToLong_uip(char const *s, char **end_ptr, int base, unsigned int *result)
> >  
> >      errno = 0;
> >      val = g_ascii_strtoull(s, &p, base);
> > +    g_assert(errno != EAGAIN);
> >      err = (memchr(s, '-', p - s) ||
> >             errno || (!end_ptr && *p) || p == s || (unsigned int) val != val);
> >      if (end_ptr)
> > @@ -160,6 +163,7 @@ virStrToLong_ulp(char const *s, char **end_ptr, int base,
> >  
> >      errno = 0;
> >      val = g_ascii_strtoull(s, &p, base);
> > +    g_assert(errno != EAGAIN);
> >      err = (memchr(s, '-', p - s) ||
> >             errno || (!end_ptr && *p) || p == s || (unsigned long) val != val);
> >      if (end_ptr)
> > @@ -202,6 +206,7 @@ virStrToLong_ull(char const *s, char **end_ptr, int base,
> >  
> >      errno = 0;
> >      val = g_ascii_strtoull(s, &p, base);
> > +    g_assert(errno != EAGAIN);
> >      err = (errno || (!end_ptr && *p) || p == s);
> >      if (end_ptr)
> >          *end_ptr = p;
> > @@ -223,6 +228,7 @@ virStrToLong_ullp(char const *s, char **end_ptr, int base,
> >  
> >      errno = 0;
> >      val = g_ascii_strtoull(s, &p, base);
> > +    g_assert(errno != EAGAIN);
> >      err = (memchr(s, '-', p - s) ||
> >             errno || (!end_ptr && *p) || p == s);
> >      if (end_ptr)
> 
> You've accidentally added some debugging code to this patch.

Doh yes, this was leftover from debugging the problem with glib mutexes
splattering errno. 

> 
> For the systemd stuff:
> 
> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
> 

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 :|