[libvirt] [PATCH] Change logrotate to be per-hypervisor logs

[Daniel Veillard]

On Thu, Mar 04, 2010 at 08:21:05AM -0700, Eric Blake wrote:
> According to Daniel Veillard on 3/4/2010 7:36 AM:
> > Original bug is:
> > 
> >   https://bugzilla.redhat.com/show_bug.cgi?id=547514
> > 
> > maybe this could be done in slightly different way, possibly more
> > generic, but I think doing a simple split is good enough for now.
> 
> Seems easy enough.
> 
> > +++ b/daemon/Makefile.am
> > @@ -25,7 +25,9 @@ EXTRA_DIST =						\
> >  	libvirtd.sasl					\
> >  	libvirtd.sysconf				\
> >  	libvirtd.aug                                    \
> > -	libvirtd.logrotate.in                           \
> > +	libvirtd.qemu.logrotate.in                      \
> > +	libvirtd.lxc.logrotate.in                       \
> > +	libvirtd.uml.logrotate.in                       \
> >  	test_libvirtd.aug                               \
> >  	$(AVAHI_SOURCES)				\
> >  	$(DAEMON_SOURCES)
> > @@ -177,20 +179,37 @@ remote_dispatch_args.h: $(srcdir)/remote_generate_stubs.pl $(REMOTE_PROTOCOL)
> >  remote_dispatch_ret.h: $(srcdir)/remote_generate_stubs.pl $(REMOTE_PROTOCOL)
> >  	perl -w $(srcdir)/remote_generate_stubs.pl -r $(REMOTE_PROTOCOL) > $@
> >  
> > -BUILT_SOURCES += libvirtd.logrotate
> > +LOGROTATE_CONFS = libvirtd.qemu.logrotate libvirtd.lxc.logrotate \
> > +                  libvirtd.uml.logrotate
> >  
> > -libvirtd.logrotate: libvirtd.logrotate.in
> > +BUILT_SOURCES += $(LOGROTATE_CONFS)
> > +
> > +libvirtd.qemu.logrotate: libvirtd.qemu.logrotate.in
> > +libvirtd.uml.logrotate: libvirtd.uml.logrotate.in
> >  	sed						\
> >  	    -e s!\@localstatedir\@!@localstatedir@!g	\
> >  	    < $< > $@-t
> >  	mv $@-t $@
> 
> Not introduced by your patch, but copied and pasted two more times.
> Unfortunately, POSIX does not define \@ in sed (GNU sed treats it like @,
> but some others treat it like \\@).  Can you guarantee that
> @localstatedir@ will not expand to any shell metacharacters?  Also,
> automake guarantees that $(localstatedir) will have the same value as
> @localstatedir@, but without triggering a 'make syntax-check' warning
> (although we must already be ignoring that particular check).  It would
> probably be better to write:
> 
> 	    -e 's![@]localstatedir[@]!$(localstatedir)!g'	\
> 
> as the substitution.
> 
> Other than that, you have my ACK

  Okay, I was afraid at first that the quoting of the expression would
defeat make variable substitution, but that works :-)
I also fixed the daemon/.gitignore file to list the new generated
logrotates and then pushed the patch,

 thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/