KVM provider: Fixed Windows support #465

Merged
merged 7 commits into from Jan 10, 2013

Conversation

Projects
None yet
3 participants
Contributor

ffeldhaus commented Jan 6, 2013

Support for Windows operating systems was broken in the KVM provider. This pull request fixes open issues with the floppy drive which is needed for the unattended installation. This pull request also adds a cdrom drive with the latest windows virtio drivers.

This fix has been tested with the Windows 7 templates and should also work with Windows 2008 and Windows 8 templates.

ffeldhaus added some commits Jan 6, 2013

@ffeldhaus ffeldhaus KVM provider: Fixed undef method 9976627
@ffeldhaus ffeldhaus KVM provider: Fixed Windows support
* fixed adding of floppy drive to KVM machines
* support for adding virtio drivers to KVM machines
827f84e

@ffeldhaus thanks! interesting work for the virtio drivers.

I'm wondering why you switched from the @raw object to the qemu:///system system?
I personally use kvm over ssh and therefore have another connections string. this is going through $HOME/.fog to specify things

Contributor

ffeldhaus commented Jan 7, 2013

For me the raw connection was not working with Fog. As I couldn't find any documentation I used qemu:///system as it is done in the methods check_default_network and check_pool. If you could point me to some documentation for the Fog code regarding the raw connection, I'd be glad to go through the KVM provider and replace all occurrences of qemu:///system with the raw connection as I guess it would make the connection handling more generic?

Contributor

ffeldhaus commented Jan 7, 2013

I found the issue with the Fog raw connection. In one of the last versions the name was changed from raw to client. As the documentation is sparse, I couldn't find it directly.
As the KVM provider mixes the usage of Fog with the direct usage of Libvirt, I would suggest to go through all KVM classes and replace all occurances of Libvirt with Fog. I would volunteer to do the work, but suggest to do it in another pull request. As this pull request shouldn't break any previous functionality, I would be glad if it could be merged before I start with the next pull request, so that I could include these classes in the next pull request (and keep additional functionality like in this pull request separate from a KVM provider code cleanup).

Owner

jedi4ever commented Jan 8, 2013

IIRC one of the reasons I was using libvirt system directly was to test if kvm had a default pool configured.

but yes I agree that it makes sense to do all via raw.

Contributor

ffeldhaus commented Jan 8, 2013

Ok, then I will go through the KVM classes, rewrite them and create a separate pull request.

Owner

jedi4ever commented Jan 8, 2013

Realized that we might have to enforce an recent enough fog version for this. to make sure it works.

ffeldhaus added some commits Jan 9, 2013

@ffeldhaus ffeldhaus Merge branch 'master' of git://github.com/jedi4ever/veewee into kvm-w…
…indows
7b9d55b
@ffeldhaus ffeldhaus KVM Provider
- improved code structure
48e613d
@ffeldhaus ffeldhaus KVM Provider
- improved checking of network and storage pool during creation of servers
- replaced not properly implemented KVM options with one option for the network name and one for the storage pool name
- replaced all occurrences of direct libvirt usage with fog.io in create
- improved documentation to reflect the changes above
de8b1a7
Contributor

ffeldhaus commented Jan 10, 2013

I updated this pull request to be in line with the changes introduced in pull request #472. As the previously available options were not listed in lib/veewee/command/kvm.rb I checked the libvirt documentation and came up with only the two options pool-name and network-name. If libvirt is properly set up with a storage pool and a network definition, these two options should be sufficient.

Owner

jedi4ever commented Jan 10, 2013

@ffeldhaus seems I can't auto merge with the current state? could you check?

Contributor

ffeldhaus commented Jan 10, 2013

There was something missing in the first merge I did. You should be able to merge the pull request now.

Owner

jedi4ever commented Jan 10, 2013

Page still shows - This pull request cannot be automatically merged.

@jedi4ever jedi4ever added a commit that referenced this pull request Jan 10, 2013

@jedi4ever jedi4ever Merge pull request #465 from ffeldhaus/kvm-windows
KVM provider: Fixed Windows support
1b78473

@jedi4ever jedi4ever merged commit 1b78473 into jedi4ever:master Jan 10, 2013

Owner

jedi4ever commented Jan 10, 2013

@ffeldhaus it worked . thanks!

This really belongs in the Autounattend.xml file in the definition rather than silently doing that behind the user's back. That's asking for all kinds of trouble.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment