Change in vdsm[master]: net: sourceroute: demote error log message
Assaf Muller has posted comments on this change. Change subject: net: sourceroute: demote error log message .. Patch Set 1: The issue of course is not a missing IP address or netmask, but is a missing default gateway. Networks without a gateway are perfectly valid, just not for source routing. I don't remember how deep this code path is (Should it expect a gateway present at this point)? Anyhow, I think that a network without a gateway is a 'supported' or 'expected' use case and perhaps even warning is too harsh. If the admin screwed up and he meant to supply a gateway when he configured his DHCP server, he'll have many other indicators that he screwed up anyway, the VDSM log will probably be the last place he'd look. -- To view, visit https://gerrit.ovirt.org/38429 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I683b8e6c45808374da322bb5418870562d6b4c81 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactor StaticSourceRoute for better testability
Assaf Muller has posted comments on this change.
Change subject: refactor StaticSourceRoute for better testability
..
Patch Set 5: Code-Review+1
(1 comment)
http://gerrit.ovirt.org/#/c/34067/5/vdsm/network/sourceroute.py
File vdsm/network/sourceroute.py:
Line 52: network = netaddr.IPNetwork('%s/%s' % (ipaddr, mask))
Line 53: return "%s/%s" % (network.network, network.prefixlen)
Line 54:
Line 55: def _generateTableId(self, ipaddr):
Line 56: # TODO: Future proof for IPv6
Just a note: Intuitively a function called generateTableId returns an int.
Line 57: return str(netaddr.IPAddress(ipaddr).value)
Line 58:
Line 59: def _buildRoutes(self):
Line 60: return [Route(network='0.0.0.0/0', via=self._gateway,
--
To view, visit http://gerrit.ovirt.org/34067
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If81c71b83670ee0c721dcce449e48ccc21e3bbec
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Ido Barkan
Gerrit-Reviewer: Ondřej Svoboda
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: make isVDSMInterface and it's helpers static
Assaf Muller has posted comments on this change. Change subject: make isVDSMInterface and it's helpers static .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/34139 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6878c3b98e6acbeae440fb29d71bddf6cadc5fae Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactor StaticSourceRoute for better testability
Assaf Muller has posted comments on this change.
Change subject: refactor StaticSourceRoute for better testability
..
Patch Set 4: Code-Review-1
(3 comments)
http://gerrit.ovirt.org/#/c/34067/4/tests/functional/networkTests.py
File tests/functional/networkTests.py:
Line 1620
Line 1621
Line 1622
Line 1623
Line 1624
The advantage of keeping these is that if someone introduces a regression to
rule/routeExists he'll get a very specific failure. Otherwise, seeing a failure
with source routing configuration can lead you on a wild goose chase (Or worse!)
Line 58: BONDING_NAME = 'bond11'
Line 59: IP_ADDRESS = '240.0.0.1'
Line 60: IP_NETWORK = '240.0.0.0'
Line 61: IP_ADDRESS_IN_NETWORK = '240.0.0.50'
Line 62: IP_CIDR = '24'
You can calculate this from IP_CIDR via netaddr.
Line 63: IP_MASK = '255.255.255.0'
Line 64: IP_NETWORK_AND_CIDR = IP_NETWORK + '/' + IP_CIDR
Line 65: IP_GATEWAY = '240.0.0.254'
Line 66: DHCP_RANGE_FROM = '240.0.0.10'
http://gerrit.ovirt.org/#/c/34067/4/vdsm/network/configurators/__init__.py
File vdsm/network/configurators/__init__.py:
Line 139:
Line 140: def _removeSourceRoute(self, netEnt, sourceRouteClass):
Line 141: if netEnt.ipConfig.bootproto != 'dhcp' and netEnt.master is
None:
Line 142: ip = netEnt.ipConfig
Line 143: logging.debug("Removing source route for device %s",
netEnt.name)
Just pass None, None, None, no?
Line 144: sourceRouteClass(netEnt.name, self, ip.ipaddr, ip.netmask,
Line 145: ip.gateway).remove()
Line 146:
Line 147: def _setNewMtu(self, iface, ifaceVlans):
--
To view, visit http://gerrit.ovirt.org/34067
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If81c71b83670ee0c721dcce449e48ccc21e3bbec
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Ido Barkan
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactor StaticSourceRoute for better testability
Assaf Muller has posted comments on this change.
Change subject: refactor StaticSourceRoute for better testability
..
Patch Set 1: Code-Review-1
(5 comments)
http://gerrit.ovirt.org/#/c/34067/1//COMMIT_MSG
Commit Message:
Line 5: CommitDate: 2014-10-13 13:02:32 +0300
Line 6:
Line 7: refactor StaticSourceRoute for better testability
Line 8:
Line 9: changed the way this object is built such that most of the relevant
creattion -> creation
(Sorry for being a putz)
Line 10: information is known at object creattion time. This allows better
Line 11: testability as the routing rules and ip rules generators can be
Line 12: always accessed directly by tests.
Line 13:
http://gerrit.ovirt.org/#/c/34067/1/tests/functional/networkTests.py
File tests/functional/networkTests.py:
Line 1617
Line 1618
Line 1619
Line 1620
Line 1621
Please don't remove these tests! We don't have coverage otherwise.
Line 328: if routeExists(route):
Line 329: raise self.failureException(
Line 330: "routing rule [%s] found. existing rules: \n%s" % (
Line 331: route, routeShowTable(routing_table)))
Line 332:
The naming scheme doesn't match with the rest of the file. I would expect pep8
to yell and shout?
Line 333: def _get_source_routing_rules(self, deviceName, ip_addr):
Line 334: return (StaticSourceRoute(deviceName, None, ip_addr,
Line 335: IP_MASK, IP_GATEWAY))._buildRules()
Line 336:
http://gerrit.ovirt.org/#/c/34067/1/vdsm/network/sourceroute.py
File vdsm/network/sourceroute.py:
Line 44: self._mask = mask
Line 45: self._gateway = gateway
Line 46: self._table = self._generateTableId(ipaddr) if ipaddr else None
Line 47: self._network = self._create_network(ipaddr, mask)
Line 48:
This method is used only in the __init__ on line 47? If so I don't think it's
needed.
Line 49: def _create_network(self, ipaddr, mask):
Line 50: if not ipaddr or not mask:
Line 51: return None
Line 52: network = netaddr.IPNetwork('%s/%s' % (ipaddr, mask))
Line 170: self.device)
Line 171: except IPRoute2Error as e:
Line 172: logging.error('ip binary failed during source
route '
Line 173: 'removal: %s' % e.message)
Line 174:
Changing these 3 functions to static doesn't belong in this patch.
Line 175: @staticmethod
Line 176: def _isLibvirtInterfaceFallback(device):
Line 177: """
Line 178: Checks whether the device belongs to libvirt when libvirt is
not yet
--
To view, visit http://gerrit.ovirt.org/34067
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If81c71b83670ee0c721dcce449e48ccc21e3bbec
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Functional test for Multiple Gateways source routing
Assaf Muller has posted comments on this change. Change subject: Functional test for Multiple Gateways source routing .. Patch Set 8: Code-Review+1 +1 assuming the tests pass. -- To view, visit http://gerrit.ovirt.org/33612 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib510bb56c205dffc98aa2bae3f55c1b4c7f6f56f Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Functional test for Multiple Gateways source routing
Assaf Muller has posted comments on this change. Change subject: Functional test for Multiple Gateways source routing .. Patch Set 6: Code-Review-1 (5 comments) http://gerrit.ovirt.org/#/c/33612/6//COMMIT_MSG Commit Message: Line 2: Author: ibarkan Line 3: AuthorDate: 2014-10-01 07:36:36 +0300 Line 4: Commit: ibarkan Line 5: CommitDate: 2014-10-06 15:21:45 +0300 Line 6: This is just personal preference / something for you to consider, but this patch could be split up into one patch that refactors the static source routing test, and another patch that adds the dynamic source routing test. Makes it easier for reviewers. Line 7: Functional test for Multiple Gateways source routing Line 8: Line 9: adding assertions on the source routing definitions that are configured Line 10: when the network is configured with DHCP. http://gerrit.ovirt.org/#/c/33612/6/tests/functional/networkTests.py File tests/functional/networkTests.py: Line 316: if devName and self.vdsm_net.config is not None: Line 317: self.assertFalse( Line 318: self.vdsm_net._vlanInRunningConfig(devName, vlanId), Line 319: '%s found unexpectedly in running config' % vlanName) Line 320: I don't understand the point of these two functions. IMO it's just extra code and you might as well use routeExists directly, but if you add them you might as well use them consistently throughout the tests. Line 321: def assertRouteExists(self, route, routing_table='all'): Line 322: if not routeExists(route): Line 323: raise self.failureException( Line 324: "routing rule [%s] wasn't found. existing rules: \n%s" % ( Line 328: if routeExists(route): Line 329: raise self.failureException( Line 330: "routing rule [%s] found. existing rules: \n%s" % ( Line 331: route, routeShowTable(routing_table))) Line 332: assertSourceRoutingConfiguration sounds better? Line 333: def assertStaticRoutingConfiguration(self, deviceName, vdsm_net_name): Line 334: """assert that the IP rules and the routing tables pointed by them Line 335: are configured correctly in order to implement source routing""" Line 336: vdsm_net = self.vdsm_net.netinfo.networks[vdsm_net_name] Line 335: are configured correctly in order to implement source routing""" Line 336: vdsm_net = self.vdsm_net.netinfo.networks[vdsm_net_name] Line 337: routing_table = str(StaticSourceRoute.generateTableId( Line 338: vdsm_net['addr'])) Line 339: The generation of the routes and rules should be in a separate helper function. You'd use it here and outside in the tests. This ties in to the next comment in this function... Line 340: routes = [Route(network='0.0.0.0/0', via=IP_GATEWAY, Line 341: device=deviceName, table=routing_table), Line 342: Route(network=IP_NETWORK_AND_CIDR, Line 343: via=str(netaddr.IPAddress(vdsm_net['addr'])), Line 348: Line 349: for route in routes: Line 350: self.assertRouteExists(route, routing_table) Line 351: for rule in rules: Line 352: self.assertTrue(ruleExists(rule)) I wouldn't expect this function to return anything? Line 353: return routes, rules Line 354: Line 355: def assertMtu(self, mtu, *elems): Line 356: for elem in elems: -- To view, visit http://gerrit.ovirt.org/33612 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib510bb56c205dffc98aa2bae3f55c1b4c7f6f56f Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Functional test for Multiple Gateways source routing
Assaf Muller has posted comments on this change.
Change subject: Functional test for Multiple Gateways source routing
..
Patch Set 4: Code-Review-1
(7 comments)
http://gerrit.ovirt.org/#/c/33612/4/AUTHORS
File AUTHORS:
Line 11: Patches have also been contributed by (ordered by lastname):
Line 12:
Line 13:Miguel Angel Ajo
Line 14:Timothy Asir
Line 15:Haim Ateya
Congrats! I always forgot to add my name here :(
Line 16:Ido Barkan
Line 17:Daniel P. Berrange
Line 18:Yaniv Bronhaim
Line 19:Humble Chirammal
http://gerrit.ovirt.org/#/c/33612/4/tests/functional/networkTests.py
File tests/functional/networkTests.py:
Line 94: dummy.remove(nic)
Line 95:
Line 96:
Line 97: @contextmanager
Line 98: def dnsmasqDhcp(interface, router=None):
> Could you remind me why the new art is unavoidable? Cannot we always pass r
+1
Line 99: """Manages the life cycle of dnsmasq as a DHCP server."""
Line 100: dhcpServer = dhcp.Dnsmasq()
Line 101: try:
Line 102: dhcpServer.start(interface, DHCP_RANGE_FROM, DHCP_RANGE_TO,
Line 1938: self.assertIn(right, devs)
Line 1939:
self.assertEqual(devs[right]['cfg']['BOOTPROTO'], 'dhcp')
Line 1940: device_name = right
Line 1941:
Line 1942: device_ip_addr = netaddr.IPAddress(net['addr'])
You're replicating logic from what code you're testing... Source routing code
should have a function that gets whatever it needs and returns the table ID.
This way when it's changed in the future (I hear IPv6 is so hot right now)
you'll only have to change it in one place.
Line 1943: routing_table = str(device_ip_addr.value)
Line 1944: rule1 = Rule(table=routing_table,
Line 1945: destination=IP_NETWORK_AND_CIDR,
Line 1946: srcDevice=str(device_name))
Line 1939:
self.assertEqual(devs[right]['cfg']['BOOTPROTO'], 'dhcp')
Line 1940: device_name = right
Line 1941:
Line 1942: device_ip_addr = netaddr.IPAddress(net['addr'])
Line 1943: routing_table = str(device_ip_addr.value)
You're repeating yourself here and in line 1656.
I wonder why not create something like assert_routes_and_rules that's used both
here and in the static test. It would get the needed parameters (IP, gateway,
etc), construct the routes and rules and assert that they exist on the system.
Line 1944: rule1 = Rule(table=routing_table,
Line 1945: destination=IP_NETWORK_AND_CIDR,
Line 1946: srcDevice=str(device_name))
Line 1947: rule2 = Rule(table=routing_table,
Line 1946: srcDevice=str(device_name))
Line 1947: rule2 = Rule(table=routing_table,
Line 1948: source=IP_NETWORK_AND_CIDR)
Line 1949:
Line 1950: # DHCP source routing configuration is async hence
we wait
> Where's the patient wait? Didn't you need to use until.retry?
'blockingdhcp' is set to True in line 1918. Very convenient for testing! The
comment can be removed.
Line 1951: # patiently
Line 1952: self.assertTrue(ruleExists(rule1))
Line 1953: self.assertTrue(ruleExists(rule2))
Line 1954: routes = [Route.fromText(r)
Line 1949:
Line 1950: # DHCP source routing configuration is async hence
we wait
Line 1951: # patiently
Line 1952: self.assertTrue(ruleExists(rule1))
Line 1953: self.assertTrue(ruleExists(rule2))
Why not use routeExists?
Line 1954: routes = [Route.fromText(r)
Line 1955: for r in routeShowTable(routing_table)]
Line 1956: default_route = Route('0.0.0.0/0', via=IP_GATEWAY,
Line 1957: device=str(device_name))
Line 1961: self.assertIn(default_route, routes)
Line 1962: self.assertIn(specific_route, routes)
Line 1963:
Line 1964: network = {NETWORK_NAME: {'remove': True}}
Line 1965: status, msg = self.vdsm_net.setupNetworks(network,
{}, NOCHK)
We really wanna check that the routes and rules are cleaned up properly. We
don't want to be like Neutron and leave mess laying everywhere :)
Line 1966: self.assertEqual(status, SUCCESS, msg)
Line 1967: self.assertNetworkDoesntExist(NETWORK_NAME)
Line 1968:
Line 1969: @permutations([['default'], ['local']])
--
To view, visit http://gerrit.ovirt.org/33612
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib510bb56c205dffc98aa2bae3f55c1b4c7f6f56f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Ge
Change in vdsm[master]: net: source route: don't attempt to configure invalid values
Assaf Muller has posted comments on this change. Change subject: net: source route: don't attempt to configure invalid values .. Patch Set 1: So the configure method is called from lots of places. Don't we want to log the bad input everywhere, and not just in the DHCP case? -- To view, visit http://gerrit.ovirt.org/32416 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0c4719629bfdc632bbc35171489670062c6c14cb Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: ipwrapper: Get rid of unused methods
Assaf Muller has posted comments on this change. Change subject: ipwrapper: Get rid of unused methods .. Patch Set 2: So sad. My babies :( -- To view, visit http://gerrit.ovirt.org/29726 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba2a8ee4a978c61f7db934e5b08cf2f74923627f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm-tool: Change upgrade mechanism
Assaf Muller has posted comments on this change. Change subject: vdsm-tool: Change upgrade mechanism .. Patch Set 11: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/27193 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6e1d28570dedfeff9fe60624b1db72d8cadf136a Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dima Kuznetsov Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Douglas Schilling Landgraf Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm-tool: Change upgrade mechanism
Assaf Muller has posted comments on this change. Change subject: vdsm-tool: Change upgrade mechanism .. Patch Set 10: Code-Review-1 No change in tests, keeping -1 for now... -- To view, visit http://gerrit.ovirt.org/27193 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6e1d28570dedfeff9fe60624b1db72d8cadf136a Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dima Kuznetsov Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Douglas Schilling Landgraf Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm-tool: Change upgrade mechanism
Assaf Muller has posted comments on this change.
Change subject: vdsm-tool: Change upgrade mechanism
..
Patch Set 9:
(1 comment)
http://gerrit.ovirt.org/#/c/27193/9/tests/toolTests.py
File tests/toolTests.py:
Line 380: def testErrorInUpgrade(self):
Line 381: bad = self.BadUpgraduratorTM()
Line 382: ret = upgrade.apply_upgrade(bad, 'foobar')
Line 383: self.assertEquals(ret, 1)
Line 384: self.assertNotSealed('bad')
> Upgrade name defined as BadUpgraduratorTM.name which is 'bad'.
Oops, confused the argument with the upgrade name :)
Line 385:
Line 386: def testRunMany(self):
Line 387: lst = [0]
Line 388: for _ in xrange(5):
--
To view, visit http://gerrit.ovirt.org/27193
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e1d28570dedfeff9fe60624b1db72d8cadf136a
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Dima Kuznetsov
Gerrit-Reviewer: Douglas Schilling Landgraf
Gerrit-Reviewer: Saggi Mizrahi
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm-tool: Change upgrade mechanism
Assaf Muller has posted comments on this change.
Change subject: vdsm-tool: Change upgrade mechanism
..
Patch Set 9: Code-Review-1
(4 comments)
Sorry for the partial reviews, I hate when people do that. Just testing
comments, rest looks good.
http://gerrit.ovirt.org/#/c/27193/9/tests/toolTests.py
File tests/toolTests.py:
Line 370: def assertNotSealed(self, name):
Line 371: self.assertFalse(self._checkSealExists(name))
Line 372:
Line 373: def testRunOnce(self):
Line 374: lst = [0]
According to the principle of least surprise, I think you should have the
number of upgrade invocations kept in the UpgraduratorTM.invocations, and not
passed in as a list.
Line 375: ret = upgrade.apply_upgrade(self.UpgraduratorTM(lst), 'test')
Line 376: self.assertEquals(ret, 0)
Line 377: self.assertEquals(lst[0], 1)
Line 378: self.assertSealed('test')
Line 380: def testErrorInUpgrade(self):
Line 381: bad = self.BadUpgraduratorTM()
Line 382: ret = upgrade.apply_upgrade(bad, 'foobar')
Line 383: self.assertEquals(ret, 1)
Line 384: self.assertNotSealed('bad')
'bad' will never be sealed because the upgrade name is 'foobar'.
Line 385:
Line 386: def testRunMany(self):
Line 387: lst = [0]
Line 388: for _ in xrange(5):
Line 388: xrange
xrange is not defined in Python 3. Are you guys using a library like six to
ease the transition?
Line 404: upgrade.apply_upgrade(self.UpgraduratorTM(lst), 'test')
Line 405: self.assertEquals(lst[0], 2)
Line 406: self.assertSealed('test')
Line 407:
Line 408: def testNoUpgrade(self):
I think you can safely remove this test...
Line 409: self.assertNotSealed('test')
Line 410:
Line 411: def testUpgradeArgs(self):
Line 412: u = self.UpgraduratorTM([0])
--
To view, visit http://gerrit.ovirt.org/27193
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e1d28570dedfeff9fe60624b1db72d8cadf136a
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Dima Kuznetsov
Gerrit-Reviewer: Douglas Schilling Landgraf
Gerrit-Reviewer: Saggi Mizrahi
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: mooli tayer
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm-tool: Change upgrade mechanism
Assaf Muller has posted comments on this change. Change subject: vdsm-tool: Change upgrade mechanism .. Patch Set 8: To make my previous question clearer: If you run the unified upgrade during VDSM start (IE: Not manually), do you see lines (in upgrade.log) like: "Adding network ovirtmgmt..."? -- To view, visit http://gerrit.ovirt.org/27193 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6e1d28570dedfeff9fe60624b1db72d8cadf136a Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dima Kuznetsov Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Douglas Schilling Landgraf Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm-tool: Change upgrade mechanism
Assaf Muller has posted comments on this change. Change subject: vdsm-tool: Change upgrade mechanism .. Patch Set 8: If you run the unified upgrade, do you see lines (in upgrade.log) like: "Adding network ovirtmgmt..."? -- To view, visit http://gerrit.ovirt.org/27193 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6e1d28570dedfeff9fe60624b1db72d8cadf136a Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dima Kuznetsov Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Douglas Schilling Landgraf Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm-tool: Change upgrade mechanism
Assaf Muller has posted comments on this change. Change subject: vdsm-tool: Change upgrade mechanism .. Patch Set 8: Code-Review-1 How is logging output to other loggers/handlers (Such as the root logger, or vdsm logger) during an upgrade logged to upgrade.log? -- To view, visit http://gerrit.ovirt.org/27193 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6e1d28570dedfeff9fe60624b1db72d8cadf136a Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dima Kuznetsov Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Douglas Schilling Landgraf Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sourcerouting: fix _getRoute not to include local routes
Assaf Muller has posted comments on this change. Change subject: sourcerouting: fix _getRoute not to include local routes .. Patch Set 2: You might have individuals placing /32 bit addresses on non-VM networks. For example, 10.0.0.1/32 on one interface, and 10.0.0.2/32 on another. Taking the first 31 bits you wouldn't be able to differentiate between them. I'd look into the routes table file instead - It would solve IPv6 as well, which the current solution does not. -- To view, visit http://gerrit.ovirt.org/27262 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5b3d43c8a2077e40b8b4314f02ea17bc3968c42b Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sourcerouting: fix _getRoute not to include local routes
Assaf Muller has posted comments on this change. Change subject: sourcerouting: fix _getRoute not to include local routes .. Patch Set 2: Why does a show on a specific table show routes that were not inserted by us? I have a vague recollection of an iproute bug where shows on tables with a large table_id show a large set of routes, and not just the routes belonging to that table. If you define a low IP on the network I suspect the bug wouldn't manifest. You could work around this using the routes table file instead (With low table IDs). -- To view, visit http://gerrit.ovirt.org/27262 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5b3d43c8a2077e40b8b4314f02ea17bc3968c42b Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sourcerouting: fix _getRoute not to include local routes
Assaf Muller has posted comments on this change.
Change subject: sourcerouting: fix _getRoute not to include local routes
..
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/27262/1/vdsm/network/sourceroute.py
File vdsm/network/sourceroute.py:
Line 110: rmFile(DynamicSourceRoute.getTrackingFilePath(device))
Line 111:
Line 112: @staticmethod
Line 113: def _getRoutes(table, device):
Line 114: routes = []
If a host has hundreds of VLAN devices it would be a shame to iterate through
all routes instead of just a specific table. I did not fully understand the
issue from the commit message (Dan already touched this), but wouldn't it be
possible to continue getting the routes from a specific table, but deal with
local routes later on?
Line 115: for entry in routeShowTable('all'):
Line 116: try:
Line 117: route = Route.fromText(entry)
Line 118: except ValueError:
--
To view, visit http://gerrit.ovirt.org/27262
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b3d43c8a2077e40b8b4314f02ea17bc3968c42b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm-tool: Change upgrade mechanism
Assaf Muller has posted comments on this change. Change subject: vdsm-tool: Change upgrade mechanism .. Patch Set 1: If you git blame and look at the patches that introduced the upgrade mechanism you'd see that Saggi NACK'd inheritance, on the basis that inheritance should never be used in Python (This is an oversimplification but was the basis for his argument). I disagreed with him but implemented it without inheritance as he asked. Let's see if you can convince him now :) -- To view, visit http://gerrit.ovirt.org/27193 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6e1d28570dedfeff9fe60624b1db72d8cadf136a Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dima Kuznetsov Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Added network directory
Assaf Muller has abandoned this change. Change subject: Added network directory .. Abandoned A lot has changed since this patch was proposed. Some interest has surfaced around this subject, so I'll abandon and Antoni will create a new patch and take ownership. -- To view, visit http://gerrit.ovirt.org/22694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: If9c7e8cfc96cc1671ed7549c2f45ca820211c7c1 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Added network directory
Assaf Muller has posted comments on this change. Change subject: Added network directory .. Patch Set 2: Antoni, Dan, Yaniv or anyone else - If you are interested in this patch please take ownership. -- To view, visit http://gerrit.ovirt.org/22694 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If9c7e8cfc96cc1671ed7549c2f45ca820211c7c1 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.4]: Fix defaultRoute handling
Assaf Muller has posted comments on this change. Change subject: Fix defaultRoute handling .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/24677 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I643c5fe9dab39833d73541b7a1bfaae9ae3e749d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.4 Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.4]: Fix defaultRoute handling
Assaf Muller has posted comments on this change. Change subject: Fix defaultRoute handling .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/24677 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I643c5fe9dab39833d73541b7a1bfaae9ae3e749d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.4 Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix defaultRoute handling
Assaf Muller has uploaded a new change for review. Change subject: Fix defaultRoute handling .. Fix defaultRoute handling defaultRoute wasn't being past correctly to objectivizeNetwork, so that defaultRoute was always None inside the function, no matter what was past to it. This meant that DEFROUTE=yes/no was never written to ifcfg files, and 'ip route add default...' never applied by the iproute2 configurator. Change-Id: I643c5fe9dab39833d73541b7a1bfaae9ae3e749d Signed-off-by: Assaf Muller --- M vdsm/configNetwork.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/31/24631/1 diff --git a/vdsm/configNetwork.py b/vdsm/configNetwork.py index 7a2206a..5bf9838 100755 --- a/vdsm/configNetwork.py +++ b/vdsm/configNetwork.py @@ -279,7 +279,7 @@ netEnt = objectivizeNetwork(network if bridged else None, vlan, bonding, bondingOptions, nics, mtu, ipaddr, netmask, gateway, bootproto, ipv6addr, ipv6gateway, -defaultRoute, _netinfo=_netinfo, +defaultRoute=defaultRoute, _netinfo=_netinfo, configurator=configurator, **options) netEnt.configure(**options) -- To view, visit http://gerrit.ovirt.org/24631 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I643c5fe9dab39833d73541b7a1bfaae9ae3e749d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf.ifcfg: include CONFFILE_HEADER in route/rule files
Assaf Muller has posted comments on this change. Change subject: netconf.ifcfg: include CONFFILE_HEADER in route/rule files .. Patch Set 1: Code-Review+1 Looks good. By the way, we could use something like for unified persistence file. There, it would only serve the purpose of knowing what version of VDSM wrote the file, but it could be useful anyway. -- To view, visit http://gerrit.ovirt.org/24103 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia49754b85797168df056ebd9acf81c151ad3980f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Ethtool_opts: fix retrieval from configwriter.
Assaf Muller has posted comments on this change. Change subject: Ethtool_opts: fix retrieval from configwriter. .. Patch Set 1: If this patch were merged and I were to browse the code, I'd ask why isn't getEthtoolOptions a method of the Configurator base class. You can see that the Iproute2 configurator is just fine, we only have an issue with the Ifcfg configurator using ConfigWriter internally. I'd have Ifcfg's configureNic pass ethtool options to ConfigWriter's addNic instead, and keep getEthtoolOptions a class method. -- To view, visit http://gerrit.ovirt.org/23792 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iac6646bbad8c7431ec1c035ebc01f0180a8338ee Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Moti Asayag Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Extend setupNetworks API to accept defaultRoute
Assaf Muller has posted comments on this change. Change subject: Extend setupNetworks API to accept defaultRoute .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.ovirt.org/22720 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9b2ff711155eb0bd79ab84e979eb82c846362374 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Moti Asayag Gerrit-Reviewer: Yaniv Bronhaim Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add netinfo.getDefaultGateway()
Assaf Muller has posted comments on this change. Change subject: Add netinfo.getDefaultGateway() .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.ovirt.org/23346 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I15be5d69b5ef8f9e4cb9a30a3739a0b060225e0e Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ondřej Svoboda Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconfig: set ETHTOOL_OPTS when a NIC goes up
Assaf Muller has posted comments on this change. Change subject: netconfig: set ETHTOOL_OPTS when a NIC goes up .. Patch Set 1: Code-Review-1 (1 comment) http://gerrit.ovirt.org/#/c/23366/1/vdsm/netconf/iproute2.py File vdsm/netconf/iproute2.py: Line 126: ( nic -> nic.name -- To view, visit http://gerrit.ovirt.org/23366 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic8a5a933e1c93f80185d61f3cd43eee53c3a7c1a Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add netinfo.getDefaultGateway()
Assaf Muller has posted comments on this change. Change subject: Add netinfo.getDefaultGateway() .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/23346 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I15be5d69b5ef8f9e4cb9a30a3739a0b060225e0e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add netinfo.getDefaultGateway()
Assaf Muller has uploaded a new change for review.
Change subject: Add netinfo.getDefaultGateway()
..
Add netinfo.getDefaultGateway()
To be used in next patch.
Change-Id: I15be5d69b5ef8f9e4cb9a30a3739a0b060225e0e
Signed-off-by: Assaf Muller
---
M lib/vdsm/ipwrapper.py
M lib/vdsm/netinfo.py
2 files changed, 12 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/23346/1
diff --git a/lib/vdsm/ipwrapper.py b/lib/vdsm/ipwrapper.py
index 500e028..7d1309c 100644
--- a/lib/vdsm/ipwrapper.py
+++ b/lib/vdsm/ipwrapper.py
@@ -462,12 +462,16 @@
return _execCmd(command)
-def routeShowAllDefaultGateways():
+def routeShowGateways(table):
command = [_IP_BINARY.cmd, 'route', 'show', 'to', '0.0.0.0/0', 'table',
- 'all']
+ table]
return _execCmd(command)
+def routeShowAllDefaultGateways():
+return routeShowGateways('all')
+
+
def routeShowTable(table):
command = [_IP_BINARY.cmd, 'route', 'show', 'table', table]
return _execCmd(command)
diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py
index d000fe4..ba86a24 100644
--- a/lib/vdsm/netinfo.py
+++ b/lib/vdsm/netinfo.py
@@ -37,7 +37,7 @@
from .ipwrapper import IPRoute2Error
from .ipwrapper import Route
from .ipwrapper import routeGet
-from .ipwrapper import routeShowAllDefaultGateways
+from .ipwrapper import routeShowGateways, routeShowAllDefaultGateways
from . import libvirtconnection
from .ipwrapper import linkShowDev
from .utils import anyFnmatch
@@ -299,6 +299,11 @@
return prefix2netmask(netmask)
+def getDefaultGateway():
+output = routeShowGateways('main')
+return Route.fromText(output[0]) if output else None
+
+
def getgateway(gateways, dev):
return gateways.get(dev, '')
--
To view, visit http://gerrit.ovirt.org/23346
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I15be5d69b5ef8f9e4cb9a30a3739a0b060225e0e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Change MANAGEMENT_NETWORK to MANAGEMENT_NETWORK*S*
Assaf Muller has posted comments on this change. Change subject: Change MANAGEMENT_NETWORK to MANAGEMENT_NETWORK*S* .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.ovirt.org/23182 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I69a39b7887267d54d4dce27d4818d32a7cc0cd68 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Extend setupNetworks API to accept defaultRoute
Assaf Muller has posted comments on this change. Change subject: Extend setupNetworks API to accept defaultRoute .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.ovirt.org/22720 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9b2ff711155eb0bd79ab84e979eb82c846362374 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Moti Asayag Gerrit-Reviewer: Yaniv Bronhaim Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Extend setupNetworks API to accept defaultRoute
Assaf Muller has posted comments on this change.
Change subject: Extend setupNetworks API to accept defaultRoute
..
Patch Set 2:
(1 comment)
http://gerrit.ovirt.org/#/c/22720/2/vdsm/configNetwork.py
File vdsm/configNetwork.py:
Line 249: # the management network.
Line 250: # TODO: When oVirt 3.3 is deprecated, change the default to False
and
Line 251: # remove reference to constants.MANAGEMENT_NETWORKS
Line 252: if defaultRoute is None:
Line 253: defaultRoute = network in constants.MANAGEMENT_NETWORKS
> needs renaming to LEGACY_MANAGEMENT_NETWORKS
Oh I didn't rebase yet.
Line 254:
Line 255: logging.info("Adding network %s with vlan=%s, bonding=%s,
nics=%s,"
Line 256: " bondingOptions=%s, mtu=%s, bridged=%s,
defaultRoute=%s,"
Line 257: "options=%s", network, vlan, bonding, nics,
bondingOptions,
--
To view, visit http://gerrit.ovirt.org/22720
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9b2ff711155eb0bd79ab84e979eb82c846362374
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Moti Asayag
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Extend setupNetworks API to accept defaultRoute
Assaf Muller has posted comments on this change. Change subject: Extend setupNetworks API to accept defaultRoute .. Patch Set 2: (1 comment) http://gerrit.ovirt.org/#/c/22720/2/vdsm/configNetwork.py File vdsm/configNetwork.py: Line 49: Line 50: def objectivizeNetwork(bridge=None, vlan=None, bonding=None, Line 51:bondingOptions=None, nics=None, mtu=None, ipaddr=None, Line 52:netmask=None, gateway=None, bootproto=None, Line 53:ipv6addr=None, ipv6gateway=None, ipv6autoconf=None, > If setupNetworks gets an explicit defaultRoute, then with patch set 1 defau Just to make this perfectly clear: Since VDSM 3.3, if the engine sends defaultRoute, then objectivize gets defaultRoute twice (And VDSM crashes), once as a named argument, and one as a keyword argument in **opts. Patch set 2 fixes that issue. Line 54:dhcpv6=None, defaultRoute=None, _netinfo=None, Line 55:configurator=None, blockingdhcp=None, Line 56:implicitBonding=None, **opts): Line 57: """ -- To view, visit http://gerrit.ovirt.org/22720 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9b2ff711155eb0bd79ab84e979eb82c846362374 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Moti Asayag Gerrit-Reviewer: Yaniv Bronhaim Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Change MANAGEMENT_NETWORK to MANAGEMENT_NETWORK*S*
Assaf Muller has posted comments on this change.
Change subject: Change MANAGEMENT_NETWORK to MANAGEMENT_NETWORK*S*
..
Patch Set 1:
(2 comments)
http://gerrit.ovirt.org/#/c/23182/1/lib/vdsm/constants.py.in
File lib/vdsm/constants.py.in:
Line 21: # Description:Constants definitions for vdsm and utilities.
Line 22:
Line 23: import os
Line 24:
Line 25: # VDSM management devices
> Actually, it's management network names, not devices (now that we have brid
Done
Line 26: MANAGEMENT_NETWORKS = ('ovirtmgmt', 'rhevm')
Line 27:
Line 28: # SMBIOS manufacturer
Line 29: SMBIOS_MANUFACTURER = '@SMBIOS_MANUFACTURER@'
Line 22:
Line 23: import os
Line 24:
Line 25: # VDSM management devices
Line 26: MANAGEMENT_NETWORKS = ('ovirtmgmt', 'rhevm')
> Could you rename this to LEGACY_MANAGEMENT_NETWORKS, so it's clear that no
Done
Line 27:
Line 28: # SMBIOS manufacturer
Line 29: SMBIOS_MANUFACTURER = '@SMBIOS_MANUFACTURER@'
Line 30: SMBIOS_OSNAME = '@SMBIOS_OSNAME@'
--
To view, visit http://gerrit.ovirt.org/23182
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69a39b7887267d54d4dce27d4818d32a7cc0cd68
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netinfo: ifcfg: ignore 0 suffix
Assaf Muller has posted comments on this change. Change subject: netinfo: ifcfg: ignore 0 suffix .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.ovirt.org/#/c/23329/1//COMMIT_MSG Commit Message: Doesn't IPADDR1 mean configuring a second IP address, IPADDR2 a third, and so on? What if the ifcfg file has both IPADDR and IPADDR0? (I understand that both of those cases would only happen with manual admin intervention, I just wanted to make sure you're aware of them...) We could make our ifcfg files parsing smarter, but since we're moving to iproute2 anyway, I'm not sure it's worth it. Line 1: Parent: 54cc6747 (vdsm: Replaced all too generic except: handlers in vm.py) Line 2: Author: Dan Kenigsberg Line 3: AuthorDate: 2014-01-16 11:51:06 + Line 4: Commit: Dan Kenigsberg -- To view, visit http://gerrit.ovirt.org/23329 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5c9a206a5b96d0b38aab9c3d62d6074424b419be Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Sandro Bonazzola Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netinfo: Determine bootproto also without ifcfg files
Assaf Muller has posted comments on this change.
Change subject: netinfo: Determine bootproto also without ifcfg files
..
Patch Set 3: Code-Review-1
(5 comments)
http://gerrit.ovirt.org/#/c/23098/3//COMMIT_MSG
Commit Message:
Line 9: DHCP version (4 or 6) is detected reliably only for dhclient,
Line 10: dhcpcd allows setting the version in its configuration file.
Line 11:
Line 12: Parsing of configuration files would get very complex.
Line 13:
I'm eagerly awaiting your findings about lease files as a better approach. I
have a hunch it'd be a significantly simpler solution.
Line 14: I will investigate if 'lease' files are a better fit.
Line 15: They certainly use less expressive syntax and we avoid
Line 16: having to simulate the interplay between cmdline arguments
Line 17: and configuration; and possibly we can even tell if an
http://gerrit.ovirt.org/#/c/23098/3/lib/vdsm/netinfo.py
File lib/vdsm/netinfo.py:
Line 445: return None
Line 446: else:
Line 447: raise NotImplementedError
Line 448:
Line 449:
This function is confusing / doesn't stand on its own at the module level. I'd
advise moving it as an inline function in 'get()'.
Line 450: def _getBootProtocols(networks):
Line 451: ifaces = {}
Line 452: for net, info in networks.iteritems():
Line 453: ifaces[info['iface']] = net
Line 528: # This possibly includes: defaulting to all interfaces; dhcpcd's
globbing;
Line 529: # explicit/implicit configuration files (of different nature,
indeed)
Line 530: #
Line 531: # A last resort: lease files created by daemons _somewhere_
Line 532:
Please add an inline comment here about this next section being a fallback for
each network. Maybe in an inline function instead.
Line 533: for netinfo in networks.itervalues():
Line 534: bootproto = None
Line 535: if 'bootproto4' not in netinfo:
Line 536: bootproto = getBootProtocol(netinfo['iface'])
Line 529: # explicit/implicit configuration files (of different nature,
indeed)
Line 530: #
Line 531: # A last resort: lease files created by daemons _somewhere_
Line 532:
Line 533: for netinfo in networks.itervalues():
netinfo for the attributes dictionary of a network is a very unfortunate name,
as it has different connotations for VDSM developers.
Line 534: bootproto = None
Line 535: if 'bootproto4' not in netinfo:
Line 536: bootproto = getBootProtocol(netinfo['iface'])
Line 537: netinfo['bootproto4'] = bootproto
Line 530: #
Line 531: # A last resort: lease files created by daemons _somewhere_
Line 532:
Line 533: for netinfo in networks.itervalues():
Line 534: bootproto = None
With how getBootProtocol is currently implemented it only gets 'bootproto' from
ifcfg / unified persistence,
and doesn't care about ipv6. With that in mind you can simplify the code to:
if 'bootproto4' not in netinfo:
netinfo['bootproto4'] = getBootProtocol(netinfo['iface'])
We'll have to track the issue of getBootProtocol and IPv6 though... Currently
that function is used once, to know how to remove source routing when a network
is deleted.
It won't work for IPv6 as it is now.
Line 535: if 'bootproto4' not in netinfo:
Line 536: bootproto = getBootProtocol(netinfo['iface'])
Line 537: netinfo['bootproto4'] = bootproto
Line 538: if 'bootproto6' not in netinfo:
--
To view, visit http://gerrit.ovirt.org/23098
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5fbc48a0adf5f40120a72ec2c4cc2fc80b7226b8
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Ondřej Svoboda
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: utils: Extend pgrep to accept multiple process names
Assaf Muller has posted comments on this change. Change subject: utils: Extend pgrep to accept multiple process names .. Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/23239/1/lib/vdsm/utils.py File lib/vdsm/utils.py: Line 264: pid = os.path.basename(path) Line 265: yield int(pid) Line 266: Line 267: Line 268: def pgrep(*names): > I think it could use a docstring now. While the change is completely hidden from current users - I agree, it should be explicitly noted that the function can accept a single or multiple names. Line 269: res = [] Line 270: for pid in iteratePids(): Line 271: try: Line 272: procName = pidStat(pid).comm -- To view, visit http://gerrit.ovirt.org/23239 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I891ee6c36b4198ce9900e56a62939b9a7bafbc6e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: utils: Moved pgrep and getCmdArgs from storage/misc
Assaf Muller has posted comments on this change. Change subject: utils: Moved pgrep and getCmdArgs from storage/misc .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/23040 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia9a933bec0df3f7773c0f8e47fc0bf0492149031 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix VDSM boot
Assaf Muller has posted comments on this change. Change subject: Fix VDSM boot .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/23218 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaa0fadae424e93b43cafc2b1519f59e88e7c5832 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix VDSM boot
Assaf Muller has uploaded a new change for review.
Change subject: Fix VDSM boot
..
Fix VDSM boot
Fix regression introduced in Gerrit 22750, hash 114692f.
The unified net persistence upgrade name was changed, changing
the name exposed to vdsm-tool. However, the call to the upgrade
was still using the old name.
Change-Id: Iaa0fadae424e93b43cafc2b1519f59e88e7c5832
Signed-off-by: Assaf Muller
---
M init/vdsmd_init_common.sh.in
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/18/23218/1
diff --git a/init/vdsmd_init_common.sh.in b/init/vdsmd_init_common.sh.in
index 071b9a6..46af454 100644
--- a/init/vdsmd_init_common.sh.in
+++ b/init/vdsmd_init_common.sh.in
@@ -181,7 +181,7 @@
}
task_unified_network_persistence_upgrade(){
-"$VDSM_TOOL" upgrade-to-unified-persistence
+"$VDSM_TOOL" upgrade-unified-persistence
}
task_upgrade_300_nets(){
--
To view, visit http://gerrit.ovirt.org/23218
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaa0fadae424e93b43cafc2b1519f59e88e7c5832
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Extend setupNetworks API to accept defaultRoute
Assaf Muller has posted comments on this change. Change subject: Extend setupNetworks API to accept defaultRoute .. Patch Set 2: (2 comments) http://gerrit.ovirt.org/#/c/22720/2/lib/vdsm/tool/unified_persistence.py File lib/vdsm/tool/unified_persistence.py: Line 29: Line 30: UPGRADE_NAME = 'UnifiedPersistence' Line 31: Line 32: Line 33: # TODO: Upgrade currently gets bootproto from ifcfg files, > I do not understand this TODO and certainty not why it's part of this patch It has nothing to do with this patch, you're right. I got carried away :) More importantly though - This upgrade isn't an upgrade from ifcfg files. It's an upgrade that builds the initial running conf for old and new hosts alike. Take a future oVirt 4.0 host - We need to build its initial running and persistent conf because otherwise it just won't exist. Meaning, we won't be able to boot from the persistent conf. Line 34: # as we assume we're upgrading from oVirt <= 3.4, where users still used Line 35: # ifcfg files. Once we start dealing with new installations on OS that don't Line 36: # use ifcfg files, we need to stop getting information from ifcfg files. Line 37: # bootproto = 'dhcp' if there's a lease on the NIC at the moment of upgrade Line 97: if netParams['gateway'] != '': Line 98: networks[network]['gateway'] = netParams['gateway'] Line 99: Line 100: networks[network]['defaultRoute'] = \ Line 101: str(network in MANAGEMENT_NETWORKS) > I do not follow. If you serialize a Boolean into json, it does not recover If you serialize True to json, it is for whatever reason converted to true (Which is different from 'true'). When I copy/paste that into setupNetworks, Python complains that true isn't defined. (As it isn't 'true', 'True', or True). On another note - It's incorrect to check if the network is the management network... I'll just get the default route from the main routing table and check if the device fits. Line 102: Line 103: # What if the 'physical device' is actually a VLAN? Line 104: if physicalDevice in netinfo.vlans: Line 105: vlanDevice = physicalDevice -- To view, visit http://gerrit.ovirt.org/22720 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9b2ff711155eb0bd79ab84e979eb82c846362374 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Moti Asayag Gerrit-Reviewer: Yaniv Bronhaim Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add --run-again option to upgrades
Assaf Muller has posted comments on this change. Change subject: Add --run-again option to upgrades .. Patch Set 1: (1 comment) File lib/vdsm/tool/upgrade.py Line 125: default=False, Line 126: action='store_true', Line 127: help='Run the upgrade again, even if it was ran before', Line 128: ) Line 129: return parser.parse_args(sys.argv[2:]) The two upgrades are currently only being called with vdsm-tool, as a prestart task. Line 130: Line 131: Line 132: def upgrade(upgradeName): Line 133: """ -- To view, visit http://gerrit.ovirt.org/22750 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e6b84bdd14b9cd3921cea187b2654be22989334 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Extend setupNetworks API to accept defaultRoute
Assaf Muller has posted comments on this change. Change subject: Extend setupNetworks API to accept defaultRoute .. Patch Set 2: (2 comments) File lib/vdsm/tool/unified_persistence.py Line 97: if netParams['gateway'] != '': Line 98: networks[network]['gateway'] = netParams['gateway'] Line 99: Line 100: networks[network]['defaultRoute'] = \ Line 101: str(network in MANAGEMENT_NETWORKS) str conversion is needed because for whatever reason when serializing to json, True is converted to true. true cannot be sent to setupNetworks (True, 'true', 'True' are all good. true is not). Line 102: Line 103: # What if the 'physical device' is actually a VLAN? Line 104: if physicalDevice in netinfo.vlans: Line 105: vlanDevice = physicalDevice File vdsm/configNetwork.py Line 49: Line 50: def objectivizeNetwork(bridge=None, vlan=None, bonding=None, Line 51:bondingOptions=None, nics=None, mtu=None, ipaddr=None, Line 52:netmask=None, gateway=None, bootproto=None, Line 53:ipv6addr=None, ipv6gateway=None, ipv6autoconf=None, If setupNetworks gets an explicit defaultRoute, then with patch set 1 defaultRoute is sent as a keyword argument and in **opts as well. Line 54:dhcpv6=None, defaultRoute=None, _netinfo=None, Line 55:configurator=None, blockingdhcp=None, Line 56:implicitBonding=None, **opts): Line 57: """ -- To view, visit http://gerrit.ovirt.org/22720 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9b2ff711155eb0bd79ab84e979eb82c846362374 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Moti Asayag Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Extend setupNetworks API to accept defaultRoute
Assaf Muller has posted comments on this change.
Change subject: Extend setupNetworks API to accept defaultRoute
..
Patch Set 1:
(5 comments)
File lib/vdsm/tool/unified_persistence.py
Line 71
Line 72
Line 73
Line 74
Line 75
Notice that I only assign into networks[network]['bootproto'] if bootproto ==
'dhcp'
Line 36: UPGRADE_NAME, networks, bondings)
Line 37: _persist(networks, bondings)
Line 38:
Line 39:
Line 40: def _ifcfgFilter(filter, value):
Filter like audio filters you can pass on sound waves... Something that changes
the output, not necessarily filtering out values. Anyway, if 'filter' has a
very specific connotation in Python I'll change it.
Line 41: """
Line 42: If value is in the dict and hashable - Get its filtered value.
Line 43: Otherwise: Return the original value
Line 44: :param filter: Dictionary of 'from' -> 'to'
Line 54: def _toIfcfgFormat(value):
Line 55: return _ifcfgFilter({'on': 'yes', 'off': 'no'}, value)
Line 56:
Line 57:
Line 58: def _fromIfcfgFormat(value):
I'll make the function assume that it can only expect 'yes' and 'no', but I
find 'fromIfcfgFormat' vastly more expressive and clear than 'yesnoToBool'. It
explains why the line of code exists, and not what it does.
Line 59: return _ifcfgFilter({'yes': True, 'no': False}, value)
Line 60:
Line 61:
Line 62: def _getNetInfo():
Line 95: networks[network]['netmask'] = netParams['netmask']
Line 96: if netParams['gateway'] != '':
Line 97: networks[network]['gateway'] = netParams['gateway']
Line 98:
Line 99: networks[network]['defaultRoute'] = _fromIfcfgFormat(
I just noticed that this bit is wrong...
What we really want is to persist defaultRoute=True only for the management
network (IE: Emulate proper engine behavior).
Line 100: ifcfg.get('DEFROUTE', False))
Line 101:
Line 102: # What if the 'physical device' is actually a VLAN?
Line 103: if physicalDevice in netinfo.vlans:
File vdsm/configNetwork.py
Line 253
Line 254
Line 255
Line 256
Line 257
Done
--
To view, visit http://gerrit.ovirt.org/22720
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9b2ff711155eb0bd79ab84e979eb82c846362374
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Moti Asayag
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Change MANAGEMENT_NETWORK to MANAGEMENT_NETWORK*S*
Assaf Muller has uploaded a new change for review.
Change subject: Change MANAGEMENT_NETWORK to MANAGEMENT_NETWORK*S*
..
Change MANAGEMENT_NETWORK to MANAGEMENT_NETWORK*S*
Before: MANAGEMENT_NETWORK was either 'ovirtmgmt' or 'rhevm',
and the value was gotten from automake
After: MANAGEMENT_NETWORKS = ('ovirtmgmt', 'rhevm')
This change allows the following patch to solve the bug.
Bug-Url: https://bugzilla.redhat.com/1015009
Change-Id: I69a39b7887267d54d4dce27d4818d32a7cc0cd68
Signed-off-by: Assaf Muller
---
M lib/vdsm/constants.py.in
M lib/vdsm/tool/upgrade_300_networks.py
M vdsm/configNetwork.py
3 files changed, 11 insertions(+), 6 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/82/23182/1
diff --git a/lib/vdsm/constants.py.in b/lib/vdsm/constants.py.in
index 9be68c2..622a28c 100644
--- a/lib/vdsm/constants.py.in
+++ b/lib/vdsm/constants.py.in
@@ -22,8 +22,8 @@
import os
-# VDSM management device
-MANAGEMENT_NETWORK = '@VDSMBRIDGE@'
+# VDSM management devices
+MANAGEMENT_NETWORKS = ('ovirtmgmt', 'rhevm')
# SMBIOS manufacturer
SMBIOS_MANUFACTURER = '@SMBIOS_MANUFACTURER@'
diff --git a/lib/vdsm/tool/upgrade_300_networks.py
b/lib/vdsm/tool/upgrade_300_networks.py
index 0f754fd..3e41b18 100644
--- a/lib/vdsm/tool/upgrade_300_networks.py
+++ b/lib/vdsm/tool/upgrade_300_networks.py
@@ -22,7 +22,7 @@
import sys
from vdsm import netinfo
-from vdsm.constants import MANAGEMENT_NETWORK
+from vdsm.constants import MANAGEMENT_NETWORKS
from vdsm.tool import expose
from vdsm.tool.upgrade import upgrade
@@ -31,8 +31,13 @@
def isNeeded(networks, bridges):
-return (MANAGEMENT_NETWORK not in networks and
-MANAGEMENT_NETWORK in bridges)
+def managementNetwork():
+return any(net in networks for net in MANAGEMENT_NETWORKS)
+
+def managementBridge():
+return any(net in bridges for net in MANAGEMENT_NETWORKS)
+
+return not managementNetwork() and managementBridge()
@upgrade('3.0.0-networks')
diff --git a/vdsm/configNetwork.py b/vdsm/configNetwork.py
index 5db2a41..dfe167a 100755
--- a/vdsm/configNetwork.py
+++ b/vdsm/configNetwork.py
@@ -254,7 +254,7 @@
bootproto = options.pop('bootproto', None)
-defaultRoute = network == constants.MANAGEMENT_NETWORK
+defaultRoute = network in constants.MANAGEMENT_NETWORKS
netEnt = objectivizeNetwork(network if bridged else None, vlan, bonding,
bondingOptions, nics, mtu, ipaddr, netmask,
--
To view, visit http://gerrit.ovirt.org/23182
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I69a39b7887267d54d4dce27d4818d32a7cc0cd68
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: BindingXMLRPC bugfix: Use getDeviceByIP() for lastClientIface
Assaf Muller has posted comments on this change. Change subject: BindingXMLRPC bugfix: Use getDeviceByIP() for lastClientIface .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.ovirt.org/22842 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic17e91c80560cd3acb73e7a64879257a6e07b1fe Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: BindingXMLRPC bugfix: Added netinfo:getDeviceByIP and a test
Assaf Muller has posted comments on this change. Change subject: BindingXMLRPC bugfix: Added netinfo:getDeviceByIP and a test .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.ovirt.org/22841 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3563ae9b3e199e29ea2227463b9e4a3752cb99a0 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: ifcfg: exclude HWADDR lines if requested
Assaf Muller has posted comments on this change. Change subject: ifcfg: exclude HWADDR lines if requested .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/22288 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifc4904251150756594502367d4bea87620dfba58 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: Gluster API verbs will now be callable again
Assaf Muller has posted comments on this change. Change subject: Gluster API verbs will now be callable again .. Patch Set 2: Verified+1 Same patch merged in master. -- To view, visit http://gerrit.ovirt.org/23045 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1513a5dca444c3e176fb31a9425da85980401b0f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: cleanup: move _updateTimestamp to API.py
Assaf Muller has posted comments on this change. Change subject: cleanup: move _updateTimestamp to API.py .. Patch Set 5: Code-Review+1 Oh, didn't notice that patch set 4 moved it back to static (Patch set 3 moved it to module level). Woo, a lot of confusion. -- To view, visit http://gerrit.ovirt.org/23050 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4270df333ff5ff56e4375742cdc526ec76ed3bf1 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: cleanup: move _updateTimestamp to API.py
Assaf Muller has posted comments on this change. Change subject: cleanup: move _updateTimestamp to API.py .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/23050 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4270df333ff5ff56e4375742cdc526ec76ed3bf1 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: cleanup: move _updateTimestamp to API.py
Assaf Muller has posted comments on this change. Change subject: cleanup: move _updateTimestamp to API.py .. Patch Set 3: (1 comment) Commit Message Line 15: Line 16: Ancient Engines used to call getVdsCaps() instead of the lightweight Line 17: ping(). Even older Engines did nothing special and depended on its Line 18: periodic list() call. For backward compatibility, we add Line 19: _updateTimestamp on these specific verbs, too. Could I ask you to state what versions of oVirt used getVdsCaps and list, and possibly add TO-DOs to remove those calls to updateTimestamp when those oVirt versions are no longer supported? Line 20: Line 21: Change-Id: I4270df333ff5ff56e4375742cdc526ec76ed3bf1 -- To view, visit http://gerrit.ovirt.org/23050 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4270df333ff5ff56e4375742cdc526ec76ed3bf1 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Gluster API verbs will now be callable again
Assaf Muller has posted comments on this change. Change subject: Gluster API verbs will now be callable again .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.ovirt.org/23044 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibab01ef654e7ac469ea0a2594c4744918ae24819 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Bala.FA Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Gluster API verbs will now be callable again
Assaf Muller has posted comments on this change.
Change subject: Gluster API verbs will now be callable again
..
Patch Set 2:
(2 comments)
File vdsm/BindingXMLRPC.py
Line 140
Line 141
Line 142
Line 143
Line 144
It is correct, but I think it's kind of deceiving to call a static method using
self. Anyway, in the next patch-set I'll move 'updateTimestamp' to be module
level, so this discussion is pointless.
Line 79: self.server.server_close()
Line 80: self._thread.join()
Line 81: return {'status': doneCode}
Line 82:
Line 83: @staticmethod
> Why is this not an instance method?
Explained in the commit message.
> I prefer to module level functions...
2v1, fair enough... I'll change it to module level.
Line 84: def updateTimestamp():
Line 85: # FIXME: The setup+editNetwork API uses this log file to
Line 86: # determine if this host is still accessible. We use a
Line 87: # file (rather than an event) because setup+editNetwork is
--
To view, visit http://gerrit.ovirt.org/23044
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibab01ef654e7ac469ea0a2594c4744918ae24819
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Bala.FA
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Gluster API verbs will now be callable again
Assaf Muller has posted comments on this change.
Change subject: Gluster API verbs will now be callable again
..
Patch Set 2:
(1 comment)
File vdsm/BindingXMLRPC.py
Line 79: self.server.server_close()
Line 80: self._thread.join()
Line 81: return {'status': doneCode}
Line 82:
Line 83: @staticmethod
Ok we had a mid-air collision between comments.
About static vs module level, here's my 2 cents: So in Python there's no real
point to static methods because you can just put them at the module level...
It's just a semantics thing. Putting it in the class suggests what is its
relevant scope. In this specific case it makes more sense to me (as a reader)
for the function to belong to the BindingXMLRPC class.
Line 84: def updateTimestamp():
Line 85: # FIXME: The setup+editNetwork API uses this log file to
Line 86: # determine if this host is still accessible. We use a
Line 87: # file (rather than an event) because setup+editNetwork is
--
To view, visit http://gerrit.ovirt.org/23044
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibab01ef654e7ac469ea0a2594c4744918ae24819
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Bala.FA
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Gluster API verbs will now be callable again
Assaf Muller has posted comments on this change.
Change subject: Gluster API verbs will now be callable again
..
Patch Set 2:
(1 comment)
File vdsm/BindingXMLRPC.py
Line 79: self.server.server_close()
Line 80: self._thread.join()
Line 81: return {'status': doneCode}
Line 82:
Line 83: @staticmethod
Why in this patch though?
I think this is the correct solution to the regression (+ the ovirt-3.3 patch),
and moving this function to API should be done in a separate patch, no?
Line 84: def updateTimestamp():
Line 85: # FIXME: The setup+editNetwork API uses this log file to
Line 86: # determine if this host is still accessible. We use a
Line 87: # file (rather than an event) because setup+editNetwork is
--
To view, visit http://gerrit.ovirt.org/23044
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibab01ef654e7ac469ea0a2594c4744918ae24819
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Bala.FA
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: Gluster API verbs will now be callable again
Assaf Muller has uploaded a new change for review.
Change subject: Gluster API verbs will now be callable again
..
Gluster API verbs will now be callable again
Fix regression introduced in #22967.
wrapApiMethod was using the function's class 'updateTimestamp',
but updateTimestamp is defined in BindingXMLRPC and doesn't exist
in the GlusterAPI class. This patch moves 'updateTimestamp' to be
a static method.
Change-Id: I1513a5dca444c3e176fb31a9425da85980401b0f
Signed-off-by: Assaf Muller
---
M vdsm/BindingXMLRPC.py
1 file changed, 4 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/45/23045/1
diff --git a/vdsm/BindingXMLRPC.py b/vdsm/BindingXMLRPC.py
index c4b4737..9542314 100644
--- a/vdsm/BindingXMLRPC.py
+++ b/vdsm/BindingXMLRPC.py
@@ -88,7 +88,8 @@
'lastClient': self.cif.threadLocal.client,
'lastClientIface': getIfaceByIP(self.cif.threadLocal.server)}
-def updateTimestamp(self):
+@staticmethod
+def updateTimestamp():
# FIXME: The setup+editNetwork API uses this log file to
# determine if this host is still accessible. We use a
# file (rather than an event) because setup+editNetwork is
@@ -150,7 +151,7 @@
def _registerFunctions(self):
def wrapIrsMethod(f):
def wrapper(*args, **kwargs):
-self.updateTimestamp()
+BindingXMLRPC.updateTimestamp()
fmt = ""
logargs = []
@@ -940,7 +941,7 @@
def wrapApiMethod(f):
def wrapper(*args, **kwargs):
try:
-f.im_self.updateTimestamp()
+BindingXMLRPC.updateTimestamp()
logLevel = logging.DEBUG
if f.__name__ in ('getVMList', 'getAllVmStats', 'getStats',
'fenceNode'):
--
To view, visit http://gerrit.ovirt.org/23045
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1513a5dca444c3e176fb31a9425da85980401b0f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.3
Gerrit-Owner: Assaf Muller
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Gluster API verbs will now be callable again
Assaf Muller has posted comments on this change. Change subject: Gluster API verbs will now be callable again .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.ovirt.org/23044 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibab01ef654e7ac469ea0a2594c4744918ae24819 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Bala.FA Gerrit-Reviewer: Dan Kenigsberg Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix regression introduced in #22837
Assaf Muller has posted comments on this change. Change subject: Fix regression introduced in #22837 .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/23044 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibab01ef654e7ac469ea0a2594c4744918ae24819 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix regression introduced in #22837
Assaf Muller has uploaded a new change for review.
Change subject: Fix regression introduced in #22837
..
Fix regression introduced in #22837
Gluster API verbs will now be callable again.
wrapApiMethod was using the function's class 'updateTimestamp',
but updateTimestamp is defined in BindingXMLRPC and doesn't exist
in the GlusterAPI class. This patch moves 'updateTimestamp' to be
a static method.
Change-Id: Ibab01ef654e7ac469ea0a2594c4744918ae24819
Signed-off-by: Assaf Muller
---
M vdsm/BindingXMLRPC.py
1 file changed, 4 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/44/23044/1
diff --git a/vdsm/BindingXMLRPC.py b/vdsm/BindingXMLRPC.py
index 2364684..56eefa6 100644
--- a/vdsm/BindingXMLRPC.py
+++ b/vdsm/BindingXMLRPC.py
@@ -80,7 +80,8 @@
self._thread.join()
return {'status': doneCode}
-def updateTimestamp(self):
+@staticmethod
+def updateTimestamp():
# FIXME: The setup+editNetwork API uses this log file to
# determine if this host is still accessible. We use a
# file (rather than an event) because setup+editNetwork is
@@ -141,7 +142,7 @@
def _registerFunctions(self):
def wrapIrsMethod(f):
def wrapper(*args, **kwargs):
-self.updateTimestamp()
+BindingXMLRPC.updateTimestamp()
fmt = ""
logargs = []
@@ -938,7 +939,7 @@
def wrapApiMethod(f):
def wrapper(*args, **kwargs):
try:
-f.im_self.updateTimestamp()
+BindingXMLRPC.updateTimestamp()
logLevel = logging.DEBUG
if f.__name__ in ('getVMList', 'getAllVmStats', 'getStats',
'fenceNode'):
--
To view, visit http://gerrit.ovirt.org/23044
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibab01ef654e7ac469ea0a2594c4744918ae24819
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: BindingXMLRPC bugfix: Return lastClientIface for current con...
Assaf Muller has posted comments on this change. Change subject: BindingXMLRPC bugfix: Return lastClientIface for current connection .. Patch Set 3: Verified+1 Waiting for z-stream ACK on the bug. -- To view, visit http://gerrit.ovirt.org/22967 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I36e48cc07a27f44a2b413f0e9159110404f1b0ca Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hooks: Security groups support for OVS
Assaf Muller has posted comments on this change. Change subject: hooks: Security groups support for OVS .. Patch Set 6: Code-Review+1 Please open a bug to track the migration issue before merging this patch. -- To view, visit http://gerrit.ovirt.org/22585 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I77e219cce9c572ebef6a8584847f517abbda224d Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mike Kolesnik Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mike Kolesnik Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: Bring back missing before_device_migrate_source hook call
Assaf Muller has posted comments on this change. Change subject: vm: Bring back missing before_device_migrate_source hook call .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/23006 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I317ae6b0ed5a7a6fa0f959169b27339c4451b35a Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: Bring back missing before_device_migrate_source hook call
Assaf Muller has posted comments on this change. Change subject: vm: Bring back missing before_device_migrate_source hook call .. Patch Set 1: Code-Review-1 Please add Bug-Url to commit message: https://bugzilla.redhat.com/show_bug.cgi?id=1048763 -- To view, visit http://gerrit.ovirt.org/23006 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I317ae6b0ed5a7a6fa0f959169b27339c4451b35a Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: BindingXMLRPC cleanup: Inline getServerInfo
Assaf Muller has abandoned this change. Change subject: BindingXMLRPC cleanup: Inline getServerInfo .. Abandoned I'll backport only the bugfix itself. -- To view, visit http://gerrit.ovirt.org/22965 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I9f19630f995c2fe9fb3e3c11e4027f22f98722ba Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: BindingXMLRPC cleanup: Remove unused variables
Assaf Muller has abandoned this change. Change subject: BindingXMLRPC cleanup: Remove unused variables .. Abandoned I'll backport only the bugfix itself. -- To view, visit http://gerrit.ovirt.org/22966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I1d36a9f71b4d192b8ba825e95055dd42fc278248 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: BindingXMLRPC bugfix: Fix netinfo:getIfaceByIP unit test
Assaf Muller has posted comments on this change. Change subject: BindingXMLRPC bugfix: Fix netinfo:getIfaceByIP unit test .. Patch Set 1: Also: This change is *not yet merged into master* -- To view, visit http://gerrit.ovirt.org/22968 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iee4757958ea5554043c6dc7169a8df05b1887687 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: BindingXMLRPC bugfix: Fix netinfo:getIfaceByIP unit test
Assaf Muller has posted comments on this change. Change subject: BindingXMLRPC bugfix: Fix netinfo:getIfaceByIP unit test .. Patch Set 1: Verified+1 Cherry pick into manual backport: Verified on ovirt-3.3 branch with this patch. I'm not sure how important it is to backport a fix for a broken unit test, but I'll let you decide. -- To view, visit http://gerrit.ovirt.org/22968 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iee4757958ea5554043c6dc7169a8df05b1887687 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: BindingXMLRPC bugfix: Return lastClientIface for current con...
Assaf Muller has posted comments on this change. Change subject: BindingXMLRPC bugfix: Return lastClientIface for current connection .. Patch Set 1: Verified+1 Cherry pick into manual backport: Verified on ovirt-3.3 branch with this patch. -- To view, visit http://gerrit.ovirt.org/22967 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I36e48cc07a27f44a2b413f0e9159110404f1b0ca Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: BindingXMLRPC cleanup: Remove unused variables
Assaf Muller has posted comments on this change. Change subject: BindingXMLRPC cleanup: Remove unused variables .. Patch Set 1: Verified+1 Cherry pick into manual backport: Verified on ovirt-3.3 branch with this patch. -- To view, visit http://gerrit.ovirt.org/22966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1d36a9f71b4d192b8ba825e95055dd42fc278248 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: BindingXMLRPC cleanup: Inline getServerInfo
Assaf Muller has posted comments on this change. Change subject: BindingXMLRPC cleanup: Inline getServerInfo .. Patch Set 1: Verified+1 Cherry pick into manual backport: Verified on ovirt-3.3 branch with this patch. -- To view, visit http://gerrit.ovirt.org/22965 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9f19630f995c2fe9fb3e3c11e4027f22f98722ba Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: BindingXMLRPC bugfix: Fix netinfo:getIfaceByIP unit test
Assaf Muller has uploaded a new change for review. Change subject: BindingXMLRPC bugfix: Fix netinfo:getIfaceByIP unit test .. BindingXMLRPC bugfix: Fix netinfo:getIfaceByIP unit test Change-Id: Iee4757958ea5554043c6dc7169a8df05b1887687 Signed-off-by: Assaf Muller --- M lib/vdsm/netinfo.py M tests/netinfoTests.py 2 files changed, 9 insertions(+), 5 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/68/22968/1 diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py index 790c099..2f54413 100644 --- a/lib/vdsm/netinfo.py +++ b/lib/vdsm/netinfo.py @@ -599,7 +599,6 @@ def getIfaceByIP(ip): for info in ethtool.get_interfaces_info(ethtool.get_active_devices()): - for ipv4addr in info.get_ipv4_addresses(): if ip == ipv4addr.address or ip == IPv4toMapped(ipv4addr.address): return info.device diff --git a/tests/netinfoTests.py b/tests/netinfoTests.py index 18e71c3..fb8c584 100644 --- a/tests/netinfoTests.py +++ b/tests/netinfoTests.py @@ -98,10 +98,15 @@ def testGetIfaceByIP(self): for dev in ethtool.get_interfaces_info(ethtool.get_active_devices()): -ipaddrs = map( -lambda etherinfo_ipv6addr: etherinfo_ipv6addr.address, -dev.get_ipv6_addresses()) -ipaddrs.append(dev.ipv4_address) +# Link-local IPv6 addresses are generated from the MAC address, +# which is shared between a nic and its bridge. Since We don't +# support having the same IP address on two different NICs, and +# link-local IPv6 addresses aren't interesting for 'getDeviceByIP' +# then ignore them in the test +ipaddrs = [ipv6.address for ipv6 in dev.get_ipv6_addresses() + if ipv6.scope != 'link'] +if dev.ipv4_address is not None: +ipaddrs.append(dev.ipv4_address) for ip in ipaddrs: self.assertEqual(dev.device, netinfo.getIfaceByIP(ip)) -- To view, visit http://gerrit.ovirt.org/22968 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iee4757958ea5554043c6dc7169a8df05b1887687 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Assaf Muller ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: BindingXMLRPC cleanup: Inline getServerInfo
Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/22965
to review the following change.
Change subject: BindingXMLRPC cleanup: Inline getServerInfo
..
BindingXMLRPC cleanup: Inline getServerInfo
Inlining getServerInfo because:
1) It's used exactly once
2) I'd like to avoid people using it at all. It only works
if there's exactly one client, and only for one command
at a time. It works for the purposes it's used, but if
(code) users expect it to work for all cases they'd be
in for a surprise
Change-Id: I9f19630f995c2fe9fb3e3c11e4027f22f98722ba
Signed-off-by: Assaf Muller
Reviewed-on: http://gerrit.ovirt.org/22835
Reviewed-by: Dan Kenigsberg
---
M vdsm/BindingXMLRPC.py
1 file changed, 3 insertions(+), 11 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/65/22965/1
diff --git a/vdsm/BindingXMLRPC.py b/vdsm/BindingXMLRPC.py
index 20aaee1..57f5ac9 100644
--- a/vdsm/BindingXMLRPC.py
+++ b/vdsm/BindingXMLRPC.py
@@ -81,16 +81,6 @@
self._thread.join()
return {'status': doneCode}
-def getServerInfo(self):
-"""
-Return the IP address and last client information
-"""
-last = self.server.lastClient
-lastserver = self.server.lastServerIP
-return {'management_ip': self.serverIP,
-'lastClient': last,
-'lastClientIface': getIfaceByIP(lastserver)}
-
def _getKeyCertFilenames(self):
"""
Get the locations of key and certificate files.
@@ -313,7 +303,9 @@
def getCapabilities(self):
api = API.Global()
ret = api.getCapabilities()
-ret['info'].update(self.getServerInfo())
+ret['info']['management_ip'] = self.serverIP
+ret['info']['lastClient'] = self.server.lastClient
+ret['info']['lastClientIface'] = getIfaceByIP(self.server.lastServerIP)
return ret
def getHardwareInfo(self):
--
To view, visit http://gerrit.ovirt.org/22965
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9f19630f995c2fe9fb3e3c11e4027f22f98722ba
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.3
Gerrit-Owner: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: BindingXMLRPC bugfix: Return lastClientIface for current con...
Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/22967
to review the following change.
Change subject: BindingXMLRPC bugfix: Return lastClientIface for current
connection
..
BindingXMLRPC bugfix: Return lastClientIface for current connection
The patch does two things:
1) Fixes a bug where lastClientIface was for the previous
connection, and not the current one. The method to retrieve
the last client was through a log_request LoggingMixIn
which is invoked at the end of a response, not at the beginning.
The patch removes the usage of that MixIn, and instead uses
values that are updated at the beginning of the connection
2) Simplifies the 'LoggingMixin' out of the picture as its no
longer needed
Change-Id: I36e48cc07a27f44a2b413f0e9159110404f1b0ca
Signed-off-by: Assaf Muller
Reviewed-on: http://gerrit.ovirt.org/22837
Reviewed-by: Dan Kenigsberg
---
M vdsm/BindingXMLRPC.py
1 file changed, 16 insertions(+), 17 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/22967/1
diff --git a/vdsm/BindingXMLRPC.py b/vdsm/BindingXMLRPC.py
index 5ee7126..666cc95 100644
--- a/vdsm/BindingXMLRPC.py
+++ b/vdsm/BindingXMLRPC.py
@@ -80,6 +80,15 @@
self._thread.join()
return {'status': doneCode}
+def updateTimestamp(self):
+# FIXME: The setup+editNetwork API uses this log file to
+# determine if this host is still accessible. We use a
+# file (rather than an event) because setup+editNetwork is
+# performed by a separate, root process. To clean this
+# up we need to move this to an API wrapper that is only
+# run for real clients (not vdsm internal API calls).
+file(constants.P_VDSM_CLIENT_LOG, 'w')
+
def _getKeyCertFilenames(self):
"""
Get the locations of key and certificate files.
@@ -97,29 +106,16 @@
threadLocal = self.cif.threadLocal
-class LoggingMixIn:
-
-def log_request(self, code='-', size='-'):
-"""Track from where client connections are coming."""
-self.server.lastClient = self.client_address[0]
-self.server.lastServerIP = self.request.getsockname()[0]
-# FIXME: The editNetwork API uses this log file to
-# determine if this host is still accessible. We use a
-# file (rather than an event) because editNetwork is
-# performed by a separate, root process. To clean this
-# up we need to move this to an API wrapper that is only
-# run for real clients (not vdsm internal API calls).
-file(constants.P_VDSM_CLIENT_LOG, 'w')
-
server_address = (self.serverIP, int(self.serverPort))
if self.enableSSL:
basehandler = SecureXMLRPCServer.SecureXMLRPCRequestHandler
else:
basehandler = SimpleXMLRPCServer.SimpleXMLRPCRequestHandler
-class LoggingHandler(LoggingMixIn, basehandler):
+class LoggingHandler(basehandler):
def setup(self):
threadLocal.client = self.client_address[0]
+threadLocal.server = self.request.getsockname()[0]
return basehandler.setup(self)
def parse_request(self):
@@ -149,6 +145,7 @@
def _registerFunctions(self):
def wrapIrsMethod(f):
def wrapper(*args, **kwargs):
+self.updateTimestamp()
fmt = ""
logargs = []
@@ -301,8 +298,9 @@
api = API.Global()
ret = api.getCapabilities()
ret['info']['management_ip'] = self.serverIP
-ret['info']['lastClient'] = self.server.lastClient
-ret['info']['lastClientIface'] = getIfaceByIP(self.server.lastServerIP)
+ret['info']['lastClient'] = self.cif.threadLocal.client
+ret['info']['lastClientIface'] = getIfaceByIP(
+self.cif.threadLocal.server)
return ret
def getHardwareInfo(self):
@@ -940,6 +938,7 @@
def wrapApiMethod(f):
def wrapper(*args, **kwargs):
try:
+f.im_self.updateTimestamp()
logLevel = logging.DEBUG
if f.__name__ in ('getVMList', 'getAllVmStats', 'getStats',
'fenceNode'):
--
To view, visit http://gerrit.ovirt.org/22967
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I36e48cc07a27f44a2b413f0e9159110404f1b0ca
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.3
Gerrit-Owner: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: BindingXMLRPC cleanup: Remove unused variables
Hello Dan Kenigsberg, I'd like you to do a code review. Please visit http://gerrit.ovirt.org/22966 to review the following change. Change subject: BindingXMLRPC cleanup: Remove unused variables .. BindingXMLRPC cleanup: Remove unused variables Change-Id: I1d36a9f71b4d192b8ba825e95055dd42fc278248 Signed-off-by: Assaf Muller Reviewed-on: http://gerrit.ovirt.org/22836 Reviewed-by: Dan Kenigsberg --- M vdsm/BindingXMLRPC.py 1 file changed, 0 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/66/22966/1 diff --git a/vdsm/BindingXMLRPC.py b/vdsm/BindingXMLRPC.py index 57f5ac9..5ee7126 100644 --- a/vdsm/BindingXMLRPC.py +++ b/vdsm/BindingXMLRPC.py @@ -18,7 +18,6 @@ # Refer to the README and COPYING files for full details of the license # -import time from errno import EINTR import SimpleXMLRPCServer from vdsm import SecureXMLRPCServer @@ -103,7 +102,6 @@ def log_request(self, code='-', size='-'): """Track from where client connections are coming.""" self.server.lastClient = self.client_address[0] -self.server.lastClientTime = time.time() self.server.lastServerIP = self.request.getsockname()[0] # FIXME: The editNetwork API uses this log file to # determine if this host is still accessible. We use a @@ -143,7 +141,6 @@ requestHandler=LoggingHandler, logRequests=True) utils.closeOnExec(server.socket.fileno()) -server.lastClientTime = 0 server.lastClient = '0.0.0.0' server.lastServerIP = '0.0.0.0' -- To view, visit http://gerrit.ovirt.org/22966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1d36a9f71b4d192b8ba825e95055dd42fc278248 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hooks: Security groups support for OVS
Assaf Muller has posted comments on this change. Change subject: hooks: Security groups support for OVS .. Patch Set 4: (1 comment) File vdsm_hooks/openstacknet/Makefile.am Line 38: install-data-hook: Line 39:chmod 440 $(DESTDIR)$(sysconfdir)/sudoers.d/50_vdsm_hook_openstacknet Line 40: Line 41: install-data-local: install-data-consts install-data-sudoers Line 42:$(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/after_device_create There's no migrate fail hook point at this time. We could implement a new after_migrate_fail_destination hook point, but I don't think it should block this patch. I think it's still definitely better to support migrations and have messy destinations on failed migrations, than not support migrations at all. Line 43:$(INSTALL_SCRIPT) $(srcdir)/after_device_create.py \ Line 44: $(DESTDIR)$(vdsmhooksdir)/after_device_create/50_openstacknet Line 45:$(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/after_device_destroy Line 46:$(INSTALL_SCRIPT) $(srcdir)/after_device_destroy.py \ -- To view, visit http://gerrit.ovirt.org/22585 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I77e219cce9c572ebef6a8584847f517abbda224d Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mike Kolesnik Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mike Kolesnik Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hooks: Security groups support for OVS
Assaf Muller has posted comments on this change. Change subject: hooks: Security groups support for OVS .. Patch Set 4: Code-Review-1 (1 comment) File vdsm_hooks/openstacknet/Makefile.am Line 38: install-data-hook: Line 39:chmod 440 $(DESTDIR)$(sysconfdir)/sudoers.d/50_vdsm_hook_openstacknet Line 40: Line 41: install-data-local: install-data-consts install-data-sudoers Line 42:$(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/after_device_create before_device_migrate_destination won't be called at the migration. Please copy before_device_create to before_device_migrate_destination as well, and include migrations in your verification. Line 43:$(INSTALL_SCRIPT) $(srcdir)/after_device_create.py \ Line 44: $(DESTDIR)$(vdsmhooksdir)/after_device_create/50_openstacknet Line 45:$(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/after_device_destroy Line 46:$(INSTALL_SCRIPT) $(srcdir)/after_device_destroy.py \ -- To view, visit http://gerrit.ovirt.org/22585 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I77e219cce9c572ebef6a8584847f517abbda224d Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mike Kolesnik Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mike Kolesnik Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hooks: Extract devName function for openstacknet
Assaf Muller has posted comments on this change. Change subject: hooks: Extract devName function for openstacknet .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/22584 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8ece3bb16ac5b9bb8b28c26f2e96d2d74defd4dc Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mike Kolesnik Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mike Kolesnik Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hooks: Extract function for command execution
Assaf Muller has posted comments on this change. Change subject: hooks: Extract function for command execution .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/22583 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id0bcc3be324d1afd9855f9d624cbe4efbf3cf6dd Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mike Kolesnik Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mike Kolesnik Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: BindingXMLRPC bugfix: Use getDeviceByIP() for lastClientIface
Assaf Muller has posted comments on this change. Change subject: BindingXMLRPC bugfix: Use getDeviceByIP() for lastClientIface .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.ovirt.org/22842 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic17e91c80560cd3acb73e7a64879257a6e07b1fe Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: BindingXMLRPC cleanup: Remove unused variables
Assaf Muller has posted comments on this change. Change subject: BindingXMLRPC cleanup: Remove unused variables .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.ovirt.org/22836 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1d36a9f71b4d192b8ba825e95055dd42fc278248 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: BindingXMLRPC bugfix: Return lastClientIface for current con...
Assaf Muller has posted comments on this change. Change subject: BindingXMLRPC bugfix: Return lastClientIface for current connection .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.ovirt.org/22837 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I36e48cc07a27f44a2b413f0e9159110404f1b0ca Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: BindingXMLRPC bugfix: Added netinfo:getDeviceByIP and a test
Assaf Muller has posted comments on this change. Change subject: BindingXMLRPC bugfix: Added netinfo:getDeviceByIP and a test .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.ovirt.org/22841 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3563ae9b3e199e29ea2227463b9e4a3752cb99a0 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: BindingXMLRPC cleanup: Inline getServerInfo
Assaf Muller has posted comments on this change. Change subject: BindingXMLRPC cleanup: Inline getServerInfo .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.ovirt.org/22835 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9f19630f995c2fe9fb3e3c11e4027f22f98722ba Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: [4/x] Added netinfo:getDeviceByIP and a test
Assaf Muller has posted comments on this change. Change subject: [4/x] Added netinfo:getDeviceByIP and a test .. Patch Set 2: (1 comment) Commit Message Line 3: AuthorDate: 2013-12-31 14:37:15 +0200 Line 4: Commit: Assaf Muller Line 5: CommitDate: 2013-12-31 15:33:57 +0200 Line 6: Line 7: [4/x] Added netinfo:getDeviceByIP and a test Done Line 8: Line 9: Change-Id: I3563ae9b3e199e29ea2227463b9e4a3752cb99a0 -- To view, visit http://gerrit.ovirt.org/22841 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3563ae9b3e199e29ea2227463b9e4a3752cb99a0 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: [3/x] Bugfix: Return lastClientIface for *current* connection
Assaf Muller has posted comments on this change. Change subject: [3/x] Bugfix: Return lastClientIface for *current* connection .. Patch Set 1: (2 comments) Commit Message Line 3: AuthorDate: 2013-12-31 14:02:49 +0200 Line 4: Commit: Assaf Muller Line 5: CommitDate: 2013-12-31 14:02:49 +0200 Line 6: Line 7: [3/x] Bugfix: Return lastClientIface for *current* connection I improved the commit message. I couldn't find an explicit bug for this, but that doesn't rule out the possibility of a bug that happens because of the current behavior. I couldn't find such bugs either case. As for backporting - I was thinking of backporting the entire chain, not just the 2 patches fixing actual bugs. (Remember I wanted exactly 2 patches, one for each bug). I know we don't normally backport cleanups, but I think for this case it'll be trivial to backport the entire chain but difficult to backport only 2 out of 5 patches. Line 8: Line 9: Change-Id: I36e48cc07a27f44a2b413f0e9159110404f1b0ca File vdsm/BindingXMLRPC.py Line 86: # file (rather than an event) because setup+editNetwork is Line 87: # performed by a separate, root process. To clean this Line 88: # up we need to move this to an API wrapper that is only Line 89: # run for real clients (not vdsm internal API calls). Line 90: file(constants.P_VDSM_CLIENT_LOG, 'w') Done Line 91: Line 92: def _getKeyCertFilenames(self): Line 93: """ Line 94: Get the locations of key and certificate files. -- To view, visit http://gerrit.ovirt.org/22837 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I36e48cc07a27f44a2b413f0e9159110404f1b0ca Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: [2/x] BindingXMLRPC cleanup: Remove unused variables
Assaf Muller has posted comments on this change. Change subject: [2/x] BindingXMLRPC cleanup: Remove unused variables .. Patch Set 1: Will be fixed in the upcoming patchset. -- To view, visit http://gerrit.ovirt.org/22836 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1d36a9f71b4d192b8ba825e95055dd42fc278248 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: [1/x] BindingXMLRPC cleanup: Remove/inline getServerInfo
Assaf Muller has posted comments on this change. Change subject: [1/x] BindingXMLRPC cleanup: Remove/inline getServerInfo .. Patch Set 1: (1 comment) Commit Message Line 3: AuthorDate: 2013-12-31 13:50:05 +0200 Line 4: Commit: Assaf Muller Line 5: CommitDate: 2013-12-31 13:56:11 +0200 Line 6: Line 7: [1/x] BindingXMLRPC cleanup: Remove/inline getServerInfo Done Line 8: Line 9: Change-Id: I9f19630f995c2fe9fb3e3c11e4027f22f98722ba -- To view, visit http://gerrit.ovirt.org/22835 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9f19630f995c2fe9fb3e3c11e4027f22f98722ba Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: allow hooks to pass down dictionaries in json format
Assaf Muller has posted comments on this change. Change subject: vdsm: allow hooks to pass down dictionaries in json format .. Patch Set 7: A humble request: Please give enough time for the thread on arch/vdsm to die down before considering merging this patch. IE: Give us slower folks time to review post thread discussions :) -- To view, visit http://gerrit.ovirt.org/20330 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie07c511e9740fd19a1c27baf87e91f9a427d0dcd Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Miguel Angel Ajo Pelayo Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Miguel Angel Ajo Pelayo Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: update NIC: having no custom properies is valid
Assaf Muller has posted comments on this change. Change subject: update NIC: having no custom properies is valid .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/22950 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If510f012039304ad16000d46d9d72a3e035539f5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: update NIC: having no custom properies is valid
Assaf Muller has posted comments on this change.
Change subject: update NIC: having no custom properies is valid
..
Patch Set 2: Code-Review+1
Looks like this is the only "params.get('custom')" usage... What a weird miss.
--
To view, visit http://gerrit.ovirt.org/22922
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If510f012039304ad16000d46d9d72a3e035539f5
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add --run-again option to upgrades
Assaf Muller has posted comments on this change. Change subject: Add --run-again option to upgrades .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.ovirt.org/22750 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e6b84bdd14b9cd3921cea187b2654be22989334 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fixing call to setLinkAndNetwork after hook
Assaf Muller has posted comments on this change.
Change subject: Fixing call to setLinkAndNetwork after hook
..
Patch Set 1:
(1 comment)
File vdsm/vm.py
Line 3237: custom)
Line 3238: self._dom.updateDeviceFlags(vnicStrXML,
Line 3239:
libvirt.VIR_DOMAIN_AFFECT_LIVE)
Line 3240: dev._deviceXML = vnicStrXML
Line 3241: self.log.debug("Nic has been updated:\n %s" %
vnicStrXML)
Well, if you're gonna use named parameters I guess it would make sense to use
them all the way... The 2nd parameter is named as well.
Line 3242: hooks.after_update_device(vnicStrXML, self.conf,
params=custom)
Line 3243: except Exception as e:
Line 3244: self.log.debug('Request failed: %s', vnicStrXML,
exc_info=True)
Line 3245: hooks.after_update_device_fail(vnicStrXML, self.conf,
--
To view, visit http://gerrit.ovirt.org/22884
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9188c6862b19a2473f8cfc5f5d207b35ca8c8507
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: [1/x] BindingXMLRPC cleanup: Remove/inline getServerInfo
Assaf Muller has posted comments on this change. Change subject: [1/x] BindingXMLRPC cleanup: Remove/inline getServerInfo .. Patch Set 1: (1 comment) Commit Message Line 3: AuthorDate: 2013-12-31 13:50:05 +0200 Line 4: Commit: Assaf Muller Line 5: CommitDate: 2013-12-31 13:56:11 +0200 Line 6: Line 7: [1/x] BindingXMLRPC cleanup: Remove/inline getServerInfo I'll remove it before it's merged - It's used so reviewers have an easier time. Line 8: Line 9: Change-Id: I9f19630f995c2fe9fb3e3c11e4027f22f98722ba -- To view, visit http://gerrit.ovirt.org/22835 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9f19630f995c2fe9fb3e3c11e4027f22f98722ba Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: [5/x] Use getDeviceByIP(server IP) for lastClientIface
Assaf Muller has posted comments on this change. Change subject: [5/x] Use getDeviceByIP(server IP) for lastClientIface .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.ovirt.org/22842 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic17e91c80560cd3acb73e7a64879257a6e07b1fe Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: [4/x] Added netinfo:getDeviceByIP and a test
Assaf Muller has posted comments on this change. Change subject: [4/x] Added netinfo:getDeviceByIP and a test .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.ovirt.org/22841 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3563ae9b3e199e29ea2227463b9e4a3752cb99a0 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
