libvirt-cim-bounces@redhat.com wrote on 2009-03-12 06:07:19:

> yunguol@cn.ibm.com wrote:
> > # HG changeset patch
> > # User Guolian Yun <yunguol@cn.ibm.com>
> > # Date 1236759196 25200
> > # Node ID 0e350cf7bbab15c07d948976435ff4fe1b4ab597
> > # Parent  a78b6f23ebaa8a38fa591e420d742aa03cd9e515
> > [TEST] Add tc to verify VSMS.RemoveResourceSettings() with correct resource
> >
> >
> > This tc verify disk and network devices now, follow up patch will
> verify input and graphics devices
> >
> > Tested for KVM/Xen/XenFV with current sources and rpm
> >
> > Signed-off-by: Guolian Yun<yunguol@cn.ibm.com>
> >
> > diff -r a78b6f23ebaa -r 0e350cf7bbab suites/libvirt-
> cim/cimtest/VirtualSystemManagementService/16_removeresource.py
> > --- /dev/null   Thu Jan 01 00:00:00 1970 +0000
> > +++ b/suites/libvirt-
> cim/cimtest/VirtualSystemManagementService/16_removeresource.py  
> Wed Mar 11 01:13:16 2009 -0700
> > @@ -0,0 +1,109 @@
> > +#!/usr/bin/python
> > +#
> > +# Copyright 2008 IBM Corp.
>
> Change date to 2009.
>
> > +
> > +import sys
> > +import pywbem
>
> Redundant import - this can be removed.
>
> > +import random
> > +from pywbem.cim_obj import CIMInstanceName
> > +from XenKvmLib import vsms
>
> Instead of importing the whole module, just import get_vsms_class().
>
> > +from XenKvmLib import vxml
>
> Instead of importing the whole module, just import get_class().
>
> > +from XenKvmLib.classes import get_typed_class
> > +from CimTest.Globals import logger
> > +from XenKvmLib.const import do_main, get_provider_version,
> default_network_name
> > +from CimTest.ReturnCodes import FAIL, PASS
> > +from XenKvmLib.common_util import create_netpool_conf, destroy_netpool
> > +from XenKvmLib.xm_virt_util import get_bridge_from_network_xml
> > +
> > +sup_types = ['Xen', 'KVM', 'XenFV']
> > +default_dom = 'domain'
> > +libvirt_cim_dev_xml = 828
>
> This variable is a bit oddly named.  Can you use something like
> rem_res_err_rev?  Or something that is more descriptive of the actual
> changeset?
>
> > +ntype = 'network'
> > +nmac = '00:11:22:33:44:55'
> > +npool_name = default_network_name + str(random.randint(1, 100))
> > +
> > +@do_main(sup_types)
> > +def main():
> > +    options = main.options
> > +
> > +    if options.virt == 'KVM':
> > +        nddev = 'hdb'
> > +    else:
> > +        nddev = 'xvdb'
> > +
> > +    cxml = vxml.get_class(options.virt)(default_dom, disk=nddev, mac=nmac)
> > +    ret = cxml.cim_define(options.ip)
> > +    if not ret:
> > +        logger.error("Failed to define the dom: %s", default_dom)
> > +        return FAIL
> > +
> > +    drasd = get_typed_class(options.virt,
> 'DiskResourceAllocationSettingData')
> > +    drasd_id = '%s/%s' % (default_dom, nddev)
> > +    dkeys = {'InstanceID' : drasd_id}
> > +  
> > +    nrasd = get_typed_class(options.virt,
> 'NetResourceAllocationSettingData')
> > +    nrasd_id = '%s/%s' % (default_dom, nmac)
> > +    nkeys = {'InstanceID' : nrasd_id}
>
> Instead of hand building the instances for the resources, use the proper
> association calls to get the RASD instances.   You know the guest, so
> you can use SystemDevice to get the resource instance, and then
> SettingsDefineState to get the RASD instances.


  I'm working on a new patch to get the RASD instances by association.
  But I can't remove "KVM_InputResourceAllocationSettingData" which output
  the device "mouse:ps2" can not be found. Actually, I get it by SettingsDefineState.
  If I remove this device from the ResourceSettings, this method works for me.
 
>
> > +
> > +    status, net_name = create_netpool_conf(options.ip, options.virt,
> > +                                           use_existing=False,
> > +                                           net_name=npool_name)
>
> Why are you creating a network?  The guest is created using the default
> network that already exists.
>
> > +    if status != PASS:
> > +        logger.error('Unable to create network pool')
> > +        return FAIL
> > +
> > +    dresource = CIMInstanceName(drasd, keybindings = dkeys)
> > +    nresource = CIMInstanceName(nrasd, keybindings = nkeys)
> > +
> > +    service = vsms.get_vsms_class(options.virt)(options.ip)
> > +    try:
> > +        service.
> RemoveResourceSettings(ResourceSettings=[dresource, nresource])
> > +    except Exception, details:
> > +        logger.error('Failed to remove % or %s', dresource, nresource)
> > +        logger.error(details)
> > +        cxml.undefine(options.ip)
> > +        destroy_netpool(options.ip, options.virt, net_name)
> > +        return FAIL
> > +    
> > +    cxml.dumpxml(options.ip)
> > +    disk = cxml.
> get_value_xpath('/domain/devices/disk/target/@dev[. = "%s"]'
> > +                                % nddev)
> > +    if options.virt == 'KVM':
> > +        net = cxml.get_value_xpath(
> > +              '/domain/devices/interface/source/@network[. = "%s"]' % nmac)
> > +    else:
> > +        br = get_bridge_from_network_xml(net_name, options.ip,
> options.virt)
> > +        net = cxml.get_value_xpath(
> > +              '/domain/devices/interface/source/@bridge[. = "%s"]' % br)
>
> You are checking for the name of the pool you created - this isn't the
> same pool that was used to define the guest.
>
> > +
> > +    curr_cim_rev, changeset = get_provider_version(options.virt,
> options.ip)
> > +    if disk != None and net != None and curr_cim_rev >=
> libvirt_cim_dev_xml:
>
> This is actually a regression - the RemoveResources() call did work
> prior to changeset 779.
>
> So, you'll want to return an XFAIL if the revision is: 779 <= rev <= 828.
>
> Instead of checking disk and net in one if, break it up so that you can
> return an error message for each resource that fails:
>
>      if disk != None:
>          logger.error('The disk device was not removed successfully')
>      if net != None:
>          logger.error('The network device was not removed successfully')
>
> > +        logger.error('The resource is not removerd successfully')
> > +        cxml.undefine(options.ip)
> > +        destroy_netpool(options.ip, options.virt, net_name)
> > +        return FAIL
> > +
> > +    cxml.undefine(options.ip)
> > +    destroy_netpool(options.ip, options.virt, net_name)
> > +    return PASS
> > +
>
> I would use exceptions so that you only need to call undefine() once.
>
> --
> Kaitlin Rupert
> IBM Linux Technology Center
> kaitlin@linux.vnet.ibm.com
>
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim@redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim