Change in vdsm[master]: Fix failed to add bridgeless network with skip libvirt

Lei Li has uploaded a new change for review.

Change subject: Fix failed to add bridgeless network with skip libvirt
......................................................................

Fix failed to add bridgeless network with skip libvirt

The bridgeless network can not be added when the options contain
skip libvirt. When I dig into the code, seems this type of network
is supported by libvirt, but it will not do its real job if skip
libvirt based on current logical in addNetwork. So set limitation
to avoid such situation.

Change-Id: Ia61fab631e8eecf3dc698a94d72858d7a72ade08
Signed-off-by: Lei 
---
M vdsm/configNetwork.py
1 file changed, 8 insertions(+), 1 deletion(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/19/3919/1
--
To view, visit http://gerrit.ovirt.org/3919
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia61fab631e8eecf3dc698a94d72858d7a72ade08
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Fix failed to add bridgeless network with skip libvirt
......................................................................

Patch Set 2: I would prefer that you didn't submit this

yes, skipLibvirt is an evil argument, born out of necessity to configure a network before libvirtd is up. We should find a way to obsolete it.

Why did you use it? Out of bootstrapping, no one should.

Please move this param validation into _addNetworkValidation()

--
To view, visit http://gerrit.ovirt.org/3919
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia61fab631e8eecf3dc698a94d72858d7a72ade08
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li 
Gerrit-Reviewer: Dan Kenigsberg

Change in vdsm[master]: Fix failure of adding bridgeless network with skipping libvi...

Lei Li has posted comments on this change.

Change subject: Fix failure of adding bridgeless network with skipping libvirt
......................................................................

Patch Set 2:

Yes, it has syntax problem. Done, thanks!

--
To view, visit http://gerrit.ovirt.org/3919
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia61fab631e8eecf3dc698a94d72858d7a72ade08
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Shu Ming

Change in vdsm[master]: Fix failure of adding bridgeless network with skipping libvi...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Fix failure of adding bridgeless network with skipping libvirt
......................................................................

Patch Set 3: Looks good to me, but someone else must approve

(2 inline comments)

....................................................
Commit Message
Line 7: Fix failure of adding bridgeless network with skipping libvirt
actually, you do not fix the failure, you make it fail gracefully.

....................................................
File vdsm/configNetwork.py
Line 508:         # Option skipLibvirt should not be true when add a bridgeless
I usually like comments, but this one says exactly what the exception text below says..

--
To view, visit http://gerrit.ovirt.org/3919
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia61fab631e8eecf3dc698a94d72858d7a72ade08
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shu Ming

Change in vdsm[master]: Fix failure of adding bridgeless network with skipping libvi...

Shahar Havivi has posted comments on this change.

Change subject: Fix failure of adding bridgeless network with skipping libvirt
......................................................................

Patch Set 3: Looks good to me, but someone else must approve

--
To view, visit http://gerrit.ovirt.org/3919
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia61fab631e8eecf3dc698a94d72858d7a72ade08
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shu Ming

Shu Ming has posted comments on this change.

Change subject: Fix failed to add bridgeless network with skip libvirt
......................................................................

Patch Set 2:

I think the subject can be changed to: Fix failure of adding bridgeless network with skip libvirt option. Does it look much better?

--
To view, visit http://gerrit.ovirt.org/3919
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia61fab631e8eecf3dc698a94d72858d7a72ade08
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Shu Ming

Change in vdsm[master]: Avoid skipping libvirt when add bridgeless network

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Avoid skipping libvirt when add bridgeless network
......................................................................

Patch Set 4: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/configNetwork.py
Line 508:         # The bridgeless network is supported by libvirt now.
I was completely misunderstood.

I meant that you should NOT have a comment that has the same content as the code below it. It is simply pointless.

--
To view, visit http://gerrit.ovirt.org/3919
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia61fab631e8eecf3dc698a94d72858d7a72ade08
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shu Ming

Change in vdsm[master]: Avoid skipping libvirt when add bridgeless network

Lei Li has posted comments on this change.

Change subject: Avoid skipping libvirt when add bridgeless network
......................................................................

Patch Set 4: (1 inline comment)

....................................................
File vdsm/configNetwork.py
Line 508:         # The bridgeless network is supported by libvirt now.
I keep this comment for that it explain why the option skipLibvirt can not be set for adding bridgeless network.

If you think it's useless, fine, I will discard it and update later...

--
To view, visit http://gerrit.ovirt.org/3919
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia61fab631e8eecf3dc698a94d72858d7a72ade08
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Lei Li 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shu Ming

Change in vdsm[master]: Avoid skipping libvirt when add bridgeless network

Shahar Havivi has posted comments on this change.

Change subject: Avoid skipping libvirt when add bridgeless network
......................................................................

Patch Set 5: Looks good to me, but someone else must approve

--
To view, visit http://gerrit.ovirt.org/3919
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia61fab631e8eecf3dc698a94d72858d7a72ade08
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Lei Li 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shu Ming

Change in vdsm[master]: Avoid skipping libvirt when add bridgeless network

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Avoid skipping libvirt when add bridgeless network
......................................................................

Patch Set 5: Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/3919
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia61fab631e8eecf3dc698a94d72858d7a72ade08
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Lei Li 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shu Ming

Change in vdsm[master]: Avoid skipping libvirt when add bridgeless network

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Avoid skipping libvirt when add bridgeless network
......................................................................

Patch Set 5: Verified

--
To view, visit http://gerrit.ovirt.org/3919
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia61fab631e8eecf3dc698a94d72858d7a72ade08
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Lei Li 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shu Ming

Change in vdsm[master]: Avoid skipping libvirt when add bridgeless network

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Avoid skipping libvirt when add bridgeless network
......................................................................

Avoid skipping libvirt when add bridgeless network

Change since V2
    - Adjust subject and comment to make it more reasonable.

Change since V1
    - Move this validation into _addNetworkValidation based
      on Dan's suggestion.

The bridgeless network can not be added when the options contain
skip libvirt. When I dig into the code, seems this type of network
is supported by libvirt, but it will not do its real job if skip
libvirt based on current logical in addNetwork. So set limitation
to avoid such situation.

Change-Id: Ia61fab631e8eecf3dc698a94d72858d7a72ade08
Signed-off-by: Lei Li 
---
M vdsm/configNetwork.py
1 file changed, 4 insertions(+), 1 deletion(-)

Approvals:
  Shahar Havivi: Looks good to me, but someone else must approve
  Dan Kenigsberg: Verified; Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/3919
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia61fab631e8eecf3dc698a94d72858d7a72ade08
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lei Li 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Lei Li 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shu Ming