machines: Move reusable code to libvirt-common.es6 #9692
Conversation
…libvirt-common libvirt-common file will contain all functions that will be shared amongst libvirt providers. Signed-off-by: Katerina Koukiou <[email protected]>
Signed-off-by: Katerina Koukiou <[email protected]>
All dependent functions were also moved. Signed-off-by: Katerina Koukiou <[email protected]>
Signed-off-by: Katerina Koukiou <[email protected]>
Signed-off-by: Katerina Koukiou <[email protected]>
Signed-off-by: Katerina Koukiou <[email protected]>
Signed-off-by: Katerina Koukiou <[email protected]>
Signed-off-by: Katerina Koukiou <[email protected]>
Signed-off-by: Katerina Koukiou <[email protected]>
Signed-off-by: Katerina Koukiou <[email protected]>
Signed-off-by: Katerina Koukiou <[email protected]>
vm.status is not defined, we need to check vm.state value. Signed-off-by: Katerina Koukiou <[email protected]>
Signed-off-by: Katerina Koukiou <[email protected]>
All these functions are going to be reused by the libvirtDBus provider that will be added in following patch. Signed-off-by: Katerina Koukiou <[email protected]>
This function will be reused in LibvirtDBus provider but there we 'll need to overwrite the 'id' attribute. Thus the extra optional parameter 'id_overwrite' was added. When not passed the function will keep its current behavior. Signed-off-by: Katerina Koukiou <[email protected]>
Signed-off-by: Katerina Koukiou <[email protected]>
…common The following functions were moved and stored in libvirt-common in alphabetical order: - canConsole, - canDelete, - canInstall, - canReset, - canRun, - canSendNMI, - canShutdown, - isRunning, - serialConsoleCommand, Signed-off-by: Katerina Koukiou <[email protected]>
Signed-off-by: Katerina Koukiou <[email protected]>
Signed-off-by: Katerina Koukiou <[email protected]>
|
All these functions that I moved can be reused in other provider files. |
| @@ -64,7 +64,7 @@ class VmDisksTabLibvirt extends React.Component { | |||
| return null; | |||
| } | |||
|
|
|||
| if (vm.status === 'running') { | |||
| if (vm.state === 'running') { | |||
There was a problem hiding this comment.
There's a similar thing here:
pkg/kubernetes/scripts/virtual-machines/components/VmsListingRow.jsx: const phase = (vm.status && vm.status.phase) || _("n/a");
but there vm.status seems to be an object, while here it's a string. Are these two different statuses really?
There was a problem hiding this comment.
I am not familiar with k8s package, I am not sure if it's problem that there as well.
For the commit here we need to check the vm.state string as set here, https://github.com/cockpit-project/cockpit/blob/master/pkg/machines/libvirt.es6#L1048
vm.status is clearly a typo, not initialized anywhere.
There was a problem hiding this comment.
Indeed, thanks! Sorry, I totally missed the /kubernetes, I just saw the "machines" there :)
There was a problem hiding this comment.
Looks all fine to me, and the justification is #9240 -- we are going to need both providers at least for some time. I just have one question, but it's unrelated to this PR.
Splitting #9240 into smaller pieces to make merge process quicker.