[libvirt] Remove functions library inclusion from libvirt-guests
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
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
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
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
* 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
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
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
Eric Blake -
Guido Günther -
Laurent Léonard