Change in vdsm[master]: migrations: change convergence schedule from time to iterations
Michal Skrivanek has posted comments on this change. Change subject: migrations: change convergence schedule from time to iterations .. Patch Set 3: we'll play with the verfication setup. I think I have an idea how to demonstrate the new behavior with a bit of artificial assumptions. It should be visible with a peak memory intensive operations. old way it will migrate sooner but with significant downtime. new way will try for few more seconds and after the peak finishes will migrate with smaller downtime -- To view, visit https://gerrit.ovirt.org/56558 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6f87c954031842c35c99888c228a34ec7f19d800 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Jelinek Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Set timeout on boot menu
Michal Skrivanek has posted comments on this change. Change subject: virt: Set timeout on boot menu .. Patch Set 3: no one explicitly asked for anything. Hence it's waste of time and yet another patch on engine for no good reason. Why over complicate things? It's not helping anyone -- To view, visit https://gerrit.ovirt.org/56393 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0f3501e8500e366e785f5a8ddfdf78fd34c997a2 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer 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]: virt: Set timeout on boot menu
Michal Skrivanek has posted comments on this change.
Change subject: virt: Set timeout on boot menu
..
Patch Set 3:
(1 comment)
https://gerrit.ovirt.org/#/c/56393/3/lib/vdsm/config.py.in
File lib/vdsm/config.py.in:
Line 240:
Line 241: ('boot_menu_timeout', '10',
Line 242: 'Boot menu timeout in seconds. '
Line 243: 'Minimum value is 0, maximum value is 65; if a different
value '
Line 244: 'is given then it is adjusted to this range.'),
> vdsm conf is not junkyard, it should contain only stuff needed on a specifi
I don't see a point spending more time on this effort.
vdsm.conf is and always was a junkyard of things not worth any API.
Line 245: ]),
Line 246:
Line 247: # Section: [rpc]
Line 248: ('rpc', [
--
To view, visit https://gerrit.ovirt.org/56393
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f3501e8500e366e785f5a8ddfdf78fd34c997a2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
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]: virt: Set timeout on boot menu
Michal Skrivanek has posted comments on this change.
Change subject: virt: Set timeout on boot menu
..
Patch Set 3: Code-Review+1
(2 comments)
https://gerrit.ovirt.org/#/c/56393/3/lib/vdsm/config.py.in
File lib/vdsm/config.py.in:
Line 240:
Line 241: ('boot_menu_timeout', '10',
Line 242: 'Boot menu timeout in seconds. '
Line 243: 'Minimum value is 0, maximum value is 65; if a different
value '
Line 244: 'is given then it is adjusted to this range.'),
> Well, increased boot menu timeout is a feature that's not strictly needed,
vdsm.conf is enough. No need to bother with UI, and still better than hardcoding
Line 245: ]),
Line 246:
Line 247: # Section: [rpc]
Line 248: ('rpc', [
https://gerrit.ovirt.org/#/c/56393/3/vdsm/virt/vmxml.py
File vdsm/virt/vmxml.py:
Line 302: timeout = config.getint('vars', 'boot_menu_timeout') *
1000
Line 303: if timeout < 0:
Line 304: timeout = 0
Line 305: elif timeout > 65535:
Line 306: timeout = 65535
> If the configuration is invalid, we should log a warning about it or fail.
or..just skip the 303-306; if you provide wrong value the wrong behavior you
get. libvirt should protect us enough, eg. failing to run if invalid value is
supplied
Line 307: oselem.appendChildWithArgs('bootmenu', enable='yes',
Line 308:timeout=unicode(timeout))
Line 309:
Line 310: if use_serial_console and cpuarch.is_x86(self.arch):
--
To view, visit https://gerrit.ovirt.org/56393
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f3501e8500e366e785f5a8ddfdf78fd34c997a2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
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]: virt: vm: Update time on VM after resume
Michal Skrivanek has posted comments on this change.
Change subject: virt: vm: Update time on VM after resume
..
Patch Set 7: Code-Review-1
(1 comment)
I don't want to get stuck for 5s when agent is not there. Any solution for that?
https://gerrit.ovirt.org/#/c/48860/7/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1204: t = time.time()
Line 1205: seconds = int(t)
Line 1206: nseconds = int((t - seconds) * 10**9)
Line 1207: try:
Line 1208: self._dom.setTime(time={'seconds': seconds, 'nseconds':
nseconds})
> We will block here for 5 seconds when qemu-guest-agent is not installed, we
Thanks for confirmation, that's what I was afraid of
Line 1209: except libvirt.libvirtError as e:
Line 1210: log_method = self.log.debug
Line 1211: code = e.get_error_code()
Line 1212: if code == libvirt.VIR_ERR_AGENT_UNRESPONSIVE:
--
To view, visit https://gerrit.ovirt.org/48860
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb583cd5d21e56d7730b0ba21d75ed93b9d34025
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
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]: virt: vm: Update time on VM after resume
Michal Skrivanek has posted comments on this change.
Change subject: virt: vm: Update time on VM after resume
..
Patch Set 7:
(1 comment)
https://gerrit.ovirt.org/#/c/48860/7/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1204: t = time.time()
Line 1205: seconds = int(t)
Line 1206: nseconds = int((t - seconds) * 10**9)
Line 1207: try:
Line 1208: self._dom.setTime(time={'seconds': seconds, 'nseconds':
nseconds})
are we sure this never blocks or delays?
We had bad experience with qemu-ga timeouts when proxied through libvirt
Line 1209: except libvirt.libvirtError as e:
Line 1210: log_method = self.log.debug
Line 1211: code = e.get_error_code()
Line 1212: if code == libvirt.VIR_ERR_AGENT_UNRESPONSIVE:
--
To view, visit https://gerrit.ovirt.org/48860
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb583cd5d21e56d7730b0ba21d75ed93b9d34025
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
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]: vm: Support for non-ascii vm name
Michal Skrivanek has posted comments on this change. Change subject: vm: Support for non-ascii vm name .. Patch Set 3: I guess it also depends on the fate of bug 1285720 (systemd fix) -- To view, visit https://gerrit.ovirt.org/48570 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2386c0f98aeb8161feaf19c2862be229f73eb0df Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Oved Ourfali Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shmuel Leib Melamud 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]: vm: Support for non-ascii vm name
Michal Skrivanek has posted comments on this change.
Change subject: vm: Support for non-ascii vm name
..
Patch Set 3:
(1 comment)
https://gerrit.ovirt.org/#/c/48570/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 384: if isinstance(name, unicode):
Line 385: # Libvirt does not support non-ascii vm name
Line 386: # See https://bugzilla.redhat.com/1260131
Line 387: try:
Line 388: name = name.encode('ascii')
don't we also need to make it unique?
Line 389: except UnicodeEncodeError:
Line 390: self.log.debug("Non-ASCII vm name '%s'",
name.encode('utf8'))
Line 391: name = None
Line 392: if name is None:
--
To view, visit https://gerrit.ovirt.org/48570
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2386c0f98aeb8161feaf19c2862be229f73eb0df
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Oved Ourfali
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Shmuel Leib Melamud
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]: vm: Support for non-ascii vm name
Michal Skrivanek has posted comments on this change.
Change subject: vm: Support for non-ascii vm name
..
Patch Set 3:
(1 comment)
I guess we can also drop the occurrences of self.name.encode('utf-8')
https://gerrit.ovirt.org/#/c/48570/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 388: name = name.encode('ascii')
Line 389: except UnicodeEncodeError:
Line 390: self.log.debug("Non-ASCII vm name '%s'",
name.encode('utf8'))
Line 391: name = None
Line 392: if name is None:
vmName is mandatory since a long time, since we already dropped the oldest 3.0
engines...we don't have to carry on with this case
Line 393: # Missing or non-ascii name
Line 394: name = 'n' + params['vmId']
Line 395: self.log.debug("Using generated vm name %r", name)
Line 396: return name
--
To view, visit https://gerrit.ovirt.org/48570
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2386c0f98aeb8161feaf19c2862be229f73eb0df
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Oved Ourfali
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Shmuel Leib Melamud
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]: vdsm: introduce cpuinfo module
Michal Skrivanek has posted comments on this change. Change subject: vdsm: introduce cpuinfo module .. Patch Set 13: are the Arch naming changes aligned with https://gerrit.ovirt.org/#/c/49299/ ? -- To view, visit https://gerrit.ovirt.org/46912 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaa702b05f3825ebdcfed16d86d39a8c38fcf224c Gerrit-PatchSet: 13 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 Betak Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim 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]: vm: snapshot - enabling memory snapshot for diskless VM
Michal Skrivanek has posted comments on this change. Change subject: vm: snapshot - enabling memory snapshot for diskless VM .. Patch Set 1: (1 comment) however, I think we do have a problem conceptually with RAM snapshots and LUN/cinder. We can't ensure consistent state unless we snapshot cinder/LUN as well (or make sure they are not there at all) I wonder if we should block that or warn at least https://gerrit.ovirt.org/#/c/49535/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 3137 Line 3138 Line 3139 Line 3140 Line 3141 > Until now, memory snapshot without disk did nothing. This looks like a bug, I think disk-less mem snapshots are merely a bug. It does deserve a fix, though it's not a high priority -- To view, visit https://gerrit.ovirt.org/49535 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia540fa9009f627f7a2943ef393084eee3f047bf5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Daniel Erez Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer 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]: lib: daemon: autodetect online cpus for affinity
Michal Skrivanek has posted comments on this change. Change subject: lib: daemon: autodetect online cpus for affinity .. Patch Set 8: I think we had the good deal of nitpicking comments..looks perfect to me -- To view, visit https://gerrit.ovirt.org/49402 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba834f67a43ce766308036cbd079107340ed69d8 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: Yaniv Bronhaim 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]: lib: daemon: autodetect online cpus for affinity
Michal Skrivanek has posted comments on this change. Change subject: lib: daemon: autodetect online cpus for affinity .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/49402 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba834f67a43ce766308036cbd079107340ed69d8 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: lib: daemon: autodetect online cpus for affinity
Michal Skrivanek has posted comments on this change.
Change subject: lib: daemon: autodetect online cpus for affinity
..
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/49402/1/vdsm/vdsm
File vdsm/vdsm:
Line 225: online_cpus = set(taskset.online_cpus())
Line 226:
Line 227: if cpu_affinity.lower() == taskset.AUTOMATIC:
Line 228: # pick any one online cpu -- see help(set().pop)
Line 229: cpu_set = frozenset((online_cpus.pop(),))
I think we still want to avoid #0 if possible?
How about getting the second from the start? I kind of like the predictability
in that case
Line 230: else:
Line 231: cpu_set = frozenset(int(cpu.strip())
Line 232: for cpu in cpu_affinity.split(","))
Line 233:
--
To view, visit https://gerrit.ovirt.org/49402
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iba834f67a43ce766308036cbd079107340ed69d8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Yaniv Bronhaim
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[ovirt-3.6]: vm: fix misleading XML log
Michal Skrivanek has posted comments on this change. Change subject: vm: fix misleading XML log .. Patch Set 3: I suppose my comments are more appropriate to the master version, not .6 backport. sorry -- To view, visit https://gerrit.ovirt.org/49139 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I50f5b8a51830c8ce2a098e4ac11dd1bd6a89aac3 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.6 Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek 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[ovirt-3.6]: vm: fix misleading XML log
Michal Skrivanek has posted comments on this change.
Change subject: vm: fix misleading XML log
..
Patch Set 3:
(3 comments)
https://gerrit.ovirt.org/#/c/49139/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1847: # on this flow. Issues:
Line 1848: # - we will also call the more specific
before_vm_dehibernate
Line 1849: # - we feed the hook with wrong XML
Line 1850: # - we ignore the output of the hook
Line 1851: hooks.before_vm_start(self._buildDomainXML(), self.conf)
there's a good chance this is a mistake or a remnant of times when we didn't
have before_vm_dehibernate.
Can we try to find out and eliminate if no longer relevant?
Line 1852:
Line 1853: fromSnapshot = self.conf.get('restoreFromSnapshot',
False)
Line 1854: srcDomXML = self.conf.pop('_srcDomXML')
Line 1855: if fromSnapshot:
Line 1857: srcDomXML =
self._correctGraphicsConfiguration(srcDomXML)
Line 1858: hooks.before_vm_dehibernate(srcDomXML, self.conf,
Line 1859: {'FROM_SNAPSHOT':
str(fromSnapshot)})
Line 1860:
Line 1861: # TODO: this is debug information. For 3.6.x we still
need to
and I think we will need it for foreseeable future:) We do want to show that in
UI, but for troubleshooting startup issues this is a very helpful log
Line 1862: # see the XML even with 'info' as default level.
Line 1863: self.log.info(srcDomXML)
Line 1864:
Line 1865: fname =
self.cif.prepareVolumePath(self.conf['restoreState'])
Line 1877: else:
Line 1878: domxml = hooks.before_vm_start(self._buildDomainXML(),
self.conf)
Line 1879: # TODO: this is debug information. For 3.6.x we still
need to
Line 1880: # see the XML even with 'info' as default level.
Line 1881: self.log.info(domxml)
it would be nice to have a note/flag next to the dump whether any hook modified
the generated xml or not. It may not be obvious whether the hook did anything
or not
Line 1882:
Line 1883: flags = libvirt.VIR_DOMAIN_NONE
Line 1884: with self._confLock:
Line 1885: if 'launchPaused' in self.conf:
--
To view, visit https://gerrit.ovirt.org/49139
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I50f5b8a51830c8ce2a098e4ac11dd1bd6a89aac3
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Michal Skrivanek
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[ovirt-3.6]: clientIF: add logs during the recovery
Michal Skrivanek has posted comments on this change. Change subject: clientIF: add logs during the recovery .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/49140 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I31dddf0a2bc760c5ad383ff6bfee9a72adc87c4f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.6 Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek 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]: image: copy - set VM metadata images format to RAW
Michal Skrivanek has posted comments on this change. Change subject: image: copy - set VM metadata images format to RAW .. Patch Set 6: (1 comment) https://gerrit.ovirt.org/#/c/49002/6/vdsm/storage/image.py File vdsm/storage/image.py: Line 67: Line 68: RENAME_RANDOM_STRING_LEN = 8 Line 69: Line 70: # Size in blocks of the conf file generated during RAM snapshot operation Line 71: VM_CONF_SIZE_BLK = 20 > The size is determined by the engine. I.e. the engine creates an image in s we don't allow to create such small images in any way, so this should be fairly safe Line 72: Line 73: # Temporary size of a volume when we optimize out the prezeroing Line 74: TEMPORARY_VOLUME_SIZE = 20480 # in sectors (10M) Line 75: -- To view, visit https://gerrit.ovirt.org/49002 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icaad6599260a2631f580e17404d25554a19ec670 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Daniel Erez Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer 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]: image: copy - set VM metadata images format to RAW
Michal Skrivanek has posted comments on this change. Change subject: image: copy - set VM metadata images format to RAW .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/49002/3/vdsm/storage/image.py File vdsm/storage/image.py: Line 67: Line 68: RENAME_RANDOM_STRING_LEN = 8 Line 69: Line 70: # Size in blocks of the conf file generated during RAM snapshot operation Line 71: VM_CONF_SIZE = 20 why is it 20? is this the minimum we allocate everytime? Line 72: Line 73: # Temporary size of a volume when we optimize out the prezeroing Line 74: TEMPORARY_VOLUME_SIZE = 20480 # in sectors (10M) Line 75: -- To view, visit https://gerrit.ovirt.org/49002 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icaad6599260a2631f580e17404d25554a19ec670 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Daniel Erez Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer 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[ovirt-3.6]: daemon: ignore cpu affinity on single processor
Michal Skrivanek has posted comments on this change.
Change subject: daemon: ignore cpu affinity on single processor
..
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/48961/1/vdsm/vdsm
File vdsm/vdsm:
Line 272: log.info('VDSM will run with cpu affinity: %s', cpu_set)
Line 273:
Line 274: # too early to use the facilities from caps.py
Line 275: if os.sysconf('SC_NPROCESSORS_ONLN') == 1:
Line 276: log.warning('Only one cpu detected: affinity disabled')
ugh, i added comment to master version which is merged. just something to
consider, no deal breaker...
https://gerrit.ovirt.org/#/c/48619/7/vdsm/vdsm@233
Line 277: return
Line 278:
Line 279: taskset.set(os.getpid(), cpu_set, all_tasks=True)
Line 280:
--
To view, visit https://gerrit.ovirt.org/48961
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0652189704cbce71d20ec809c9c26f081516758a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
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]: daemon: ignore cpu affinity on single processor
Michal Skrivanek has posted comments on this change.
Change subject: daemon: ignore cpu affinity on single processor
..
Patch Set 7:
(1 comment)
https://gerrit.ovirt.org/#/c/48619/7/vdsm/vdsm
File vdsm/vdsm:
Line 229: log.info('VDSM will run with cpu affinity: %s', cpu_set)
Line 230:
Line 231: # too early to use the facilities from caps.py
Line 232: if os.sysconf('SC_NPROCESSORS_ONLN') == 1:
Line 233: log.warning('Only one cpu detected: affinity disabled')
if possible i would like to avoid a warning in harmless/expected situations.
Can we just exit early na bit above with an info?
Line 234: return
Line 235:
Line 236: taskset.set(os.getpid(), cpu_set, all_tasks=True)
Line 237:
--
To view, visit https://gerrit.ovirt.org/48619
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0652189704cbce71d20ec809c9c26f081516758a
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Petr Horáček
Gerrit-Reviewer: Piotr Kliczewski
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]: virt: memory hotplug for vm
Michal Skrivanek has posted comments on this change. Change subject: virt: memory hotplug for vm .. Patch Set 6: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/40549 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib2cdea311c3ff010b1d232abf6cc0a7d60937b1e Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Omer Frenkel Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Omer Frenkel Gerrit-Reviewer: Vitor de Lima 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]: virt: Trigger event on guest agent status changes
Michal Skrivanek has posted comments on this change. Change subject: virt: Trigger event on guest agent status changes .. Patch Set 11: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/41108 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaceca8e42720df50d5eb1e20670b6ed733db8b50 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan 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]: status: logging status values before sending event
Michal Skrivanek has posted comments on this change.
Change subject: status: logging status values before sending event
..
Patch Set 8: Code-Review+1
(1 comment)
https://gerrit.ovirt.org/#/c/41380/8/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 401: if 'exitReason' in self.conf:
Line 402: stats['exitReason'] = self.conf['exitReason']
Line 403:
Line 404: self.log.debug('Last status %s and evaluated status %s',
Line 405:self.lastStatus, vm_status)
> I was asked by mskrivanek to log both.
yeah, to actually understand what is the reported status from libvirt. Point is
to understand where we (by mistake) hide/change things incorrectly
Line 406: self._notify('VM_status', stats)
Line 407:
Line 408: def _notify(self, operation, params):
Line 409: sub_id = '|virt|%s|%s' % (operation, self.id)
--
To view, visit https://gerrit.ovirt.org/41380
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e266759ba0310dd83d6b65d9e3a63caacc0b637
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski
Gerrit-Reviewer: Dima Kuznetsov
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yeela Kaplan
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]: caps: workaround for libvirt's canonical name in machine types
Michal Skrivanek has posted comments on this change.
Change subject: caps: workaround for libvirt's canonical name in machine types
..
Patch Set 3: Code-Review+1
(1 comment)
https://gerrit.ovirt.org/#/c/42082/3/vdsm/caps.py
File vdsm/caps.py:
Line 388: caps = ET.fromstring(capabilities)
Line 389:
Line 390: for archTag in caps.iter(tag='arch'):
Line 391: if archTag.get('name') == arch:
Line 392: return list(set((itertools.chain.from_iterable(
please add libvirt's bug reference so we know when to revert
Line 393: (
Line 394: (m.text, m.get('canonical')) if
Line 395: m.get('canonical') else (m.text,)
Line 396: )
--
To view, visit https://gerrit.ovirt.org/42082
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0bade940d7d548be18eb779cde324e37231ce73
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
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]: ppc64le: Fix CPU map parsing
Michal Skrivanek has posted comments on this change. Change subject: ppc64le: Fix CPU map parsing .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/42037 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d7762d23118149ec47a962110bfecc66b9ead57 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek 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]: events: vm status notifications
Michal Skrivanek has posted comments on this change.
Change subject: events: vm status notifications
..
Patch Set 21:
(1 comment)
https://gerrit.ovirt.org/#/c/38937/21/tests/vmfakelib.py
File tests/vmfakelib.py:
Line 57: self.channelListener = None
Line 58: self.vmContainerLock = threading.Lock()
Line 59: self.vmContainer = {}
Line 60:
Line 61: def notify(self, event_id, **kwargs):
> We need it for the tests to pass. There is bunch of tests which check statu
oh I'm sorry, I missed this is the fake vdsm. Of course
Line 62: pass
Line 63:
Line 64:
Line 65: class Domain(object):
--
To view, visit https://gerrit.ovirt.org/38937
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I746299f9f1e2f49831a05072f19af1d242796276
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Dima Kuznetsov
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yeela Kaplan
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]: hostdev: destroy VM with hostdev even without IOMMU support
Michal Skrivanek has posted comments on this change. Change subject: hostdev: destroy VM with hostdev even without IOMMU support .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/41886 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I857ec18c0932de247469f6fb780d1b28aa3873e8 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Alona Kaplan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek 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]: serial console: add code to prepare the path
Michal Skrivanek has posted comments on this change.
Change subject: serial console: add code to prepare the path
..
Patch Set 3:
(1 comment)
https://gerrit.ovirt.org/#/c/41896/3/vdsm/vdsm
File vdsm/vdsm:
Line 90: utils.panic("Error initializing IRS")
how about using this for bailing out from vmconsoledir issue?
It may sound a bit harsh, OTOH if you don't touch it it should keep working,
And if you screwed it then we should scream loud
--
To view, visit https://gerrit.ovirt.org/41896
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f851d7f7233265d33896b3aad5604e84c8af53b
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani
Gerrit-Reviewer: Alon Bar-Lev
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Yaniv Bronhaim
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]: serial console: add code to prepare the path
Michal Skrivanek has posted comments on this change. Change subject: serial console: add code to prepare the path .. Patch Set 3: I'm for bailing out. Ignoring issues never pays out in the long run. I don't mind different solution if someone has a strong preference -- To view, visit https://gerrit.ovirt.org/41896 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6f851d7f7233265d33896b3aad5604e84c8af53b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Alon Bar-Lev Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek 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]: hostdev: add support for usb devices
Michal Skrivanek has posted comments on this change. Change subject: hostdev: add support for usb devices .. Patch Set 5: if we have to work around libvirt's deficiency of not being able to report the address then I still think it's a better approach to make up some address(maybe use the name), persist it in engine, etc, and only discard on VM creation This way once libvirt fix it we can easily enable the proper address persistence -- To view, visit https://gerrit.ovirt.org/39715 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf9b4f302353f3006e1f945dd342d351039ba387 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Michal Skrivanek 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]: events: vm status notifications
Michal Skrivanek has posted comments on this change. Change subject: events: vm status notifications .. Patch Set 22: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/38937 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I746299f9f1e2f49831a05072f19af1d242796276 Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan 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]: events: vm status notifications
Michal Skrivanek has posted comments on this change.
Change subject: events: vm status notifications
..
Patch Set 21:
(3 comments)
https://gerrit.ovirt.org/#/c/38937/21/tests/vmfakelib.py
File tests/vmfakelib.py:
Line 57: self.channelListener = None
Line 58: self.vmContainerLock = threading.Lock()
Line 59: self.vmContainer = {}
Line 60:
Line 61: def notify(self, event_id, **kwargs):
why do we need this?
Line 62: pass
Line 63:
Line 64:
Line 65: class Domain(object):
https://gerrit.ovirt.org/#/c/38937/21/vdsm/clientIF.py
File vdsm/clientIF.py:
Line 572: vmid = dom.UUIDString()
Line 573: v = self.vmContainer.get(vmid)
Line 574:
Line 575: if not v:
Line 576: self.log.debug('unknown vm %s eventid %s args %s',
@Francesco, I would say we want to log all the events (at debug level which we
drop from default), can you please add it to your "logging" patches?
Line 577:vmid, eventid, args)
Line 578: return
Line 579:
Line 580: if eventid == libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE:
https://gerrit.ovirt.org/#/c/38937/21/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 378: if self._lastStatus != value:
Line 379: self.saveState()
Line 380: self._lastStatus = value
Line 381:
Line 382: def send_status_event(self):
> there is no pooling here. I agree that it do not send the status always bec
I'm fine with the name as is (but also don't mind if you prefer to change
it...unless it's going to be send_status_event_when_changed() which sounds too
javaish to me)
Line 383: vm_status = self._getVmStatus()['status']
Line 384: if vm_status != self._evaluatedStatus:
Line 385: self._evaluatedStatus = vm_status
Line 386: stats = {
--
To view, visit https://gerrit.ovirt.org/38937
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I746299f9f1e2f49831a05072f19af1d242796276
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Dima Kuznetsov
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yeela Kaplan
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]: sampling: more detailed stale sampling reporting
Michal Skrivanek has posted comments on this change. Change subject: sampling: more detailed stale sampling reporting .. Patch Set 10: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/40390 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I00cb8f602fc3f26d62dd34bafe05b8169e184b91 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek 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]: virt: bind console to unix domain socket
Michal Skrivanek has posted comments on this change. Change subject: virt: bind console to unix domain socket .. Patch Set 7: @Yaniv: what's your preferred solution to initialize/create that /var/run/ovirt-vmconsole-console directory and permissions? We'd need to do that on vdsm service startup I guess -- To view, visit https://gerrit.ovirt.org/40704 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0a876cd7f3b848aa9dde8123d459dc292729b303 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: Vitor de Lima 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]: virt: bind console to unix domain socket
Michal Skrivanek has posted comments on this change. Change subject: virt: bind console to unix domain socket .. Patch Set 7: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/40704 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0a876cd7f3b848aa9dde8123d459dc292729b303 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: Vitor de Lima 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]: status: adding elapsed time in millis
Michal Skrivanek has posted comments on this change.
Change subject: status: adding elapsed time in millis
..
Patch Set 8:
(1 comment)
https://gerrit.ovirt.org/#/c/40904/8/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 404: def _notify(self, operation, params):
Line 405: sub_id = '|virt|%s|%s' % (operation, self.id)
Line 406: self.cif.notify(sub_id, **{self.id: params})
Line 407:
Line 408: def _get_status_time(self):
> We could provide it as part of infrastructure code. In this situation this
+1
Line 409: """
Line 410: Value provided by this method is used to order messages
Line 411: containing changed status on the engine side.
Line 412: """
--
To view, visit https://gerrit.ovirt.org/40904
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I38727e0eac7cb29c1dc7c9696568540b6545461c
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: Yaniv Bronhaim
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]: status: adding elapsed time in millis
Michal Skrivanek has posted comments on this change.
Change subject: status: adding elapsed time in millis
..
Patch Set 8:
(1 comment)
not sure about naming
https://gerrit.ovirt.org/#/c/40904/8/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 404: def _notify(self, operation, params):
Line 405: sub_id = '|virt|%s|%s' % (operation, self.id)
Line 406: self.cif.notify(sub_id, **{self.id: params})
Line 407:
Line 408: def _get_status_time(self):
do we want to have this as a single-purpose field (e.g. for "status" event) or
do we plan to re-use similar concept in other events?
I would think that actually we will need this for many(if not all)
eventsthen it would be better to use a more generic name
Thoughts?
Line 409: """
Line 410: Value provided by this method is used to order messages
Line 411: containing changed status on the engine side.
Line 412: """
--
To view, visit https://gerrit.ovirt.org/40904
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I38727e0eac7cb29c1dc7c9696568540b6545461c
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: Yaniv Bronhaim
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]: migration: Support SPICE seamless migration over NAT
Michal Skrivanek has posted comments on this change. Change subject: migration: Support SPICE seamless migration over NAT .. Patch Set 6: Code-Review+1 copy score. Dan, seems ready -- To view, visit https://gerrit.ovirt.org/41223 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7b2b765c62aaf1cc694990205b518558e808516 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Tomas Jelinek Gerrit-Reviewer: Yeela Kaplan 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]: virt: allow a single dev from IOMMU group to be passed through
Michal Skrivanek has posted comments on this change. Change subject: virt: allow a single dev from IOMMU group to be passed through .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/41508 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic266e74b5ffc4ae5c4627b004ade55a19c969511 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek 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]: migration: Support SPICE seamless migration over NAT
Michal Skrivanek has posted comments on this change.
Change subject: migration: Support SPICE seamless migration over NAT
..
Patch Set 4:
(1 comment)
https://gerrit.ovirt.org/#/c/41223/4/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 348: # TODO: use libvirt constants when bz#1222795 is fixed
Line 349: params = {VIR_MIGRATE_PARAM_URI: str(muri),
Line 350: VIR_MIGRATE_PARAM_BANDWIDTH: maxBandwidth}
Line 351: if self._consoleAddress:
Line 352: if self._vm.hasSpice:
> Do we have a way of knowing about this?
not easily. It's too much effort and not worth it. I'll document it in the bug
as a known issue/behavior
Line 353: graphics = 'spice'
Line 354: else:
Line 355: graphics = 'vnc'
Line 356: params[VIR_MIGRATE_PARAM_GRAPHICS_URI] = \
--
To view, visit https://gerrit.ovirt.org/41223
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7b2b765c62aaf1cc694990205b518558e808516
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Tomas Jelinek
Gerrit-Reviewer: Yeela Kaplan
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]: migration: Support SPICE seamless migration over NAT
Michal Skrivanek has posted comments on this change. Change subject: migration: Support SPICE seamless migration over NAT .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/41223 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7b2b765c62aaf1cc694990205b518558e808516 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Tomas Jelinek Gerrit-Reviewer: Yeela Kaplan 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]: migration: Support SPICE seamless migration over NAT
Michal Skrivanek has posted comments on this change.
Change subject: migration: Support SPICE seamless migration over NAT
..
Patch Set 4: Code-Review+1
(1 comment)
https://gerrit.ovirt.org/#/c/41223/4/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 348: # TODO: use libvirt constants when bz#1222795 is fixed
Line 349: params = {VIR_MIGRATE_PARAM_URI: str(muri),
Line 350: VIR_MIGRATE_PARAM_BANDWIDTH: maxBandwidth}
Line 351: if self._consoleAddress:
Line 352: if self._vm.hasSpice:
I think this is the best we can do
Though note that with today's support of VNC+SPICE it will behave wrongly when
you connect via VNC and migrate, here we switch to SPICE (since VM has both)
and the client will fail (probably, hopefully:)
I believe that's "good enough"
Line 353: graphics = 'spice'
Line 354: else:
Line 355: graphics = 'vnc'
Line 356: params[VIR_MIGRATE_PARAM_GRAPHICS_URI] = \
--
To view, visit https://gerrit.ovirt.org/41223
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7b2b765c62aaf1cc694990205b518558e808516
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Tomas Jelinek
Gerrit-Reviewer: Yeela Kaplan
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]: migration: Support SPICE seamless migration over NAT
Michal Skrivanek has posted comments on this change.
Change subject: migration: Support SPICE seamless migration over NAT
..
Patch Set 2: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/41223/2/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 349: params = {VIR_MIGRATE_PARAM_URI: str(muri),
Line 350: VIR_MIGRATE_PARAM_BANDWIDTH: maxBandwidth}
Line 351: if self._consoleAddress:
Line 352: params[VIR_MIGRATE_PARAM_GRAPHICS_URI] = \
Line 353: 'spice://%s' % self._consoleAddress
all looks goodexcept I wonder how does this behave with VNC. We have to
check that:-/
Line 354:
Line 355: flags = (libvirt.VIR_MIGRATE_LIVE |
Line 356: libvirt.VIR_MIGRATE_PEER2PEER |
Line 357: (libvirt.VIR_MIGRATE_TUNNELLED if
--
To view, visit https://gerrit.ovirt.org/41223
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7b2b765c62aaf1cc694990205b518558e808516
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Tomas Jelinek
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]: virt: Remove QEMU passthrough hack in ppc64
Michal Skrivanek has posted comments on this change. Change subject: virt: Remove QEMU passthrough hack in ppc64 .. Patch Set 7: Code-Review+1 should be ok for 3.6 -- To view, visit https://gerrit.ovirt.org/33871 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I64d660bf7534203d5d5cdbc318ffd1429a16f954 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vitor de Lima Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Vitor de Lima 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]: vdsm: hook for booting from an ISO image gathered via https
Michal Skrivanek has posted comments on this change. Change subject: vdsm: hook for booting from an ISO image gathered via https .. Patch Set 11: Code-Review+1 (didn't review code in detail, but conceptually it's fine) -- To view, visit https://gerrit.ovirt.org/41076 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I22b190b5d3d43cab7bdbfa81bdf0904b2988b2bc Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Simone Tiraboschi Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eldan Shachar Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Simone Tiraboschi 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]: vdsm: hook for booting from an ISO image gathered via https
Michal Skrivanek has posted comments on this change. Change subject: vdsm: hook for booting from an ISO image gathered via https .. Patch Set 11: I agree with your explanation in https://bugzilla.redhat.com/show_bug.cgi?id=917026#c17 one question, it's adding one more CDROM. How does it interact with the one we add and potentially another one from cloud-init? -- To view, visit https://gerrit.ovirt.org/41076 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I22b190b5d3d43cab7bdbfa81bdf0904b2988b2bc Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Simone Tiraboschi Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eldan Shachar Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Simone Tiraboschi 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]: migration: Support SPICE seamless migration over NAT
Michal Skrivanek has posted comments on this change.
Change subject: migration: Support SPICE seamless migration over NAT
..
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/41223/1/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 347: # TODO: use libvirt constants when bz#1222795 is fixed
Line 348: params = {VIR_MIGRATE_PARAM_URI: str(muri),
Line 349: VIR_MIGRATE_PARAM_BANDWIDTH: maxBandwidth}
Line 350: if self._consoleAddress:
Line 351: params[VIR_MIGRATE_PARAM_GRAPHICS_URI] = \
> libvirt calls this graphicsURI. Can we follow suit?
what's wrong with the name? Seems ok to me
+1 for vdsClient
Line 352: str(self._consoleAddress)
Line 353:
Line 354: flags = (libvirt.VIR_MIGRATE_LIVE |
Line 355: libvirt.VIR_MIGRATE_PEER2PEER |
--
To view, visit https://gerrit.ovirt.org/41223
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7b2b765c62aaf1cc694990205b518558e808516
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Tomas Jelinek
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]: migration: Start using migrate3 instead of migrate2
Michal Skrivanek has posted comments on this change. Change subject: migration: Start using migrate3 instead of migrate2 .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/41101 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I57a1d9aeb796998c5100e0c2ec3fb05e775022ed Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Tomas Jelinek Gerrit-Reviewer: Yeela Kaplan 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]: virt: Trigger event on guest agent status changes
Michal Skrivanek has posted comments on this change. Change subject: virt: Trigger event on guest agent status changes .. Patch Set 2: (1 comment) https://gerrit.ovirt.org/#/c/41108/2/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 354: self._vmJobs = None Line 355: Line 356: def _agent_status_changed(self): Line 357: vm_status = self._getVmStatus() Line 358: if self.lastStatus != vm_status: > we override the reported status with the guest status only if status is 'UP another alternative is to step back and simply fire the event (with _getVmStatus()) every time guest agent tells you there's a status change. Since it is called only on a change it won't happen blindly and it would be reasonably safe to just send it here Line 359: self._send_status_event(vm_status['status']) Line 360: Line 361: def _get_lastStatus(self): Line 362: # note that we don't use _statusLock here. One of the reasons is the -- To view, visit https://gerrit.ovirt.org/41108 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaceca8e42720df50d5eb1e20670b6ed733db8b50 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan 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]: virt: Trigger event on guest agent status changes
Michal Skrivanek has posted comments on this change. Change subject: virt: Trigger event on guest agent status changes .. Patch Set 2: (2 comments) https://gerrit.ovirt.org/#/c/41108/2/vdsm/virt/guestagent.py File vdsm/virt/guestagent.py: Line 133: self._channelListener = channelListener Line 134: self._messageState = MessageState.NORMAL Line 135: Line 136: @property Line 137: def guestStatus(self): I know the varialbe was there already, but since now you're building an interface it would be better to resemble the VM's status a bit - how about lastGuestStatus? Line 138: return self._guestStatus Line 139: Line 140: @guestStatus.setter Line 141: def guestStatus(self, value): https://gerrit.ovirt.org/#/c/41108/2/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 354: self._vmJobs = None Line 355: Line 356: def _agent_status_changed(self): Line 357: vm_status = self._getVmStatus() Line 358: if self.lastStatus != vm_status: you're comparing different set of values I'd prefer to set the same lastStatus (lastStatus = lastStatus) and let it be the only plae we send events from However since it has a custom getter as well...it's just a mess Line 359: self._send_status_event(vm_status['status']) Line 360: Line 361: def _get_lastStatus(self): Line 362: # note that we don't use _statusLock here. One of the reasons is the -- To view, visit https://gerrit.ovirt.org/41108 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaceca8e42720df50d5eb1e20670b6ed733db8b50 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan 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]: Add support for specifying vm maximum memory
Michal Skrivanek has posted comments on this change.
Change subject: Add support for specifying vm maximum memory
..
Patch Set 2: Code-Review+1
(2 comments)
https://gerrit.ovirt.org/#/c/40547/2/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 3586: # @maxMemSize:#optional The maximum amount of memory
that can be
Line 3587: # assigned to the VM in MB
Line 3588: #
Line 3589: # @maxMemSlots: #optional Maximum number of memory slots
available
Line 3590: # This is the upper boundry for hot plug
memory action
s/boundry/boundary/
Line 3591: #
Line 3592: # Since: 4.10.0
Line 3593: ##
Line 3594: {'type': 'VmDefinition',
https://gerrit.ovirt.org/#/c/40547/2/vdsm/virt/vmxml.py
File vdsm/virt/vmxml.py:
Line 168: self.dom.appendChildWithArgs('memory', text=memSizeKB)
Line 169: self.dom.appendChildWithArgs('currentMemory', text=memSizeKB)
Line 170: if 'maxMemSize' in self.conf:
Line 171: maxMemSizeKB = str(int(self.conf['maxMemSize']) * 1024)
Line 172: maxMemSlots = str(self.conf.get('maxMemSlots', '16'))
document "16" at least in the schema...or here...either way it does deserve one
line of comment somewhere:)
Line 173: self.dom.appendChildWithArgs('maxMemory',
text=maxMemSizeKB,
Line 174: slots=maxMemSlots)
Line 175: vcpu = self.dom.appendChildWithArgs('vcpu',
text=self._getMaxVCpus())
Line 176: vcpu.setAttrs(**{'current': self._getSmp()})
--
To view, visit https://gerrit.ovirt.org/40547
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I54a698fbd45d6605f24b3641c541ff10a332d9f8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Omer Frenkel
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Omer Frenkel
Gerrit-Reviewer: Vitor de Lima
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: hook for booting from an ISO image gathered via https
Michal Skrivanek has posted comments on this change. Change subject: vdsm: hook for booting from an ISO image gathered via https .. Patch Set 9: Code-Review-1 problem here is that you are duplicating a lot of code which already is in vdsm. It's not that big deal as hook can do whatever...but still since we do have a valid RFE and will want to incorporate this feature properly I don't see an excuse for a hook...the code directly in vdsm would be way more simple. And then whether we actually expose/use it in engine right away is a completely different matter. We don't have to -- To view, visit https://gerrit.ovirt.org/41076 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I22b190b5d3d43cab7bdbfa81bdf0904b2988b2bc Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Simone Tiraboschi Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eldan Shachar Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Simone Tiraboschi 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]: events: vm status notifications
Michal Skrivanek has posted comments on this change.
Change subject: events: vm status notifications
..
Patch Set 12:
(1 comment)
https://gerrit.ovirt.org/#/c/38937/12/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 392: stats['exitCode'] = self.conf['exitCode']
Line 393: if 'exitMessage' in self.conf:
Line 394: stats['exitMessage'] = self.conf['exitMessage']
Line 395: if 'exitReason' in self.conf:
Line 396: stats['exitReason'] = self.conf['exitReason']
> I was suggested to have it that way as a result of long conversation.
suggestion was to return the stats looking like that, seems the function is
good enough
Line 397:
Line 398: self.cif.notify('|virt|VM_status|' + self.id,
Line 399: **{self.id: stats})
Line 400:
--
To view, visit https://gerrit.ovirt.org/38937
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I746299f9f1e2f49831a05072f19af1d242796276
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Dima Kuznetsov
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yeela Kaplan
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]: vm: Detect RNG device by type
Michal Skrivanek has posted comments on this change. Change subject: vm: Detect RNG device by type .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/40095 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I31a1c38eec5be96e298e0f1dd5a7598c2212cb21 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shmuel Leib Melamud Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Omer Frenkel Gerrit-Reviewer: Shmuel Leib Melamud 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]: Prevent KeyError in migrateStatus
Michal Skrivanek has posted comments on this change. Change subject: Prevent KeyError in migrateStatus .. Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/40657 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9277b70432d1d9ecc3c39ee7deabbbd89d179715 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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]: revent KeyError in migrateStatus
Michal Skrivanek has posted comments on this change. Change subject: revent KeyError in migrateStatus .. Patch Set 3: "revent" - you mean revert? -- To view, visit https://gerrit.ovirt.org/40657 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9277b70432d1d9ecc3c39ee7deabbbd89d179715 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shahar Havivi 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]: Getting migration statistics NPE
Michal Skrivanek has posted comments on this change.
Change subject: Getting migration statistics NPE
..
Patch Set 2:
(1 comment)
https://gerrit.ovirt.org/#/c/40657/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1299: if 'pauseCode' in self.conf:
Line 1300: stats['pauseCode'] = self.conf['pauseCode']
Line 1301: if self.isMigrating():
Line 1302: mig_status = self.migrateStatus()
Line 1303: if 'progress' in mig_status:
I'd prefer to fix migrateStatus to return something meaningful based on the
existence of the src thread.
It should be encapsulated
Line 1304: stats['migrationProgress'] = mig_status['progress']
Line 1305:
Line 1306: decStats = {}
Line 1307: try:
--
To view, visit https://gerrit.ovirt.org/40657
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9277b70432d1d9ecc3c39ee7deabbbd89d179715
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Shahar Havivi
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]: events: vm status notifications
Michal Skrivanek has posted comments on this change.
Change subject: events: vm status notifications
..
Patch Set 10:
(3 comments)
https://gerrit.ovirt.org/#/c/38937/10/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 357:
Line 358: def _set_lastStatus(self, value):
Line 359: with self._statusLock:
Line 360: if self._lastStatus != value:
Line 361: self._send_status_event(value)
this should go to line 372-374
Line 362: if self._lastStatus == vmstatus.DOWN:
Line 363: self.log.warning(
Line 364: 'trying to set state to %s when already Down',
Line 365: value)
Line 380: self.guestAgent.diskMappingHash))),
Line 381: 'elapsedTime': str(int(time.time() - self._startTime)),
Line 382: }
Line 383:
Line 384: if value == vmstatus.DOWN:
is _getExitedVmStats no good enough? Looking at the optional timeOffset there,
it wouldn't hurt even though it's not used on engine side(it's still stored
though)
Better than duplicate code here...
Line 385: if 'exitCode' in self.conf:
Line 386: stats['exitCode'] = self.conf['exitCode']
Line 387: if 'exitMessage' in self.conf:
Line 388: stats['exitMessage'] = self.conf['exitMessage']
Line 388: stats['exitMessage'] = self.conf['exitMessage']
Line 389: if 'exitReason' in self.conf:
Line 390: stats['exitReason'] = self.conf['exitReason']
Line 391:
Line 392: self.cif.notify('|virt|VM_status|' + self.id,
since this is done under _statusLock, what's going on inside cif.notify?
Line 393: **{self.id: stats})
Line 394:
Line 395: lastStatus = property(_get_lastStatus, _set_lastStatus)
Line 396:
--
To view, visit https://gerrit.ovirt.org/38937
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I746299f9f1e2f49831a05072f19af1d242796276
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski
Gerrit-Reviewer: Dima Kuznetsov
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yeela Kaplan
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]: RFE: Report downtime for each live migration
Michal Skrivanek has posted comments on this change.
Change subject: RFE: Report downtime for each live migration
..
Patch Set 2:
(1 comment)
https://gerrit.ovirt.org/#/c/40103/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1717: 'network': {},
Line 1718: 'disks': {}}
Line 1719: if 'pauseCode' in self.conf:
Line 1720: stats['pauseCode'] = self.conf['pauseCode']
Line 1721: if self.isMigrating() and
self.migrateStatus().has_key('progress'):
> There is not always progress and we had NPE when calling migrateStat verb.
I'd say it should always have a progress when isMigrating()
what NPE you get? There should have been an exception here on vdsm side
Line 1722: stats['migrationProgress'] =
self.migrateStatus()['progress']
Line 1723:
Line 1724: decStats = {}
Line 1725: try:
--
To view, visit https://gerrit.ovirt.org/40103
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ff421c489ef24869502bd80461018f0aca2144d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Shahar Havivi
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]: v2v: add format for disk conversion
Michal Skrivanek has posted comments on this change. Change subject: v2v: add format for disk conversion .. Patch Set 2: Code-Review+1 (1 comment) https://gerrit.ovirt.org/#/c/40343/2//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2015-04-28 11:28:15 +0300 Line 4: Commit: Shahar Havivi Line 5: CommitDate: 2015-05-03 14:42:11 +0300 Line 6: Line 7: v2v: add format for disk conversion > Why this was not included in the original patch? this is to add sparse/thin support Line 8: Line 9: Change-Id: I1da72771c9b02fa741cc84aa7159e862675fb577 -- To view, visit https://gerrit.ovirt.org/40343 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1da72771c9b02fa741cc84aa7159e862675fb577 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi 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]: virt: graphdev: support headless VM
Michal Skrivanek has posted comments on this change. Change subject: virt: graphdev: support headless VM .. Patch Set 22: Code-Review+1 how about merging this finally?:-) -- To view, visit https://gerrit.ovirt.org/27846 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iafeb0bebfb43c089614127d94c054175c111ce54 Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Vinzenz Feenstra 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]: events: sending stats with vm status change event
Michal Skrivanek has posted comments on this change.
Change subject: events: sending stats with vm status change event
..
Patch Set 2:
(3 comments)
https://gerrit.ovirt.org/#/c/40204/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 368: return status
Line 369:
Line 370: def _set_lastStatus(self, value):
Line 371: with self._statusLock:
Line 372: self._send_status_event(value)
> During testing of my changes I have never received the same status twice. I
possibly. though till now it didn't really matter as it has no significance to
override lastStatus with the same thing. The only special handling is below for
DoubleDown (and yes, I do see it sometimes;-)
should be harmless to send only on a change, no?
Line 373: if self._lastStatus == vmstatus.DOWN:
Line 374: self.log.warning(
Line 375: 'trying to set state to %s when already Down',
Line 376: value)
Line 386:
Line 387: def _send_status_event(self, value):
Line 388: if self.lastStatus == vmstatus.DOWN:
Line 389: stats = self._getExitedVmStats()
Line 390: else:
> I checked the code and I believe that applist is added during _getGuestStat
we would need to dissect it more as some of the guest stats are currently
needed.
I do think this is the right place where to do it.
I'd like to ultimately have a custom structure with the minimum set of fields
engine needs to make an update. Some engine-side changes could be needed
Line 391: stats = self._getConfigVmStats()
Line 392: stats.update(self._getRunningVmStats())
Line 393: stats.update(self._getVmStatus())
Line 394:
Line 4635: self.log.warning('monitor become unresponsive'
Line 4636: ' (command timeout, age=%s)',
Line 4637: statsAge)
Line 4638: stats['monitorResponse'] = '-1'
Line 4639: self._send_status_event('NotResponding')
> NotResponding is part of status enum on the engine side. I think this one a
yes, but vdsm never sent this status. did you check engine's behavior in this
case? (I think just follow the code path is enough at this point)
Line 4640:
Line 4641: def updateNumaInfo(self):
Line 4642: self._numaInfo = numaUtils.getVmNumaNodeRuntimeInfo(self)
Line 4643:
--
To view, visit https://gerrit.ovirt.org/40204
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a1bdea997ec24342bbb25b3baeb3e7f8313321
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Dima Kuznetsov
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yeela Kaplan
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]: events: sending stats with vm status change event
Michal Skrivanek has posted comments on this change. Change subject: events: sending stats with vm status change event .. Patch Set 2: *tweak the engine code* -- To view, visit https://gerrit.ovirt.org/40204 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I61a1bdea997ec24342bbb25b3baeb3e7f8313321 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan 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]: events: sending stats with vm status change event
Michal Skrivanek has posted comments on this change.
Change subject: events: sending stats with vm status change event
..
Patch Set 2:
(3 comments)
https://gerrit.ovirt.org/#/c/40204/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 368: return status
Line 369:
Line 370: def _set_lastStatus(self, value):
Line 371: with self._statusLock:
Line 372: self._send_status_event(value)
would be better if we send it only on a status change, not all the time. I'd
suspect sometimes we may set the same status twice
Line 373: if self._lastStatus == vmstatus.DOWN:
Line 374: self.log.warning(
Line 375: 'trying to set state to %s when already Down',
Line 376: value)
Line 386:
Line 387: def _send_status_event(self, value):
Line 388: if self.lastStatus == vmstatus.DOWN:
Line 389: stats = self._getExitedVmStats()
Line 390: else:
this is a nice improvement. Still we should drop few fields - this still does
contain applist I think. Also net and disk stats. Let's talk about the content
outside of the review
Line 391: stats = self._getConfigVmStats()
Line 392: stats.update(self._getRunningVmStats())
Line 393: stats.update(self._getVmStatus())
Line 394:
Line 4635: self.log.warning('monitor become unresponsive'
Line 4636: ' (command timeout, age=%s)',
Line 4637: statsAge)
Line 4638: stats['monitorResponse'] = '-1'
Line 4639: self._send_status_event('NotResponding')
This is aproblematic one...vdsm doesn't have a NotResponding status, it's only
derived in engine when monitorResponse==-1.
Since this is a new code maybe we can tweak the code and introduce this state
on engine side
Line 4640:
Line 4641: def updateNumaInfo(self):
Line 4642: self._numaInfo = numaUtils.getVmNumaNodeRuntimeInfo(self)
Line 4643:
--
To view, visit https://gerrit.ovirt.org/40204
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a1bdea997ec24342bbb25b3baeb3e7f8313321
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Dima Kuznetsov
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yeela Kaplan
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]: hostdev: add support for usb devices
Michal Skrivanek has posted comments on this change. Change subject: hostdev: add support for usb devices .. Patch Set 5: Code-Review-1 I'd like to push the libvirt fix instead. Let's keep vdsm code nice and clean if possible:-) -- To view, visit https://gerrit.ovirt.org/39715 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf9b4f302353f3006e1f945dd342d351039ba387 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Michal Skrivanek 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]: hostdev: add support for usb devices
Michal Skrivanek has posted comments on this change. Change subject: hostdev: add support for usb devices .. Patch Set 4: (1 comment) https://gerrit.ovirt.org/#/c/39715/4/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 3808: # both addresses and determine the correct one. Line 3809: if hostdev.address_to_name(device_type, address) == device: Line 3810: try: Line 3811: address = self._getUnderlyingDeviceAddress(x, 1) Line 3812: except IndexError: > It was guaranteed that the second address would exist, that fact is gone wi why wouldn't it have address? type='usb' USB addresses have the following additional attributes: bus (a hex value between 0 and 0xfff, inclusive), and port (a dotted notation of up to four octets, such as 1.2 or 2.1.3.1). Line 3813: address = None Line 3814: Line 3815: known_device = False Line 3816: for dev in self.conf['devices']: -- To view, visit https://gerrit.ovirt.org/39715 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf9b4f302353f3006e1f945dd342d351039ba387 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Michal Skrivanek 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]: stomp: server side subscriptions
Michal Skrivanek has posted comments on this change.
Change subject: stomp: server side subscriptions
..
Patch Set 15:
(1 comment)
https://gerrit.ovirt.org/#/c/38451/15/lib/yajsonrpc/stompReactor.py
File lib/yajsonrpc/stompReactor.py:
Line 120: ack = frame.headers.get("ack", stomp.AckMode.AUTO)
Line 121: subscription = stomp._Subscription(dispatcher.connection,
destination,
Line 122:sub_id, ack, None)
Line 123:
Line 124: # TODO make sure to cleanup when connection lost
> No, this is done in https://gerrit.ovirt.org/#/c/39969
good, then do not put TODO here or drop it in that other patch:-)
Line 125: self._sub_dests[destination].append(subscription)
Line 126: self._sub_ids[sub_id] = subscription
Line 127:
Line 128: def _send_error(self, msg, connection):
--
To view, visit https://gerrit.ovirt.org/38451
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1493070f2ba66ca9d39a6661876c82c4727cad62
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski
Gerrit-Reviewer: Dima Kuznetsov
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Oved Ourfali
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yeela Kaplan
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]: stomp: server side subscriptions
Michal Skrivanek has posted comments on this change.
Change subject: stomp: server side subscriptions
..
Patch Set 15:
(1 comment)
https://gerrit.ovirt.org/#/c/38451/15/lib/yajsonrpc/stompReactor.py
File lib/yajsonrpc/stompReactor.py:
Line 120: ack = frame.headers.get("ack", stomp.AckMode.AUTO)
Line 121: subscription = stomp._Subscription(dispatcher.connection,
destination,
Line 122:sub_id, ack, None)
Line 123:
Line 124: # TODO make sure to cleanup when connection lost
don't we want to address the TODO as part of this patch?
Line 125: self._sub_dests[destination].append(subscription)
Line 126: self._sub_ids[sub_id] = subscription
Line 127:
Line 128: def _send_error(self, msg, connection):
--
To view, visit https://gerrit.ovirt.org/38451
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1493070f2ba66ca9d39a6661876c82c4727cad62
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski
Gerrit-Reviewer: Dima Kuznetsov
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Oved Ourfali
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yeela Kaplan
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]: events: sending stats with vm status change event
Michal Skrivanek has posted comments on this change.
Change subject: events: sending stats with vm status change event
..
Patch Set 1: Code-Review-1
(1 comment)
Vinzenz, Francesco, please help with getting the right content to send.
Each omitted field should be checked against latest engine monitoring code
https://gerrit.ovirt.org/#/c/40204/1/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 785:
Line 786: def _send_status_event(self, value):
Line 787: self.cif.notify('|virt|VM_status|' + self.id,
Line 788: **{self.id: {'status': value,
Line 789: 'stats': self.getStats()}})
I think we should rather assemble our own set of data instead generic
getStats(). We clearly do not need applist (potentially huge), and quite likely
other things.
I'd start with minimum and check the engine side what's really needed. Our
advantage is that we can consider only 3.6 engine, many legacy fields can be
dropped
Line 790:
Line 791: lastStatus = property(_get_lastStatus, _set_lastStatus)
Line 792:
Line 793: def __getNextIndex(self, used):
--
To view, visit https://gerrit.ovirt.org/40204
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a1bdea997ec24342bbb25b3baeb3e7f8313321
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski
Gerrit-Reviewer: Dima Kuznetsov
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yeela Kaplan
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]: events: vm status notifications
Michal Skrivanek has posted comments on this change.
Change subject: events: vm status notifications
..
Patch Set 2:
(1 comment)
https://gerrit.ovirt.org/#/c/38937/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1578: self._guestCpuRunning = isRunning
Line 1579: if (not self._guestCpuRunning and
Line 1580: self._lastStatus in vmstatus.PAUSED_STATES):
Line 1581: self.cif.notify('|virt|VM_status|' + self.id,
Line 1582: **{self.id: 'Paused'})
> When exploring the option of removal I noticed that when removed Paused sta
yeah, because lastStatus is not really a reliable way. I'm wondering if it
wouldn't be better to only trigger the event from well known safe places
instead. Especially if we want to send stats data with it, we need to make sure
we won't send nonsense or the same event multiple times. We can do that relying
on the 15s getAllVmStats which will "fix" corner cases we miss here
Line 1583: finally:
Line 1584: if not guestCpuLocked:
Line 1585: self._guestCpuLock.release()
Line 1586:
--
To view, visit https://gerrit.ovirt.org/38937
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I746299f9f1e2f49831a05072f19af1d242796276
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski
Gerrit-Reviewer: Dima Kuznetsov
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yeela Kaplan
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]: events: vm status notifications
Michal Skrivanek has posted comments on this change.
Change subject: events: vm status notifications
..
Patch Set 3:
(1 comment)
another idea to explore. Not suggesting to change it right away.
https://gerrit.ovirt.org/#/c/38937/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1524: if not guestCpuLocked:
Line 1525: self._acquireCpuLockWithTimeout()
Line 1526: try:
Line 1527: self._guestCpuRunning = isRunning
Line 1528: if (not self._guestCpuRunning and
> Looking at the code of dispatchLibvirtEvents it looks like we use _onLibvir
I'm not so sure about the watchdog events there, apparently they mean VM is not
running, but we don't track it.
Overall it seems to me the lastStatus variable is not really a good indicator,
also on that event when NonResponsive we don't actually recognize it as a state
within vdsm
So, hmm, maybe it is better to send the event from the places we are sure
about(since we now expect/plan to send getStats data we need to make sure it's
grabbed at the right time). Francesco, can you think about it?
Line 1529: self._lastStatus in vmstatus.PAUSED_STATES):
Line 1530: self.cif.notify('|virt|VM_status|' + self.id,
Line 1531: **{self.id: 'Paused'})
Line 1532: finally:
--
To view, visit https://gerrit.ovirt.org/38937
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I746299f9f1e2f49831a05072f19af1d242796276
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski
Gerrit-Reviewer: Dima Kuznetsov
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yeela Kaplan
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
