Change in vdsm[master]: hostdev: add support for hotplug

mpolednik at redhat.com mpolednik at redhat.com
Thu Jul 16 12:13:33 UTC 2015


Martin Polednik has posted comments on this change.

Change subject: hostdev: add support for hotplug
......................................................................


Patch Set 3:

(3 comments)

https://gerrit.ovirt.org/#/c/42661/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2026:             return errCode['migInProgress']
Line 2027: 
Line 2028:         dev_specs = params['devices']
Line 2029:         dev_objects = []
Line 2030:         for dev_spec in dev_specs:
> maybe wrap the body into a try/except block?
this is what we have to figure out - imho we should go ahead and report back the vmList to engine, so if there were any failures the engine can determine what to do with them (because you might be left with detached device <group> that wouldn't be used)
Line 2031:             dev_objects.append(vmdevices.hostdevice.HostDevice(
Line 2032:                 self.conf, self.log, **dev_spec
Line 2033:             ))
Line 2034:             dev_object = dev_objects[-1]


Line 2031:             dev_objects.append(vmdevices.hostdevice.HostDevice(
Line 2032:                 self.conf, self.log, **dev_spec
Line 2033:             ))
Line 2034:             dev_object = dev_objects[-1]
Line 2035:             dev_object.detach()
> why not simply
leftover from previous code, thanks! the dev_objects aren't even needed in this version
Line 2036: 
Line 2037:             try:
Line 2038:                 dev_xml = dev_object.getXML().toprettyxml(encoding='utf-8')
Line 2039:             except vmdevices.core.SkipDevice:


Line 2049:                 self._dom.attachDevice(dev_xml)
Line 2050:             except libvirt.libvirtError as e:
Line 2051:                 self.log.exception("Hotplug failed")
Line 2052:                 if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
Line 2053:                     return errCode['noVM']
> while you're at it, you can use response.error() also here :)
will look into it (and below)
Line 2054:                 return response.error('hotplugHostdev', e.message)
Line 2055:             else:
Line 2056:                 # FIXME!  We may have a problem here if vdsm dies right after
Line 2057:                 # we sent command to libvirt and before save conf. In this case


-- 
To view, visit https://gerrit.ovirt.org/42661
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2465360664ef9b659c52dc610a95d2c2f1555c54
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list