Change in vdsm[master]: configNetwork: remove skipLibvirt flag
Douglas Schilling Landgraf has posted comments on this change. Change subject: configNetwork: remove skipLibvirt flag .. Patch Set 3: (2 inline comments) Hi Dan, my fault, I will split the patch. File vdsm/configNetwork.py Line 322: self._removeFile(self.NET_CONF_PREF + bridge) You are correct, I will split this patch. This is not required for the bug that I am attacking. Line 655: if netName not in conn.listNetworks(): Indeed, not required for now. I will split the patch. -- To view, visit http://gerrit.ovirt.org/4342 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie98ba38da64e34aedfd88b11880929ee4d99d0d6 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Douglas Schilling Landgraf Gerrit-Reviewer: Igor Lvovsky ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: configNetwork: remove skipLibvirt flag
Dan Kenigsberg has posted comments on this change. Change subject: configNetwork: remove skipLibvirt flag .. Patch Set 3: I would prefer that you didn't submit this (2 inline comments) I'm still confused. How your change solves the problem? It includes more than simple removal of skipLibvirt. I very much prefer that you re-write the commit message, and split this patch, if needed. File vdsm/configNetwork.py Line 322: self._removeFile(self.NET_CONF_PREF + bridge) I do not know why this check was here in the first place, but why are you moving it now? Line 655: if netName not in conn.listNetworks(): that's not good enough, since the network may be defined (but unactive), or defined wrongly (bridged/bridgeless). why are you including this change in this commit? -- To view, visit http://gerrit.ovirt.org/4342 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie98ba38da64e34aedfd88b11880929ee4d99d0d6 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Douglas Schilling Landgraf Gerrit-Reviewer: Igor Lvovsky ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: configNetwork: remove skipLibvirt flag
Douglas Schilling Landgraf has posted comments on this change. Change subject: configNetwork: remove skipLibvirt flag .. Patch Set 3: Verified It's working nice with manual install and autoinstall, of course, I will do more tests here. -- To view, visit http://gerrit.ovirt.org/4342 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie98ba38da64e34aedfd88b11880929ee4d99d0d6 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Douglas Schilling Landgraf Gerrit-Reviewer: Igor Lvovsky ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: configNetwork: remove skipLibvirt flag
Dan Kenigsberg has posted comments on this change.
Change subject: configNetwork: remove skipLibvirt flag
..
Patch Set 2: (2 inline comments)
File vdsm_reg/deployUtil.py.in
Line 49: sys.path.append("/usr/share/vdsm")
I do not think configNetwork has to be touched directly.
Line 896: configNetwork.createLibvirtNetwork(mgtBridge,
bridged=True)
> humm, I see clientIF.py using it but ok..
but of which rhev version? I bet your looking at 3.1.
> Because when the bridge was created (breth0) by RHEV-H it doesn't added it to
> libvirt db and if we try to remove line 904 it will break the code.
We can change the delNetwork not to explode if the libvirt net does not exists.
that's more reasonable imo.
--
To view, visit http://gerrit.ovirt.org/4342
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie98ba38da64e34aedfd88b11880929ee4d99d0d6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Douglas Schilling Landgraf
Gerrit-Reviewer: Igor Lvovsky
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: configNetwork: remove skipLibvirt flag
Douglas Schilling Landgraf has posted comments on this change. Change subject: configNetwork: remove skipLibvirt flag .. Patch Set 2: > but are we sure that libvirt is up and running and > responding in all use cases? I have tried manual install and autoinstall options, everything working out of the box. -- To view, visit http://gerrit.ovirt.org/4342 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie98ba38da64e34aedfd88b11880929ee4d99d0d6 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Douglas Schilling Landgraf Gerrit-Reviewer: Igor Lvovsky ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: configNetwork: remove skipLibvirt flag
Douglas Schilling Landgraf has posted comments on this change.
Change subject: configNetwork: remove skipLibvirt flag
..
Patch Set 2: (2 inline comments)
File vdsm_reg/deployUtil.py.in
Line 49: sys.path.append("/usr/share/vdsm")
I can move it to vdsm site-package...
Line 896: configNetwork.createLibvirtNetwork(mgtBridge,
bridged=True)
> not good - rhev-2 does not have this, and I don't think rhev-3.0 has
> bridge=True
humm, I see clientIF.py using it but ok..
> why is this even needed?
Because when the bridge was created (breth0) by RHEV-H it doesn't added it to
libvirt db and if we try to remove line 904 it will break the code.
> libvirt network should have been created by line 904 below
You meant, line 911 right? However, mgtBridge (806) will be vdsm-breth0 and the
line 911 is MGT_BRIDGE_NAME whici is vdsm-ovirtmgmt or vdsm-rhevm.
So, two different creations.
--
To view, visit http://gerrit.ovirt.org/4342
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie98ba38da64e34aedfd88b11880929ee4d99d0d6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Douglas Schilling Landgraf
Gerrit-Reviewer: Igor Lvovsky
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: configNetwork: remove skipLibvirt flag
Dan Kenigsberg has posted comments on this change.
Change subject: configNetwork: remove skipLibvirt flag
..
Patch Set 2: (2 inline comments)
skipLibvirt was an ugly hack. I am happy that you remove it - but are we sure
that libvirt is up and running and responding in all use cases?
File vdsm_reg/deployUtil.py.in
Line 49: sys.path.append("/usr/share/vdsm")
messing with sys.path in a module is evil.
this should not be hard-coded, anyway.
Line 896: configNetwork.createLibvirtNetwork(mgtBridge,
bridged=True)
not good - rhev-2 does not have this, and I don't think rhev-3.0 has
bridge=True.
why is this even needed? libvirt network should have been created by line 904
below
--
To view, visit http://gerrit.ovirt.org/4342
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie98ba38da64e34aedfd88b11880929ee4d99d0d6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Douglas Schilling Landgraf
Gerrit-Reviewer: Igor Lvovsky
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: configNetwork: remove skipLibvirt flag
Dan Kenigsberg has posted comments on this change. Change subject: configNetwork: remove skipLibvirt flag .. Patch Set 2: I would prefer that you didn't submit this -- To view, visit http://gerrit.ovirt.org/4342 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie98ba38da64e34aedfd88b11880929ee4d99d0d6 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Douglas Schilling Landgraf Gerrit-Reviewer: Igor Lvovsky ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: configNetwork: remove skipLibvirt flag
Douglas Schilling Landgraf has posted comments on this change. Change subject: configNetwork: remove skipLibvirt flag .. Patch Set 2: Verified -- To view, visit http://gerrit.ovirt.org/4342 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie98ba38da64e34aedfd88b11880929ee4d99d0d6 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Douglas Schilling Landgraf Gerrit-Reviewer: Igor Lvovsky ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/vdsm-patches
