On 14-10-2010 08:26, Chiradeep Vittal wrote:
> === added file 'nova/tests/hyperv_unittest.py'
> --- nova/tests/hyperv_unittest.py	1970-01-01 00:00:00 +0000
> +++ nova/tests/hyperv_unittest.py	2010-10-14 06:26:41 +0000
> @@ -0,0 +1,67 @@
> +# vim: tabstop=4 shiftwidth=4 softtabstop=4
> +#
> +#    Copyright 2010 Cloud.com, Inc
> +#
> +#    Licensed under the Apache License, Version 2.0 (the "License");
you may
> +#    not use this file except in compliance with the License. You may
obtain
> +#    a copy of the License at
> +#
> +#         http://www.apache.org/licenses/LICENSE-2.0
> +#
> +#    Unless required by applicable law or agreed to in writing, software
> +#    distributed under the License is distributed on an "AS IS"
BASIS, WITHOUT
> +#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the
> +#    License for the specific language governing permissions and
limitations
> +#    under the License.
> +"""
> +Tests For Hyper-V driver
> +"""
> +
> +import random
> +
> +from nova import db
> +from nova import flags
> +from nova import test
> +
> +from nova.virt import hyperv
> +
> +FLAGS = flags.FLAGS
> +FLAGS.connection_type = 'hyperv'
> +# Redis is probably not  running on Hyper-V host.
> +# Change this to the actual Redis host
> +FLAGS.redis_host = '127.0.0.1'
> +
> +
> +class HyperVTestCase(test.TrialTestCase):
> +    """Test cases for the Hyper-V driver"""
> +    def setUp(self):  # pylint: disable-msg=C0103
> +        pass

Why define it at all?

> +
> +    def test_create_destroy(self):
> +        """Create a VM and destroy it"""
> +        instance = {'internal_id'     : random.randint(1, 1000000),
> +                     'memory_mb'      : '1024',
> +                     'mac_address'    : '02:12:34:46:56:67',
> +                     'vcpus'          : 2,
> +                     'project_id'     : 'fake',
> +                     'instance_type'  : 'm1.small'}
> +
> +        instance_ref = db.instance_create(None, instance)
> +
> +        conn = hyperv.get_connection(False)
> +        conn._create_vm(instance_ref)  # pylint: disable-msg=W0212
> +        found = [n  for n in conn.list_instances()
> +                      if n == instance_ref['name']]
> +        self.assertTrue(len(found) == 1)
> +        info = conn.get_info(instance_ref['name'])
> +        #Unfortunately since the vm is not running at this point,
> +        #we cannot obtain memory information from get_info
> +        self.assertEquals(info['num_cpu'], instance_ref['vcpus'])
> +
> +        conn.destroy(instance_ref)
> +        found = [n  for n in conn.list_instances()
> +                      if n == instance_ref['name']]
> +        self.assertTrue(len(found) == 0)

Wouldn't this actually run the VM? The purpose of unit tests is to be
able to test things in units, so to verify that each piece of the puzzle
acts the way it should. Ideally, you should include a mock wmi module
that stubs out all the relevant methods so that we take the actual
hypervisor out of the equation.

> +
> +    def tearDown(self):  # pylint: disable-msg=C0103
> +        pass

Again, why define it at all?


> === modified file 'nova/virt/connection.py'
> --- nova/virt/connection.py	2010-08-30 13:19:14 +0000
> +++ nova/virt/connection.py	2010-10-14 06:26:41 +0000
> @@ -26,6 +26,7 @@
>  from nova.virt import fake
>  from nova.virt import libvirt_conn
>  from nova.virt import xenapi
> +from nova.virt import hyperv
>
>
>  FLAGS = flags.FLAGS
> @@ -49,6 +50,8 @@
>          conn = libvirt_conn.get_connection(read_only)
>      elif t == 'xenapi':
>          conn = xenapi.get_connection(read_only)
> +    elif t == 'hyperv':
> +        conn = hyperv.get_connection(read_only)
>      else:
>          raise Exception('Unknown connection type "%s"' % t)
>
>
> === added file 'nova/virt/hyperv.py'
> --- nova/virt/hyperv.py	1970-01-01 00:00:00 +0000
> +++ nova/virt/hyperv.py	2010-10-14 06:26:41 +0000
> @@ -0,0 +1,483 @@
> +# vim: tabstop=4 shiftwidth=4 softtabstop=4
> +
> +# Copyright (c) 2010 Cloud.com, Inc
> +#
> +#    Licensed under the Apache License, Version 2.0 (the "License");
you may
> +#    not use this file except in compliance with the License. You may
obtain
> +#    a copy of the License at
> +#
> +#         http://www.apache.org/licenses/LICENSE-2.0
> +#
> +#    Unless required by applicable law or agreed to in writing, software
> +#    distributed under the License is distributed on an "AS IS"
BASIS, WITHOUT
> +#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the
> +#    License for the specific language governing permissions and
limitations
> +#    under the License.
> +
> +"""
> +A connection to Hyper-V .
> +Uses Windows Management Instrumentation (WMI) calls to interact with
Hyper-V
> +Hyper-V WMI usage:
> +    http://msdn.microsoft.com/en-us/library/cc723875%28v=VS.85%29.aspx
> +The Hyper-V object model briefly:
> +    The physical computer and its hosted virtual machines are each
represented
> +    by the Msvm_ComputerSystem class.
> +
> +    Each virtual machine is associated with a
> +    Msvm_VirtualSystemGlobalSettingData (vs_gs_data) instance and one
or more
> +    Msvm_VirtualSystemSettingData (vmsetting) instances. For each
vmsetting
> +    there is a series of Msvm_ResourceAllocationSettingData (rasd)
objects.
> +    The rasd objects describe the settings for each device in a VM.
> +    Together, the vs_gs_data, vmsettings and rasds describe the
configuration
> +    of the virtual machine.
> +
> +    Creating new resources such as disks and nics involves cloning a
default
> +    rasd object and appropriately modifying the clone and calling the
> +    AddVirtualSystemResources WMI method
> +    Changing resources such as memory uses the
ModifyVirtualSystemResources
> +    WMI method
> +
> +Using the Python WMI library:
> +    Tutorial:
> +        http://timgolden.me.uk/python/wmi/tutorial.html
> +    Hyper-V WMI objects can be retrieved simply by using the class name
> +    of the WMI object and optionally specifying a column to filter the
> +    result set. More complex filters can be formed using WQL (sql-like)
> +    queries.
> +    The parameters and return tuples of WMI method calls can gleaned by
> +    examining the doc string. For example:
> +    >>> vs_man_svc.ModifyVirtualSystemResources.__doc__
> +    ModifyVirtualSystemResources (ComputerSystem, ResourceSettingData[])
> +                 => (Job, ReturnValue)'
> +    When passing setting data (ResourceSettingData) to the WMI method,
> +    an XML representation of the data is passed in using GetText_(1).
> +    Available methods on a service can be determined using method.keys():
> +    >>> vs_man_svc.methods.keys()
> +    vmsettings and rasds for a vm can be retrieved using the
'associators'
> +    method with the appropriate return class.
> +    Long running WMI commands generally return a Job (an instance of
> +    Msvm_ConcreteJob) whose state can be polled to determine when it
finishes
> +
> +"""
> +
> +import os
> +import logging
> +import time
> +
> +from twisted.internet import defer
> +
> +from nova import exception
> +from nova import flags
> +from nova.auth import manager
> +from nova.compute import power_state
> +from nova.virt import images
> +
> +wmi = None
> +
> +
> +FLAGS = flags.FLAGS
> +
> +
> +HYPERV_POWER_STATE = {
> +    3 : power_state.SHUTDOWN,
> +    2 : power_state.RUNNING,
> +    32768 : power_state.PAUSED,
> +}
> +
> +
> +REQ_POWER_STATE = {
> +    'Enabled' : 2,
> +    'Disabled': 3,
> +    'Reboot' : 10,
> +    'Reset' : 11,
> +    'Paused' : 32768,
> +    'Suspended': 32769
> +}
> +
> +
> +WMI_JOB_STATUS_STARTED = 4096
> +WMI_JOB_STATE_RUNNING = 4
> +WMI_JOB_STATE_COMPLETED = 7
> +
> +##### Exceptions
> +
> +
> +class HyperVError(Exception):
> +    """Base Exception class for all hyper-v errors."""
> +    def __init__(self, *args):
> +        Exception.__init__(self, *args)
> +
> +
> +class VmResourceAllocationError(HyperVError):
> +    """Raised when Hyper-V is unable to create or add a resource to
> +    a VM
> +    """
> +    def __init__(self, *args):
> +        HyperVError.__init__(self, *args)
> +
> +
> +class VmOperationError(HyperVError):
> +    """Raised when Hyper-V is unable to change the state of
> +    a VM (start/stop/reboot/destroy)
> +    """
> +    def __init__(self, *args):
> +        HyperVError.__init__(self, *args)
> +
> +

What's the purpose of defining these __init__ methods? The only thing
they seem to do is to refuse kwargs?

I don't think we want driver specific exceptions like this. Exceptions
should generally be generic enough to be shared (at least) across hypervisor
drivers. Can you make them more generic and move them to
nova.exceptions?

> +def get_connection(_):
> +    global wmi
> +    if wmi is None:
> +        wmi = __import__('wmi')
> +    return HyperVConnection()
> +
> +
> +class HyperVConnection(object):
> +    def __init__(self):
> +        self._conn = wmi.WMI(moniker='//./root/virtualization')
> +        self._cim_conn = wmi.WMI(moniker='//./root/cimv2')
> +
> +    def list_instances(self):
> +        """ Return the names of all the instances known to Hyper-V. """
> +        vms = [v.ElementName \
> +                for v in self._conn.Msvm_ComputerSystem(['ElementName'])]
> +        return vms
> +
> +    @defer.inlineCallbacks
> +    def spawn(self, instance):
> +        """ Create a new VM and start it."""
> +        vm = yield self._lookup(instance.name)
> +        if vm is not None:
> +            raise exception.Duplicate('Attempted to create duplicate
name %s' %
> +                            instance.name)
> +
> +        user = manager.AuthManager().get_user(instance['user_id'])
> +        project =
manager.AuthManager().get_project(instance['project_id'])

Hmm, yes, as you point out, libvirt_conn does the same. It's still not
the right thing to do, though :) There's no point instantiating
AuthManager for each operation.

> +        #Fetch the file, assume it is a VHD file.
> +        base_vhd_filename = os.path.join(FLAGS.instances_path,
> +                                         instance['str_id'])
> +        vhdfile = "%s.vhd" % (base_vhd_filename)
> +        yield images.fetch(instance['image_id'], vhdfile, user, project)
> +
> +        try:
> +            yield self._create_vm(instance)
> +
> +            yield self._create_disk(instance['name'], vhdfile)
> +            yield self._create_nic(instance['name'],
instance['mac_address'])
> +
> +            logging.debug('Starting VM %s ', instance.name)
> +            yield self._set_vm_state(instance['name'], 'Enabled')
> +            logging.info('Started VM %s ', instance.name)
> +        except Exception as exn:
> +            logging.error('spawn vm failed: %s', exn)
> +            self.destroy(instance)
> +
> +    def _create_vm(self, instance):
> +        """Create a VM but don't start it.  """
> +        vs_man_svc = self._conn.Msvm_VirtualSystemManagementService()[0]
> +
> +        vs_gs_data = self._conn.Msvm_VirtualSystemGlobalSettingData.new()
> +        vs_gs_data.ElementName = instance['name']
> +        (job, ret_val) = vs_man_svc.DefineVirtualSystem(
> +                                        [], None,
vs_gs_data.GetText_(1))[1:]
> +        if ret_val == WMI_JOB_STATUS_STARTED:
> +            success = self._check_job_status(job)
> +        else:
> +            success = (ret_val == 0)
> +
> +        if not success:
> +            raise VmResourceAllocationException('Failed to create VM %s',
> +                                                 instance.name)
> +
> +        logging.debug('Created VM %s...', instance.name)
> +        vm = self._conn.Msvm_ComputerSystem(ElementName=instance.name)[0]
> +
> +        vmsettings = vm.associators(
> +
wmi_result_class='Msvm_VirtualSystemSettingData')
> +        vmsetting = [s for s in vmsettings
> +                        if s.SettingType == 3][0]  # avoid snapshots
> +        memsetting = vmsetting.associators(wmi_result_class=
> +                                           'Msvm_MemorySettingData')[0]
> +        #No Dynamic Memory, so reservation, limit and quantity are
identical.
> +        mem = long(str(instance['memory_mb']))
> +        memsetting.VirtualQuantity = mem
> +        memsetting.Reservation = mem
> +        memsetting.Limit = mem
> +
> +        (job, ret_val) = vs_man_svc.ModifyVirtualSystemResources(
> +                                        vm.path_(),
[memsetting.GetText_(1)])
> +        logging.debug('Set memory for vm %s...', instance.name)
> +        procsetting = vmsetting.associators(wmi_result_class=
> +
'Msvm_ProcessorSettingData')[0]
> +        vcpus = long(instance['vcpus'])
> +        procsetting.VirtualQuantity = vcpus
> +        procsetting.Reservation = vcpus
> +        procsetting.Limit = vcpus
> +
> +        (job, ret_val) = vs_man_svc.ModifyVirtualSystemResources(
> +                                        vm.path_(),
[procsetting.GetText_(1)])
> +        logging.debug('Set vcpus for vm %s...', instance.name)
> +
> +    def _create_disk(self, vm_name, vhdfile):
> +        """Create a disk and attach it to the vm"""
> +        logging.debug("Creating disk for %s by attaching disk file %s",
> +                        vm_name, vhdfile)
> +        #Find the IDE controller for the vm.
> +        vms = self._conn.MSVM_ComputerSystem(ElementName=vm_name)
> +        vm = vms[0]
> +        vmsettings = vm.associators(
> +                        wmi_result_class='Msvm_VirtualSystemSettingData')
> +        rasds = vmsettings[0].associators(
> +
wmi_result_class='MSVM_ResourceAllocationSettingData')
> +        ctrller = [r for r in rasds
> +                   if r.ResourceSubType == 'Microsoft Emulated IDE
Controller'\
> +                                         and r.Address == "0"]
> +        #Find the default disk drive object for the vm and clone it.
> +        diskdflt = self._conn.query(
> +                  "SELECT * FROM Msvm_ResourceAllocationSettingData \
> +                   WHERE ResourceSubType LIKE 'Microsoft Synthetic
Disk Drive'\
> +                   AND InstanceID LIKE '%Default%'")[0]
> +        diskdrive = self._clone_wmi_obj(
> +                    'Msvm_ResourceAllocationSettingData', diskdflt)
> +        #Set the IDE ctrller as parent.
> +        diskdrive.Parent = ctrller[0].path_()
> +        diskdrive.Address = 0
> +        #Add the cloned disk drive object to the vm.
> +        new_resources = self._add_virt_resource(diskdrive, vm)
> +        if new_resources is None:
> +            raise VmResourceAllocationError('Failed to add diskdrive
to VM %s',
> +                                             vm_name)
> +        diskdrive_path = new_resources[0]
> +        logging.debug("New disk drive path is %s", diskdrive_path)
> +        #Find the default VHD disk object.
> +        vhddefault = self._conn.query(
> +                "SELECT * FROM Msvm_ResourceAllocationSettingData \
> +                 WHERE ResourceSubType LIKE 'Microsoft Virtual Hard
Disk' AND \
> +                 InstanceID LIKE '%Default%' ")[0]
> +
> +        #Clone the default and point it to the image file.
> +        vhddisk = self._clone_wmi_obj(
> +                'Msvm_ResourceAllocationSettingData', vhddefault)
> +        #Set the new drive as the parent.
> +        vhddisk.Parent = diskdrive_path
> +        vhddisk.Connection = [vhdfile]
> +
> +        #Add the new vhd object as a virtual hard disk to the vm.
> +        new_resources = self._add_virt_resource(vhddisk, vm)
> +        if new_resources is None:
> +            raise VmResourceAllocationError('Failed to add vhd file
to VM %s',
> +                                             vm_name)
> +        logging.info("Created disk for %s ", vm_name)
> +
> +    def _create_nic(self, vm_name, mac):
> +        """Create a (emulated) nic and attach it to the vm"""
> +        logging.debug("Creating nic for %s ", vm_name)
> +        #Find the vswitch that is connected to the physical nic.
> +        vms = self._conn.Msvm_ComputerSystem(ElementName=vm_name)
> +        extswitch = self._find_external_network()
> +        vm = vms[0]
> +        switch_svc = self._conn.Msvm_VirtualSwitchManagementService()[0]
> +        #Find the default nic and clone it to create a new nic for
the vm.
> +        #Use Msvm_SyntheticEthernetPortSettingData for Windows or
Linux with
> +        #Linux Integration Components installed.
> +        emulatednics_data =
self._conn.Msvm_EmulatedEthernetPortSettingData()
> +        default_nic_data = [n for n in emulatednics_data
> +                            if n.InstanceID.rfind('Default') > 0]
> +        new_nic_data = self._clone_wmi_obj(
> +
'Msvm_EmulatedEthernetPortSettingData',
> +                                      default_nic_data[0])
> +        #Create a port on the vswitch.
> +        (new_port, ret_val) = switch_svc.CreateSwitchPort(vm_name,
vm_name,
> +                                            "", extswitch.path_())
> +        if ret_val != 0:
> +            logging.error("Failed creating a new port on the external
vswitch")
> +            raise VmResourceAllocationError('Failed creating port for
%s',
> +                                             vm_name)
> +        logging.debug("Created switch port %s on switch %s",
> +                        vm_name, extswitch.path_())
> +        #Connect the new nic to the new port.
> +        new_nic_data.Connection = [new_port]
> +        new_nic_data.ElementName = vm_name + ' nic'
> +        new_nic_data.Address = ''.join(mac.split(':'))
> +        new_nic_data.StaticMacAddress = 'TRUE'
> +        #Add the new nic to the vm.
> +        new_resources = self._add_virt_resource(new_nic_data, vm)
> +        if new_resources is None:
> +            raise VmResourceAllocationError('Failed to add nic to VM %s',
> +                                             vm_name)
> +        logging.info("Created nic for %s ", vm_name)
> +
> +    def _add_virt_resource(self, res_setting_data, target_vm):
> +        """Add a new resource (disk/nic) to the VM"""
> +        vs_man_svc = self._conn.Msvm_VirtualSystemManagementService()[0]
> +        (job, new_resources, ret_val) = vs_man_svc.\
> +
AddVirtualSystemResources([res_setting_data.GetText_(1)],
> +                                                target_vm.path_())
> +        success = True
> +        if ret_val == WMI_JOB_STATUS_STARTED:
> +            success = self._check_job_status(job)
> +        else:
> +            success = (ret_val == 0)
> +        if success:
> +            return new_resources
> +        else:
> +            return None
> +
> +    #TODO: use the reactor to poll instead of sleep
> +    def _check_job_status(self, jobpath):
> +        """Poll WMI job state for completion"""
> +        #Jobs have a path of the form:
> +
#\\WIN-P5IG7367DAG\root\virtualization:Msvm_ConcreteJob.InstanceID="8A496B9C-AF4D-4E98-BD3C-1128CD85320D"
> +        inst_id = jobpath.split(':')[1].split('=')[1].strip('\"')

Why not just

   jobpath.split('=')[1].strip('"')

?

> +        jobs = self._conn.Msvm_ConcreteJob(InstanceID=inst_id)
> +        if len(jobs) == 0:
> +            return False
> +        job = jobs[0]
> +        while job.JobState == WMI_JOB_STATE_RUNNING:
> +            time.sleep(0.1)
> +            job = self._conn.Msvm_ConcreteJob(InstanceID=inst_id)[0]
> +        if job.JobState != WMI_JOB_STATE_COMPLETED:
> +            logging.debug("WMI job failed: %s",
job.ErrorSummaryDescription)
> +            return False
> +        logging.debug("WMI job succeeded: %s, Elapsed=%s ",
job.Description,
> +                      job.ElapsedTime)
> +        return True
> +
> +    def _find_external_network(self):
> +        """Find the vswitch that is connected to the physical nic.
> +           Assumes only one physical nic on the host
> +        """
> +        #If there are no physical nics connected to networks, return.
> +        bound = self._conn.Msvm_ExternalEthernetPort(IsBound='TRUE')
> +        if len(bound) == 0:
> +            return None
> +        return self._conn.Msvm_ExternalEthernetPort(IsBound='TRUE')[0]\
> +            .associators(wmi_result_class='Msvm_SwitchLANEndpoint')[0]\
> +            .associators(wmi_result_class='Msvm_SwitchPort')[0]\
> +            .associators(wmi_result_class='Msvm_VirtualSwitch')[0]
> +
> +    def _clone_wmi_obj(self, wmi_class, wmi_obj):
> +        """Clone a WMI object"""
> +        cl = self._conn.__getattr__(wmi_class)  # get the class
> +        newinst = cl.new()
> +        #Copy the properties from the original.
> +        for prop in wmi_obj._properties:
> +            newinst.Properties_.Item(prop).Value =\
> +                    wmi_obj.Properties_.Item(prop).Value
> +        return newinst
> +
> +    @defer.inlineCallbacks
> +    def reboot(self, instance):
> +        """Reboot the specified instance."""
> +        vm = yield self._lookup(instance.name)
> +        if vm is None:
> +            raise exception.NotFound('instance not present %s' %
instance.name)
> +        self._set_vm_state(instance.name, 'Reboot')
> +
> +    @defer.inlineCallbacks
> +    def destroy(self, instance):
> +        """Destroy the VM. Also destroy the associated VHD disk files"""
> +        logging.debug("Got request to destroy vm %s", instance.name)
> +        vm = yield self._lookup(instance.name)
> +        if vm is None:
> +            defer.returnValue(None)
> +        vm = self._conn.Msvm_ComputerSystem(ElementName=instance.name)[0]
> +        vs_man_svc = self._conn.Msvm_VirtualSystemManagementService()[0]
> +        #Stop the VM first.
> +        self._set_vm_state(instance.name, 'Disabled')
> +        vmsettings = vm.associators(wmi_result_class=
> +
'Msvm_VirtualSystemSettingData')
> +        rasds = vmsettings[0].associators(wmi_result_class=
> +
'MSVM_ResourceAllocationSettingData')
> +        disks = [r for r in rasds \
> +                    if r.ResourceSubType == 'Microsoft Virtual Hard
Disk']
> +        diskfiles = []
> +        #Collect disk file information before destroying the VM.
> +        for disk in disks:
> +            diskfiles.extend([c for c in disk.Connection])
> +        #Nuke the VM. Does not destroy disks.
> +        (job, ret_val) = vs_man_svc.DestroyVirtualSystem(vm.path_())
> +        if ret_val == WMI_JOB_STATUS_STARTED:
> +            success = self._check_job_status(job)
> +        elif ret_val == 0:
> +            success = True
> +        if not success:
> +            raise VmOperationError('Failed to destroy vm %s' %
instance.name)
> +        #Delete associated vhd disk files.
> +        for disk in diskfiles:
> +            vhdfile = self._cim_conn.CIM_DataFile(Name=disk)
> +            for vf in vhdfile:
> +                vf.Delete()
> +                logging.debug("Deleted disk %s vm %s", vhdfile,
instance.name)
> +
> +    def get_info(self, instance_id):
> +        """Get information about the VM"""
> +        vm = self._lookup(instance_id)
> +        if vm is None:
> +            raise exception.NotFound('instance not present %s' %
instance_id)
> +        vm = self._conn.Msvm_ComputerSystem(ElementName=instance_id)[0]
> +        vs_man_svc = self._conn.Msvm_VirtualSystemManagementService()[0]
> +        vmsettings = vm.associators(wmi_result_class=
> +                                        'Msvm_VirtualSystemSettingData')
> +        settings_paths = [v.path_() for v in vmsettings]
> +        #See
http://msdn.microsoft.com/en-us/library/cc160706%28VS.85%29.aspx
> +        summary_info = vs_man_svc.GetSummaryInformation(
> +                                       [4, 100, 103, 105],
settings_paths)[1]
> +        info = summary_info[0]
> +        logging.debug("Got Info for vm %s: state=%s, mem=%s,
num_cpu=%s, \
> +                    cpu_time=%s", instance_id,
> +                    str(HYPERV_POWER_STATE[info.EnabledState]),
> +                    str(info.MemoryUsage),
> +                    str(info.NumberOfProcessors),
> +                    str(info.UpTime))
> +
> +        return {'state': HYPERV_POWER_STATE[info.EnabledState],
> +                'max_mem': info.MemoryUsage,
> +                'mem': info.MemoryUsage,
> +                'num_cpu': info.NumberOfProcessors,
> +                'cpu_time': info.UpTime}
> +
> +    def _lookup(self, i):
> +        vms = self._conn.Msvm_ComputerSystem(ElementName=i)
> +        n = len(vms)
> +        if n == 0:
> +            return None
> +        elif n > 1:
> +            raise Exception('duplicate name found: %s' % i)
> +        else:
> +            return vms[0].ElementName
> +
> +    def _set_vm_state(self, vm_name, req_state):
> +        """Set the desired state of the VM"""
> +        vms = self._conn.Msvm_ComputerSystem(ElementName=vm_name)
> +        if len(vms) == 0:
> +            return False
> +        (job, ret_val) =
vms[0].RequestStateChange(REQ_POWER_STATE[req_state])
> +        success = False
> +        if ret_val == WMI_JOB_STATUS_STARTED:
> +            success = self._check_job_status(job)
> +        elif ret_val == 0:
> +            success = True
> +        elif ret_val == 32775:
> +            #Invalid state for current operation. Typically means it is
> +            #already in the state requested
> +            success = True
> +        if success:
> +            logging.info("Successfully changed vm state of %s to %s",
> +                                vm_name, req_state)
> +        else:
> +            logging.error("Failed to change vm state of %s to %s",
> +                                vm_name, req_state)
> +            raise VmOperationError("Failed to change vm state of %s
to %s",
> +                                        vm_name, req_state)
> +
> +    def attach_volume(self, instance_name, device_path, mountpoint):
> +        vm = self._lookup(instance_name)
> +        if vm is None:
> +            raise exception.NotFound('Cannot attach volume to missing
%s vm' %
> +                            instance_name)
> +
> +    def detach_volume(self, instance_name, mountpoint):
> +        vm = self._lookup(instance_name)
> +        if vm is None:
> +            raise exception.NotFound('Cannot detach volume from
missing %s ' %
> +                            instance_name)
>
> === modified file 'nova/virt/images.py'
> --- nova/virt/images.py	2010-10-07 14:03:43 +0000
> +++ nova/virt/images.py	2010-10-14 06:26:41 +0000
> @@ -21,8 +21,12 @@
>  Handling of VM disk images.
>  """
>
> +import logging
>  import os.path
> +import shutil
> +import sys
>  import time
> +import urllib2
>  import urlparse
>
>  from nova import flags
> @@ -45,8 +49,28 @@
>      return f(image, path, user, project)
>
>
> +def _fetch_image_no_curl(url, path, headers):
> +    request = urllib2.Request(url)
> +    for (k, v) in headers.iteritems():
> +        request.add_header(k, v)
> +
> +    def urlretrieve(urlfile, fpath):
> +        chunk = 1 * 1024 * 1024
> +        f = open(fpath, "wb")
> +        while 1:
> +            data = urlfile.read(chunk)
> +            if not data:
> +                break
> +            f.write(data)
> +
> +    urlopened = urllib2.urlopen(request)
> +    urlretrieve(urlopened, path)
> +    logging.debug("Finished retreving %s -- placed in %s", url, path)
> +
> +
>  def _fetch_s3_image(image, path, user, project):
>      url = image_url(image)
> +    logging.debug("About to retrieve %s and place it in %s", url, path)
>
>      # This should probably move somewhere else, like e.g. a download_as
>      # method on User objects and at the same time get rewritten to use
> @@ -61,17 +85,23 @@
>
url_path)
>      headers['Authorization'] = 'AWS %s:%s' % (access, signature)
>
> -    cmd = ['/usr/bin/curl', '--fail', '--silent', url]
> -    for (k,v) in headers.iteritems():
> -        cmd += ['-H', '%s: %s' % (k,v)]
> -
> -    cmd += ['-o', path]
> -    return process.SharedPool().execute(executable=cmd[0], args=cmd[1:])
> +    if sys.platform.startswith('win'):
> +        return _fetch_image_no_curl(url, path, headers)
> +    else:
> +        cmd = ['/usr/bin/curl', '--fail', '--silent', url]
> +        for (k,v) in headers.iteritems():
> +            cmd += ['-H', '%s: %s' % (k,v)]
> +            cmd += ['-o', path]
> +            return process.SharedPool().execute(executable=cmd[0],
args=cmd[1:])

The two last lines don't belong inside the for loop.

>
>
>  def _fetch_local_image(image, path, user, project):
> -    source = _image_path('%s/image' % image)
> -    return process.simple_execute('cp %s %s' % (source, path))
> +    source = _image_path(os.path.join(image, 'image'))
> +    logging.debug("About to copy %s to %s", source, path)
> +    if sys.platform.startswith('win'):
> +        return shutil.copy(source, path)
> +    else:
> +        return process.simple_execute('cp %s %s' % (source, path))
>
>
>  def _image_path(path):
>


-- 
Soren Hansen
Ubuntu Developer    http://www.ubuntu.com/
OpenStack Developer http://www.openstack.org/