Dan Kenigsberg has uploaded a new change for review.
Change subject: configNet: atomicBackup: it's fine if a new file disappears ......................................................................
configNet: atomicBackup: it's fine if a new file disappears
If restoreAtomicBackup cannot find a file that it needs to remove anyway, it is not an error -- as long as the initial state is restored.
Change-Id: I2925bc9b4db86d9a38fad761e58d164b3ad7bfd4 Signed-off-by: Dan Kenigsberg danken@redhat.com --- M tests/configNetworkTests.py M vdsm/configNetwork.py 2 files changed, 10 insertions(+), 8 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/54/5954/1 -- To view, visit http://gerrit.ovirt.org/5954 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I2925bc9b4db86d9a38fad761e58d164b3ad7bfd4 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg danken@redhat.com
Igor Lvovsky has posted comments on this change.
Change subject: configNet: atomicBackup: it's fine if a new file disappears ......................................................................
Patch Set 1: I would prefer that you didn't submit this
ACK, but Please, split out the test (for downstream sake)
Dan, I think the split these patches to 3 is too artificial. I would prefer, two patches, one with test (squash both tests fixes together) and second with code fix (again squash all 3 fixes together)
-- To view, visit http://gerrit.ovirt.org/5954 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I2925bc9b4db86d9a38fad761e58d164b3ad7bfd4 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: configNetTests: test atomicBackup ......................................................................
Patch Set 2:
I actually prefer multiple, short, topical patches. I think it makes the life of the reviewer (and of the future git-blamer) much simpler.
I also prefer having a functional change and its test on the same commit.
However, to make my only reviewer happier, I've rehashed my patches, and splited code from test.
-- To view, visit http://gerrit.ovirt.org/5954 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I2925bc9b4db86d9a38fad761e58d164b3ad7bfd4 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com
Igor Lvovsky has posted comments on this change.
Change subject: configNetTests: test atomicBackup ......................................................................
Patch Set 2: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/5954 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I2925bc9b4db86d9a38fad761e58d164b3ad7bfd4 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: configNetTests: test atomicBackup ......................................................................
Patch Set 2: Verified; Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/5954 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I2925bc9b4db86d9a38fad761e58d164b3ad7bfd4 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com
Dan Kenigsberg has submitted this change and it was merged.
Change subject: configNetTests: test atomicBackup ......................................................................
configNetTests: test atomicBackup
The sematics of this function were far from clear to me before I've written this unit test. While writing this test, I've found the issues that are fixed in the preceding patches.
I hope this test makes the function of _atomicBackup() clearer not only to me.
Change-Id: I2925bc9b4db86d9a38fad761e58d164b3ad7bfd4 Signed-off-by: Dan Kenigsberg danken@redhat.com --- M tests/configNetworkTests.py 1 file changed, 50 insertions(+), 0 deletions(-)
Approvals: Dan Kenigsberg: Verified; Looks good to me, approved Igor Lvovsky: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/5954 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: I2925bc9b4db86d9a38fad761e58d164b3ad7bfd4 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com
vdsm-patches@lists.fedorahosted.org