On Tue, Mar 22, 2011 at 12:00:02PM -0600, Eric Blake wrote:
Child processes don't always reach _exit(); if they die from a
signal, then any messages should still be accurate. Most users
either expect a 0 status (thankfully, if status==0, then
WIFEXITED(status) is true and WEXITSTATUS(status)==0 for all
known platforms) or were filtering on WIFEXITED before printing
a status, but a few were missing this check. Additionally,
nwfilter_ebiptables_driver was making an assumption that works
on Linux (where WEXITSTATUS shifts and WTERMSIG just masks)
but fails on other platforms (where WEXITSTATUS just masks and
WTERMSIG shifts).
* src/util/command.h (virCommandTranslateStatus): New helper.
* src/libvirt_private.syms (command.h): Export it.
* src/util/command.c (virCommandTranslateStatus): New function.
(virCommandWait): Use it to also diagnose status from signals.
* src/security/security_apparmor.c (load_profile): Likewise.
* src/storage/storage_backend.c
(virStorageBackendQEMUImgBackingFormat): Likewise.
* src/util/util.c (virExecDaemonize, virRunWithHook)
(virFileOperation, virDirCreate): Likewise.
* daemon/remote.c (remoteDispatchAuthPolkit): Likewise.
* src/nwfilter/nwfilter_ebiptables_driver.c (ebiptablesExecCLI):
Likewise.
---
In response to my finding here:
https://www.redhat.com/archives/libvir-list/2011-March/msg01029.html
> status includes normal exit and signals. This should probably use
> WIFEXITED and WEXITSTATUS to avoid printing values shifted by 8. For
> that matter, I just noticed that virCommandWait should probably be more
> careful in how it interprets status.
daemon/remote.c | 11 +++++++-
src/libvirt_private.syms | 1 +
src/nwfilter/nwfilter_ebiptables_driver.c | 16 ++++++++++---
src/security/security_apparmor.c | 22 ++++++++++++------
src/storage/storage_backend.c | 18 ++++++---------
src/util/command.c | 34 ++++++++++++++++++++++++++--
src/util/command.h | 7 +++++-
src/util/util.c | 27 ++++++++++------------
8 files changed, 92 insertions(+), 44 deletions(-)
if (status != 0) {
- VIR_ERROR(_("Policy kit denied action %s from pid %d, uid %d, result:
%d"),
- action, callerPid, callerUid, status);
+ char *tmp = virCommandTranslateStatus(status);
+ if (!tmp) {
+ virReportOOMError();
+ goto authfail;
+ }
+ VIR_ERROR(_("Policy kit denied action %s from pid %d, uid %d: %s"),
+ action, callerPid, callerUid, tmp);
+ VIR_FREE(tmp);
goto authdeny;
Hmm, I rather think we ought to keep the VIR_ERROR log message, even
if virCommandTranslateStatus does OOM. eg just use NULLSTR(tmp)
Daniel
--
|:
http://berrange.com -o-
http://www.flickr.com/photos/dberrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|:
http://entangle-photo.org -o-
http://live.gnome.org/gtk-vnc :|