5:13 a.m.
Hi,
The attached patch removes functions library inclusion from libvirt-guests.
No function from this library is used by the script so there is no need to
include it.
Thank you,
--
Laurent Léonard
8:10 a.m.
On 12/17/2010 04:13 AM, Laurent Léonard wrote:
Hi,
The attached patch removes functions library inclusion from libvirt-guests.
No function from this library is used by the script so there is no need to
include it.
NACK. It's better to follow the init script standards and blindly
include the library, even if we currently do not use functions from the
library in the rest of the script. For all you know, the mere inclusion
of the library may have some system registration side effects, where
omitting the inclusion would change the behavior of the init script.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:56 a.m.
On Fri, Dec 17, 2010 at 07:10:27AM -0700, Eric Blake wrote:
On 12/17/2010 04:13 AM, Laurent Léonard wrote:
> Hi,
>
> The attached patch removes functions library inclusion from libvirt-guests.
>
> No function from this library is used by the script so there is no need to
> include it.
NACK. It's better to follow the init script standards and blindly
include the library, even if we currently do not use functions from the
library in the rest of the script. For all you know, the mere inclusion
of the library may have some system registration side effects, where
omitting the inclusion would change the behavior of the init script.
Something like
[ ! -f "$sysconfdir"/rc.d/init.d/functions ] || .
"$sysconfdir"/rc.d/init.d/functions
would avoid a distro specific patch.
Cheers,
-- Guido
12:10 p.m.
On 12/17/2010 10:56 AM, Guido Günther wrote:ehavior of the init script.
Something like
[ ! -f "$sysconfdir"/rc.d/init.d/functions ] || .
"$sysconfdir"/rc.d/init.d/functions
would avoid a distro specific patch.
Yes, I can live with a patch like that, which checks that it will be
safe to source the script before actually doing so. I'd use -r instead
of -f, though.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
6:26 p.m.
New subject: [libvirt] [PATCH] libvirt-guests: avoid sourcing failure
* tools/libvirt-guests.init.in (libvirtd): Skip function library
if it is not present.
Suggested by Guido Günther.
---
tools/libvirt-guests.init.in | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/tools/libvirt-guests.init.in b/tools/libvirt-guests.init.in
index 5dab36b..e28938d 100644
--- a/tools/libvirt-guests.init.in
+++ b/tools/libvirt-guests.init.in
@@ -29,7 +29,8 @@ localstatedir=@localstatedir@
libvirtd=@sbindir@/libvirtd
# Source function library.
-. "$sysconfdir"/rc.d/init.d/functions
+test ! -r "$sysconfdir"/rc.d/init.d/functions ||
+ . "$sysconfdir"/rc.d/init.d/functions
URIS=default
ON_BOOT=start
--
1.7.3.3
7:41 a.m.
New subject: [libvirt] [PATCH] libvirt-guests: avoid sourcing failure
On Fri, Dec 17, 2010 at 05:26:52PM -0700, Eric Blake wrote:
* tools/libvirt-guests.init.in (libvirtd): Skip function library
if it is not present.
Suggested by Guido Günther.
---
tools/libvirt-guests.init.in | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/tools/libvirt-guests.init.in b/tools/libvirt-guests.init.in
index 5dab36b..e28938d 100644
--- a/tools/libvirt-guests.init.in
+++ b/tools/libvirt-guests.init.in
@@ -29,7 +29,8 @@ localstatedir=@localstatedir@
libvirtd=@sbindir@/libvirtd
# Source function library.
-. "$sysconfdir"/rc.d/init.d/functions
+test ! -r "$sysconfdir"/rc.d/init.d/functions ||
+ . "$sysconfdir"/rc.d/init.d/functions
ACK. Thanks!
-- Guido
10:13 a.m.
New subject: [libvirt] [PATCH] libvirt-guests: avoid sourcing failure
On 12/18/2010 06:41 AM, Guido Günther wrote:
On Fri, Dec 17, 2010 at 05:26:52PM -0700, Eric Blake wrote:
> * tools/libvirt-guests.init.in (libvirtd): Skip function library
> if it is not present.
> Suggested by Guido Günther.
> ---
> tools/libvirt-guests.init.in | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/tools/libvirt-guests.init.in b/tools/libvirt-guests.init.in
> index 5dab36b..e28938d 100644
> --- a/tools/libvirt-guests.init.in
> +++ b/tools/libvirt-guests.init.in
> @@ -29,7 +29,8 @@ localstatedir=@localstatedir@
> libvirtd=@sbindir@/libvirtd
>
> # Source function library.
> -. "$sysconfdir"/rc.d/init.d/functions
> +test ! -r "$sysconfdir"/rc.d/init.d/functions ||
> + . "$sysconfdir"/rc.d/init.d/functions
ACK. Thanks!
Likewise, for your review and input. Now pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
5087
days inactive
5090
days old
6 comments
3 participants
participants (3)
-
Eric Blake -
Guido Günther -
Laurent Léonard