Use IDE as the bus type for root disks and VIRTIO for data disks on platforms without support for para virtualization when using managed storage - #3319
Conversation
|
@skattoju3 thanks we will test it and come back tomorrow. |
|
@skattoju4 so -1 for me. |
|
Maybe we talk past each other. It should only set the first disk to ide device and all other to virtio like in the bug Report #3089 described. We don’t want to break things and we don’t want only virtio disks. Simple it should be The bug with solidfire / iscsi we got is Cdrom ide @skattoju4 is that what I understand it Correctly? @syed what do you think? |
|
@svenvogel |
|
You are right. That should not be the solution. I understand. Let’s investigate and fix that. What I wrote above the goal was another. |
|
I agree with @svenvogel. The first disk should always be IDE. The subsequent disks can be virtio. |
… or platform emulator string
|
@skattoju4 @svenvogel @syed |
|
@ustcweizhou the thinking behind always setting the root disk to ide was to be able to boot a windows vm regardless of whether or not virtio drivers were installed. That said setting all disks to virtio when selecting "Windows PV" when registering the template is one way to move forward. @svenvogel what do you think ? |
|
@skattoju4 @ustcweizhou @syed i will try to explain it.
first i have to say is yes! let me explain. as written in #3089 we have "NOT" only the problem with template! When we select "Windows Server 2016 (64-bit)" as OS Type for an W2K16 cd image the resulting
this pr should "NOT" remove/break or add functionality. it should enable the normal functionality for managed storage according to NFS works and it is needed by QEMU if we choose "Windows Server 201X..." as OS Type. it is a bug that will affect only managed storage (as far as i i know). If any user wants "ONLY" virtio driver he can choose "other pv ..." OS Type. there is no discussion about performance of IDE or move all Microsoft Windows OS Types to virtio. it should be clear for everybody here that virtio is faster than ide and yes we dont need a driver for the root disk. but this content is not part of this discussion here. i hope it is more clear now? |
|
@svenvogel thanks for you explanation. so please change the code to following if possible. |
|
@ustcweizhou no we will check this before and have a look on the PR that we dont break the virtio. @syed @skattoju4 what do think, can we add this? |
|
Sounds good to me. @skattoju4 can you take care of it? |
|
Also note that as of 4.12, kvm enlightenment is also enabled via Windows
PV. I'm working on a PR to also allow it to be enabled by the vmdetails
feature.
…On Sun, May 12, 2019, 7:06 PM Syed Mushtaq Ahmed ***@***.***> wrote:
Sounds good to me. @skattoju4 <https://github.com/skattoju4> can you take
care of it?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#3319 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AED2J4TGINWFF2S5Z2HFHDDPVCWHRANCNFSM4HLS5PTA>
.
|
|
@kiwiflyer Such a feature would a way in the direction of further untangling this os-type problem. I am looking forward to your pr. |
@skattoju4 @syed do you think the fix from @ustcweizhou will fix this? @kiwiflyer better configuration options for the os type sounds good. |
|
@sven , @ustcweizhou 's suggestion sounds reasonable. I think this is ok for now but ideally this should be handle in a better way. @kiwiflyer could you link your PR please ? |
skattoju4
changed the title
skattoju4
changed the title
|
@skattoju4 do you mean we should test it? i think @kiwiflyer 's PR is out of scope of this one. right? |
|
@svenvogel now we should have the same behavior as nfs when selecting os type Windows* (except Windows PV) I think it might be better to select Windows PV and have all disk be virtio when para-virtualization is supported. With the current code vm created from iso templates with os type Windows* (Except Windows PV) will have an ide root disk but there is no way to attach a data disk as IDE. (one additional ide disk is possible before the error is triggered if i am not mistaken) I'm guessing @kiwiflyer 's PR will enable setting the disk controller when registering an iso similar to how this can be specified when registering a template. I think this would be a cleaner solution than trying to guess the controller / bus type based on the os type string. |
|
@skattoju4 we will test it
okay there should not be a second ide disk right? |
|
yes, just the root disk will be ide now.
…On Tue., May 14, 2019, 8:09 a.m. Sven Vogel, ***@***.***> wrote:
@skattoju4 <https://github.com/skattoju4> we will test it
(one additional ide disk is possible before the error is triggered if i am
not mistaken)
unfortunately no. only one ide disk so what i know.
okay there should not be a second ide disk right?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3319?email_source=notifications&email_token=AK5V4KW2DKCJIWHQZFVNC5LPVKTZJA5CNFSM4HLS5PTKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVLIT2Y#issuecomment-492210667>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AK5V4KUTLQ4CKOTQVAP3METPVKTZJANCNFSM4HLS5PTA>
.
|
|
@skattoju4 @svenvogel I will build this today and testing can happen the following days. |
|
To be clear, the feature that allows selection of root disk controller for a template using vmdetails has been in cloudstack for a couple of major versions now -
|
|
@svenvogel yeah @ustcweizhou suggestion is cleaner / more generic the windows specific logic can be removed. As pointed out the issue is guest OSs without pv support using managed storage. (sorry missed previous messages) |
|
@skattoju4 should we change it like the suggestion of @ustcweizhou? i mean the ide managed storage... |
|
@svenvogel it sounds logical will test and update the pr |
|
@skattoju4 sound good. thx! |
|
@skattoju4 good, looking forward to your update. |
Are there any Ubuntu versions which don't support PV ? |
…isks so that data disks are always virtio (or iscsi) like with nfs (or file based disks)
as far as I know, pv is supported by all ubuntu versions in past 10 years. |
|
Looks like there are still on-going discussions with no clear consensus. Because of this I've removed the milestone on this PR, kindly ping me when a consensus is reached. If this could make into 4.13.0.0 before the freeze I'll be happy to help run tests, otherwise we can revisit this after 4.13.0.0, either in 4.13.1.0 or 4.14.0.0. |
| platformEmulator.startsWith("Debian GNU/Linux") || | ||
| platformEmulator.startsWith("FreeBSD") || | ||
| platformEmulator.startsWith("Oracle") || | ||
| platformEmulator.startsWith("Windows PV") || |
There was a problem hiding this comment.
actually the platformEmulator is "Other PV" if guest os type is "Windows PV".
so line 3212 is not needed
mysql> select * from guest_os where display_name='Windows PV';
+-----+-------------+------+--------------------------------------+--------------+---------------------+---------+-----------------+---------+
| id | category_id | name | uuid | display_name | created | removed | is_user_defined | display |
+-----+-------------+------+--------------------------------------+--------------+---------------------+---------+-----------------+---------+
| 160 | 6 | NULL | e1990eb3-4b9d-11e3-8346-2c44fd7a3378 | Windows PV | 2016-07-27 10:48:33 | NULL | 0 | 0 |
+-----+-------------+------+--------------------------------------+--------------+---------------------+---------+-----------------+---------+
1 row in set (0.00 sec)
mysql> select * from guest_os_hypervisor where guest_os_id=160;
+------+-----------------+---------------+-------------+--------------------+--------------------------------------+---------------------+---------+-----------------+
| id | hypervisor_type | guest_os_name | guest_os_id | hypervisor_version | uuid | created | removed | is_user_defined |
+------+-----------------+---------------+-------------+--------------------+--------------------------------------+---------------------+---------+-----------------+
| 1264 | KVM | Other PV | 160 | default | e768e05d-53d6-11e6-8d7a-2c44fd7a3454 | 2016-07-27 10:48:35 | NULL | 0 |
| 2114 | LXC | Other PV | 160 | default | e965179c-53d6-11e6-8d7a-2c44fd7a3454 | 2016-07-27 10:48:38 | NULL | 0 |
+------+-----------------+---------------+-------------+--------------------+--------------------------------------+---------------------+---------+-----------------+
2 rows in set (0.00 sec)
There was a problem hiding this comment.
@ustcweizhou does it select "display_name" or "guest_os_name"? you mean we dont need "Windows PV" in the list right?
There was a problem hiding this comment.
@svenvogel yes, we do not need "Windows PV" in the line.
the platformEmulator is "Other PV" when guest os is "Windows PV".
@rhtyd |
skattoju4
changed the title
ustcweizhou
left a comment
ustcweizhou
left a comment
There was a problem hiding this comment.
approved
please squash the commits into one commit.
|
@skattoju4 is it ready for merge? |
|
Let me also help review and test it @svenvogel |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-114 |
|
@blueorangutan test |
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-146)
|
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-150)
|
Correção no _script_ de _upgrade_ para a versão `4.20.0.8-scclouds` Closes #3319 See merge request scclouds/scclouds!1371
9 participants
Description
This change addresses #3089. There was an issue when disks were being added with bus type IDE when creating windows VMs from ISOs. It is not possible to select bus type when creating a VM with an ISO. The bus type is inferred based on the platform emulator string provided to the KVM agent. Currently when creating a VM with managed storage (ex: Solidfire) and OS type string Windows*, all disks are added as IDE. Qemu currently does not support multiple IDE controllers and this configuration results in VMs that cannot be started. This issue does not occur when using NFS as the storage provider due to logic in that KVM agent that makes all data volumes (non root) use a virtio controller for file based disk. Similar logic was added for raw physical disks so that managed storage has the same behavior as NFS. In addition specific versions were removed from the code that guesses the disk controller to be used based on the platform emulator string since most modern operating systems support virtio.
steps to reproduce are described in #3089
Types of changes
How Has This Been Tested?
In a Cloudstack system with Solidfire managed primary storage and a KVM host the following steps were performed:
virsh dumpxml <id>on the KVM host