Francesco Romani has uploaded a new change for review.
Change subject: virt: tolerate missing ovirt-vmconsole group ......................................................................
virt: tolerate missing ovirt-vmconsole group
To support the virtio serial console feature, VDSM must bind the virtio channel to an unix domain socket, and fix its permissions and group in order to let ovirt-vmconsole-host connect to that socket.
ovirt-vmconsole-host it is a soft dependency. VDSM should happily run without that package, but incorrectly assumed the ovirt-vmconsole group existed in the system
This patch lift this requirement: VDSM still try to fix the group of the socket, but if the group doesn't exist, it happily goes ahead.
Change-Id: I55b708953e5871053f0063bb82434c5c0dcb6103 Related-To: https://bugzilla.redhat.com/show_bug.cgi?id=1223671 Signed-off-by: Francesco Romani fromani@redhat.com --- M vdsm/virt/vmdevices/core.py 1 file changed, 8 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/51/42451/1
diff --git a/vdsm/virt/vmdevices/core.py b/vdsm/virt/vmdevices/core.py index a847ce0..4d56005 100644 --- a/vdsm/virt/vmdevices/core.py +++ b/vdsm/virt/vmdevices/core.py @@ -99,9 +99,14 @@
def prepare(self): if self._path: - supervdsm.getProxy().prepareVmChannel( - self._path, - constants.OVIRT_VMCONSOLE_GROUP) + try: + supervdsm.getProxy().prepareVmChannel( + self._path, + constants.OVIRT_VMCONSOLE_GROUP) + except KeyError: + self.log.warn('unknown group %s, permissions not updated', + constants.OVIRT_VMCONSOLE_GROUP) +
def cleanup(self): if self._path:
automation@ovirt.org has posted comments on this change.
Change subject: virt: tolerate missing ovirt-vmconsole group ......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Francesco Romani has posted comments on this change.
Change subject: virt: tolerate missing ovirt-vmconsole group ......................................................................
Patch Set 1: Verified+1
without this patch: VM startup triggers getgrnam() with fails, and KeyError is raised. With this patch: warning in the logs, socket bound but with "wrong" group, VM boots.
Alternative option: just add dependency on ovirt-vmconsole-host package, which owns the ovirt-vmconsole user/group.
automation@ovirt.org has posted comments on this change.
Change subject: virt: tolerate missing ovirt-vmconsole group ......................................................................
Patch Set 2:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Francesco Romani has posted comments on this change.
Change subject: virt: tolerate missing ovirt-vmconsole group ......................................................................
Patch Set 2: Verified+1
V2 fixes a pep8 violation, copied score.
Francesco Romani has posted comments on this change.
Change subject: virt: tolerate missing ovirt-vmconsole group ......................................................................
Patch Set 2:
the package we could depend on is ovirt-vmconsole, which owns user, group, and provides common utilities for -proxy and -host. It doesn't start any service.
Nir Soffer has posted comments on this change.
Change subject: virt: tolerate missing ovirt-vmconsole group ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://gerrit.ovirt.org/#/c/42451/2/vdsm/virt/vmdevices/core.py File vdsm/virt/vmdevices/core.py:
Line 102: try: Line 103: supervdsm.getProxy().prepareVmChannel( Line 104: self._path, Line 105: constants.OVIRT_VMCONSOLE_GROUP) Line 106: except KeyError: KeyError is pretty bad error to check here, but this is stdlib fault.
We should fix this later in fileUtils, raise an error that cannot mask a bug in the calling code. Line 107: self.log.warn('unknown group %s, permissions not updated', Line 108: constants.OVIRT_VMCONSOLE_GROUP) Line 109: Line 110: def cleanup(self):
automation@ovirt.org has posted comments on this change.
Change subject: virt: tolerate missing ovirt-vmconsole group ......................................................................
Patch Set 3:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Dan Kenigsberg has posted comments on this change.
Change subject: virt: tolerate missing ovirt-vmconsole group ......................................................................
Patch Set 3: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/42451/3//COMMIT_MSG Commit Message:
Line 12: group in order to let ovirt-vmconsole-host Line 13: connect to that socket. Line 14: Line 15: ovirt-vmconsole-host it is a soft dependency. Line 16: VDSM should happily run without that package, ...however, Vdsm must not happily start a VM where this new feature was explicitly requested by Engine, but Vdsm cannot deliver. Line 17: but incorrectly assumed the ovirt-vmconsole Line 18: group existed in the system Line 19: Line 20: This patch lift this requirement:
automation@ovirt.org has posted comments on this change.
Change subject: virt: tolerate missing ovirt-vmconsole group ......................................................................
Patch Set 4:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Francesco Romani has posted comments on this change.
Change subject: virt: tolerate missing ovirt-vmconsole group ......................................................................
Patch Set 3:
(1 comment)
https://gerrit.ovirt.org/#/c/42451/3//COMMIT_MSG Commit Message:
Line 12: group in order to let ovirt-vmconsole-host Line 13: connect to that socket. Line 14: Line 15: ovirt-vmconsole-host it is a soft dependency. Line 16: VDSM should happily run without that package,
...however, Vdsm must not happily start a VM where this new feature was exp
Right, than the real fix is engine-side. Will abandon this patch (set) once Engine fix is in Line 17: but incorrectly assumed the ovirt-vmconsole Line 18: group existed in the system Line 19: Line 20: This patch lift this requirement:
automation@ovirt.org has posted comments on this change.
Change subject: virt: tolerate missing ovirt-vmconsole group ......................................................................
Patch Set 5:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Francesco Romani has abandoned this change.
Change subject: virt: tolerate missing ovirt-vmconsole group ......................................................................
Abandoned
VDSM should not take initiatives. Alternative approaches: 42479, 42542
automation@ovirt.org has posted comments on this change.
Change subject: virt: tolerate missing ovirt-vmconsole group ......................................................................
Patch Set 5:
* Update tracker::IGNORE, no Bug-Url found
Francesco Romani has restored this change.
Change subject: virt: tolerate missing ovirt-vmconsole group ......................................................................
Restored
automation@ovirt.org has posted comments on this change.
Change subject: virt: abort if missing ovirt-vmconsole group ......................................................................
Patch Set 6:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Dan Kenigsberg has posted comments on this change.
Change subject: virt: abort if missing ovirt-vmconsole group ......................................................................
Patch Set 6: Code-Review+2
Francesco Romani has posted comments on this change.
Change subject: virt: abort if missing ovirt-vmconsole group ......................................................................
Patch Set 7: Verified+1
Works. - run patched VDSM. - renamed ovirt-vmconsole group (same as deleting it, I was lazy). - tricked VDSM to always start new console - booted VM with console device enabled - VM boot fails:
Thread-122::DEBUG::2015-06-22 15:26:29,484::vmchannels::203::vds::(register) Add fileno 28 to listener's channels. Thread-122::WARNING::2015-06-22 15:26:29,485::core::122::vm.Vm::(prepare) vmId=`995a5f50-9f77-479d-9f05-acc00965dd3e`::unknown group ovirt-vmconsole, cannot create console Thread-122::INFO::2015-06-22 15:26:29,492::vm::1294::vm.Vm::(setDownStatus) vmId=`995a5f50-9f77-479d-9f05-acc00965dd3e`::Changed state to Down: VM cannot run on this host due to unsupported device (code=10)
later:
Thread-131::DEBUG::2015-06-22 15:26:41,129::__init__::496::jsonrpc.JsonRpcServer::(_serveRequest) Calling 'VM.destroy' in bridge with {u'vmID': u'995a5f50-9f77-479d-9f05-acc00965dd3e'} JsonRpcServer::DEBUG::2015-06-22 15:26:41,129::__init__::533::jsonrpc.JsonRpcServer::(serve_requests) Waiting for request Thread-131::INFO::2015-06-22 15:26:41,129::API::335::vds::(destroy) vmContainerLock acquired by vm 995a5f50-9f77-479d-9f05-acc00965dd3e Thread-131::DEBUG::2015-06-22 15:26:41,129::vm::3684::vm.Vm::(destroy) vmId=`995a5f50-9f77-479d-9f05-acc00965dd3e`::destroy Called Thread-131::INFO::2015-06-22 15:26:41,130::vm::3616::vm.Vm::(releaseVm) vmId=`995a5f50-9f77-479d-9f05-acc00965dd3e`::Release VM resources Thread-131::WARNING::2015-06-22 15:26:41,130::vm::372::vm.Vm::(_set_lastStatus) vmId=`995a5f50-9f77-479d-9f05-acc00965dd3e`::trying to set state to Powering down when already Down Thread-131::DEBUG::2015-06-22 15:26:41,627::task::595::Storage.TaskManager.Task::(_updateState) Task=`6950b9af-bf28-4cdd-9dc5-f0f0867968d4`::moving from state init -> state preparing libvirtEventLoop::DEBUG::2015-06-22 15:26:41,627::vm::4344::vm.Vm::(_onLibvirtLifecycleEvent) vmId=`995a5f50-9f77-479d-9f05-acc00965dd3e`::event Stopped detail 1 opaque None Thread-131::INFO::2015-06-22 15:26:41,627::logUtils::48::dispatcher::(wrapper) Run and protect: teardownImage(sdUUID=u'12f68692-2a5a-4e48-af5e-4679bca7fd44', spUUID=u'00000002-0002-0002-0002-00000000014b', imgUUID=u'ee1295ee-7ddc-4030-be5e-4557538bc4d2', volUUID=None) libvirtEventLoop::INFO::2015-06-22 15:26:41,627::vm::815::vm.Vm::(_onQemuDeath) vmId=`995a5f50-9f77-479d-9f05-acc00965dd3e`::underlying process disconnected
also:
BENji> 15:26:36 root [~]$ vdsClient -s 0 getAllVmStats
995a5f50-9f77-479d-9f05-acc00965dd3e Status = Down exitMessage = VM cannot run on this host due to unsupported device exitReason = 10 timeOffset = 0 statusTime = 4296290010 exitCode = 1
also verified with virsh -r list that no leftovers remains.
Francesco Romani has posted comments on this change.
Change subject: virt: abort if missing ovirt-vmconsole group ......................................................................
Patch Set 7: Code-Review-1
We discussed the vmconsole upgrade path inside the virt group, so please let's wait for Michal's ACK here.
automation@ovirt.org has posted comments on this change.
Change subject: virt: abort if missing ovirt-vmconsole group ......................................................................
Patch Set 7:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: virt: abort if missing ovirt-vmconsole group ......................................................................
Patch Set 8:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Michal Skrivanek has posted comments on this change.
Change subject: virt: abort if missing ovirt-vmconsole group ......................................................................
Patch Set 8: Code-Review+1
Francesco Romani has posted comments on this change.
Change subject: virt: abort if missing ovirt-vmconsole group ......................................................................
Patch Set 8: -Code-Review
automation@ovirt.org has posted comments on this change.
Change subject: virt: abort if missing ovirt-vmconsole group ......................................................................
Patch Set 9:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Francesco Romani has posted comments on this change.
Change subject: virt: abort if missing ovirt-vmconsole group ......................................................................
Patch Set 9: Verified+1
forgot to document the new vmexitreason in schema. Now fixed. re-verified with make-check the new tiny change since last upload.
Michal Skrivanek has posted comments on this change.
Change subject: virt: abort if missing ovirt-vmconsole group ......................................................................
Patch Set 9: Code-Review-1
I would favor solution in 42479. Then this is not needed
Francesco Romani has abandoned this change.
Change subject: virt: abort if missing ovirt-vmconsole group ......................................................................
Abandoned
automation@ovirt.org has posted comments on this change.
Change subject: virt: abort if missing ovirt-vmconsole group ......................................................................
Patch Set 9:
* Update tracker::IGNORE, no Bug-Url found
vdsm-patches@lists.fedorahosted.org