On 01/09/2017 07:58 AM, Michal Privoznik wrote: > v2 of: > > https://www.redhat.com/archives/libvir-list/2016-December/msg00907.html > > diff to v1: > - Pushed 1/6 from the original series as it was ACKed > - Added more comments > - Fixed return value of virSecurityDACTransactionAppend and virSecuritySELinuxTransactionAppend > - Dropped ignore_value() around virThreadLocalSet() > - Unset thread local just in transactionCommit APIs > > I have not implemented any rollback yet, but I've added comments > where the implementation should go ;-) > > Michal Privoznik (5): > security_dac: Resolve virSecurityDACSetOwnershipInternal const > correctness > security driver: Introduce transaction APIs > security_dac: Implement transaction APIs > security_selinux: Implement transaction APIs > qemu: Use transactions from security driver > > src/libvirt_private.syms | 3 + > src/qemu/qemu_driver.c | 25 ++-- > src/qemu/qemu_security.c | 100 ++++--------- > src/security/security_dac.c | 261 +++++++++++++++++++++++++++++++++- > src/security/security_driver.h | 9 ++ > src/security/security_manager.c | 68 +++++++++ > src/security/security_manager.h | 7 +- > src/security/security_selinux.c | 256 ++++++++++++++++++++++++++++++++- > src/security/security_stack.c | 49 +++++++ > src/storage/storage_backend.h | 2 +- > src/storage/storage_backend_fs.c | 2 +- > src/storage/storage_backend_gluster.c | 2 +- > src/storage/storage_driver.c | 6 +- > src/storage/storage_driver.h | 4 +- > src/util/virstoragefile.c | 2 +- > src/util/virstoragefile.h | 2 +- > 16 files changed, 703 insertions(+), 95 deletions(-) > BTW: Perhaps an existing bug... If *Launch calls SetAllLabel and fails a -1 is returned... Once SetAllLabel succeeds, all returns afterwards are -2. When the caller detects a -2 return, then it forces the relabel in the exit path. So if we have a partial success/failure, then do we really restore anything? Should the setting of ret = -2 go before the call to qemuSecuritySetAllLabel? Not related to this set of changes, but since I was looking at the code... I guess for now not having Rollback will be no worse than what exists. It would seem to matter more on Set processing and much harder on Restore processing (other than ignoring errors and trying to restore as many as possible). I think the Rollback thoughts came mostly because I started thinking about other transaction applications which don't process "All", but rather process "Some". Not rolling back on Abort or forcing a caller to Rollback may be unexpected. Consider that the virSecurity*TransactionRun could "save" the status of the call to each list entry and then if there is a failure, go through each successful one, recall the saved data label data, and restore it (ok conceptually at least). Taking this path means not worrying about the partial failure path I noted above. If ignoring failures on Restore is important, then Start could save in list whether this was a "Set" or "Restore" operation... In any case, I'm not going to hold things up for a Rollback... As long as the notes from patch 1 and 5 are addressed, then ACK series. John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
