Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[danken]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: hostdev: expose parameters needed to support hotunplug
..


hostdev: expose parameters needed to support hotunplug

This patch prepares HostDevice for hostdevHotunplug verb. We need an
xpath to uniquely identify the device in libvirt XML to properly wait
for device's unplug, and 'device' attribute (the libvirt name) to have
unique identifier we can return.

Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1234283
Signed-off-by: Martin Polednik 
Reviewed-on: https://gerrit.ovirt.org/54939
Reviewed-by: Francesco Romani 
Continuous-Integration: Jenkins CI
---
M vdsm/virt/vmdevices/hostdevice.py
1 file changed, 35 insertions(+), 1 deletion(-)

Approvals:
  Jenkins CI: Passed CI tests
  Francesco Romani: Looks good to me, approved
  Martin Polednik: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 27
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[mpolednik]

Martin Polednik has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 26: Verified+1

tested on real VM incl. timeouts w/ followup patch /pci & usb, scsi needs 
additional work/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 26
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[fromani]

Francesco Romani has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 26: Code-Review+2

thanks!

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 26
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 26:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 26
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 25:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 25
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[fromani]

Francesco Romani has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 24:

To make my comfortable and happier, please just rewrite it in more verbose and 
less compact war ("Sparse is better than dense"!).

Bonus points if you could avoid using nested functions. No big deal, however.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[fromani]

Francesco Romani has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 24: Code-Review-1

(1 comment)

I'm fine with the idea, but the implementation is too terse and hard to read.

https://gerrit.ovirt.org/#/c/54939/24/vdsm/virt/vmdevices/hostdevice.py
File vdsm/virt/vmdevices/hostdevice.py:

PS24, Line 64: address = ''.join([address_format_string.format(
 : key=key, value=int(value), **_padding(key)) for
 : key, value in self.hostAddress.items()])
nope, this is still too clever. Please try to make it more explicit and more 
readable.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 24:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 23:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 22:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 21:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 20:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[mpolednik]

Martin Polednik has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 19: Verified+1

Tested whole hostdev + hotplug/hotunplug flow VM flow.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[mpolednik]

Martin Polednik has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 19:

(1 comment)

The trend of _xpath being tested.

https://gerrit.ovirt.org/#/c/54939/19/vdsm/virt/vmdevices/hostdevice.py
File vdsm/virt/vmdevices/hostdevice.py:

Line 59: return {'base': '0x', 'padding': ''}
Line 60: return {'base': '0x', 'padding': '02'}
Line 61: return {'base': '', 'padding': ''}
Line 62: 
Line 63: address_format_string = '[@{key}=\'{base}{value:{padding}}\']'
> Why not using double quotes for the string to avoid the need of backslashes
Personal preference.
Line 64: address = ''.join([address_format_string.format(
Line 65: key=key, value=int(value), **_padding(key)) for
Line 66: key, value in self.hostAddress.items()])
Line 67: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[mzamazal]

Milan Zamazal has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 19: Code-Review+1

(1 comment)

Better now.

> I don't feel like adding xpath test in this patch. It is good idea for future 
> patch, with other devices following the trend.

The trend of adding tests or the trend of postponing addition tests? ;-)

https://gerrit.ovirt.org/#/c/54939/19/vdsm/virt/vmdevices/hostdevice.py
File vdsm/virt/vmdevices/hostdevice.py:

Line 59: return {'base': '0x', 'padding': ''}
Line 60: return {'base': '0x', 'padding': '02'}
Line 61: return {'base': '', 'padding': ''}
Line 62: 
Line 63: address_format_string = '[@{key}=\'{base}{value:{padding}}\']'
Why not using double quotes for the string to avoid the need of backslashes?
Line 64: address = ''.join([address_format_string.format(
Line 65: key=key, value=int(value), **_padding(key)) for
Line 66: key, value in self.hostAddress.items()])
Line 67: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 19:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[mpolednik]

Martin Polednik has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 18:

I don't feel like adding xpath test in this patch. It is good idea for future 
patch, with other devices following the trend.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[mpolednik]

Martin Polednik has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 17:

(3 comments)

https://gerrit.ovirt.org/#/c/54939/17/vdsm/virt/vmdevices/hostdevice.py
File vdsm/virt/vmdevices/hostdevice.py:

Line 48: logging.debug('Detaching device %s from the host.' % 
self.device)
Line 49: self._deviceParams = detach_detachable(self.device)
Line 50: 
Line 51: @property
Line 52: def _xpath(self):
> Why @property?
It is consistent with other devices.

$ git --no-pager grep -B1 "def _xpath"
vdsm/virt/vmdevices/hostdevice.py-@property
vdsm/virt/vmdevices/hostdevice.py:def _xpath(self):
--
vdsm/virt/vmdevices/network.py-@property
vdsm/virt/vmdevices/network.py:def _xpath(self):
--
vdsm/virt/vmdevices/storage.py-@property
vdsm/virt/vmdevices/storage.py:def _xpath(self):
Line 53: """
Line 54: Returns xpath to the device in libvirt dom xml
Line 55: The path is relative to the root element
Line 56: """


Line 52: def _xpath(self):
Line 53: """
Line 54: Returns xpath to the device in libvirt dom xml
Line 55: The path is relative to the root element
Line 56: """
> Missing dots at the ends of the sentences.
Will fix in some iteration.
Line 57: def _padding(key):
Line 58: if CAPABILITY_TO_XML_ATTR[
Line 59: self._deviceParams['capability']] == 'pci':
Line 60: if key == 'domain':


Line 66: 
Line 67: return ('./devices/hostdev/source/address{}'.format(
Line 68: ''.join(['[@{key}=\'{base}{value:{padding}}\']'.format(
Line 69: key=key, value=int(value), **_padding(key)) for
Line 70: key, value in self.hostAddress.items()])))
> Am I the only one who finds this unreadable?  How about something like
You are not, it is too terse. Will fix in one of the future iterations.
Line 71: 
Line 72: def is_attached_to(self, xml_string):
Line 73: dom = ET.fromstring(xml_string)
Line 74: return bool(dom.findall(self._xpath))


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[mzamazal]

Milan Zamazal has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 18: Code-Review-1

Some cosmetic issues. And how about adding a simple test to check we build the 
xpath expression correctly?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[mzamazal]

Milan Zamazal has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 17:

(3 comments)

https://gerrit.ovirt.org/#/c/54939/17/vdsm/virt/vmdevices/hostdevice.py
File vdsm/virt/vmdevices/hostdevice.py:

Line 48: logging.debug('Detaching device %s from the host.' % 
self.device)
Line 49: self._deviceParams = detach_detachable(self.device)
Line 50: 
Line 51: @property
Line 52: def _xpath(self):
Why @property?
Line 53: """
Line 54: Returns xpath to the device in libvirt dom xml
Line 55: The path is relative to the root element
Line 56: """


Line 52: def _xpath(self):
Line 53: """
Line 54: Returns xpath to the device in libvirt dom xml
Line 55: The path is relative to the root element
Line 56: """
Missing dots at the ends of the sentences.
Line 57: def _padding(key):
Line 58: if CAPABILITY_TO_XML_ATTR[
Line 59: self._deviceParams['capability']] == 'pci':
Line 60: if key == 'domain':


Line 66: 
Line 67: return ('./devices/hostdev/source/address{}'.format(
Line 68: ''.join(['[@{key}=\'{base}{value:{padding}}\']'.format(
Line 69: key=key, value=int(value), **_padding(key)) for
Line 70: key, value in self.hostAddress.items()])))
Am I the only one who finds this unreadable?  How about something like

  attributes = ["[@{key}='{base}{value:{padding}}']"
.format(key=key, value=int(value), **_padding(key))
for key, value in self.hostAddress.items()]
  return './devices/hostdev/source/address' + ''.join(attributes)
Line 71: 
Line 72: def is_attached_to(self, xml_string):
Line 73: dom = ET.fromstring(xml_string)
Line 74: return bool(dom.findall(self._xpath))


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 18:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 17:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 16:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 15:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 14:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 13:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 12:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-3.5 ovirt-3.4 
ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[fromani]

Francesco Romani has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 11:

and tests (either new or document where is already exercised) are a nice plus :)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[fromani]

Francesco Romani has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 11: Code-Review+1

(1 comment)

fine, but too hard to read, hence partial ACK.
please make easier to read to get full ACK.

https://gerrit.ovirt.org/#/c/54939/11/vdsm/virt/vmdevices/hostdevice.py
File vdsm/virt/vmdevices/hostdevice.py:

PS11, Line 65: return ('./devices/hostdev/source/address{}'.format(
 : 
''.join(['[@{key}=\'{base}{value:{padding}}\']'.format(
 : key=key, value=int(value), **_padding(key)) for
 : key, value in self.hostAddress.items()])))
this is too packed, please decompress this (clever) line in something easier to 
digest


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[mpolednik]

Martin Polednik has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 11: Verified+1

Tested at real vm with GPU passthrough.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 11:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-3.5 ovirt-3.4 
ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 10:

* #1234283::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1234283::OK, public bug
* Check Product::#1234283::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-3.5 ovirt-3.4 
ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 9:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 8:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 7:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 6:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 5:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 4:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[fromani]

Francesco Romani has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 2: Code-Review+1

(1 comment)

partial review. Looks OK, albeit a bit hard to parse. Partial ACK.
Please check the inline comments and let's try to make it easier to read.

https://gerrit.ovirt.org/#/c/54939/2/vdsm/virt/vmdevices/hostdevice.py
File vdsm/virt/vmdevices/hostdevice.py:

Line 65: return ('./devices/hostdev/source/address{}'.format(
   : ''.join(['[@{key}=\'{base}{value:{padding}}\']'.format(
   : key=key, value=int(value), **_padding(key)) for
   : key, value in self.hostAddress.items()])))
this is a bit too terse. Could you please unzip it to make easier to parse for 
humans?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[automation]

gerrit-hooks has posted comments on this change.

Change subject: hostdev: expose parameters needed to support hotunplug
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hostdev: expose parameters needed to support hotunplug

[mpolednik]

Martin Polednik has uploaded a new change for review.

Change subject: hostdev: expose parameters needed to support hotunplug
..

hostdev: expose parameters needed to support hotunplug

Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Signed-off-by: Martin Polednik 
---
M vdsm/virt/vmdevices/hostdevice.py
1 file changed, 30 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/39/54939/1

diff --git a/vdsm/virt/vmdevices/hostdevice.py 
b/vdsm/virt/vmdevices/hostdevice.py
index 37aff3d..3a5a719 100644
--- a/vdsm/virt/vmdevices/hostdevice.py
+++ b/vdsm/virt/vmdevices/hostdevice.py
@@ -18,6 +18,8 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
+import xml.etree.ElementTree as ET
+
 from vdsm import utils
 from vdsm.hostdev import get_device_params, detach_detachable, \
 CAPABILITY_TO_XML_ATTR
@@ -26,13 +28,15 @@
 
 
 class HostDevice(core.Base):
-__slots__ = ('address', 'hostAddress', 'bootOrder', '_deviceParams')
+__slots__ = ('address', 'hostAddress', 'bootOrder', '_deviceParams',
+ 'name')
 
 def __init__(self, conf, log, **kwargs):
 super(HostDevice, self).__init__(conf, log, **kwargs)
 
 self._deviceParams = get_device_params(self.device)
 self.hostAddress = self._deviceParams.get('address')
+self.name = self.device
 
 def detach(self):
 """
@@ -41,6 +45,31 @@
 """
 self._deviceParams = detach_detachable(self.device)
 
+@property
+def _xpath(self):
+"""
+Returns xpath to the device in libvirt dom xml
+The path is relative to the root element
+"""
+def _padding(key):
+if CAPABILITY_TO_XML_ATTR[
+self._deviceParams['capability']] == 'pci':
+if key == 'domain':
+return {'base': '0x', 'padding': '04'}
+if key == 'function':
+return {'base': '0x', 'padding': ''}
+return {'base': '0x', 'padding': '02'}
+return {'base': '', 'padding': ''}
+
+return ('./devices/hostdev/source/address{}'.format(
+''.join(['[@{key}=\'{base}{value:{padding}}\']'.format(
+key=key, value=int(value), **_padding(key)) for
+key, value in self.hostAddress.items()])))
+
+def is_attached_to(self, xml_string):
+dom = ET.fromstring(xml_string)
+return bool(dom.findall(self._xpath))
+
 def getXML(self):
 """
 Create domxml for a host device.


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I09e879051b1d47e48e9ae73c1f7d9bfbea8f1237
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches