CLOUDSTACK-10160: Fix typo in Libvirt XML definition for Virtio-SCSI by wido · Pull Request #2341 · apache/cloudstack

wido · 2017-11-27T12:02:06Z

The attribute for the XML element 'controller' should be 'model' and
not 'mode'.

Source: https://libvirt.org/formatdomain.html#elementsControllers

A scsi controller has an optional attribute model, which is one of
'auto', 'buslogic', 'ibmvscsi', 'lsilogic', 'lsisas1068', 'lsisas1078',
'virtio-scsi' or 'vmpvscsi'.

In the current state a regular SCSI device is attached and not a Virtio-SCSI
device.

Afterwards this is what lspci reports:

root@ub-scsi:~# lspci |grep -i scsi
00:09.0 SCSI storage controller: Red Hat, Inc Virtio SCSI
root@ub-scsi:~#

The attribute for the XML element 'controller' should be 'model' and not 'mode'. Source: https://libvirt.org/formatdomain.html#elementsControllers A scsi controller has an optional attribute model, which is one of 'auto', 'buslogic', 'ibmvscsi', 'lsilogic', 'lsisas1068', 'lsisas1078', 'virtio-scsi' or 'vmpvscsi'. In the current state a regular SCSI device is attached and not a Virtio-SCSI device. Signed-off-by: Wido den Hollander <wido@widodh.nl>

wido · 2017-11-27T12:04:07Z

Preferably this one would be merged to 4.10 as well so that we could fix it in a hotfix there.

Otherwise 4.11

yadvr

LGTM, simple change.

yadvr · 2017-11-27T12:18:29Z

@wido the 4.10 maintainer may choose to backport (cherry-pick it)
@blueorangutan package

blueorangutan · 2017-11-27T12:19:31Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

To make sure the XML output string is correct Signed-off-by: Wido den Hollander <wido@widodh.nl>

blueorangutan · 2017-11-27T12:54:32Z

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1308

wido · 2017-11-27T12:55:03Z

Thanks @rhtyd !

I've also added a UnitTest in a second commit to make sure this is tested.

yadvr · 2017-11-27T14:04:05Z

@blueorangutan test

blueorangutan · 2017-11-27T14:04:31Z

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

nathanejohnson

Good catch!

wido · 2017-11-27T15:30:44Z

@nathanejohnson: Not to point a finger, but without this it never worked properly. Although it seemed to work as VirtIO-SCSI it was actually just plain SCSI.

nathanejohnson · 2017-11-27T16:35:45Z

@wido when using linux, it recognizes as a VIRTIO-Scsi HBA, and when spinning up Windows guests with the Virtio SCSI drivers installed it recognizes the storage appropriately. That doesn't "prove" it's actually using it as virtio-scsi, but it's possible when the "model" is not specified (or incorrectly specified) it might default to "auto", which might do the right thing based on OS support? I'm guessing here.

blueorangutan · 2017-11-28T00:05:26Z

Trillian test result (tid-1704)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36050 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2341-t1704-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_host_annotations.py
Intermitten failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_templates.py
Intermitten failure detected: /marvin/tests/smoke/test_volumes.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Test completed. 63 look OK, 4 have error(s)

Test	Result	Time (s)	Test File
test_01_vpc_remote_access_vpn	`Failure`	45.87	test_vpc_vpn.py
test_07_resize_fail	`Failure`	15.39	test_volumes.py
test_04_rvpc_privategw_static_routes	`Failure`	228.46	test_privategw_acl.py
test_03_vpc_privategw_restart_vpc_cleanup	`Failure`	112.75	test_privategw_acl.py
test_02_vpc_privategw_static_routes	`Failure`	218.42	test_privategw_acl.py
test_01_vpc_privategw_acl	`Failure`	61.70	test_privategw_acl.py
test_05_add_annotation_for_invalid_entityType	`Error`	0.07	test_host_annotations.py
test_change_service_offering_for_vm_with_snapshots	Skipped	0.00	test_vm_snapshots.py
test_09_copy_delete_template	Skipped	0.03	test_templates.py
test_06_copy_template	Skipped	0.00	test_templates.py
test_static_role_account_acls	Skipped	0.02	test_staticroles.py
test_11_ss_nfs_version_on_ssvm	Skipped	0.02	test_ssvm.py
test_01_scale_vm	Skipped	0.00	test_scale_vm.py
test_01_primary_storage_iscsi	Skipped	0.14	test_primary_storage.py
test_vm_nic_adapter_vmxnet3	Skipped	0.00	test_nic_adapter_type.py
test_03_nic_multiple_vmware	Skipped	1.14	test_nic.py
test_nested_virtualization_vmware	Skipped	0.00	test_nested_virtualization.py
test_06_copy_iso	Skipped	0.00	test_iso.py
test_list_ha_for_host_valid	Skipped	0.02	test_hostha_simulator.py
test_list_ha_for_host_invalid	Skipped	0.02	test_hostha_simulator.py
test_list_ha_for_host	Skipped	0.03	test_hostha_simulator.py
test_hostha_enable_feature_without_setting_provider	Skipped	0.03	test_hostha_simulator.py
test_hostha_enable_feature_valid	Skipped	0.02	test_hostha_simulator.py
test_hostha_disable_feature_valid	Skipped	0.02	test_hostha_simulator.py
test_hostha_configure_invalid_provider	Skipped	0.03	test_hostha_simulator.py
test_hostha_configure_default_driver	Skipped	0.02	test_hostha_simulator.py
test_ha_verify_fsm_recovering	Skipped	0.03	test_hostha_simulator.py
test_ha_verify_fsm_fenced	Skipped	0.02	test_hostha_simulator.py
test_ha_verify_fsm_degraded	Skipped	0.02	test_hostha_simulator.py
test_ha_verify_fsm_available	Skipped	0.02	test_hostha_simulator.py
test_ha_multiple_mgmt_server_ownership	Skipped	0.02	test_hostha_simulator.py
test_ha_list_providers	Skipped	0.03	test_hostha_simulator.py
test_ha_enable_feature_invalid	Skipped	0.03	test_hostha_simulator.py
test_ha_disable_feature_invalid	Skipped	0.04	test_hostha_simulator.py
test_ha_configure_enabledisable_across_clusterzones	Skipped	0.11	test_hostha_simulator.py
test_configure_ha_provider_valid	Skipped	0.03	test_hostha_simulator.py
test_configure_ha_provider_invalid	Skipped	0.04	test_hostha_simulator.py
test_deploy_vgpu_enabled_vm	Skipped	0.04	test_deploy_vgpu_enabled_vm.py
test_3d_gpu_support	Skipped	0.04	test_deploy_vgpu_enabled_vm.py

yadvr · 2017-11-28T04:11:16Z

Lgtm, any further improvements may be submitted in separate PRs. Merging now, thanks.

…ouds' Alteração nas exceções quando nenhuma estratégia suporta a criação de VM _snapshot_ Closes apache#2341 See merge request scclouds/scclouds!1191

wido added component:kvm component:libvirt labels Nov 27, 2017

wido added this to the 4.11 milestone Nov 27, 2017

wido requested review from nathanejohnson and yadvr November 27, 2017 12:02

yadvr approved these changes Nov 27, 2017

View reviewed changes

rafaelweingartner approved these changes Nov 27, 2017

View reviewed changes

CLOUDSTACK-10160: Add UnitTest for LibvirtVMDef.SCSIDef

2e747ff

To make sure the XML output string is correct Signed-off-by: Wido den Hollander <wido@widodh.nl>

nathanejohnson approved these changes Nov 27, 2017

View reviewed changes

yadvr merged commit fdf2509 into apache:master Nov 28, 2017

Conversation

wido commented Nov 27, 2017

Uh oh!

wido commented Nov 27, 2017

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr commented Nov 27, 2017

Uh oh!

blueorangutan commented Nov 27, 2017

Uh oh!

blueorangutan commented Nov 27, 2017

Uh oh!

wido commented Nov 27, 2017

Uh oh!

yadvr commented Nov 27, 2017

Uh oh!

blueorangutan commented Nov 27, 2017

Uh oh!

nathanejohnson left a comment

Choose a reason for hiding this comment

Uh oh!

wido commented Nov 27, 2017

Uh oh!

nathanejohnson commented Nov 27, 2017

Uh oh!

blueorangutan commented Nov 28, 2017

Uh oh!

yadvr commented Nov 28, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants