Change in vdsm[master]: Fix crash on vm start with netdevs some w/out info

List overview All Threads
Download

newer

older

Change in vdsm[master]: Multiply...

Change in vdsm[master]: Per device...

asegurap＠redhat.com

2 Apr 2013 2 Apr '13

8:47 p.m.

Antoni Segura Puimedon has uploaded a new change for review.

Change subject: Fix crash on vm start with netdevs some w/out info ......................................................................

Fix crash on vm start with netdevs some w/out info

Some devices, like sr-iov do not have have information in the domxml to represent alias and name.

This patch is a first version to get around the crash. It would maybe be preferrable to add support for some hook call in the except that would allow us to ask for the missing information to the hook (which could return something specific or default to the empty string, for example).

Change-Id: I5a90fba21075ab2c7ecb0cb75f14ca07f090829e Signed-off-by: Antoni S. Puimedon asegurap@redhat.com --- M vdsm/libvirtvm.py 1 file changed, 12 insertions(+), 3 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/40/13540/1

diff --git a/vdsm/libvirtvm.py b/vdsm/libvirtvm.py index cb792b5..2837b8f 100644 --- a/vdsm/libvirtvm.py +++ b/vdsm/libvirtvm.py @@ -2966,10 +2966,19 @@ getElementsByTagName('interface') for x in ifsxml: devType = x.getAttribute('type') - name = x.getElementsByTagName('target')[0].getAttribute('dev') + try: + name = x.getElementsByTagName('target')[0].getAttribute('dev') + except IndexError: + name = '' + try: + alias = x.getElementsByTagName('alias')[0].getAttribute('name') + except IndexError: + alias = '' + try: + model = x.getElementsByTagName('model')[0].getAttribute('type') + except IndexError: + model = '' mac = x.getElementsByTagName('mac')[0].getAttribute('address') - alias = x.getElementsByTagName('alias')[0].getAttribute('name') - model = x.getElementsByTagName('model')[0].getAttribute('type')

network = None try:

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

Gerrit-MessageType: newchange Gerrit-Change-Id: I5a90fba21075ab2c7ecb0cb75f14ca07f090829e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com

Show replies by date

Jenkins CI RO

2 Apr 2 Apr

8:47 p.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Fix crash on vm start with netdevs some w/out info ......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1834/ (1/2)

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

Gerrit-MessageType: comment Gerrit-Change-Id: I5a90fba21075ab2c7ecb0cb75f14ca07f090829e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

Jenkins CI RO

8:47 p.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Fix crash on vm start with netdevs some w/out info ......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1783/ (2/2)

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

Jenkins CI RO

8:49 p.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Fix crash on vm start with netdevs some w/out info ......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1783/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1834/ : SUCCESS

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

Jenkins CI RO

8:56 p.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1835/ (1/2)

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

Gerrit-MessageType: comment Gerrit-Change-Id: I5a90fba21075ab2c7ecb0cb75f14ca07f090829e Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Dan Yasny dyasny@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

Jenkins CI RO

8:56 p.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1784/ (2/2)

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

Jenkins CI RO

8:58 p.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Patch Set 2:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1784/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1835/ : SUCCESS

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

wudxw＠linux.vnet.ibm.com

11:17 p.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

Mark Wu has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

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

(2 inline comments)

.................................................... File vdsm/libvirtvm.py Line 2964: ifsxml = _domParseStr(self._lastXMLDesc).childNodes[0]. \ Line 2965: getElementsByTagName('devices')[0]. \ Line 2966: getElementsByTagName('interface') Line 2967: for x in ifsxml: Line 2968: devType = x.getAttribute('type') I suggest to determine whether to fetch info about target dev and model according to the devType. If it's 'hostdev', then we could skip it. Line 2969: try: Line 2970: name = x.getElementsByTagName('target')[0].getAttribute('dev') Line 2971: except IndexError: Line 2972: name = ''

Line 2970: name = x.getElementsByTagName('target')[0].getAttribute('dev') Line 2971: except IndexError: Line 2972: name = '' Line 2973: try: Line 2974: alias = x.getElementsByTagName('alias')[0].getAttribute('name') The 'alias' entry should be also available for SRIOV device. Something like: <alias name='hostdev0'/> Line 2975: except IndexError: Line 2976: alias = '' Line 2977: try: Line 2978: model = x.getElementsByTagName('model')[0].getAttribute('type')

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

wudxw＠linux.vnet.ibm.com

11:33 p.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

Mark Wu has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Patch Set 2: No score

BTW, have we considered to support SRIOV by using vhost-net+macvtap over it instead of a hook for it? It could give good performance, no limitation on live migration and guest memory swapping.

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

Dan Kenigsberg

3 Apr 3 Apr

7:37 a.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

Dan Kenigsberg has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

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

(2 inline comments)

Mark, you have good comments. Please use your -1 powers to make them more visible.

.................................................... File vdsm/libvirtvm.py Line 2970: name = x.getElementsByTagName('target')[0].getAttribute('dev') Line 2971: except IndexError: Line 2972: name = '' Line 2973: try: Line 2974: alias = x.getElementsByTagName('alias')[0].getAttribute('name') yes, "alias" is my favorite key into the device list. Let's not ignore it if possible. Line 2975: except IndexError: Line 2976: alias = '' Line 2977: try: Line 2978: model = x.getElementsByTagName('model')[0].getAttribute('type')

Line 2999: # Get nic address Line 3000: address = self._getUnderlyingDeviceAddress(x) Line 3001: for nic in self._devices[vm.NIC_DEVICES]: Line 3002: if nic.macAddr.lower() == mac.lower(): Line 3003: nic.name = name I have a feeling that it would be better not to set the nic name, over putting a false empty string here. Remember that the "name" is used as a key into nic statistics dictionary. Who knows what would happen if you have two sriov cards per VM. Line 3004: nic.alias = alias Line 3005: nic.address = address Line 3006: nic.linkActive = linkActive Line 3007: # Update vm's conf with address for known nic devices

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

Jenkins CI RO

6:19 p.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Patch Set 3:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1847/ (1/2)

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

Gerrit-MessageType: comment Gerrit-Change-Id: I5a90fba21075ab2c7ecb0cb75f14ca07f090829e Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Dan Yasny dyasny@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server

Jenkins CI RO

6:19 p.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Patch Set 3:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1796/ (2/2)

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

asegurap＠redhat.com

6:19 p.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

Antoni Segura Puimedon has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Patch Set 2: (2 inline comments)

.................................................... File vdsm/libvirtvm.py Line 2964: ifsxml = _domParseStr(self._lastXMLDesc).childNodes[0]. \ Line 2965: getElementsByTagName('devices')[0]. \ Line 2966: getElementsByTagName('interface') Line 2967: for x in ifsxml: Line 2968: devType = x.getAttribute('type') Good idea. Line 2969: try: Line 2970: name = x.getElementsByTagName('target')[0].getAttribute('dev') Line 2971: except IndexError: Line 2972: name = ''

Line 2970: name = x.getElementsByTagName('target')[0].getAttribute('dev') Line 2971: except IndexError: Line 2972: name = '' Line 2973: try: Line 2974: alias = x.getElementsByTagName('alias')[0].getAttribute('name') Totally right. I have been doing some tests with libvirt and indeed what we get is: <interface type='hostdev' managed='yes'> <mac address='52:54:00:47:ed:f0'/> <source> <address type='pci' domain='0x0000' bus='0x02' slot='0x10' function='0x2'/> </source> <alias name='hostdev0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </interface> Line 2975: except IndexError: Line 2976: alias = '' Line 2977: try: Line 2978: model = x.getElementsByTagName('model')[0].getAttribute('type')

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

asegurap＠redhat.com

6:21 p.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

Antoni Segura Puimedon has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Patch Set 3: (1 inline comment)

And thanks everyone for the reviews.

.................................................... File vdsm/libvirtvm.py Line 2968: devType = x.getAttribute('type') Line 2969: mac = x.getElementsByTagName('mac')[0].getAttribute('address') Line 2970: alias = x.getElementsByTagName('alias')[0].getAttribute('name') Line 2971: if devType == 'hostdev': Line 2972: name = alias I thought that for statistical purposes it might be better to have a nic.name be like the alias for the passed through devices, but I'm open to change it if somebody has strong objections. Line 2973: model = 'passthrough' Line 2974: else: Line 2975: name = x.getElementsByTagName('target')[0].getAttribute('dev') Line 2976: model = x.getElementsByTagName('model')[0].getAttribute('type')

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

Jenkins CI RO

6:21 p.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Patch Set 3:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1796/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1847/ : SUCCESS

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

Dan Kenigsberg

11:24 p.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

Dan Kenigsberg has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

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

Looks lovely - but now I recall that there has been an issue about extracting statistics from a device that was granted to a guest... Please verify, we may need to turn off stats polling on hostdevs.

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

wudxw＠linux.vnet.ibm.com

4 Apr 4 Apr

12:45 a.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

Mark Wu has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

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

(1 inline comment)

I am also fine if you would like to fix the statistics problem in another patch.

.................................................... File vdsm/libvirtvm.py Line 2968: devType = x.getAttribute('type') Line 2969: mac = x.getElementsByTagName('mac')[0].getAttribute('address') Line 2970: alias = x.getElementsByTagName('alias')[0].getAttribute('name') Line 2971: if devType == 'hostdev': Line 2972: name = alias virDomainInterfaceStats uses the interface name (like vnet0, vnet1, etc.) to find the interface. 'alias' doesn't work. And the more important thing is that libvirt can't get the statistics of the interface which has been detached from host via parsing '/proc/net/dev'. It's the way to get statistics for tap interface. I think it's ok to use alias for its name, but you need filter out the passthrough interface in the net sampling code as Dan concerned about. Line 2973: model = 'passthrough' Line 2974: else: Line 2975: name = x.getElementsByTagName('target')[0].getAttribute('dev') Line 2976: model = x.getElementsByTagName('model')[0].getAttribute('type')

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

Jenkins CI RO

4:22 a.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Patch Set 4:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1799/ (1/2)

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

Gerrit-MessageType: comment Gerrit-Change-Id: I5a90fba21075ab2c7ecb0cb75f14ca07f090829e Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Dan Yasny dyasny@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server

Jenkins CI RO

4:22 a.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Patch Set 4:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1850/ (2/2)

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

asegurap＠redhat.com

4:24 a.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

Antoni Segura Puimedon has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Patch Set 4:

Totally right Mark! Thanks a lot for reporting this issue.

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

Jenkins CI RO

4:27 a.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Patch Set 4:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1799/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1850/ : SUCCESS

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

Dan Kenigsberg

8:56 a.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

Dan Kenigsberg has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Patch Set 4: Looks good to me, approved

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

asegurap＠redhat.com

1 p.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

Antoni Segura Puimedon has posted comments on this change.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Patch Set 4: Verified

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

Dan Kenigsberg

4:52 p.m.

New subject: Change in vdsm[master]: Fix crash on vm start with netdevs w/out some info

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Fix crash on vm start with netdevs w/out some info ......................................................................

Fix crash on vm start with netdevs w/out some info

Some devices, like sr-iov do not have information in the domxml to represent name and model.

This patch fixes the issue by detecting when we are dealing with a passed through device and setting special values for name and model.

Change-Id: I5a90fba21075ab2c7ecb0cb75f14ca07f090829e Signed-off-by: Antoni S. Puimedon asegurap@redhat.com --- M vdsm/libvirtvm.py 1 file changed, 8 insertions(+), 2 deletions(-)

Approvals: Antoni Segura Puimedon: Verified Dan Kenigsberg: Looks good to me, approved

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

Gerrit-MessageType: merged Gerrit-Change-Id: I5a90fba21075ab2c7ecb0cb75f14ca07f090829e Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Dan Yasny dyasny@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server

4903

Age (days ago)

4904

Last active (days ago)

vdsm-patches@lists.fedorahosted.org

23 comments

4 participants

tags (0)

participants (4)

asegurap＠redhat.com
Dan Kenigsberg
Jenkins CI RO
wudxw＠linux.vnet.ibm.com