giuseppe/reboot-after-upgrade

[giuseppe]

No description provided.

[stefwalter]

I would suggest adding an XML comment here that documents the various flags.

[mbarnes]

So the rpmostree command doesn't appear to be passing the opt_reboot flag to the daemon for either method, and is still rebooting from the client-side. Is that intentional? I expected to see the client-side reboot logic removed.

[giuseppe]

yes, the rpm-ostree program is not using the new reboot flag because it has some logic between the upgrade/rollback and the reboot. I'll amend the other changes and submit an updated version

[giuseppe]

I have not refactored the reboot in a single function yet, as it is used both in rpm-ostree and in rpm-ostreed. Should it go in libglnx or in a common library used by rpm-ostree and rpm-ostreed?

[mbarnes]

I have not refactored the reboot in a single function yet, as it is used both in rpm-ostree and in rpm-ostreed. Should it go in libglnx or in a common library used by rpm-ostree and rpm-ostreed?

I'd say in the daemon since it requires root privilege. But I was also assuming the reboot calls would be removed from the command-line layer.

Looking there, I don't really see anything between rpmostree_transaction_get_response_sync() and handling --reboot (expect maybe where --check-diff is handled, but that in combination with --reboot should be rejected just after option parsing). Am I missing the logic you referred to?

[giuseppe]

thanks for the review. Yes I was referring to --check-diff. If it is fine to not allow --check-diff together with --reboot then it is all set and I can proceed with the refactoring.

[mbarnes]

Doesn't make much sense to me to print a package diff and then immediately reboot. @cgwalters, agree?

I think currently if --check-diff is given then --reboot is just ignored. But I'd prefer an error message.

[cgwalters]

Yeah, seems like they should conflict.

[giuseppe]

I pushed some new commits

[mbarnes]

Looks great to me! 👍