Francesco Romani has posted comments on this change.

Change subject: virt: utils: add is_kvm helper
......................................................................


Patch Set 28:

(4 comments)

https://gerrit.ovirt.org/#/c/55647/28//COMMIT_MSG
Commit Message:

PS28, Line 9: Proper container support needs some changes into oVirt Engine.
            : First of all, Engine should send a different vmType from the
            : default (hardcoded) 'kvm' to select the container runtime to be 
used.
            : 
            : Until the Engine patch is ready, we use this hack to get
            : the container runtime from a custom property.
this is stale and needs to be updated.


https://gerrit.ovirt.org/#/c/55647/28/lib/vdsm/virt/utils.py
File lib/vdsm/virt/utils.py:

PS28, Line 205: conf
> s/conf/vm_conf
Yes, better, will fix


PS28, Line 210: vm_type = conf.get('vmType')
> Idea: is there an issue with 'vmType': 'container'?
The initial plan was to abuse vmType like

vmType=container:rkt

or something like this.
But now I'm abusing custom properties, which cause much less trouble and it is 
a good way to go.
I'm keeping the 'vmType'=='kvm' check mostly as fallback, the real deal is the 
inspection of custom properties above.


https://gerrit.ovirt.org/#/c/55647/28/tests/vmUtilsTests.py
File tests/vmUtilsTests.py:

PS28, Line 224:     def test_container_type(self):
              :         self.assertFalse(utils.is_kvm({
              :             'vmType': 'kvm',
              :             'custom': {
              :                 'containerType': 'rkt',
              :             },
              :         }))
> The usage of different key and it's implicit exclusivity with vmType makes 
Sorry, I don't follow. What would you like me to do here?

'vmType'=='kvm' is supposed to be here - and always here;
custom.containerType == 'something' is supposed to override it, to let Vdsm 
understand what Engine is asking.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f9e1b8f9326e565ee7324d0b328100ca86c6967
Gerrit-PatchSet: 28
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Reply via email to