[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#991491: unblock: runit/2.1.2-41 (pre-approval)



Control: tags -1 -moreinfo

On Mon, 26 Jul 2021 23:30:19 +0200
Sebastian Ramacher <sramacher@debian.org> wrote:

> Control: tags -1 moreinfo confirmed
> 
> On 2021-07-25 20:31:25 +0200, Lorenzo Puliti wrote:
> > Package: release.debian.org
> > Severity: normal
> > User: release.debian.org@packages.debian.org
> > Usertags: unblock
> > X-Debbugs-Cc: plorenzo@disroot.org
> > 
> > Dear release team,
> > this is a pre-approval request for a runit upload.
> > 
> > [ Reason ]
> > The switch for SysVinit users is broken, the current code works
> > only if a user types 'reboot', then login into the emergency shell
> > and types 'reboot' again (see #991227).
> > With the attached patch:
> >  * the -f and -r flags work as expected
> >  * the shutdown interface is documented in the manpage. 
> > 
> > [ Impact ]
> > Without this patch SysVinit users will likely need to press the
> > reset button (see #991227), which is a regression compared to the
> > runit's version in Buster - the bug was introduced in runit
> > 2.1.2-27 (#919699). Poweroff instead of reboot when the -r flag is
> > passed (#990774) can be bad if one does 'shutdown -r now' on a
> > remote machine expecting to be able to login again..
> > 
> > [ Tests ]
> > I've manually run the qemu autopkgtest to ensure that there is no
> > regression for the systemd-->runit-init switch.
> > I've tested in Virtualbox that the patch fixes the
> > SysVinit-->runit-init switch, which is currently broken in
> > unstable/testing. I'm using the runit package from experimental
> > that include the proposed fix and shutdown or reboot works as
> > expected.
> > 
> > [ Risks ]
> > This change affects runit-init users (popcon ~ 20) and users of
> > other init systems but only when they perform the switch to runit.
> > 
> > [ Checklist ]
> >   [x] all changes are documented in the d/changelog
> >   [x] I reviewed all changes and I approve them
> >   [x] attach debdiff against the package in testing
> > 
> > 
> > unblock runit/2.1.2-41
> > 
> > Lorenzo
> > 
> > diff -Nru ./runit/debian/changelog ./runit-bullseye/debian/changelog
> > --- ./runit/debian/changelog	2021-07-25 15:48:55.494007628
> > +0200 +++ ./runit-bullseye/debian/changelog	2021-07-25
> > 17:03:59.122707529 +0200 @@ -1,3 +1,14 @@
> > +runit (2.1.2-41) unstable; urgency=medium
> > +
> > +  * Cherry-pick shutdown.c fixes from experimental
> > +    for incoming stable release:
> > +    - fix broken switch from SysVinit due to wrong
> > +      command line parsing logic
> > +    - reboot the system with -r flag instead of poweroff
> > +    - Update shutdown(8) manpage
> 
> Is this missing some Closes: #NNNNNN?

I've added Closes for #990774 and #991227 to the changelog as you
suggested.

> In any case, please go ahead and remove the moreinfo tag once the new
> version is available in unstable.

Runit 2.1.2-41 has been uploaded to unstable.

Thanks,
Lorenzo

> 
> Cheers
> 
> > +
> > + -- Lorenzo Puliti <plorenzo@disroot.org>  Sun, 25 Jul 2021
> > 16:35:08 +0200 +
> >  runit (2.1.2-40) unstable; urgency=medium
> >  
> >    * Ack previous NMU, thanks Boyuan Yang for the upload
> > diff -Nru ./runit/debian/contrib/shutdown.8
> > ./runit-bullseye/debian/contrib/shutdown.8 ---
> > ./runit/debian/contrib/shutdown.8	2021-07-25
> > 15:48:55.494007628 +0200 +++
> > ./runit-bullseye/debian/contrib/shutdown.8	2021-07-25
> > 16:23:23.731732301 +0200 @@ -1,15 +1,66 @@ -.TH SHUTDOWN 8 "Oct 10,
> > 2016" "" "GNU/Linux System Adminstrator's manual" +.TH SHUTDOWN 8
> > "July 16, 2021" "" "GNU/Linux System Adminstrator's manual" .SH
> > NAME -shutdown, reboot, poweroff \- stop the system +shutdown,
> > reboot, poweroff \- poweroff or reboot the system .SH SYNOPSIS
> > -.B /sbin/shutdown
> > +.B /sbin/shutdown [-w] [-f] [-r]
> >  .br
> > -.B /sbin/reboot
> > +.B /sbin/reboot [-w] [-f] [-r]
> >  .br
> > -.B /sbin/halt
> > +.B /sbin/halt [-w] [-f] [-r]
> > +.br
> > +.B /sbin/poweroff [-w] [-f] [-r]
> >  .SH DESCRIPTION
> > -SysV-init compatibility scripts, that just invokes
> > -.BR /sbin/init ,
> > -which are expected by graphical desktop environments.
> > +.BR Shutdown
> > +is a program to poweroff or reboot the system that maintains some
> > compatibility with +original SysV-init halt, poweroff, reboot and
> > shutdown programs. +These programs are expected by some
> > initscripts, graphical desktop environments and tools like acpi.
> > +.RE +When called as shutdown, halt or poweroff without options,
> > +.BR runit(8)
> > +is told to shutdown the system and poweroff.
> > +.RE
> > +When called as reboot
> > +.BR runit(8)
> > +is told to reboot the system.
> > +.RE
> > +When
> > +.BR runit(8)
> > +is not the current init system this program sends data in the
> > appropriate format to perform the requested action to the initctl
> > pipe, if it exists. +.SH OPTIONS +
> > +.TP
> > +.B \-f, \-\-force
> > +Force unsafe reboot or poweroff immediately without signaling the
> > init system. +This will likely result in an unclean shutdown an can
> > cause data loss or corruption. +
> > +.TP
> > +.B \-w, \-\-wtmp\-only
> > +No-Op, maintained for compatibility with initscripts. See #919699
> > +
> > +.TP
> > +.B \-r
> > +Reboot the system regardless of how the command is called.
> > +.SH SWITCHING FORM OTHER INIT SYSTEMS
> > +This program maintains a compatibility layer with SysV-init's
> > initctl pipe according to the spec described in SysV-init's
> > initctl(5). This allow one to reboot the system when switching from
> > another init to runit-init. +.RE +Currently only switching from
> > systemd and SysV-init is tested but any other init system that
> > maintains an initctl pipe compatible with SysV's one should work. +
> > +.SH BUGS +Non existent or unsupported options are silently
> > ignored. +.RE +Combining flags, like
> > +.B halt -wf
> > +is not supported, all merged short options will be ignored.
> > +.RE
> > +The
> > +.B -r
> > +flag will always reboot the system, even if called as poweroff or
> > halt; this is counterintuitive. +.RE
> > +The
> > +.B -f
> > +flag is used as in
> > +.B halt -f
> > +but it should have a different effect when the program is called
> > as shutdown. +
> >  .SH SEE ALSO
> >  .BR init (8)
> > diff -Nru ./runit/debian/contrib/shutdown.c
> > ./runit-bullseye/debian/contrib/shutdown.c ---
> > ./runit/debian/contrib/shutdown.c	2021-07-25
> > 15:48:55.494007628 +0200 +++
> > ./runit-bullseye/debian/contrib/shutdown.c	2021-07-25
> > 16:19:07.389733631 +0200 @@ -134,14 +134,16 @@ } 
> >  	for (i = 1; i != argc; ++i) {
> > -		if (strcmp(argv[i], "-f"))
> > +		if (strcmp(argv[i], "-f") == 0)
> >  			cfg->force = true;
> > -		if (strcmp(argv[i], "--force"))
> > +		if (strcmp(argv[i], "--force") == 0)
> >  			cfg->force = true;
> > -		if (strcmp(argv[i], "-w"))
> > +		if (strcmp(argv[i], "-w") == 0)
> >  			cfg->wtmp_only = true;
> > -		if (strcmp(argv[i], "--wtmp-only"))
> > +		if (strcmp(argv[i], "--wtmp-only") == 0)
> >  			cfg->wtmp_only = true;
> > +		if (strcmp(argv[i], "-r") == 0)
> > +			cfg->action = ACTION_REBOOT;
> >  	}
> >  }
> >  
> > 
> 


Reply to: