Change in vdsm[master]: configNet: atomicBackup: it's fine if a new file disappears

[Dan Kenigsberg]

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(a)redhat.com&gt;
---
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(a)redhat.com&gt;

Change in vdsm[master]: configNetTests: test atomicBackup

[Dan Kenigsberg]

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(a)redhat.com&gt;
---
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(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;
Gerrit-Reviewer: Livnat Peer <lpeer(a)redhat.com&gt;

Change in vdsm[master]: configNetTests: test atomicBackup

[Dan Kenigsberg]

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(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;
Gerrit-Reviewer: Livnat Peer <lpeer(a)redhat.com&gt;

Change in vdsm[master]: configNetTests: test atomicBackup

[Igor Lvovsky]

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(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;
Gerrit-Reviewer: Livnat Peer <lpeer(a)redhat.com&gt;

Change in vdsm[master]: configNetTests: test atomicBackup

[Dan Kenigsberg]

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(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;
Gerrit-Reviewer: Livnat Peer <lpeer(a)redhat.com&gt;

[Igor Lvovsky]

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(a)redhat.com&gt;
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com&gt;
Gerrit-Reviewer: Livnat Peer <lpeer(a)redhat.com&gt;