Re: [libvirt] [test-API PATCHv2 1/2] sharemod: Add a new file for variable sharing in testcases
s/sharing in/sharing among/, but given it's already pushed. let's live with it. On 2012年04月10日 21:38, Guannan Ren wrote: sharedmod.py --- sharedmod.py | 16 1 files changed, 16 insertions(+), 0 deletions(-) create mode 100644 sharedmod.py diff --git a/sharedmod.py b/sharedmod.py new file mode 100644 index 000..8af26d8 --- /dev/null +++ b/sharedmod.py @@ -0,0 +1,16 @@ +# This is a module for variable sharing across testcases during +# running. You have to import it in each of testcases which want +# to share data. The framwork have already set {'conn': connobj} +# in libvirtobj dictionary for use in testcases. + +# The libvirtobj dictionary is only set and used by framework Any checking? I.e could testcase r/w it too? +# in testcases you could use sharedmod.libvirtobj['conn'] to get +# the connection object in libvirt.py, you need not to close it, IMHO need not should be should never. How about the follow up use of connection if it's already closed, reopen one again? +# the framework do it. snip The framwork have already set {'conn': connobj} +# in libvirtobj dictionary for use in testcases. + /snip The comment above should resides here. +libvirtobj = {} + +# shared variables for customized use in testcases +# set variable: sharedmod.data['my_test_variable'] = 'test_value' +# check the variable: sharedmod.data.has_key('my_test_variable') +# get the varialbe: sharedmod.data.get('my_test_variable', 'test_variable_default_value') From my p.o.v, libvirtobj is a confused name, also data is not enough to tell what the meaning is. How about use framework_shares and case_shares instead? And implement get/set/has functions for both framework_shares and case_shares, but not access them directly. e.g. def case_shares_get(key): pass def case_shares_set(key, value): pass def case_shares_has(key) pass Even perhaps a class is good. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [test-API PATCH 2/3] improvement on comments function name and destruct logger handlers
generator.py libvirt-test-api.py mapper.py utils/log.py: add __del__() to destruct logger handlers when it go out of context --- generator.py|2 +- libvirt-test-api.py |1 + mapper.py | 14 +- utils/log.py|6 ++ 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/generator.py b/generator.py index 0c3aec4..62b0d66 100644 --- a/generator.py +++ b/generator.py @@ -61,7 +61,7 @@ class FuncGen(object): self.env = env_parser.Envparser(env.cfg) mapper_obj = mapper.Mapper(activity) -pkg_casename_func = mapper_obj.package_casename_func_map() +pkg_casename_func = mapper_obj.module_casename_func_map() for test_procedure in pkg_casename_func: log_xml_parser.add_testprocedure_xml(testrunid, diff --git a/libvirt-test-api.py b/libvirt-test-api.py index 877104c..c171276 100644 --- a/libvirt-test-api.py +++ b/libvirt-test-api.py @@ -205,6 +205,7 @@ class Main(object): logname = log.Log.get_log_name() logfile = os.path.join('log/%s' % testrunid, logname) env_clear.EnvClear(cases_clearfunc_ref_dict, activity, logfile, self.loglevel)() +print Done elif options_list[0]['options'][cleanup] == disable: pass else: diff --git a/mapper.py b/mapper.py index f0b675a..c2c44da 100644 --- a/mapper.py +++ b/mapper.py @@ -22,10 +22,12 @@ import copy class Mapper(object): def __init__(self, testcases_list): -self.testcases_list = copy.deepcopy(testcases_list) +self.testcases_list = testcases_list -def package_casename_func_map(self): - Remove the package information to form a new dictionary +def module_casename_func_map(self): + generate a new list of dictionary +change key from module:casename to module:casename:func + tripped_cases_list = [] prev_testcasename = '' prev_testcases_params = '' @@ -56,8 +58,10 @@ class Mapper(object): return tripped_cases_list -def clean_package_casename_func_map(self): -get testcase function maping without cleaning ones +def module_casename_cleanfunc_map(self): +generate a new data format + keys of dictionay are of module:casename:casename_clean + tripped_cases_list = [] for testcase in self.testcases_list: tripped_case = {} diff --git a/utils/log.py b/utils/log.py index bd6f816..b146956 100644 --- a/utils/log.py +++ b/utils/log.py @@ -56,6 +56,9 @@ class CaseLog(Log): self.logger.setLevel(logging.DEBUG) super(CaseLog, self).__init__(logname, loglevel) +def __del__(self): +self.logger.handlers = [] + def case_log(self): Initialize log file fmt = {'file_formatter': @@ -94,6 +97,9 @@ class EnvLog(Log): self.logger.setLevel(logging.DEBUG) super(EnvLog, self).__init__(logname, loglevel) +def __del__(self): +self.logger.handlers = [] + def env_log(self): Initialize log file fmt = {'file_formatter':'%(message)s', -- 1.7.7.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [test-API PATCH 1/3] cleanup:make clean function optional
The patch make the writing of clean function optional If a testcase makes testing environment dirty after running, it must write a CASENAME_clean function to restore back the testing environment, otherwise, the clean function could be omitted --- env_clear.py |9 - proxy.py |7 --- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/env_clear.py b/env_clear.py index 3efc9be..2e5bcf5 100644 --- a/env_clear.py +++ b/env_clear.py @@ -30,7 +30,7 @@ class EnvClear(object): self.loglevel = loglevel mapper_obj = mapper.Mapper(activity) -clean_pkg_casename_func = mapper_obj.clean_package_casename_func_map() +clean_pkg_casename_func = mapper_obj.module_casename_cleanfunc_map() self.cases_ref_names = [] for case in clean_pkg_casename_func: @@ -47,7 +47,7 @@ class EnvClear(object): return retflag def env_clear(self): - Run each clearing function with the corresponding arguments + Run each clean function with the corresponding arguments envlog = log.EnvLog(self.logfile, self.loglevel) logger = envlog.env_log() @@ -60,8 +60,7 @@ class EnvClear(object): case_params = self.cases_params_list[i] case_params['logger'] = logger -self.cases_clearfunc_ref_dict[case_ref_name](case_params) - -del envlog +if self.cases_clearfunc_ref_dict.has_key(case_ref_name): +self.cases_clearfunc_ref_dict[case_ref_name](case_params) return 0 diff --git a/proxy.py b/proxy.py index 3c4cd63..fdbffd9 100644 --- a/proxy.py +++ b/proxy.py @@ -85,12 +85,13 @@ class Proxy(object): var_func_names = dir(casemod_ref) key = module + ':' + casename + ':' + func + +# the clean function is optional, we get its reference +# only if it exists in testcases if func in var_func_names: func_ref = getattr(casemod_ref, func) func_dict[key] = func_ref -else: -raise exception.TestCaseError(clean function not found in %s % \ - (func, testcase_name)) + return func_dict def get_params_variables(self): -- 1.7.7.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [test-API PATCH 0/3] make clean function optional
The set of patches is to make the writing of clean function optional. If a testcase which doesn't dirty the testing environment, then the TESTCASE_clean function could be optional or omitted. Because loggers in python are static objects managed by the module itself. When you create one, it won't be removed until the shell quit. In test-API there are two loggers which are configured with two handlers for each. we need to find a way to destruct these two loggers in order no to affect other application. The idea is to clear the handlers of each logger in destructor of CaseLog and EnvLog. def __del__(self): self.logger.handlers = [] rename some unclear function and variables. for example: repos/domain/start.py domain - mod start.py - case start() - func -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [test-API PATCH 2/2] proxy: import each testcase file only once, initialize proxy once
*libvirt-test-api.py: initialize proxy module only once *casecfgcheck.py: use proxy object rather than initialize it by itself *proxy.py: make get_func_call_dict more flexible --- casecfgcheck.py |5 + libvirt-test-api.py | 12 +--- proxy.py| 22 ++ 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/casecfgcheck.py b/casecfgcheck.py index 40d7c6e..9a4f8e6 100644 --- a/casecfgcheck.py +++ b/casecfgcheck.py @@ -18,13 +18,10 @@ import proxy class CaseCfgCheck(object): validate the options in testcase config file -def __init__(self, unique_testcases, activities_list): -self.unique_testcases = unique_testcases - +def __init__(self, proxy_obj, activities_list): # XXX to check the first testcase list in activities_list self.activity = activities_list[0] -proxy_obj = proxy.Proxy(self.unique_testcases) self.case_params = proxy_obj.get_params_variables() def check(self): diff --git a/libvirt-test-api.py b/libvirt-test-api.py index 385b52d..7b38aaa 100644 --- a/libvirt-test-api.py +++ b/libvirt-test-api.py @@ -112,20 +112,18 @@ class Main(object): unique_testcases = filterobj.unique_testcases() +# __import__ TESTCASE.py once for duplicate testcase names +proxy_obj = proxy.Proxy(unique_testcases) + # check the options to each testcase in case config file -casechk = CaseCfgCheck(unique_testcases, activities_list) +casechk = CaseCfgCheck(proxy_obj, activities_list) if casechk.check(): return 1 # get a list of unique testcase # with 'clean' flag appended to its previous testcase unique_testcase_keys = filterobj.unique_testcase_cleansuffix() - -# call and initilize proxy component to -# get a list of reference of testcases -proxy_obj = proxy.Proxy(unique_testcase_keys) - -cases_func_ref_dict = proxy_obj.get_func_call_dict() +cases_func_ref_dict = proxy_obj.get_func_call_dict(unique_testcase_keys) # create a null list, then, initilize generator to # get the callable testcase function diff --git a/proxy.py b/proxy.py index bc82a84..49a0420 100644 --- a/proxy.py +++ b/proxy.py @@ -1,6 +1,6 @@ #!/usr/bin/env python # -# libvirt-test-API is copyright 2010 Red Hat, Inc. +# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc. # # libvirt-test-API is free software: you can redistribute it and/or modify it # under the terms of the GNU General Public License as published by @@ -37,12 +37,13 @@ class Proxy(object): casename = elements[1] casemod_ref = self.get_call_dict(module, casename) -self.testcase_ref_dict[testcase_name] = casemod_ref +modcase = module + ':' + casename +self.testcase_ref_dict[modcase] = casemod_ref -def get_func_call_dict(self): -Return running function reference dictionary +def get_func_call_dict(self, unique_testcase_keys): +get reference to functions defined in testcase file func_dict = {} -for testcase_name in self.testcases_names: +for testcase_name in unique_testcase_keys: # Get module, casename elements = testcase_name.split(':') module = elements[0] @@ -55,16 +56,21 @@ class Proxy(object): flag = elements[2] func = casename + flag -casemod_ref = self.testcase_ref_dict[testcase_name] +# use modcase key to get the reference to corresponding +# testcase module +modcase = module + ':' + casename +casemod_ref = self.testcase_ref_dict[modcase] var_func_names = dir(casemod_ref) -key = module + ':' + casename + ':' + func +key = modcase + ':' + func +# check if the expected function is present in +# the list of string name from dir() if func in var_func_names: func_ref = getattr(casemod_ref, func) func_dict[key] = func_ref else: raise exception.TestCaseError(function %s not found in %s % \ - (func, testcase_name)) + (func, modcase)) return func_dict def get_clearfunc_call_dict(self): -- 1.7.7.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [test-API PATCH 1/2] add a new option -t to print template of testcase config file
If a given testcase has clean function, the clean flag will be used automatically # python libvirt-test-api.py -t repos/domain/attach_disk.py \ repos/storage/create_netfs_pool.py \ repos/domain/save.py output: domain:attach_disk guestname GUESTNAME guesttype GUESTTYPE imagename IMAGENAME imagesize IMAGESIZE hdmodel HDMODEL storage:create_netfs_pool poolname POOLNAME sourcename SOURCENAME sourcepath SOURCEPATH pooltype POOLTYPE [targetpath] TARGETPATH domain:save guestname GUESTNAME filepath FILEPATH clean --- libvirt-test-api.py | 49 +++-- proxy.py| 17 + 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/libvirt-test-api.py b/libvirt-test-api.py index c171276..385b52d 100644 --- a/libvirt-test-api.py +++ b/libvirt-test-api.py @@ -35,6 +35,7 @@ def usage(): print Usage: libvirt_test_api.py OPTIONS ARGUS print \noptions: -h, --help : Display usage information \ \n -c, --casefile: Specify configuration file \ + \n -t, --template: Print testcase config file template \ \n -f, --logxml: Specify log file with type xml \ \n -l, --log-level: 0 or 1 currently \ \n -d, --delete-log: Delete log items \ @@ -45,6 +46,7 @@ def usage(): print example: \ \n python libvirt-test-api.py -l 0|1 -c TEST.CONF\ \n python libvirt-test-api.py -c TEST.CONF -f TEST.XML \ + \n python libvirt-test-api.py -t repos/domain/start.py ... \ \n python libvirt-test-api.py -m TESTONE.XML TESTTWO.XML \ \n python libvirt-test-api.py -d TEST.XML TESTRUNID TESTID \ \n python libvirt-test-api.py -d TEST.XML TESTRUNID \ @@ -215,6 +217,41 @@ class Main(object): return 1 return 0 +def print_casefile(self, testcases): +print testcase file template +modcasename = [] +for case in testcases: +if not os.path.isfile(case) or not case.endswith('.py'): +print testcase %s couldn't be recognized % case +return 1 + +paths = case.split('/') +modcasename.append(paths[1] + ':' + paths[2][:-3]) + +proxy_obj = proxy.Proxy(modcasename) +case_params = proxy_obj.get_params_variables() + +string = # the file is generated automatically, please\n \ + # make some modifications before the use of it\n \ + # params in [] are optional to its testcase\n +for key in modcasename: +string += %s\n % key +required_params, optional_params = case_params[key] +for p in required_params: +string += * 4 + p + \n +string += * 8 + p.upper() + \n +for p in optional_params: +string += * 4 + [ + p + ]\n +string += * 8 + p.upper() + \n + +if proxy_obj.has_clean_function(key): +string += clean\n + +string += \n + +print string +return 0 + def remove_log(self, testrunid, testid = None): to remove log item in the log xmlfile log_xml_parser = LogXMLParser(self.logxml) @@ -274,8 +311,8 @@ if __name__ == __main__: loglevel = 0 try: -opts, args = getopt.getopt(sys.argv[1:], hc:l:dmr, -[help, casefile=, logxml=, +opts, args = getopt.getopt(sys.argv[1:], hc:tl:dmr, +[help, casefile=, template, logxml=, delete-log=, merge=, rerun=]) except getopt.GetoptError, err: print str(err) @@ -288,6 +325,14 @@ if __name__ == __main__: sys.exit(0) if o == -c or o == --casefile: casefile = v +if o == -t or o == --template: +if len(args) = 0: +usage() +sys.exit(1) +main = Main('', '', '', '') +if main.print_casefile(args): +sys.exit(1) +sys.exit(0) if o == -f or o == --logxml: logxml = v if o == -l or o == --log-level: diff --git a/proxy.py b/proxy.py index fdbffd9..bc82a84 100644 --- a/proxy.py +++ b/proxy.py @@ -120,6 +120,23 @@ class Proxy(object): (required_params or optional_params not found in %s % testcase_name) return case_params +def has_clean_function(self, testcase_name): + Return true if the testcase have clean function + +if testcase_name not in self.testcases_names: +return False + +elements = testcase_name.split(':') +
[libvirt] [test-API PATCH 3/3] repo: cleanup clean functions that are not used
--- repos/domain/attach_disk.py|4 repos/domain/autostart.py |4 repos/domain/balloon_memory.py |4 repos/domain/blkstats.py |4 repos/domain/console_mutex.py |4 repos/domain/cpu_affinity.py |4 repos/domain/cpu_topology.py |5 - repos/domain/define.py |4 repos/domain/destroy.py|4 repos/domain/detach_disk.py|4 repos/domain/eventhandler.py |4 repos/domain/ifstats.py|4 repos/domain/install_linux_check.py|5 - repos/domain/restore.py|4 repos/domain/resume.py |4 repos/domain/shutdown.py |4 repos/domain/start.py |4 repos/domain/suspend.py|4 repos/domain/undefine.py |4 repos/libvirtd/restart.py |5 - .../multiple_thread_block_on_domain_create.py | 14 -- repos/snapshot/delete.py |4 repos/snapshot/file_flag.py|4 repos/snapshot/flag_check.py |4 repos/snapshot/internal_create.py |4 repos/snapshot/revert.py |5 - repos/snapshot/snapshot_list.py|5 - repos/storage/build_dir_pool.py|3 --- 28 files changed, 0 insertions(+), 126 deletions(-) diff --git a/repos/domain/attach_disk.py b/repos/domain/attach_disk.py index 4385df7..9d8426d 100644 --- a/repos/domain/attach_disk.py +++ b/repos/domain/attach_disk.py @@ -99,7 +99,3 @@ def attach_disk(params): return 1 return 0 - -def attach_disk_clean(params): - clean testing environment -pass diff --git a/repos/domain/autostart.py b/repos/domain/autostart.py index 930a04b..1906706 100644 --- a/repos/domain/autostart.py +++ b/repos/domain/autostart.py @@ -73,7 +73,3 @@ def autostart(params): return 1 return 0 - -def autostart_clean(params): - clean testing environment -pass diff --git a/repos/domain/balloon_memory.py b/repos/domain/balloon_memory.py index 7cef074..25b8318 100644 --- a/repos/domain/balloon_memory.py +++ b/repos/domain/balloon_memory.py @@ -259,7 +259,3 @@ def balloon_memory(params): if count: return 1 return 0 - -def balloon_memory_clean(params): - clean testing environment -pass diff --git a/repos/domain/blkstats.py b/repos/domain/blkstats.py index 1500a1a..a208887 100644 --- a/repos/domain/blkstats.py +++ b/repos/domain/blkstats.py @@ -66,7 +66,3 @@ def blkstats(params): return 1 return 0 - -def blkstats_clean(params): - clean testing environment -pass diff --git a/repos/domain/console_mutex.py b/repos/domain/console_mutex.py index 409861b..c67aec6 100644 --- a/repos/domain/console_mutex.py +++ b/repos/domain/console_mutex.py @@ -87,7 +87,3 @@ def console_mutex(params): logger.info(Done) return ret - -def console_mutex_clean(params): -clean testing environment -pass diff --git a/repos/domain/cpu_affinity.py b/repos/domain/cpu_affinity.py index 833cfd2..1332c7e 100644 --- a/repos/domain/cpu_affinity.py +++ b/repos/domain/cpu_affinity.py @@ -280,7 +280,3 @@ def cpu_affinity(params): if retflag: return 1 return 0 - -def cpu_affinity_clean(params): - clean testing environment -pass diff --git a/repos/domain/cpu_topology.py b/repos/domain/cpu_topology.py index 2caf919..af41f62 100644 --- a/repos/domain/cpu_topology.py +++ b/repos/domain/cpu_topology.py @@ -199,8 +199,3 @@ def cpu_topology(params): return 1 return 0 - -def cpu_topology_clean(params): -clean testing enviorment -return 0 - diff --git a/repos/domain/define.py b/repos/domain/define.py index 29d2c15..9d3b2eb 100644 --- a/repos/domain/define.py +++ b/repos/domain/define.py @@ -92,7 +92,3 @@ def define(params): return 1 return 0 - -def define_clean(params): - clean testing environment -pass diff --git a/repos/domain/destroy.py b/repos/domain/destroy.py index 9d71835..c41f0f8 100644 --- a/repos/domain/destroy.py +++ b/repos/domain/destroy.py @@ -91,7 +91,3 @@ def destroy(params): return 1 return 0 - -def destroy_clean(params): - clean testing environment -pass diff --git a/repos/domain/detach_disk.py b/repos/domain/detach_disk.py index ddba2ca..1b0b7f9 100644 --- a/repos/domain/detach_disk.py +++ b/repos/domain/detach_disk.py @@ -75,7 +75,3 @@ def detach_disk(params):
Re: [libvirt] [test-API PATCH 0/3] make clean function optional
On 2012年04月16日 14:11, Guannan Ren wrote: The set of patches is to make the writing of clean function optional. If a testcase which doesn't dirty the testing environment, then the TESTCASE_clean function could be optional or omitted. Good, this was one of the TODO thing in my mind. Because loggers in python are static objects managed by the module itself. When you create one, it won't be removed until the shell quit. In test-API there are two loggers which are configured with two handlers for each. we need to find a way to destruct these two loggers in order no to affect other application. The idea is to clear the handlers of each logger in destructor of CaseLog and EnvLog. def __del__(self): self.logger.handlers = [] rename some unclear function and variables. for example: repos/domain/start.py IIUC, you do 3 different things in these 3 patches, it will be good to document them separately here for easy reviewing. domain - mod start.py - case start() - func What does these mean? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API PATCH 3/3] repo: cleanup clean functions that are not used
On 2012年04月16日 14:11, Guannan Ren wrote: --- repos/domain/attach_disk.py|4 repos/domain/autostart.py |4 repos/domain/balloon_memory.py |4 repos/domain/blkstats.py |4 repos/domain/console_mutex.py |4 repos/domain/cpu_affinity.py |4 repos/domain/cpu_topology.py |5 - repos/domain/define.py |4 repos/domain/destroy.py|4 repos/domain/detach_disk.py|4 repos/domain/eventhandler.py |4 repos/domain/ifstats.py|4 repos/domain/install_linux_check.py|5 - repos/domain/restore.py|4 repos/domain/resume.py |4 repos/domain/shutdown.py |4 repos/domain/start.py |4 repos/domain/suspend.py|4 repos/domain/undefine.py |4 repos/libvirtd/restart.py |5 - .../multiple_thread_block_on_domain_create.py | 14 -- repos/snapshot/delete.py |4 repos/snapshot/file_flag.py|4 repos/snapshot/flag_check.py |4 repos/snapshot/internal_create.py |4 repos/snapshot/revert.py |5 - repos/snapshot/snapshot_list.py|5 - repos/storage/build_dir_pool.py|3 --- 28 files changed, 0 insertions(+), 126 deletions(-) diff --git a/repos/domain/attach_disk.py b/repos/domain/attach_disk.py index 4385df7..9d8426d 100644 --- a/repos/domain/attach_disk.py +++ b/repos/domain/attach_disk.py @@ -99,7 +99,3 @@ def attach_disk(params): return 1 return 0 - -def attach_disk_clean(params): - clean testing environment -pass diff --git a/repos/domain/autostart.py b/repos/domain/autostart.py index 930a04b..1906706 100644 --- a/repos/domain/autostart.py +++ b/repos/domain/autostart.py @@ -73,7 +73,3 @@ def autostart(params): return 1 return 0 - -def autostart_clean(params): - clean testing environment -pass diff --git a/repos/domain/balloon_memory.py b/repos/domain/balloon_memory.py index 7cef074..25b8318 100644 --- a/repos/domain/balloon_memory.py +++ b/repos/domain/balloon_memory.py @@ -259,7 +259,3 @@ def balloon_memory(params): if count: return 1 return 0 - -def balloon_memory_clean(params): - clean testing environment -pass diff --git a/repos/domain/blkstats.py b/repos/domain/blkstats.py index 1500a1a..a208887 100644 --- a/repos/domain/blkstats.py +++ b/repos/domain/blkstats.py @@ -66,7 +66,3 @@ def blkstats(params): return 1 return 0 - -def blkstats_clean(params): - clean testing environment -pass diff --git a/repos/domain/console_mutex.py b/repos/domain/console_mutex.py index 409861b..c67aec6 100644 --- a/repos/domain/console_mutex.py +++ b/repos/domain/console_mutex.py @@ -87,7 +87,3 @@ def console_mutex(params): logger.info(Done) return ret - -def console_mutex_clean(params): -clean testing environment -pass diff --git a/repos/domain/cpu_affinity.py b/repos/domain/cpu_affinity.py index 833cfd2..1332c7e 100644 --- a/repos/domain/cpu_affinity.py +++ b/repos/domain/cpu_affinity.py @@ -280,7 +280,3 @@ def cpu_affinity(params): if retflag: return 1 return 0 - -def cpu_affinity_clean(params): - clean testing environment -pass diff --git a/repos/domain/cpu_topology.py b/repos/domain/cpu_topology.py index 2caf919..af41f62 100644 --- a/repos/domain/cpu_topology.py +++ b/repos/domain/cpu_topology.py @@ -199,8 +199,3 @@ def cpu_topology(params): return 1 return 0 - -def cpu_topology_clean(params): -clean testing enviorment -return 0 - diff --git a/repos/domain/define.py b/repos/domain/define.py index 29d2c15..9d3b2eb 100644 --- a/repos/domain/define.py +++ b/repos/domain/define.py @@ -92,7 +92,3 @@ def define(params): return 1 return 0 - -def define_clean(params): - clean testing environment -pass diff --git a/repos/domain/destroy.py b/repos/domain/destroy.py index 9d71835..c41f0f8 100644 --- a/repos/domain/destroy.py +++ b/repos/domain/destroy.py @@ -91,7 +91,3 @@ def destroy(params): return 1 return 0 - -def destroy_clean(params): - clean testing environment -pass diff --git a/repos/domain/detach_disk.py b/repos/domain/detach_disk.py index ddba2ca..1b0b7f9 100644 --- a/repos/domain/detach_disk.py
[libvirt] [test-API PATCH 0/5] add TESTCASE_check optional function support
In some cases, we need to check if the testing environment is satisfied to run a certain testcase. The testcase only will be executed If some specific hardware is present on box. The patches add a optional check function support. For example: a testcase named testa.py with testa_check() defined in this file. The framework will run test_check(params) first, return 0 means pass, 1 means failure. If the check function failed. the testa(params) will not be run. Skip is marked in output. # testa.py required_params = () optional_params = ('options') def testa_check(params): logger = params['logger'] logger.info(I am from testa_check) return 1 def testa(params): logger = params['logger'] logger.info(I am from testa) return 1 The Output: Checking Testing Environment... Linux localhost.localdomain 3.2.5-3.fc16.x86_64 #1 SMP Thu Feb 9 01:24:38 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux Virsh command line tool of libvirt: 0.9.10 libvirtd (libvirt) 0.9.10 Default URI: qemu:///system QEMU emulator version 0.15.0 (qemu-kvm-0.15.0), Copyright (c) 2003-2008 Fabrice Bellard Start Testing: Case Count: 1 Log File: log/20120416142755/libvirt_test001 test:testa 14:27:56|INFO |I am from testa_check 14:27:56|INFO |Failed to meet testing requirement Result: Skip Summary: Total:1 [Pass:0 Fail:0 Skip:1] The testcase.conf file: test:testa options value1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [test-API PATCH 2/5] generator: calling testcase_check if it is present in testcase
*calling testcase_check() if testcase defined it. On pass, run testcase(), otherwise, skip the testcase. *add Skip counting *because we don't treat clean flag as a seprate call, so we run clean function based on the check of clean flag on testcase name that returned by mapper. --- generator.py | 125 + 1 files changed, 64 insertions(+), 61 deletions(-) diff --git a/generator.py b/generator.py index 62b0d66..5059587 100644 --- a/generator.py +++ b/generator.py @@ -39,18 +39,19 @@ for dist in os.listdir('dist'): class FuncGen(object): To generate a callable testcase def __init__(self, cases_func_ref_dict, + cases_checkfunc_ref_dict, activity, logfile, testrunid, testid, log_xml_parser, lockfile, bugstxt, loglevel): self.cases_func_ref_dict = cases_func_ref_dict +self.cases_checkfunc_ref_dict = cases_checkfunc_ref_dict self.logfile = logfile self.testrunid = testrunid self.testid = testid self.lockfile = lockfile self.bugstxt = bugstxt self.loglevel = loglevel -self.testcase_number = 0 self.fmt = format.Format(logfile) self.log_xml_parser = log_xml_parser @@ -61,23 +62,21 @@ class FuncGen(object): self.env = env_parser.Envparser(env.cfg) mapper_obj = mapper.Mapper(activity) -pkg_casename_func = mapper_obj.module_casename_func_map() +case_list = mapper_obj.module_casename_func_map() -for test_procedure in pkg_casename_func: +for test_procedure in case_list: log_xml_parser.add_testprocedure_xml(testrunid, testid, test_procedure) -self.cases_ref_names = [] -for case in pkg_casename_func: -case_ref_name = case.keys()[0] -if case_ref_name[-6:] != _clean: -self.testcase_number += 1 -self.cases_ref_names.append(case_ref_name) - -self.cases_params_list = [] -for case in pkg_casename_func: +self.case_name_list = [] +for case in case_list: +mod_case_func = case.keys()[0] +self.case_name_list.append(mod_case_func) + +self.case_params_list = [] +for case in case_list: case_params = case.values()[0] -self.cases_params_list.append(case_params) +self.case_params_list.append(case_params) def __call__(self): retflag = self.generator() @@ -115,7 +114,7 @@ class FuncGen(object): envlog = log.EnvLog(self.logfile, self.loglevel) env_logger = envlog.env_log() -loop_number = len(self.cases_ref_names) +casenumber = len(self.case_name_list) start_time = time.strftime(%Y-%m-%d %H:%M:%S) env_logger.info(Checking Testing Environment... ) @@ -125,92 +124,96 @@ class FuncGen(object): sys.exit(1) else: env_logger.info(\nStart Testing:) -env_logger.info(Case Count: %s % self.testcase_number) +env_logger.info(Case Count: %s % casenumber) env_logger.info(Log File: %s\n % self.logfile) caselog = log.CaseLog(self.logfile, self.loglevel) case_logger = caselog.case_log() -retflag = 0 -for i in range(loop_number): +# retflag: [pass, fail, skip] +retflag = [0, 0, 0] +for i in range(casenumber): -case_ref_name = self.cases_ref_names[i] -pkg_casename = case_ref_name.rsplit(:, 1)[0] -funcname = case_ref_name.rsplit(:, 1)[-1] +clean_flag = False -if _clean not in funcname: -cleanoper = 0 -else: -cleanoper = 1 +mod_case_func = self.case_name_list[i] +mod_case = mod_case_func.rsplit(:, 1)[0] +if mod_case_func.endswith(':clean'): +mod_case_func = mod_case_func[:-6] +clean_flag = True +self.fmt.print_start(mod_case, env_logger) -if not cleanoper: -self.fmt.print_start(pkg_casename, env_logger) -else: -self.fmt.print_string(12* + Cleaning..., env_logger) +case_params = self.case_params_list[i] +case_params['logger'] = case_logger -case_params = self.cases_params_list[i] +if self.cases_checkfunc_ref_dict.has_key(mod_case_func): +if self.cases_checkfunc_ref_dict[mod_case_func](case_params): +case_logger.info(Failed to meet testing requirement) +self.fmt.print_end(mod_case, 2, env_logger) +retflag[2] += 1 +continue
[libvirt] [test-API PATCH 1/5] mapper: don't treat clean flag as a testcase
Original, the clean flag work with previous testcase to form a separate call request to generator. the patch will append :clean to previous testcase to mark that the previous testcase needs to clean after running. It make counting of testcase number easier. And rename the function and variable, add more comments --- mapper.py | 71 +--- 1 files changed, 34 insertions(+), 37 deletions(-) diff --git a/mapper.py b/mapper.py index c2c44da..5cf12e3 100644 --- a/mapper.py +++ b/mapper.py @@ -1,6 +1,6 @@ #!/usr/bin/env python # -# libvirt-test-API is copyright 2010 Red Hat, Inc. +# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc. # # libvirt-test-API is free software: you can redistribute it and/or modify it # under the terms of the GNU General Public License as published by @@ -17,8 +17,6 @@ # the purpose is to get useful information about a testrun. # -import copy - class Mapper(object): def __init__(self, testcases_list): @@ -27,54 +25,53 @@ class Mapper(object): def module_casename_func_map(self): generate a new list of dictionary change key from module:casename to module:casename:func +if clean flag is set, key will be module:casename:func:clean -tripped_cases_list = [] -prev_testcasename = '' -prev_testcases_params = '' +new_case_list = [] for testcase in self.testcases_list: -tripped_case = {} -testcase_name = testcase.keys()[0] -if : in testcase_name: -casename = testcase_name.split(:)[1] +case = {} +mod_case = testcase.keys()[0] +if : in mod_case: +casename = mod_case.split(:)[1] func = casename -if testcase_name == 'sleep': -tripped_cases_list.append(testcase) +if mod_case == 'sleep': +new_case_list.append(testcase) continue -if testcase_name == 'clean': -func = casename + _clean -tripped_case[prev_testcasename + : + func] = prev_testcases_params -tripped_cases_list.append(tripped_case) -continue +if mod_case == 'clean': +if not new_case_list: +return None -testcases_params = testcase.values()[0] -tripped_case[testcase_name + : + func] = testcases_params -tripped_cases_list.append(tripped_case) +previous_case = new_case_list.pop() +key = previous_case.keys()[0] + ':clean' +case[key] = previous_case.values()[0] +new_case_list.append(case) +continue -prev_testcasename = testcase_name -prev_testcases_params = testcases_params +cases_params = testcase.values()[0] +case[mod_case + : + func] = cases_params +new_case_list.append(case) -return tripped_cases_list +return new_case_list -def module_casename_cleanfunc_map(self): -generate a new data format - keys of dictionay are of module:casename:casename_clean +def module_casename_func_noflag(self): +remove sleep and clean data + generate a new data format -tripped_cases_list = [] +new_case_list = [] for testcase in self.testcases_list: -tripped_case = {} -testcase_name = testcase.keys()[0] -if : in testcase_name: -casename = testcase_name.split(:)[1] -func = casename + _clean +case = {} +mod_case = testcase.keys()[0] -if testcase_name == 'sleep' or testcase_name == 'clean': +if mod_case == 'sleep' or mod_case == 'clean': continue -testcases_params = testcase.values()[0] -tripped_case[testcase_name + : + func] = testcases_params -tripped_cases_list.append(tripped_case) +func = mod_case.split(:)[1] + +cases_params = testcase.values()[0] +case[mod_case + : + func] = cases_params +new_case_list.append(case) -return tripped_cases_list +return new_case_list -- 1.7.7.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [test-API PATCH 4/5] libvirt-test-api: get reference to check function in testcases
*proxy_obj.get_optionalfunc_call_dict('check') to get references to 'check' function that defined in testcases, and pass them to generator for running later --- libvirt-test-api.py |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/libvirt-test-api.py b/libvirt-test-api.py index 7b38aaa..c53a3b6 100644 --- a/libvirt-test-api.py +++ b/libvirt-test-api.py @@ -125,6 +125,9 @@ class Main(object): unique_testcase_keys = filterobj.unique_testcase_cleansuffix() cases_func_ref_dict = proxy_obj.get_func_call_dict(unique_testcase_keys) +# get check function reference if that is defined in testcase file +cases_checkfunc_ref_dict = proxy_obj.get_optionalfunc_call_dict('check') + # create a null list, then, initilize generator to # get the callable testcase function # and put it into procs list for running. @@ -142,6 +145,7 @@ class Main(object): else: logfile = os.path.join('log/%s' % testrunid, logname) procs.append(generator.FuncGen(cases_func_ref_dict, + cases_checkfunc_ref_dict, activity, logfile, testrunid, @@ -199,7 +203,7 @@ class Main(object): if options_list[0]['options'].has_key(cleanup): if options_list[0]['options'][cleanup] == enable: print Clean up Testing Environment... -cases_clearfunc_ref_dict = proxy_obj.get_clearfunc_call_dict() +cases_clearfunc_ref_dict = proxy_obj.get_optionalfunc_call_dict('clean') log.Log.counter = 0 for activity in activities_list: logname = log.Log.get_log_name() -- 1.7.7.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [test-API PATCH 3/5] proxy: generalize function get_optionalfunc_call_dict
*make get_optionalfunc_call_dict work for both testcase_check() and testcase_clean() *imporve the code readability --- proxy.py | 26 ++ 1 files changed, 14 insertions(+), 12 deletions(-) diff --git a/proxy.py b/proxy.py index 49a0420..32a8eb2 100644 --- a/proxy.py +++ b/proxy.py @@ -21,10 +21,10 @@ import exception class Proxy(object): - The Proxy class is used for getting real function call reference + The Proxy class is used for getting function reference def __init__(self, testcases_names): - Argument case_list is test case list + initialize a list of references to testcases module self.testcases_names = testcases_names self.testcase_ref_dict = {} @@ -73,8 +73,8 @@ class Proxy(object): (func, modcase)) return func_dict -def get_clearfunc_call_dict(self): - Return a clearing function reference dictionary. +def get_optionalfunc_call_dict(self, suffix): + get optional function that is present in testcase func_dict = {} for testcase_name in self.testcases_names: # Get module, casename @@ -85,14 +85,15 @@ class Proxy(object): module = elements[0] casename = elements[1] -func = casename + '_clean' +func = casename + '_' + suffix -casemod_ref = self.testcase_ref_dict[testcase_name] -var_func_names = dir(casemod_ref) +modcase = module + ':' + casename +key = modcase + ':' + casename -key = module + ':' + casename + ':' + func +casemod_ref = self.testcase_ref_dict[modcase] +var_func_names = dir(casemod_ref) -# the clean function is optional, we get its reference +# function is optional, we get its reference # only if it exists in testcases if func in var_func_names: func_ref = getattr(casemod_ref, func) @@ -114,16 +115,17 @@ class Proxy(object): module = elements[0] casename = elements[1] -casemod_ref = self.testcase_ref_dict[testcase_name] +modcase = module + ':' + casename +casemod_ref = self.testcase_ref_dict[modcase] var_func_names = dir(casemod_ref) if 'required_params' in var_func_names \ and 'optional_params' in var_func_names: -case_params[testcase_name] = \ +case_params[modcase] = \ [casemod_ref.required_params, casemod_ref.optional_params] else: raise exception.TestCaseError\ - (required_params or optional_params not found in %s % testcase_name) + (required_params or optional_params not found in %s % modcase) return case_params def has_clean_function(self, testcase_name): -- 1.7.7.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [test-API PATCH 5/5] function and variable name improvement
*env_clear.py *utils/format.py we use 2 for skip flag --- env_clear.py| 26 +- utils/format.py |2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/env_clear.py b/env_clear.py index 2e5bcf5..a5dc4fa 100644 --- a/env_clear.py +++ b/env_clear.py @@ -30,17 +30,17 @@ class EnvClear(object): self.loglevel = loglevel mapper_obj = mapper.Mapper(activity) -clean_pkg_casename_func = mapper_obj.module_casename_cleanfunc_map() +mod_casename_func = mapper_obj.module_casename_func_noflag() -self.cases_ref_names = [] -for case in clean_pkg_casename_func: -case_ref_name = case.keys()[0] -self.cases_ref_names.append(case_ref_name) +self.case_name_list = [] +for case in mod_casename_func: +mod_case_func = case.keys()[0] +self.case_name_list.append(mod_case_func) -self.cases_params_list = [] -for case in clean_pkg_casename_func: +self.case_params_list = [] +for case in mod_casename_func: case_params = case.values()[0] -self.cases_params_list.append(case_params) +self.case_params_list.append(case_params) def __call__(self): retflag = self.env_clear() @@ -52,15 +52,15 @@ class EnvClear(object): envlog = log.EnvLog(self.logfile, self.loglevel) logger = envlog.env_log() -testcase_number = len(self.cases_ref_names) +testcase_number = len(self.case_name_list) for i in range(testcase_number): -case_ref_name = self.cases_ref_names[i] -case_params = self.cases_params_list[i] +mod_case_func = self.case_name_list[i] +case_params = self.case_params_list[i] case_params['logger'] = logger -if self.cases_clearfunc_ref_dict.has_key(case_ref_name): -self.cases_clearfunc_ref_dict[case_ref_name](case_params) +if self.cases_clearfunc_ref_dict.has_key(mod_case_func): +self.cases_clearfunc_ref_dict[mod_case_func](case_params) return 0 diff --git a/utils/format.py b/utils/format.py index 7ee5eca..9f228c4 100644 --- a/utils/format.py +++ b/utils/format.py @@ -57,7 +57,7 @@ class Format(object): if flag == 1: result = 'FAIL' console_result = '\033[1;31mFAIL\033[1;m' -if flag == 100: +if flag == 2: result = 'Skip' console_result = '\033[1;38mSkip\033[1;m' -- 1.7.7.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API PATCH 1/3] cleanup:make clean function optional
s/cleanup:make/cleanup: Make/ On 2012年04月16日 14:11, Guannan Ren wrote: The patch make the writing of clean function optional If a testcase makes testing environment dirty after running, it If a case dirties the testing environment, must write a CASENAME_clean function to restore back the Could the $CASENAME_ be omitted? IIRC, the scripts under repos are imported by framework when executing. And thus we can invoke the cleanup functions by $script_name.cleanup. testing environment, otherwise, the clean function could be omitted --- env_clear.py |9 - proxy.py |7 --- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/env_clear.py b/env_clear.py index 3efc9be..2e5bcf5 100644 --- a/env_clear.py +++ b/env_clear.py @@ -30,7 +30,7 @@ class EnvClear(object): self.loglevel = loglevel mapper_obj = mapper.Mapper(activity) -clean_pkg_casename_func = mapper_obj.clean_package_casename_func_map() +clean_pkg_casename_func = mapper_obj.module_casename_cleanfunc_map() self.cases_ref_names = [] for case in clean_pkg_casename_func: @@ -47,7 +47,7 @@ class EnvClear(object): return retflag def env_clear(self): - Run each clearing function with the corresponding arguments + Run each clean function with the corresponding arguments envlog = log.EnvLog(self.logfile, self.loglevel) logger = envlog.env_log() @@ -60,8 +60,7 @@ class EnvClear(object): case_params = self.cases_params_list[i] case_params['logger'] = logger -self.cases_clearfunc_ref_dict[case_ref_name](case_params) - -del envlog +if self.cases_clearfunc_ref_dict.has_key(case_ref_name): To be consistent, clear should be clean. +self.cases_clearfunc_ref_dict[case_ref_name](case_params) return 0 diff --git a/proxy.py b/proxy.py index 3c4cd63..fdbffd9 100644 --- a/proxy.py +++ b/proxy.py @@ -85,12 +85,13 @@ class Proxy(object): var_func_names = dir(casemod_ref) key = module + ':' + casename + ':' + func + +# the clean function is optional, we get its reference +# only if it exists in testcases s/testcases/test cases/, or cases is enough, we should have a consistent term across the project, but not different from here and there. if func in var_func_names: func_ref = getattr(casemod_ref, func) func_dict[key] = func_ref -else: -raise exception.TestCaseError(clean function not found in %s % \ - (func, testcase_name)) + return func_dict def get_params_variables(self): -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API PATCH 0/2] support variables share across testcases
On 04/16/2012 10:03 AM, Osier Yang wrote: On 2012年04月06日 17:53, Guannan Ren wrote: An example: #sharedvar.cfg domain:testa domain:testb #testa.py import sharedmod def testa(params): conn = sharedmod.conn logger = params['logger'] Given that the connection object is shared among test cases now, why not logger object too? It's good idea, but if put the logger object in sharedmod.py the sharedmod.py will have to be included in each testcases right now, it is optional, only the testcase that wants to use the connection object offered by framework should import it. Maybe let me wait for some times until the sharedmod become necessary mod for all testcase, then move logger into it. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain:screenshot: Added cleanup function
On 04/16/2012 04:43 AM, Osier Yang wrote: On 2012年04月13日 23:44, Martin Kletzander wrote: Added cleanup function to the screeshot testcase. This makes use of the new sharedmod module. --- WARNING: don't push this before the patch with sharedmod is pushed in the repo, otherwise this will not work. Thanks. repos/domain/screenshot.py |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 3e727a7..5a12c4b 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -6,6 +6,7 @@ import os import mimetypes import libvirt +import sharedmod required_params = ('guestname', 'filename',) optional_params = ('screen',) @@ -36,6 +37,7 @@ def screenshot(params): logger.debug('Mimetype of the file is %s' % mime) ret = st.finish() +sharedmod.dict['screenshot_filename'] = filename Is it neccessary to set a shared variable in the same case? IIUC the purpose of sharemod.py, it's for variable sharing among different test cases. Though the case will work fine, but it's just redundant, and let's keep things simple. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list We can share just the extension if you want. I can also change it to share it in the module global variable. However, the parameter can be used by other tests this way, just in case. We talked about what came to this patch here a little bit: https://www.redhat.com/archives/libvir-list/2012-April/msg00172.html I'm willing to change it to whatever suits the others but I'd like to have it asap so I can finish part of documentation based on this test. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] openvz: wire up more node information functions
On Mon, Apr 16, 2012 at 09:48:38AM +0800, Osier Yang wrote: On 2012年04月15日 04:20, Guido Günther wrote: in detail nodeGetCPUStats, nodeGetMemoryStats, nodeGetCellsFreeMemory and nodeGetFreeMemory --- src/openvz/openvz_driver.c |4 1 file changed, 4 insertions(+) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 3b2ffea..41fd8e4 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1704,6 +1704,10 @@ static virDriver openvzDriver = { .version = openvzGetVersion, /* 0.5.0 */ .getMaxVcpus = openvzGetMaxVCPUs, /* 0.4.6 */ .nodeGetInfo = nodeGetInfo, /* 0.3.2 */ +.nodeGetCPUStats = nodeGetCPUStats, /* 0.9.11 */ +.nodeGetMemoryStats = nodeGetMemoryStats, /* 0.9.11 */ +.nodeGetCellsFreeMemory = nodeGetCellsFreeMemory, /* 0.9.11 */ +.nodeGetFreeMemory = nodeGetFreeMemory, /* 0.9.11 */ .getCapabilities = openvzGetCapabilities, /* 0.4.6 */ .listDomains = openvzListDomains, /* 0.3.1 */ .numOfDomains = openvzNumDomains, /* 0.3.1 */ ACK Pushed. Thanks, -- Guido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] numad: Convert node list to cpumap before setting affinity
On Wed, Apr 11, 2012 at 10:40:32PM +0800, Osier Yang wrote: Instead of returning a CPUs list, numad returns NUMA node list instead, this patch is to convert the node list to cpumap before affinity setting. Otherwise, the domain processes will be pinned only to CPU[$numa_cell_num], which will cause significiant performance losing. s/losing/losses/ Also because numad will balance the affinity dynamically, reflecting the cpuset from numad back doesn't make much sense then, and it may just could produce confusion for users' eyes. Thus the better way is not to reflect it back confusion for the users. to XML. And in this case, it's better to ignore the cpuset when parsing XML. The codes to update the cpuset is removed in this patch incidentally, and there will be a follow up patch to ignore the manually specified cpuset if placement is auto, and document will be updated too. --- The patch is tested on a NUMA box with 4 cells, 24 CPUs. --- src/qemu/qemu_process.c | 33 - 1 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 481b4f3..0bf743b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1819,36 +1819,43 @@ qemuProcessInitCpuAffinity(struct qemud_driver *driver, } if (vm-def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { -char *tmp_cpumask = NULL; char *nodeset = NULL; +char *nodemask = NULL; nodeset = qemuGetNumadAdvice(vm-def); if (!nodeset) return -1; -if (VIR_ALLOC_N(tmp_cpumask, VIR_DOMAIN_CPUMASK_LEN) 0) { +if (VIR_ALLOC_N(nodemask, VIR_DOMAIN_CPUMASK_LEN) 0) { virReportOOMError(); return -1; } -if (virDomainCpuSetParse(nodeset, 0, tmp_cpumask, +if (virDomainCpuSetParse(nodeset, 0, nodemask, VIR_DOMAIN_CPUMASK_LEN) 0) { -VIR_FREE(tmp_cpumask); +VIR_FREE(nodemask); VIR_FREE(nodeset); return -1; } -for (i = 0; i maxcpu i VIR_DOMAIN_CPUMASK_LEN; i++) { -if (tmp_cpumask[i]) -VIR_USE_CPU(cpumap, i); +/* numad returns the NUMA node list, convert it to cpumap */ +int prev_total_ncpus = 0; +for (i = 0; i driver-caps-host.nnumaCell; i++) { +int j; +int cur_ncpus = driver-caps-host.numaCell[i]-ncpus; +if (nodemask[i]) { +for (j = prev_total_ncpus; + j cur_ncpus + prev_total_ncpus + j maxcpu + j VIR_DOMAIN_CPUMASK_LEN; + j++) { +VIR_USE_CPU(cpumap, j); +} +} +prev_total_ncpus += cur_ncpus; } -VIR_FREE(vm-def-cpumask); -vm-def-cpumask = tmp_cpumask; -if (virDomainSaveStatus(driver-caps, driver-stateDir, vm) 0) { -VIR_WARN(Unable to save status on vm %s after state change, - vm-def-name); -} +VIR_FREE(nodemask); VIR_FREE(nodeset); } else { if (vm-def-cpumask) { ACK, but fix the commit message, and wait to see the feedback on 2/3 and 3/3 as this should not be commited in isolation Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] numad: Ignore cpuset if placement is auto
On Wed, Apr 11, 2012 at 10:40:33PM +0800, Osier Yang wrote: As explained in previous patch, numad will balance the affinity dynamically, so reflecting the cpuset from numad at the first time doesn't make much case, and may just could cause confusion. --- docs/formatdomain.html.in | 10 +- src/conf/domain_conf.c| 28 +++- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a382d30..bb67cd1 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -365,11 +365,11 @@ auto, defaults to static if codecpuset/code is specified, auto indicates the domain process will be pinned to the advisory nodeset from querying numad, and the value of attribute -codecpuset/code will be overridden by the advisory nodeset -from numad if it's specified. If both codecpuset/code and -codeplacement/code are not specified, or if codeplacement/code -is static, but no codecpuset/code is specified, the domain -process will be pinned to all the available physical CPUs. +codecpuset/code will be ignored if it's specified. If both +codecpuset/code and codeplacement/code are not specified, +or if codeplacement/code is static, but no codecpuset/code +is specified, the domain process will be pinned to all the +available physical CPUs. /dd /dl diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c6b97e1..07dcc89 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7884,19 +7884,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } } -tmp = virXPathString(string(./vcpu[1]/@cpuset), ctxt); -if (tmp) { -char *set = tmp; -def-cpumasklen = VIR_DOMAIN_CPUMASK_LEN; -if (VIR_ALLOC_N(def-cpumask, def-cpumasklen) 0) { -goto no_memory; -} -if (virDomainCpuSetParse(set, 0, def-cpumask, - def-cpumasklen) 0) -goto error; -VIR_FREE(tmp); -} - tmp = virXPathString(string(./vcpu[1]/@placement), ctxt); if (tmp) { if ((def-placement_mode = @@ -7913,6 +7900,21 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, def-placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC; } +if (def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC) { +tmp = virXPathString(string(./vcpu[1]/@cpuset), ctxt); +if (tmp) { +char *set = tmp; +def-cpumasklen = VIR_DOMAIN_CPUMASK_LEN; +if (VIR_ALLOC_N(def-cpumask, def-cpumasklen) 0) { +goto no_memory; +} +if (virDomainCpuSetParse(set, 0, def-cpumask, + def-cpumasklen) 0) +goto error; +VIR_FREE(tmp); +} +} + /* Extract cpu tunables. */ if (virXPathULong(string(./cputune/shares[1]), ctxt, def-cputune.shares) 0) Okay since we default to static in case the user didn't explicitely expressed the placement method, I'm fine with this this should not break existing setups. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: Avoid the memory allocation and freeing
On Wed, Apr 11, 2012 at 10:40:34PM +0800, Osier Yang wrote: --- src/qemu/qemu_process.c | 14 +- 1 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0bf743b..7a48c12 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1759,23 +1759,19 @@ static char * qemuGetNumadAdvice(virDomainDefPtr def) { virCommandPtr cmd = NULL; -char *args = NULL; char *output = NULL; -if (virAsprintf(args, %d:%llu, def-vcpus, def-mem.cur_balloon) 0) { -virReportOOMError(); -goto out; -} -cmd = virCommandNewArgList(NUMAD, -w, args, NULL); +cmd = virCommandNewArgList(NUMAD, -w, NULL); +virCommandAddArgFormat(cmd, %d:%llu, def-vcpus, + def-mem.cur_balloon); virCommandSetOutputBuffer(cmd, output); if (virCommandRun(cmd, NULL) 0) qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, -_(Failed to query numad for the advisory nodeset)); +_(Failed to query numad for the + advisory nodeset)); -out: -VIR_FREE(args); virCommandFree(cmd); return output; } -- 1.7.1 ACK, that's a pure cleanup, should not change any functionality, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API PATCH 2/2] proxy: import each testcase file only once, initialize proxy once
On 04/16/2012 08:15 AM, Guannan Ren wrote: *libvirt-test-api.py: initialize proxy module only once *casecfgcheck.py: use proxy object rather than initialize it by itself *proxy.py: make get_func_call_dict more flexible --- casecfgcheck.py |5 + libvirt-test-api.py | 12 +--- proxy.py| 22 ++ 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/casecfgcheck.py b/casecfgcheck.py index 40d7c6e..9a4f8e6 100644 --- a/casecfgcheck.py +++ b/casecfgcheck.py @@ -18,13 +18,10 @@ import proxy class CaseCfgCheck(object): validate the options in testcase config file -def __init__(self, unique_testcases, activities_list): -self.unique_testcases = unique_testcases - +def __init__(self, proxy_obj, activities_list): # XXX to check the first testcase list in activities_list self.activity = activities_list[0] -proxy_obj = proxy.Proxy(self.unique_testcases) self.case_params = proxy_obj.get_params_variables() def check(self): diff --git a/libvirt-test-api.py b/libvirt-test-api.py index 385b52d..7b38aaa 100644 --- a/libvirt-test-api.py +++ b/libvirt-test-api.py @@ -112,20 +112,18 @@ class Main(object): unique_testcases = filterobj.unique_testcases() +# __import__ TESTCASE.py once for duplicate testcase names +proxy_obj = proxy.Proxy(unique_testcases) + # check the options to each testcase in case config file -casechk = CaseCfgCheck(unique_testcases, activities_list) +casechk = CaseCfgCheck(proxy_obj, activities_list) if casechk.check(): return 1 # get a list of unique testcase # with 'clean' flag appended to its previous testcase unique_testcase_keys = filterobj.unique_testcase_cleansuffix() - -# call and initilize proxy component to -# get a list of reference of testcases -proxy_obj = proxy.Proxy(unique_testcase_keys) - -cases_func_ref_dict = proxy_obj.get_func_call_dict() +cases_func_ref_dict = proxy_obj.get_func_call_dict(unique_testcase_keys) # create a null list, then, initilize generator to # get the callable testcase function diff --git a/proxy.py b/proxy.py index bc82a84..49a0420 100644 --- a/proxy.py +++ b/proxy.py @@ -1,6 +1,6 @@ #!/usr/bin/env python # -# libvirt-test-API is copyright 2010 Red Hat, Inc. +# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc. # # libvirt-test-API is free software: you can redistribute it and/or modify it # under the terms of the GNU General Public License as published by @@ -37,12 +37,13 @@ class Proxy(object): casename = elements[1] casemod_ref = self.get_call_dict(module, casename) -self.testcase_ref_dict[testcase_name] = casemod_ref +modcase = module + ':' + casename +self.testcase_ref_dict[modcase] = casemod_ref -def get_func_call_dict(self): -Return running function reference dictionary +def get_func_call_dict(self, unique_testcase_keys): +get reference to functions defined in testcase file func_dict = {} -for testcase_name in self.testcases_names: +for testcase_name in unique_testcase_keys: # Get module, casename elements = testcase_name.split(':') module = elements[0] @@ -55,16 +56,21 @@ class Proxy(object): flag = elements[2] func = casename + flag -casemod_ref = self.testcase_ref_dict[testcase_name] +# use modcase key to get the reference to corresponding +# testcase module +modcase = module + ':' + casename +casemod_ref = self.testcase_ref_dict[modcase] var_func_names = dir(casemod_ref) -key = module + ':' + casename + ':' + func +key = modcase + ':' + func +# check if the expected function is present in +# the list of string name from dir() if func in var_func_names: func_ref = getattr(casemod_ref, func) func_dict[key] = func_ref else: raise exception.TestCaseError(function %s not found in %s % \ - (func, testcase_name)) + (func, modcase)) return func_dict def get_clearfunc_call_dict(self): ACK both. Martin P.S.: This is very useful, I don't have to type it manually =) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain:screenshot: Added cleanup function
On 2012年04月16日 14:52, Martin Kletzander wrote: On 04/16/2012 04:43 AM, Osier Yang wrote: On 2012年04月13日 23:44, Martin Kletzander wrote: Added cleanup function to the screeshot testcase. This makes use of the new sharedmod module. --- WARNING: don't push this before the patch with sharedmod is pushed in the repo, otherwise this will not work. Thanks. repos/domain/screenshot.py |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 3e727a7..5a12c4b 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -6,6 +6,7 @@ import os import mimetypes import libvirt +import sharedmod required_params = ('guestname', 'filename',) optional_params = ('screen',) @@ -36,6 +37,7 @@ def screenshot(params): logger.debug('Mimetype of the file is %s' % mime) ret = st.finish() +sharedmod.dict['screenshot_filename'] = filename Is it neccessary to set a shared variable in the same case? IIUC the purpose of sharemod.py, it's for variable sharing among different test cases. Though the case will work fine, but it's just redundant, and let's keep things simple. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list We can share just the extension if you want. I can also change it to share it in the module global variable. However, the parameter can be used by other tests this way, just in case. I don't see any other test case will need the screenshot_filename now, if we have some ones in future, change it to shared var at that time then. The principle is not to put the parameter in shared module if it's no need. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API PATCHv2 1/2] sharemod: Add a new file for variable sharing in testcases
On 04/16/2012 08:03 AM, Osier Yang wrote: s/sharing in/sharing among/, but given it's already pushed. let's live with it. On 2012年04月10日 21:38, Guannan Ren wrote: sharedmod.py --- sharedmod.py | 16 1 files changed, 16 insertions(+), 0 deletions(-) create mode 100644 sharedmod.py diff --git a/sharedmod.py b/sharedmod.py new file mode 100644 index 000..8af26d8 --- /dev/null +++ b/sharedmod.py @@ -0,0 +1,16 @@ +# This is a module for variable sharing across testcases during +# running. You have to import it in each of testcases which want +# to share data. The framwork have already set {'conn': connobj} +# in libvirtobj dictionary for use in testcases. + +# The libvirtobj dictionary is only set and used by framework Any checking? I.e could testcase r/w it too? +# in testcases you could use sharedmod.libvirtobj['conn'] to get +# the connection object in libvirt.py, you need not to close it, IMHO need not should be should never. How about the follow up use of connection if it's already closed, reopen one again? +# the framework do it. snip The framwork have already set {'conn': connobj} +# in libvirtobj dictionary for use in testcases. + /snip The comment above should resides here. +libvirtobj = {} + +# shared variables for customized use in testcases +# set variable: sharedmod.data['my_test_variable'] = 'test_value' +# check the variable: sharedmod.data.has_key('my_test_variable') +# get the varialbe: sharedmod.data.get('my_test_variable', 'test_variable_default_value') From my p.o.v, libvirtobj is a confused name, also data is not enough to tell what the meaning is. How about use framework_shares and case_shares instead? And implement get/set/has functions for both framework_shares and case_shares, but not access them directly. e.g. def case_shares_get(key): pass def case_shares_set(key, value): pass def case_shares_has(key) pass Even perhaps a class is good. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list That's getting closer and closer to what I recommended as a redesign for the whole test-API. Class would be better, tests shouldn't need to import the mod every time, there should be checking etc. However I feel the need to make this stable asap and after some release, we can go ahead with some major redesigns (or at least that's how I understood our talk about this with Dave few weeks ago). Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain:screenshot: Added cleanup function
On 04/16/2012 10:00 AM, Osier Yang wrote: On 2012年04月16日 14:52, Martin Kletzander wrote: On 04/16/2012 04:43 AM, Osier Yang wrote: On 2012年04月13日 23:44, Martin Kletzander wrote: Added cleanup function to the screeshot testcase. This makes use of the new sharedmod module. --- WARNING: don't push this before the patch with sharedmod is pushed in the repo, otherwise this will not work. Thanks. repos/domain/screenshot.py |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 3e727a7..5a12c4b 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -6,6 +6,7 @@ import os import mimetypes import libvirt +import sharedmod required_params = ('guestname', 'filename',) optional_params = ('screen',) @@ -36,6 +37,7 @@ def screenshot(params): logger.debug('Mimetype of the file is %s' % mime) ret = st.finish() +sharedmod.dict['screenshot_filename'] = filename Is it neccessary to set a shared variable in the same case? IIUC the purpose of sharemod.py, it's for variable sharing among different test cases. Though the case will work fine, but it's just redundant, and let's keep things simple. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list We can share just the extension if you want. I can also change it to share it in the module global variable. However, the parameter can be used by other tests this way, just in case. I don't see any other test case will need the screenshot_filename now, if we have some ones in future, change it to shared var at that time then. The principle is not to put the parameter in shared module if it's no need. Osier OK then, I'll save it in the test and send v2. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API PATCH 1/3] cleanup:make clean function optional
On 04/16/2012 02:34 PM, Osier Yang wrote: s/cleanup:make/cleanup: Make/ On 2012年04月16日 14:11, Guannan Ren wrote: The patch make the writing of clean function optional If a testcase makes testing environment dirty after running, it If a case dirties the testing environment, must write a CASENAME_clean function to restore back the Could the $CASENAME_ be omitted? IIRC, the scripts under repos are imported by framework when executing. And thus we can invoke the cleanup functions by $script_name.cleanup. If we add clean flag after testcase in config file like this, the framework will invoke the testa_clean() after running the testa() in testa.py Note: we only add the clean for testcase which has _clean function in it. ... test:testa options value1 clean ... testing environment, otherwise, the clean function could be omitted --- env_clear.py |9 - proxy.py |7 --- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/env_clear.py b/env_clear.py index 3efc9be..2e5bcf5 100644 --- a/env_clear.py +++ b/env_clear.py @@ -30,7 +30,7 @@ class EnvClear(object): self.loglevel = loglevel mapper_obj = mapper.Mapper(activity) -clean_pkg_casename_func = mapper_obj.clean_package_casename_func_map() +clean_pkg_casename_func = mapper_obj.module_casename_cleanfunc_map() self.cases_ref_names = [] for case in clean_pkg_casename_func: @@ -47,7 +47,7 @@ class EnvClear(object): return retflag def env_clear(self): - Run each clearing function with the corresponding arguments + Run each clean function with the corresponding arguments envlog = log.EnvLog(self.logfile, self.loglevel) logger = envlog.env_log() @@ -60,8 +60,7 @@ class EnvClear(object): case_params = self.cases_params_list[i] case_params['logger'] = logger -self.cases_clearfunc_ref_dict[case_ref_name](case_params) - -del envlog +if self.cases_clearfunc_ref_dict.has_key(case_ref_name): To be consistent, clear should be clean. + self.cases_clearfunc_ref_dict[case_ref_name](case_params) return 0 diff --git a/proxy.py b/proxy.py index 3c4cd63..fdbffd9 100644 --- a/proxy.py +++ b/proxy.py @@ -85,12 +85,13 @@ class Proxy(object): var_func_names = dir(casemod_ref) key = module + ':' + casename + ':' + func + +# the clean function is optional, we get its reference +# only if it exists in testcases s/testcases/test cases/, or cases is enough, we should have a consistent term across the project, but not different from here and there. if func in var_func_names: func_ref = getattr(casemod_ref, func) func_dict[key] = func_ref -else: -raise exception.TestCaseError(clean function not found in %s % \ - (func, testcase_name)) + return func_dict def get_params_variables(self): Thanks for the review. I will push it after these problems fixed. Guannan Ren -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API PATCH 3/3] repo: cleanup clean functions that are not used
On 04/16/2012 02:26 PM, Osier Yang wrote: On 2012年04月16日 14:11, Guannan Ren wrote: --- repos/domain/attach_disk.py|4 repos/domain/autostart.py |4 repos/domain/balloon_memory.py |4 repos/domain/blkstats.py |4 repos/domain/console_mutex.py |4 repos/domain/cpu_affinity.py |4 repos/domain/cpu_topology.py |5 - repos/domain/define.py |4 repos/domain/destroy.py|4 repos/domain/detach_disk.py|4 repos/domain/eventhandler.py |4 repos/domain/ifstats.py|4 repos/domain/install_linux_check.py|5 - repos/domain/restore.py|4 repos/domain/resume.py |4 repos/domain/shutdown.py |4 repos/domain/start.py |4 repos/domain/suspend.py|4 repos/domain/undefine.py |4 repos/libvirtd/restart.py |5 - .../multiple_thread_block_on_domain_create.py | 14 -- repos/snapshot/delete.py |4 repos/snapshot/file_flag.py|4 repos/snapshot/flag_check.py |4 repos/snapshot/internal_create.py |4 repos/snapshot/revert.py |5 - repos/snapshot/snapshot_list.py|5 - repos/storage/build_dir_pool.py|3 --- 28 files changed, 0 insertions(+), 126 deletions(-) Please create a separate patch for this. It's of different purpose. Need a v2. Osier Hi Osier It is for the same purpose. After we make the clean function optional, so I cleanup these testcases where the clean function is not used. like the following. -def restart_clean(params): - clean testing environment -pass Guannan Ren -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API PATCH 0/3] make clean function optional
On 04/16/2012 02:20 PM, Osier Yang wrote: On 2012年04月16日 14:11, Guannan Ren wrote: The set of patches is to make the writing of clean function optional. If a testcase which doesn't dirty the testing environment, then the TESTCASE_clean function could be optional or omitted. Good, this was one of the TODO thing in my mind. Because loggers in python are static objects managed by the module itself. When you create one, it won't be removed until the shell quit. In test-API there are two loggers which are configured with two handlers for each. we need to find a way to destruct these two loggers in order no to affect other application. The idea is to clear the handlers of each logger in destructor of CaseLog and EnvLog. def __del__(self): self.logger.handlers = [] rename some unclear function and variables. for example: repos/domain/start.py IIUC, you do 3 different things in these 3 patches, it will be good to document them separately here for easy reviewing. get it, thanks. domain - mod start.py - case start() - func What does these mean? just a naming convention If you see a variable like mod_case_func, that refers to damain:start:start The second start is the function in start.py variable mod_case means domain:start that is the testcase file all of code will use the same naming convention. Guannan Ren -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API PATCH 2/2] proxy: import each testcase file only once, initialize proxy once
On 04/16/2012 03:54 PM, Martin Kletzander wrote: On 04/16/2012 08:15 AM, Guannan Ren wrote: *libvirt-test-api.py: initialize proxy module only once *casecfgcheck.py: use proxy object rather than initialize it by itself *proxy.py: make get_func_call_dict more flexible --- casecfgcheck.py |5 + libvirt-test-api.py | 12 +--- proxy.py| 22 ++ 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/casecfgcheck.py b/casecfgcheck.py index 40d7c6e..9a4f8e6 100644 --- a/casecfgcheck.py +++ b/casecfgcheck.py @@ -18,13 +18,10 @@ import proxy class CaseCfgCheck(object): validate the options in testcase config file -def __init__(self, unique_testcases, activities_list): -self.unique_testcases = unique_testcases - +def __init__(self, proxy_obj, activities_list): # XXX to check the first testcase list in activities_list self.activity = activities_list[0] -proxy_obj = proxy.Proxy(self.unique_testcases) self.case_params = proxy_obj.get_params_variables() def check(self): diff --git a/libvirt-test-api.py b/libvirt-test-api.py index 385b52d..7b38aaa 100644 --- a/libvirt-test-api.py +++ b/libvirt-test-api.py @@ -112,20 +112,18 @@ class Main(object): unique_testcases = filterobj.unique_testcases() +# __import__ TESTCASE.py once for duplicate testcase names +proxy_obj = proxy.Proxy(unique_testcases) + # check the options to each testcase in case config file -casechk = CaseCfgCheck(unique_testcases, activities_list) +casechk = CaseCfgCheck(proxy_obj, activities_list) if casechk.check(): return 1 # get a list of unique testcase # with 'clean' flag appended to its previous testcase unique_testcase_keys = filterobj.unique_testcase_cleansuffix() - -# call and initilize proxy component to -# get a list of reference of testcases -proxy_obj = proxy.Proxy(unique_testcase_keys) - -cases_func_ref_dict = proxy_obj.get_func_call_dict() +cases_func_ref_dict = proxy_obj.get_func_call_dict(unique_testcase_keys) # create a null list, then, initilize generator to # get the callable testcase function diff --git a/proxy.py b/proxy.py index bc82a84..49a0420 100644 --- a/proxy.py +++ b/proxy.py @@ -1,6 +1,6 @@ #!/usr/bin/env python # -# libvirt-test-API is copyright 2010 Red Hat, Inc. +# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc. # # libvirt-test-API is free software: you can redistribute it and/or modify it # under the terms of the GNU General Public License as published by @@ -37,12 +37,13 @@ class Proxy(object): casename = elements[1] casemod_ref = self.get_call_dict(module, casename) -self.testcase_ref_dict[testcase_name] = casemod_ref +modcase = module + ':' + casename +self.testcase_ref_dict[modcase] = casemod_ref -def get_func_call_dict(self): -Return running function reference dictionary +def get_func_call_dict(self, unique_testcase_keys): +get reference to functions defined in testcase file func_dict = {} -for testcase_name in self.testcases_names: +for testcase_name in unique_testcase_keys: # Get module, casename elements = testcase_name.split(':') module = elements[0] @@ -55,16 +56,21 @@ class Proxy(object): flag = elements[2] func = casename + flag -casemod_ref = self.testcase_ref_dict[testcase_name] +# use modcase key to get the reference to corresponding +# testcase module +modcase = module + ':' + casename +casemod_ref = self.testcase_ref_dict[modcase] var_func_names = dir(casemod_ref) -key = module + ':' + casename + ':' + func +key = modcase + ':' + func +# check if the expected function is present in +# the list of string name from dir() if func in var_func_names: func_ref = getattr(casemod_ref, func) func_dict[key] = func_ref else: raise exception.TestCaseError(function %s not found in %s % \ - (func, testcase_name)) + (func, modcase)) return func_dict def get_clearfunc_call_dict(self): ACK both. Martin P.S.: This is very useful, I don't have to type it manually =) Thanks and pushed. Guannan Ren -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-API PATCH] domain:screenshot: Added cleanup function
--- v2: - removed sharedmod for persistence of the filename repos/domain/screenshot.py |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 82425f3..2761dc5 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -10,6 +10,8 @@ import libvirt required_params = ('guestname', 'filename',) optional_params = ('screen',) +last_filename = None + def saver(stream, data, file_): return file_.write(data) @@ -27,7 +29,7 @@ def screenshot(params): mime = dom.screenshot(st, int(screen), 0) ext = mimetypes.guess_extension(mime) or '.ppm' -filename = params['filename'] + ext +last_filename = params['filename'] + ext f = file(filename, 'w') logger.debug('Saving screenshot into %s' % filename) @@ -37,3 +39,7 @@ def screenshot(params): ret = st.finish() return ret + +def cleanup(params): +if last_filename: +os.remove(sharedmod['last_filename']) -- 1.7.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] numad: Convert node list to cpumap before setting affinity
On 2012年04月16日 15:45, Daniel Veillard wrote: On Wed, Apr 11, 2012 at 10:40:32PM +0800, Osier Yang wrote: Instead of returning a CPUs list, numad returns NUMA node list instead, this patch is to convert the node list to cpumap before affinity setting. Otherwise, the domain processes will be pinned only to CPU[$numa_cell_num], which will cause significiant performance losing. s/losing/losses/ Also because numad will balance the affinity dynamically, reflecting the cpuset from numad back doesn't make much sense then, and it may just could produce confusion for users' eyes. Thus the better way is not to reflect it back confusion for the users. to XML. And in this case, it's better to ignore the cpuset when parsing XML. The codes to update the cpuset is removed in this patch incidentally, and there will be a follow up patch to ignore the manually specified cpuset if placement is auto, and document will be updated too. --- The patch is tested on a NUMA box with 4 cells, 24 CPUs. --- src/qemu/qemu_process.c | 33 - 1 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 481b4f3..0bf743b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1819,36 +1819,43 @@ qemuProcessInitCpuAffinity(struct qemud_driver *driver, } if (vm-def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { -char *tmp_cpumask = NULL; char *nodeset = NULL; +char *nodemask = NULL; nodeset = qemuGetNumadAdvice(vm-def); if (!nodeset) return -1; -if (VIR_ALLOC_N(tmp_cpumask, VIR_DOMAIN_CPUMASK_LEN) 0) { +if (VIR_ALLOC_N(nodemask, VIR_DOMAIN_CPUMASK_LEN) 0) { virReportOOMError(); return -1; } -if (virDomainCpuSetParse(nodeset, 0, tmp_cpumask, +if (virDomainCpuSetParse(nodeset, 0, nodemask, VIR_DOMAIN_CPUMASK_LEN) 0) { -VIR_FREE(tmp_cpumask); +VIR_FREE(nodemask); VIR_FREE(nodeset); return -1; } -for (i = 0; i maxcpu i VIR_DOMAIN_CPUMASK_LEN; i++) { -if (tmp_cpumask[i]) -VIR_USE_CPU(cpumap, i); +/* numad returns the NUMA node list, convert it to cpumap */ +int prev_total_ncpus = 0; +for (i = 0; i driver-caps-host.nnumaCell; i++) { +int j; +int cur_ncpus = driver-caps-host.numaCell[i]-ncpus; +if (nodemask[i]) { +for (j = prev_total_ncpus; + j cur_ncpus + prev_total_ncpus + j maxcpu + j VIR_DOMAIN_CPUMASK_LEN; + j++) { +VIR_USE_CPU(cpumap, j); +} +} +prev_total_ncpus += cur_ncpus; } -VIR_FREE(vm-def-cpumask); -vm-def-cpumask = tmp_cpumask; -if (virDomainSaveStatus(driver-caps, driver-stateDir, vm) 0) { -VIR_WARN(Unable to save status on vm %s after state change, - vm-def-name); -} +VIR_FREE(nodemask); VIR_FREE(nodeset); } else { if (vm-def-cpumask) { ACK, but fix the commit message, and wait to see the feedback on 2/3 and 3/3 as this should not be commited in isolation Daniel Thanks, pushed series with nits fixed. Also with conflicts with commit 354e6d4ed. Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-test-API PATCH] domain:screenshot: Added cleanup function
On 2012年04月16日 17:32, Martin Kletzander wrote: --- v2: - removed sharedmod for persistence of the filename repos/domain/screenshot.py |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 82425f3..2761dc5 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -10,6 +10,8 @@ import libvirt required_params = ('guestname', 'filename',) optional_params = ('screen',) +last_filename = None + def saver(stream, data, file_): return file_.write(data) @@ -27,7 +29,7 @@ def screenshot(params): mime = dom.screenshot(st, int(screen), 0) ext = mimetypes.guess_extension(mime) or '.ppm' -filename = params['filename'] + ext +last_filename = params['filename'] + ext f = file(filename, 'w') logger.debug('Saving screenshot into %s' % filename) @@ -37,3 +39,7 @@ def screenshot(params): ret = st.finish() return ret + +def cleanup(params): +if last_filename: +os.remove(sharedmod['last_filename']) Shoud this be the following instead? os.remove(last_filename) ACK with the nit fixed. Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-test-API PATCH] domain:screenshot: Added cleanup function
On 04/16/2012 12:14 PM, Osier Yang wrote: On 2012年04月16日 17:32, Martin Kletzander wrote: --- v2: - removed sharedmod for persistence of the filename repos/domain/screenshot.py |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index 82425f3..2761dc5 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -10,6 +10,8 @@ import libvirt required_params = ('guestname', 'filename',) optional_params = ('screen',) +last_filename = None + def saver(stream, data, file_): return file_.write(data) @@ -27,7 +29,7 @@ def screenshot(params): mime = dom.screenshot(st, int(screen), 0) ext = mimetypes.guess_extension(mime) or '.ppm' -filename = params['filename'] + ext +last_filename = params['filename'] + ext f = file(filename, 'w') logger.debug('Saving screenshot into %s' % filename) @@ -37,3 +39,7 @@ def screenshot(params): ret = st.finish() return ret + +def cleanup(params): +if last_filename: +os.remove(sharedmod['last_filename']) Shoud this be the following instead? os.remove(last_filename) ACK with the nit fixed. Regards, Osier Yes, of course, stupid error, sorry. Fixed and pushed. Thanks, Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC
On 04/13/2012 09:14 AM, Stefan Bader wrote: I think it would be better if we just centralized this logic, as in, only set that (type ioemu) bit in conditional rather than 2. Should be pretty straightforward. Did you have something like below in mind? -Stefan From a0e214fa6dea9bd66616f37246067a2c631f4184 Mon Sep 17 00:00:00 2001 From: Stefan Bader stefan.ba...@canonical.com Date: Thu, 12 Apr 2012 15:32:41 +0200 Subject: [PATCH] libvirt: xen: do not use ioemu type for any emulated NIC When using the xm/xend stack to manage instances there is a bug that causes the emulated interfaces to be unusable when the vif config contains type=ioemu. The current code already has a special quirk to not use this keyword if no specific model is given for the emulated NIC (defaulting to rtl8139). Essentially it works because regardless of the type argument,i the Xen stack always creates emulated and paravirt interfaces and lets the guest decide which one to use. So neither xl nor xm stack actually require the type keyword for emulated NICs. [v2: move code around to have the exception in one case together] Signed-off-by: Stefan Bader stefan.ba...@canonical.com --- src/xenxs/xen_sxpr.c | 28 +++- src/xenxs/xen_xm.c | 27 ++- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index e1bbd76..3a56345 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1999,20 +1999,22 @@ xenFormatSxprNet(virConnectPtr conn, if (def-model != NULL) virBufferEscapeSexpr(buf, (model '%s'), def-model); } -else if (def-model == NULL) { -/* - * apparently (type ioemu) breaks paravirt drivers on HVM so skip - * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, (type ioemu)); -} -else if (STREQ(def-model, netfront)) { -virBufferAddLit(buf, (type netfront)); -} else { -virBufferEscapeSexpr(buf, (model '%s'), def-model); -virBufferAddLit(buf, (type ioemu)); +if (def-model != NULL STREQ(def-model, netfront)) { +virBufferAddLit(buf, (type netfront)); +} +else { +if (def-model != NULL) { +virBufferEscapeSexpr(buf, (model '%s'), def-model); +} +/* + * apparently (type ioemu) breaks paravirt drivers on HVM so skip + * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) { +virBufferAddLit(buf, (type ioemu)); +} +} } if (!isAttach) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index d65e97a..93a26f9 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1381,20 +1381,21 @@ static int xenFormatXMNet(virConnectPtr conn, if (net-model != NULL) virBufferAsprintf(buf, ,model=%s, net-model); } -else if (net-model == NULL) { -/* - * apparently type ioemu breaks paravirt drivers on HVM so skip this - * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, ,type=ioemu); -} -else if (STREQ(net-model, netfront)) { -virBufferAddLit(buf, ,type=netfront); -} else { -virBufferAsprintf(buf, ,model=%s, net-model); -virBufferAddLit(buf, ,type=ioemu); +if (net-model != NULL STREQ(net-model, netfront)) { +virBufferAddLit(buf, ,type=netfront); +} +else { +if (net-model != NULL) +virBufferAsprintf(buf, ,model=%s, net-model); + +/* + * apparently type ioemu breaks paravirt drivers on HVM so skip this + * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) +virBufferAddLit(buf, ,type=ioemu); +} } if (net-ifname) Looks good. Did you verify 'make check' passes as well? If so, ACK - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] cgroup cpuset limited by default?
Hi, I noticed recently that my system VMs feel much slower, and that seems to be because of cpuset cgroup which constrain the VM on the first cpu. This doesn't happen with session VMs, since they are not ruled by cgroup. I run f17 with libvirt git, the VMs are created with virt-manager, using mostly default settings vcpu4/vcpu os type arch='x86_64' machine='pc-0.15'hvm/type boot dev='hd'/ /os features acpi/ apic/ pae/ /features clock offset='localtime'/ Is that intended or is it a bug? -- Marc-André Lureau -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: lvm: use correct lv* command parameters
On 04/15/2012 09:22 PM, Osier Yang wrote: On 2012幎04æ13æ¥ 22:09, Cole Robinson wrote: On 04/13/2012 09:03 AM, Osier Yang wrote: On 04/13/2012 07:50 PM, Cole Robinson wrote: lvcreate want's the parent pool's name, not the pool path lvchange and lvremove want lv specified as $vgname/$lvname This largely worked before because these commands strip off a starting /dev. But https://bugzilla.redhat.com/show_bug.cgi?id=714986 is from a user using a 'nested VG' that was having problems. I couldn't find any info on nested LVM and the reporter never responded, but I reproduced with XML that specified a valid source name, and set target path to a symlink. Signed-off-by: Cole Robinsoncrobi...@redhat.com --- src/storage/storage_backend_logical.c | 21 +++-- 1 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 6a235f6..9a91dd9 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -672,7 +672,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, char size[100]; const char *cmdargvnew[] = { LVCREATE, --name, vol-name, -L, size, -pool-def-target.path, NULL +pool-def-source.name, NULL This makes sense. }; const char *cmdargvsnap[] = { LVCREATE, --name, vol-name, -L, size, @@ -778,23 +778,23 @@ virStorageBackendLogicalDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned int flags) { int ret = -1; +char *volpath = NULL; virCommandPtr lvchange_cmd = NULL; virCommandPtr lvremove_cmd = NULL; virCheckFlags(0, -1); -virFileWaitForDevices(); +if (virAsprintf(volpath, %s/%s, +pool-def-source.name, vol-name) 0) { +virReportOOMError(); +goto cleanup; +} -lvchange_cmd = virCommandNewArgList(LVCHANGE, --aln, -vol-target.path, -NULL); +virFileWaitForDevices(); -lvremove_cmd = virCommandNewArgList(LVREMOVE, --f, -vol-target.path, -NULL); +lvchange_cmd = virCommandNewArgList(LVCHANGE, -aln, volpath, NULL); +lvremove_cmd = virCommandNewArgList(LVREMOVE, -f, volpath, NULL); I tried with both vol-target.path, and $vgname/$lvname, both of them work. So do we really need these changes? They both work, just like lvcreate /dev/myvgname also works. But we do need this if this 'nested lvm' thing actually exists as the reporter mentioned, or user uses a symlink as the target path (I'm not saying that's a valid use case but it's a data point). Also the man pages for lvremove and lvchange but use that format in their examples. Thanks, Cole Makes sense, and ACK then. Thanks, pushed now. - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] cgroup cpuset limited by default?
On 04/16/2012 05:34 PM, Marc-André Lureau wrote: Hi, I noticed recently that my system VMs feel much slower, and that seems to be because of cpuset cgroup which constrain the VM on the first cpu. This doesn't happen with session VMs, since they are not ruled by cgroup. I run f17 with libvirt git, the VMs are created with virt-manager, using mostly default settings vcpu4/vcpu os type arch='x86_64' machine='pc-0.15'hvm/type boot dev='hd'/ /os features acpi/ apic/ pae/ /features clock offset='localtime'/ Is that intended or is it a bug? Did you happen to perform a suspend/resume or a hibernation/restore on your computer? (Or did you do CPU hotplug manually?) If yes, you might be seeing the problem reported at: https://bugzilla.redhat.com/show_bug.cgi?id=714271 Unfortunately, as of now the kernel still doesn't handle this properly.. IOW, we don't have a kernel fix (yet). Regards, Srivatsa S. Bhat IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC
On 16.04.2012 13:58, Cole Robinson wrote: On 04/13/2012 09:14 AM, Stefan Bader wrote: I think it would be better if we just centralized this logic, as in, only set that (type ioemu) bit in conditional rather than 2. Should be pretty straightforward. Did you have something like below in mind? -Stefan From a0e214fa6dea9bd66616f37246067a2c631f4184 Mon Sep 17 00:00:00 2001 From: Stefan Bader stefan.ba...@canonical.com Date: Thu, 12 Apr 2012 15:32:41 +0200 Subject: [PATCH] libvirt: xen: do not use ioemu type for any emulated NIC When using the xm/xend stack to manage instances there is a bug that causes the emulated interfaces to be unusable when the vif config contains type=ioemu. The current code already has a special quirk to not use this keyword if no specific model is given for the emulated NIC (defaulting to rtl8139). Essentially it works because regardless of the type argument,i the Xen stack always creates emulated and paravirt interfaces and lets the guest decide which one to use. So neither xl nor xm stack actually require the type keyword for emulated NICs. [v2: move code around to have the exception in one case together] Signed-off-by: Stefan Bader stefan.ba...@canonical.com --- src/xenxs/xen_sxpr.c | 28 +++- src/xenxs/xen_xm.c | 27 ++- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index e1bbd76..3a56345 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1999,20 +1999,22 @@ xenFormatSxprNet(virConnectPtr conn, if (def-model != NULL) virBufferEscapeSexpr(buf, (model '%s'), def-model); } -else if (def-model == NULL) { -/* - * apparently (type ioemu) breaks paravirt drivers on HVM so skip - * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, (type ioemu)); -} -else if (STREQ(def-model, netfront)) { -virBufferAddLit(buf, (type netfront)); -} else { -virBufferEscapeSexpr(buf, (model '%s'), def-model); -virBufferAddLit(buf, (type ioemu)); +if (def-model != NULL STREQ(def-model, netfront)) { +virBufferAddLit(buf, (type netfront)); +} +else { +if (def-model != NULL) { +virBufferEscapeSexpr(buf, (model '%s'), def-model); +} +/* + * apparently (type ioemu) breaks paravirt drivers on HVM so skip + * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) { +virBufferAddLit(buf, (type ioemu)); +} +} } if (!isAttach) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index d65e97a..93a26f9 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1381,20 +1381,21 @@ static int xenFormatXMNet(virConnectPtr conn, if (net-model != NULL) virBufferAsprintf(buf, ,model=%s, net-model); } -else if (net-model == NULL) { -/* - * apparently type ioemu breaks paravirt drivers on HVM so skip this - * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, ,type=ioemu); -} -else if (STREQ(net-model, netfront)) { -virBufferAddLit(buf, ,type=netfront); -} else { -virBufferAsprintf(buf, ,model=%s, net-model); -virBufferAddLit(buf, ,type=ioemu); +if (net-model != NULL STREQ(net-model, netfront)) { +virBufferAddLit(buf, ,type=netfront); +} +else { +if (net-model != NULL) +virBufferAsprintf(buf, ,model=%s, net-model); + +/* + * apparently type ioemu breaks paravirt drivers on HVM so skip this + * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) +virBufferAddLit(buf, ,type=ioemu); +} } if (net-ifname) Looks good. Did you verify 'make check' passes as well? If so, ACK No :( And it fails. TEST: libvirtdconftest .!!...!!!37 FAIL FAIL: libvirtdconftest I guess this is expected as the change really does modify the generated configs. So I need to find out how to make it expect that... - Cole signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC
On 04/16/2012 08:36 AM, Stefan Bader wrote: On 16.04.2012 13:58, Cole Robinson wrote: On 04/13/2012 09:14 AM, Stefan Bader wrote: I think it would be better if we just centralized this logic, as in, only set that (type ioemu) bit in conditional rather than 2. Should be pretty straightforward. Did you have something like below in mind? -Stefan From a0e214fa6dea9bd66616f37246067a2c631f4184 Mon Sep 17 00:00:00 2001 From: Stefan Bader stefan.ba...@canonical.com Date: Thu, 12 Apr 2012 15:32:41 +0200 Subject: [PATCH] libvirt: xen: do not use ioemu type for any emulated NIC When using the xm/xend stack to manage instances there is a bug that causes the emulated interfaces to be unusable when the vif config contains type=ioemu. The current code already has a special quirk to not use this keyword if no specific model is given for the emulated NIC (defaulting to rtl8139). Essentially it works because regardless of the type argument,i the Xen stack always creates emulated and paravirt interfaces and lets the guest decide which one to use. So neither xl nor xm stack actually require the type keyword for emulated NICs. [v2: move code around to have the exception in one case together] Signed-off-by: Stefan Bader stefan.ba...@canonical.com --- src/xenxs/xen_sxpr.c | 28 +++- src/xenxs/xen_xm.c | 27 ++- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index e1bbd76..3a56345 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1999,20 +1999,22 @@ xenFormatSxprNet(virConnectPtr conn, if (def-model != NULL) virBufferEscapeSexpr(buf, (model '%s'), def-model); } -else if (def-model == NULL) { -/* - * apparently (type ioemu) breaks paravirt drivers on HVM so skip - * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, (type ioemu)); -} -else if (STREQ(def-model, netfront)) { -virBufferAddLit(buf, (type netfront)); -} else { -virBufferEscapeSexpr(buf, (model '%s'), def-model); -virBufferAddLit(buf, (type ioemu)); +if (def-model != NULL STREQ(def-model, netfront)) { +virBufferAddLit(buf, (type netfront)); +} +else { +if (def-model != NULL) { +virBufferEscapeSexpr(buf, (model '%s'), def-model); +} +/* + * apparently (type ioemu) breaks paravirt drivers on HVM so skip + * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) { +virBufferAddLit(buf, (type ioemu)); +} +} } if (!isAttach) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index d65e97a..93a26f9 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1381,20 +1381,21 @@ static int xenFormatXMNet(virConnectPtr conn, if (net-model != NULL) virBufferAsprintf(buf, ,model=%s, net-model); } -else if (net-model == NULL) { -/* - * apparently type ioemu breaks paravirt drivers on HVM so skip this - * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, ,type=ioemu); -} -else if (STREQ(net-model, netfront)) { -virBufferAddLit(buf, ,type=netfront); -} else { -virBufferAsprintf(buf, ,model=%s, net-model); -virBufferAddLit(buf, ,type=ioemu); +if (net-model != NULL STREQ(net-model, netfront)) { +virBufferAddLit(buf, ,type=netfront); +} +else { +if (net-model != NULL) +virBufferAsprintf(buf, ,model=%s, net-model); + +/* + * apparently type ioemu breaks paravirt drivers on HVM so skip this + * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) +virBufferAddLit(buf, ,type=ioemu); +} } if (net-ifname) Looks good. Did you verify 'make check' passes as well? If so, ACK No :( And it fails. TEST: libvirtdconftest .!!...!!!37 FAIL FAIL: libvirtdconftest I guess this is expected as the change really does modify the generated configs. So I need to find out how to make it expect that... Make sure it's actually your patch causing those issues. I wouldn't expect your change to affect that particular test. The first section of HACKING has some info about things to run before submitting a patch, and ways to debug test failures. Thanks,
Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC
On 16.04.2012 14:45, Cole Robinson wrote: On 04/16/2012 08:36 AM, Stefan Bader wrote: On 16.04.2012 13:58, Cole Robinson wrote: On 04/13/2012 09:14 AM, Stefan Bader wrote: I think it would be better if we just centralized this logic, as in, only set that (type ioemu) bit in conditional rather than 2. Should be pretty straightforward. Did you have something like below in mind? -Stefan From a0e214fa6dea9bd66616f37246067a2c631f4184 Mon Sep 17 00:00:00 2001 From: Stefan Bader stefan.ba...@canonical.com Date: Thu, 12 Apr 2012 15:32:41 +0200 Subject: [PATCH] libvirt: xen: do not use ioemu type for any emulated NIC When using the xm/xend stack to manage instances there is a bug that causes the emulated interfaces to be unusable when the vif config contains type=ioemu. The current code already has a special quirk to not use this keyword if no specific model is given for the emulated NIC (defaulting to rtl8139). Essentially it works because regardless of the type argument,i the Xen stack always creates emulated and paravirt interfaces and lets the guest decide which one to use. So neither xl nor xm stack actually require the type keyword for emulated NICs. [v2: move code around to have the exception in one case together] Signed-off-by: Stefan Bader stefan.ba...@canonical.com --- src/xenxs/xen_sxpr.c | 28 +++- src/xenxs/xen_xm.c | 27 ++- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index e1bbd76..3a56345 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1999,20 +1999,22 @@ xenFormatSxprNet(virConnectPtr conn, if (def-model != NULL) virBufferEscapeSexpr(buf, (model '%s'), def-model); } -else if (def-model == NULL) { -/* - * apparently (type ioemu) breaks paravirt drivers on HVM so skip - * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, (type ioemu)); -} -else if (STREQ(def-model, netfront)) { -virBufferAddLit(buf, (type netfront)); -} else { -virBufferEscapeSexpr(buf, (model '%s'), def-model); -virBufferAddLit(buf, (type ioemu)); +if (def-model != NULL STREQ(def-model, netfront)) { +virBufferAddLit(buf, (type netfront)); +} +else { +if (def-model != NULL) { +virBufferEscapeSexpr(buf, (model '%s'), def-model); +} +/* + * apparently (type ioemu) breaks paravirt drivers on HVM so skip + * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) { +virBufferAddLit(buf, (type ioemu)); +} +} } if (!isAttach) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index d65e97a..93a26f9 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1381,20 +1381,21 @@ static int xenFormatXMNet(virConnectPtr conn, if (net-model != NULL) virBufferAsprintf(buf, ,model=%s, net-model); } -else if (net-model == NULL) { -/* - * apparently type ioemu breaks paravirt drivers on HVM so skip this - * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, ,type=ioemu); -} -else if (STREQ(net-model, netfront)) { -virBufferAddLit(buf, ,type=netfront); -} else { -virBufferAsprintf(buf, ,model=%s, net-model); -virBufferAddLit(buf, ,type=ioemu); +if (net-model != NULL STREQ(net-model, netfront)) { +virBufferAddLit(buf, ,type=netfront); +} +else { +if (net-model != NULL) +virBufferAsprintf(buf, ,model=%s, net-model); + +/* + * apparently type ioemu breaks paravirt drivers on HVM so skip this + * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) +virBufferAddLit(buf, ,type=ioemu); +} } if (net-ifname) Looks good. Did you verify 'make check' passes as well? If so, ACK No :( And it fails. TEST: libvirtdconftest .!!...!!!37 FAIL FAIL: libvirtdconftest I guess this is expected as the change really does modify the generated configs. So I need to find out how to make it expect that... Make sure it's actually your patch causing those issues. I wouldn't expect your change to affect that particular test. Right, I just ran the checks with my patch reverted and it fails exactly the same way.
Re: [libvirt] [PATCHv4 08/18] blockjob: react to active block copy
On Fri, Apr 13, 2012 at 09:28:08 -0600, Eric Blake wrote: On 04/13/2012 05:35 AM, Jiri Denemark wrote: On Mon, Apr 09, 2012 at 21:52:17 -0600, Eric Blake wrote: For now, disk migration via block copy job is not implemented. But when we do implement it, we have to deal with the fact that qemu does not provide an easy way to re-start a qemu process with mirroring still intact (it _might_ be possible by using qemu -S then an initial 'drive-mirror' with disk reuse before starting the domain, but that gets hairy). Even something like 'virDomainSave' becomes hairy, if you realize the implications that 'virDomainRestore' would be stuck with recreating the same mirror layout. @@ -4947,6 +4952,12 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { goto cleanup; } def = NULL; +if (virDomainHasDiskMirror(vm)) { +qemuReportError(VIR_ERR_BLOCK_COPY_ACTIVE, %s, +_(domain has active block copy job)); +virDomainObjAssignDef(vm, NULL, false); +goto cleanup; +} vm-persistent = 1; I think it would be better to do this check a bit earlier in the process to avoid the need to undo virDomainObjAssignDef(). I see where you are coming from in the limited context shown in this patch (and it even matches my initial thoughts when first trying to write the patch), but look at the bigger picture: if (!(vm = virDomainAssignDef(driver-caps, driver-domains, def, false))) { goto cleanup; } def = NULL; +if (virDomainHasDiskMirror(vm)) { +qemuReportError(VIR_ERR_BLOCK_COPY_ACTIVE, %s, +_(domain has active block copy job)); +virDomainObjAssignDef(vm, NULL, false); +goto cleanup; +} vm-persistent = 1; That is, we don't know what vm is, to check if it has a disk mirror, until after we have already associated a potential persistent definition Oh, right. Although I was looking at this change in the context of complete qemudDomainDefine(), I somehow missed the fact that virDomainAssignDef() does the lookup. OK, then. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Do not enforce source type of console[0]
On 2012-03-13 16:48, Jan Kiszka wrote: If console[0] is an alias for serial[0], do not enforce the former to have a PTY source type. This breaks serial consoles on stdio and makes no sense. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- src/qemu/qemu_process.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ef311d1..216b594 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1376,7 +1376,6 @@ qemuProcessFindCharDevicePTYs(virDomainObjPtr vm, if ((ret = virDomainChrSourceDefCopy(chr-source, ((vm-def-serials[0])-source))) != 0) return ret; -chr-source.type = VIR_DOMAIN_CHR_TYPE_PTY; } else { if (chr-source.type == VIR_DOMAIN_CHR_TYPE_PTY chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) { Ping. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 12/18] blockjob: implement block copy for qemu
On Fri, Apr 13, 2012 at 09:15:18 -0600, Eric Blake wrote: On 04/13/2012 08:46 AM, Jiri Denemark wrote: I guess I wasn't paying enough attention somewhere but why do we forbid block copy for persistent domains? I understand why we want to forbid certain operations when block copy is active but I don't see a reason for penalizing persistent domains. It was in patch 8/18 where I added the restrictions, and hopefully documented in that commit message why limiting things to transient is a good first step: 1. the first client of live migration is oVirt, which uses transient domains 2. qemu does not (yet) provide a way to resume a mirror when restarting a domain, so anything that would require restoring a domain from saved state is broken: incoming migration, virsh restore, and virsh start after a managed save. But users would be upset if they saved a domain, only to find out that they cannot then restore it, so I squelched things one step earlier in the process, by preventing any save of a domain so that we never have a broken save image in the first place. My worry now comes from the fact that managedsave is on the list of forbidden operations. If a domain is transient, managedsave is already forbidden (it is assumed that you either don't care about the domain if the host dies, or that you are running a higher-level app like oVirt that knows how to rebuild the guest on a different host). But if a guest is persistent, and you use the libvirt-guests init script, then Yeah, this was the part I didn't see. Failing manually issued commands is fine but failing managedsave called from libvirt-guests during host shutdown/reboot is not nice. I agree with forbidding block copy for persistent domains then. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC
On 16.04.2012 14:54, Stefan Bader wrote: On 16.04.2012 14:45, Cole Robinson wrote: On 04/16/2012 08:36 AM, Stefan Bader wrote: On 16.04.2012 13:58, Cole Robinson wrote: On 04/13/2012 09:14 AM, Stefan Bader wrote: I think it would be better if we just centralized this logic, as in, only set that (type ioemu) bit in conditional rather than 2. Should be pretty straightforward. Did you have something like below in mind? -Stefan From a0e214fa6dea9bd66616f37246067a2c631f4184 Mon Sep 17 00:00:00 2001 From: Stefan Bader stefan.ba...@canonical.com Date: Thu, 12 Apr 2012 15:32:41 +0200 Subject: [PATCH] libvirt: xen: do not use ioemu type for any emulated NIC When using the xm/xend stack to manage instances there is a bug that causes the emulated interfaces to be unusable when the vif config contains type=ioemu. The current code already has a special quirk to not use this keyword if no specific model is given for the emulated NIC (defaulting to rtl8139). Essentially it works because regardless of the type argument,i the Xen stack always creates emulated and paravirt interfaces and lets the guest decide which one to use. So neither xl nor xm stack actually require the type keyword for emulated NICs. [v2: move code around to have the exception in one case together] Signed-off-by: Stefan Bader stefan.ba...@canonical.com --- src/xenxs/xen_sxpr.c | 28 +++- src/xenxs/xen_xm.c | 27 ++- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index e1bbd76..3a56345 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1999,20 +1999,22 @@ xenFormatSxprNet(virConnectPtr conn, if (def-model != NULL) virBufferEscapeSexpr(buf, (model '%s'), def-model); } -else if (def-model == NULL) { -/* - * apparently (type ioemu) breaks paravirt drivers on HVM so skip - * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, (type ioemu)); -} -else if (STREQ(def-model, netfront)) { -virBufferAddLit(buf, (type netfront)); -} else { -virBufferEscapeSexpr(buf, (model '%s'), def-model); -virBufferAddLit(buf, (type ioemu)); +if (def-model != NULL STREQ(def-model, netfront)) { +virBufferAddLit(buf, (type netfront)); +} +else { +if (def-model != NULL) { +virBufferEscapeSexpr(buf, (model '%s'), def-model); +} +/* + * apparently (type ioemu) breaks paravirt drivers on HVM so skip + * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) { +virBufferAddLit(buf, (type ioemu)); +} +} } if (!isAttach) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index d65e97a..93a26f9 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1381,20 +1381,21 @@ static int xenFormatXMNet(virConnectPtr conn, if (net-model != NULL) virBufferAsprintf(buf, ,model=%s, net-model); } -else if (net-model == NULL) { -/* - * apparently type ioemu breaks paravirt drivers on HVM so skip this - * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, ,type=ioemu); -} -else if (STREQ(net-model, netfront)) { -virBufferAddLit(buf, ,type=netfront); -} else { -virBufferAsprintf(buf, ,model=%s, net-model); -virBufferAddLit(buf, ,type=ioemu); +if (net-model != NULL STREQ(net-model, netfront)) { +virBufferAddLit(buf, ,type=netfront); +} +else { +if (net-model != NULL) +virBufferAsprintf(buf, ,model=%s, net-model); + +/* + * apparently type ioemu breaks paravirt drivers on HVM so skip this + * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) +virBufferAddLit(buf, ,type=ioemu); +} } if (net-ifname) Looks good. Did you verify 'make check' passes as well? If so, ACK No :( And it fails. TEST: libvirtdconftest .!!...!!!37 FAIL FAIL: libvirtdconftest I guess this is expected as the change really does modify the generated configs. So I need to find out how to make it expect that... Make sure it's actually your patch causing those issues. I wouldn't expect your change to affect that particular test. Right, I just ran the checks with my patch
[libvirt] [PATCH] docs: fix path to openvz network configuration file
It's vznet.conf not vznetctl.conf, see e.g.: http://git.openvz.org/?p=vzctl;a=blob;f=bin/vznetcfg.in;h=e91f5c4a0744c1ea149e1b8c241b666052e10b12;hb=HEAD --- docs/drvopenvz.html.in |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/drvopenvz.html.in b/docs/drvopenvz.html.in index bb437d3..a8f9f28 100644 --- a/docs/drvopenvz.html.in +++ b/docs/drvopenvz.html.in @@ -67,7 +67,7 @@ openvz+ssh://r...@example.com/system (remote access, SSH tunnelled) script must be created manually by the host OS administrator. The simplest way is to just download the latest version of this script from a newer OpenVZ release, or upstream source repository. Then -a generic configuration file code/etc/vz/vznetctl.conf/code +a generic configuration file code/etc/vz/vznet.conf/code must be created containing /p -- 1.7.10 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: fix path to openvz network configuration file
On 16.04.2012 16:21, Guido Günther wrote: It's vznet.conf not vznetctl.conf, see e.g.: http://git.openvz.org/?p=vzctl;a=blob;f=bin/vznetcfg.in;h=e91f5c4a0744c1ea149e1b8c241b666052e10b12;hb=HEAD --- docs/drvopenvz.html.in |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Qualifies as trivial. Anyway, ACK. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 16/18] blockjob: add virDomainBlockCopy
On Fri, Apr 13, 2012 at 15:42:20 -0600, Eric Blake wrote: On 04/13/2012 03:25 PM, Jiri Denemark wrote: On Mon, Apr 09, 2012 at 21:52:25 -0600, Eric Blake wrote: This new API provides additional flexibility over what can be crammed on top of virDomainBlockRebase (namely, the ability to specify an arbitrary destination format, for things like copying qcow2 into qed without having to pre-create the destination), at the expense that it cannot be backported without bumping the .so version. +typedef enum { +VIR_DOMAIN_BLOCK_COPY_SHALLOW = 1 0, /* Limit copy to top of source + backing chain */ +VIR_DOMAIN_BLOCK_COPY_REUSE_EXT = 1 1, /* Reuse existing external + file for a copy */ +} virDomainBlockCopyFlags; Hmm, we have several flags enums that end up being passed to a single internal API and one has to be extra careful when adding new flags. Should we make a note about this to the affected enums? Yes, a note would be helpful (it's only the two least-significant bits that we are playing fast and loose with at the moment, but if we ever expand to a third common bit, I see where it could get confusing). /** + * virDomainBlockCopy: + * @dom: pointer to domain object + * @disk: path to the block device, or device shorthand + * @dest: path to the copy destination + * @format: format of the destination + * @bandwidth: (optional) specify copy bandwidth limit in Mbps + * @flags: bitwise-OR of virDomainBlockCopyFlags OK, so this new API may be used to avoid format guessing involved in virDomainBlockRebase. Shouldn't we introduce an enhanced version of virDomainBlockRebase with format parameter instead of introducing an API with a different name that does almost the same as virDomainBlockRebase? And what would you name it? I'm saying that virDomainBlockCopy _is_ an enhanced virDomainBlockRebase, and the name BlockCopy was the name I picked, as it looks nicer than virDomainBlockRebase2(). I don't know, I was probably expecting something like virDomainBlockRebaseExt :-P I'm just missing a clear link between virDomainBlockRebase and virDomainBlockCopy. I guess a note to virDomainBlockRebase documentation mentioning virDomainBlockCopy as an enhanced version would work too. I guess I should wait for more feedback on the qemu front before committing to the final form of this proposed libvirt API. Yeah, I think that's wise. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: fix path to openvz network configuration file
On Mon, Apr 16, 2012 at 04:21:01PM +0200, Guido Günther wrote: It's vznet.conf not vznetctl.conf, see e.g.: http://git.openvz.org/?p=vzctl;a=blob;f=bin/vznetcfg.in;h=e91f5c4a0744c1ea149e1b8c241b666052e10b12;hb=HEAD --- docs/drvopenvz.html.in |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/drvopenvz.html.in b/docs/drvopenvz.html.in index bb437d3..a8f9f28 100644 --- a/docs/drvopenvz.html.in +++ b/docs/drvopenvz.html.in @@ -67,7 +67,7 @@ openvz+ssh://r...@example.com/system (remote access, SSH tunnelled) script must be created manually by the host OS administrator. The simplest way is to just download the latest version of this script from a newer OpenVZ release, or upstream source repository. Then -a generic configuration file code/etc/vz/vznetctl.conf/code +a generic configuration file code/etc/vz/vznet.conf/code must be created containing /p ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RESEND] qemuProcessStart: Switch to flags instead of bunch booleans
Currently, we have 3 boolean arguments we have to pass to qemuProcessStart(). As libvirt grows it is harder and harder to remember them and their position. Therefore we should switch to flags instead. --- This is just rebased version of: http://www.redhat.com/archives/libvir-list/2012-March/msg00331.html src/qemu/qemu_driver.c| 45 + src/qemu/qemu_migration.c |7 --- src/qemu/qemu_process.c | 21 + src/qemu/qemu_process.h | 12 4 files changed, 54 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 65ed290..436ef37 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1351,10 +1351,16 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, virDomainPtr dom = NULL; virDomainEventPtr event = NULL; virDomainEventPtr event2 = NULL; +unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD; virCheckFlags(VIR_DOMAIN_START_PAUSED | VIR_DOMAIN_START_AUTODESTROY, NULL); +if (flags VIR_DOMAIN_START_PAUSED) +start_flags |= VIR_QEMU_PROCESS_START_PAUSED; +if (flags VIR_DOMAIN_START_AUTODESTROY) +start_flags |= VIR_QEMU_PROCESS_START_AUTODESROY; + qemuDriverLock(driver); if (!(def = virDomainDefParseString(driver-caps, xml, QEMU_EXPECTED_VIRT_TYPES, @@ -1383,10 +1389,9 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0) goto cleanup; /* free the 'vm' we created ? */ -if (qemuProcessStart(conn, driver, vm, NULL, true, - (flags VIR_DOMAIN_START_PAUSED) != 0, - (flags VIR_DOMAIN_START_AUTODESTROY) != 0, - -1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE) 0) { +if (qemuProcessStart(conn, driver, vm, NULL, -1, NULL, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE, + start_flags) 0) { virDomainAuditStart(vm, booted, false); if (qemuDomainObjEndJob(driver, vm) 0) qemuDomainRemoveInactive(driver, vm); @@ -4138,9 +4143,9 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, } /* Set the migration source and start it up. */ -ret = qemuProcessStart(conn, driver, vm, stdio, false, true, - false, *fd, path, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_RESTORE); +ret = qemuProcessStart(conn, driver, vm, stdio, *fd, path, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, + VIR_QEMU_PROCESS_START_PAUSED); if (intermediatefd != -1) { if (ret 0) { @@ -4710,6 +4715,10 @@ qemuDomainObjStart(virConnectPtr conn, bool autodestroy = (flags VIR_DOMAIN_START_AUTODESTROY) != 0; bool bypass_cache = (flags VIR_DOMAIN_START_BYPASS_CACHE) != 0; bool force_boot = (flags VIR_DOMAIN_START_FORCE_BOOT) != 0; +unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD; + +start_flags |= start_paused ? VIR_QEMU_PROCESS_START_PAUSED : 0; +start_flags |= autodestroy ? VIR_QEMU_PROCESS_START_AUTODESROY : 0; /* * If there is a managed saved state restore it instead of starting @@ -4741,9 +4750,8 @@ qemuDomainObjStart(virConnectPtr conn, } } -ret = qemuProcessStart(conn, driver, vm, NULL, true, start_paused, - autodestroy, -1, NULL, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_CREATE); +ret = qemuProcessStart(conn, driver, vm, NULL, -1, NULL, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags); virDomainAuditStart(vm, booted, ret = 0); if (ret = 0) { virDomainEventPtr event = @@ -11027,9 +11035,10 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, if (config) virDomainObjAssignDef(vm, config, false); -rc = qemuProcessStart(snapshot-domain-conn, driver, vm, NULL, - false, true, false, -1, NULL, snap, - VIR_NETDEV_VPORT_PROFILE_OP_CREATE); +rc = qemuProcessStart(snapshot-domain-conn, + driver, vm, NULL, -1, NULL, snap, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE, + VIR_QEMU_PROCESS_START_PAUSED); virDomainAuditStart(vm, from-snapshot, rc = 0); detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT; event = virDomainEventNewFromObj(vm, @@ -4,12 +11123,16 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) { /* Flush first event, now do transition 2 or 3
Re: [libvirt] [PATCH RESEND] qemuProcessStart: Switch to flags instead of bunch booleans
On Mon, Apr 16, 2012 at 04:55:41PM +0200, Michal Privoznik wrote: Currently, we have 3 boolean arguments we have to pass to qemuProcessStart(). As libvirt grows it is harder and harder to remember them and their position. Therefore we should switch to flags instead. --- This is just rebased version of: http://www.redhat.com/archives/libvir-list/2012-March/msg00331.html src/qemu/qemu_driver.c| 45 + src/qemu/qemu_migration.c |7 --- src/qemu/qemu_process.c | 21 + src/qemu/qemu_process.h | 12 4 files changed, 54 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 692fc32..d18d721 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3633,7 +3638,7 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG(Handshake complete, child running); if (migrateFrom) -start_paused = true; +flags |= VIR_QEMU_PROCESS_START_PAUSED; if (ret == -1) /* The VM failed to start; tear filters before taps */ virDomainConfVMNWFilterTeardown(vm); @@ -3708,7 +3713,7 @@ int qemuProcessStart(virConnectPtr conn, } qemuDomainObjExitMonitorWithDriver(driver, vm); -if (!start_paused) { +if (flags ~VIR_QEMU_PROCESS_START_PAUSED) { Those are different semantics. What you have requires that at least one flag is set that isn't START_PAUSED, which is not the same as requiring that START_PAUSED is not set. You want: if (!(flags VIR_QEMU_PROCESS_START_PAUSED)) Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: fix path to openvz network configuration file
On Mon, Apr 16, 2012 at 03:27:28PM +0100, Daniel P. Berrange wrote: On Mon, Apr 16, 2012 at 04:21:01PM +0200, Guido Günther wrote: It's vznet.conf not vznetctl.conf, see e.g.: http://git.openvz.org/?p=vzctl;a=blob;f=bin/vznetcfg.in;h=e91f5c4a0744c1ea149e1b8c241b666052e10b12;hb=HEAD --- docs/drvopenvz.html.in |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/drvopenvz.html.in b/docs/drvopenvz.html.in index bb437d3..a8f9f28 100644 --- a/docs/drvopenvz.html.in +++ b/docs/drvopenvz.html.in @@ -67,7 +67,7 @@ openvz+ssh://r...@example.com/system (remote access, SSH tunnelled) script must be created manually by the host OS administrator. The simplest way is to just download the latest version of this script from a newer OpenVZ release, or upstream source repository. Then -a generic configuration file code/etc/vz/vznetctl.conf/code +a generic configuration file code/etc/vz/vznet.conf/code must be created containing /p ACK Pushed. Thanks, -- Guido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [ANNOUNCE] Perl binding Sys-Virt release 0.9.11
I am pleased to announce that release 0.9.11 of Sys-Virt, the libvirt Perl API binding is now available for download: http://search.cpan.org/CPAN/authors/id/D/DA/DANBERR/Sys-Virt-0.9.11.tar.gz Changed in this release: - Add all new APIs in libvirt 0.9.11 - Add test case to validate API coverage - Fix misc POD docs bugs - Fix reference handling in block stats - Add handling of VIR_TYPED_PARAM_STRING Further information, including online API documentation previous release archives, is available from the CPAN home page http://search.cpan.org/dist/Sys-Virt/ Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V10 6/7] nwfilter: Add multiple IP address support to DHCP snooping
With support for multiple IP addresses per interface in place, this patch now adds support for multiple IP addresses per interface for the DHCP snooping code. Testing: Since the infrastructure I tested this with does not provide multiple IP addresses per MAC address (anymore), I either had to plug the VM's interface from the virtual bride connected directly to the infrastructure to virbr0 to get a 2nd IP address from dnsmasq (kill and run dhclient inside the VM) or changed the lease file (/var/run/libvirt/network/nwfilter.leases) and restart libvirtd to have a 2nd IP address on an existing interface. Note that dnsmasq can take a lease timeout parameter as part of the --dhcp-range command line parameter, so that timeouts can be tested that way (--dhcp-range 192.168.122.2,192.168.122.254,120). So, terminating and restarting dnsmasq with that parameter is another choice to watch an IP address disappear after 120 seconds. Regards, Stefan --- src/nwfilter/nwfilter_dhcpsnoop.c | 103 +- 1 file changed, 70 insertions(+), 33 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.c === --- libvirt-acl.orig/src/nwfilter/nwfilter_dhcpsnoop.c +++ libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.c @@ -59,6 +59,7 @@ #include conf/domain_conf.h #include nwfilter_gentech_driver.h #include nwfilter_dhcpsnoop.h +#include nwfilter_ipaddrmap.h #include virnetdev.h #include virfile.h #include viratomic.h @@ -251,7 +252,8 @@ struct _virNWFilterDHCPDecodeJob { /* local function prototypes */ static int virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req, - uint32_t ipaddr, bool update_leasefile); + uint32_t ipaddr, bool update_leasefile, + bool instantiate); static void virNWFilterSnoopReqLock(virNWFilterSnoopReqPtr req); static void virNWFilterSnoopReqUnlock(virNWFilterSnoopReqPtr req); @@ -427,8 +429,8 @@ virNWFilterSnoopIPLeaseInstallRule(virNW { char ipbuf[INET_ADDRSTRLEN]; int rc = -1; -virNWFilterVarValuePtr ipVar; virNWFilterSnoopReqPtr req; +char *ipaddr; if (!inet_ntop(AF_INET, ipl-IPAddress, ipbuf, sizeof(ipbuf))) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, @@ -437,20 +439,19 @@ virNWFilterSnoopIPLeaseInstallRule(virNW __func__, ipl-IPAddress); return -1; } -ipVar = virNWFilterVarValueCreateSimpleCopyValue(ipbuf); -if (!ipVar) { + +ipaddr = strdup(ipbuf); +if (ipaddr == NULL) { virReportOOMError(); return -1; } -if (virNWFilterHashTablePut(ipl-SnoopReq-vars, NWFILTER_VARNAME_IP, -ipVar, 1) 0) { -virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, - _(Could not add variable \ - NWFILTER_VARNAME_IP \ to hashmap)); -virNWFilterVarValueFree(ipVar); + +if (virNWFilterIPAddrMapAddIPAddr(ipl-SnoopReq-ifname, ipaddr) 0) { +VIR_FREE(ipaddr); return -1; } + if (!instantiate) return 0; @@ -511,12 +512,18 @@ static unsigned int virNWFilterSnoopReqLeaseTimerRun(virNWFilterSnoopReqPtr req) { time_t now = time(0); +bool is_last = false; /* protect req-start */ virNWFilterSnoopReqLock(req); -while (req-start req-start-Timeout = now) -virNWFilterSnoopReqLeaseDel(req, req-start-IPAddress, true); +while (req-start req-start-Timeout = now) { +if (req-start-next == NULL || +req-start-next-Timeout now) +is_last = true; +virNWFilterSnoopReqLeaseDel(req, req-start-IPAddress, true, +is_last); +} virNWFilterSnoopReqUnlock(req); @@ -587,7 +594,7 @@ virNWFilterSnoopReqFree(virNWFilterSnoop /* free all leases */ for (ipl = req-start; ipl; ipl = req-start) -virNWFilterSnoopReqLeaseDel(req, ipl-IPAddress, false); +virNWFilterSnoopReqLeaseDel(req, ipl-IPAddress, false, false); /* free all req data */ VIR_FREE(req-ifname); @@ -720,15 +727,6 @@ virNWFilterSnoopReqLeaseAdd(virNWFilterS virNWFilterSnoopReqUnlock(req); -/* support for multiple addresses requires the ability to add filters - * to existing chains, or to instantiate address lists via - * virNWFilterInstantiateFilterLate(). Until one of those capabilities - * is added, don't allow a new address when one is already assigned to - * this interface. - */ -if (req-start) - return 0;/* silently ignore multiple addresses */ - if (VIR_ALLOC(pl) 0) { virReportOOMError(); return -1; @@ -796,34 +794,64 @@ virNWFilterSnoopReqRestore(virNWFilterSn *memory or when calling this function while reading *
[libvirt] [PATCH V10 5/7] nwfilter: move code for IP address map into separate file
The goal of this patch is to prepare for support for multiple IP addresses per interface in the DHCP snooping code. Move the code for the IP address map that maps interface names to IP addresses into their own file. Rename the functions on the way but otherwise leave the code as-is. Initialize this new layer separately before dependent layers (iplearning, dhcpsnooping) and shut it down after them. --- src/Makefile.am|4 src/conf/nwfilter_ipaddrmap.c | 167 + src/conf/nwfilter_ipaddrmap.h | 37 +++ src/libvirt_private.syms |8 + src/nwfilter/nwfilter_driver.c | 11 +- src/nwfilter/nwfilter_gentech_driver.c |5 src/nwfilter/nwfilter_learnipaddr.c| 126 src/nwfilter/nwfilter_learnipaddr.h|3 8 files changed, 229 insertions(+), 132 deletions(-) Index: libvirt-acl/src/Makefile.am === --- libvirt-acl.orig/src/Makefile.am +++ libvirt-acl/src/Makefile.am @@ -159,9 +159,11 @@ NETWORK_CONF_SOURCES = \ # Network filter driver generic impl APIs NWFILTER_PARAM_CONF_SOURCES = \ conf/nwfilter_params.c conf/nwfilter_params.h \ + conf/nwfilter_ipaddrmap.c \ + conf/nwfilter_ipaddrmap.h \ conf/nwfilter_conf.h -NWFILTER_CONF_SOURCES =\ +NWFILTER_CONF_SOURCES =\ $(NWFILTER_PARAM_CONF_SOURCES) \ conf/nwfilter_conf.c conf/nwfilter_conf.h Index: libvirt-acl/src/nwfilter/nwfilter_driver.c === --- libvirt-acl.orig/src/nwfilter/nwfilter_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_driver.c @@ -39,6 +39,7 @@ #include nwfilter_gentech_driver.h #include configmake.h +#include nwfilter_ipaddrmap.h #include nwfilter_dhcpsnoop.h #include nwfilter_learnipaddr.h @@ -67,10 +68,12 @@ static int nwfilterDriverStartup(int privileged) { char *base = NULL; -if (virNWFilterDHCPSnoopInit() 0) +if (virNWFilterIPAddrMapInit() 0) return -1; if (virNWFilterLearnInit() 0) -return -1; +goto err_exit_ipaddrmapshutdown; +if (virNWFilterDHCPSnoopInit() 0) +goto err_exit_learnshutdown; virNWFilterTechDriversInit(privileged); @@ -131,7 +134,10 @@ alloc_err_exit: conf_init_err: virNWFilterTechDriversShutdown(); virNWFilterDHCPSnoopShutdown(); +err_exit_learnshutdown: virNWFilterLearnShutdown(); +err_exit_ipaddrmapshutdown: +virNWFilterIPAddrMapShutdown(); return -1; } @@ -210,6 +216,7 @@ nwfilterDriverShutdown(void) { virNWFilterTechDriversShutdown(); virNWFilterDHCPSnoopShutdown(); virNWFilterLearnShutdown(); +virNWFilterIPAddrMapShutdown(); nwfilterDriverLock(driverState); Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c === --- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c @@ -33,6 +33,7 @@ #include nwfilter_gentech_driver.h #include nwfilter_ebiptables_driver.h #include nwfilter_dhcpsnoop.h +#include nwfilter_ipaddrmap.h #include nwfilter_learnipaddr.h #include virnetdev.h #include datatypes.h @@ -870,7 +871,7 @@ __virNWFilterInstantiateFilter(const uns goto err_exit; } -ipaddr = virNWFilterGetIpAddrForIfname(ifname); +ipaddr = virNWFilterIPAddrMapGetIPAddr(ifname); vars1 = virNWFilterCreateVarHashmap(str_macaddr, ipaddr); if (!vars1) { @@ -1132,7 +1133,7 @@ _virNWFilterTeardownFilter(const char *i techdriver-allTeardown(ifname); -virNWFilterDelIpAddrForIfname(ifname, NULL); +virNWFilterIPAddrMapDelIPAddr(ifname, NULL); virNWFilterUnlockIface(ifname); Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c === --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c +++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c @@ -52,6 +52,7 @@ #include conf/domain_conf.h #include nwfilter_gentech_driver.h #include nwfilter_ebiptables_driver.h +#include nwfilter_ipaddrmap.h #include nwfilter_learnipaddr.h #define VIR_FROM_THIS VIR_FROM_NWFILTER @@ -118,9 +119,6 @@ struct ether_vlan_header static virMutex pendingLearnReqLock; static virHashTablePtr pendingLearnReq; -static virMutex ipAddressMapLock; -static virNWFilterHashTablePtr ipAddressMap; - static virMutex ifaceMapLock; static virHashTablePtr ifaceLockMap; @@ -310,113 +308,8 @@ virNWFilterDeregisterLearnReq(int ifinde return res; } -/* Add an IP address to the list of IP addresses an
[libvirt] [PATCH V10 7/7] nwfilter: Display detected IP address in domain XML
Display detected IP addresses in the domain XML using the IP_LEASE variable name. This variable name now becomes a reserved variable name that can be read only but not set by the user. The format of the value is: ip addresss,lease timeout in seconds An example of a displayed XML may then be: interface type='bridge' mac address='52:54:00:68:e3:90'/ source bridge='virbr0'/ target dev='vnet1'/ model type='virtio'/ filterref filter='clean-traffic' parameter name='ip_learning' value='dhcp'/ parameter name='IP_LEASE' value='192.168.122.210,100'/ /filterref alias name='net0'/ address type='pci' domain='0x' bus='0x00' slot='0x04' function='0x0'/ /interface --- docs/formatnwfilter.html.in| 27 +++ src/conf/domain_conf.c | 16 +++-- src/conf/domain_nwfilter.c | 11 ++ src/conf/domain_nwfilter.h | 12 +++ src/conf/nwfilter_conf.c |3 + src/conf/nwfilter_ipaddrmap.c | 29 + src/conf/nwfilter_ipaddrmap.h |5 ++ src/conf/nwfilter_params.c | 45 +- src/conf/nwfilter_params.h |9 - src/libvirt_private.syms |3 + src/nwfilter/nwfilter_dhcpsnoop.c | 56 + src/nwfilter/nwfilter_dhcpsnoop.h |4 ++ src/nwfilter/nwfilter_driver.c | 10 + src/nwfilter/nwfilter_gentech_driver.c | 24 ++ src/nwfilter/nwfilter_gentech_driver.h |6 +++ src/nwfilter/nwfilter_learnipaddr.c| 14 src/nwfilter/nwfilter_learnipaddr.h|2 + 17 files changed, 267 insertions(+), 9 deletions(-) Index: libvirt-acl/src/conf/nwfilter_params.h === --- libvirt-acl.orig/src/conf/nwfilter_params.h +++ libvirt-acl/src/conf/nwfilter_params.h @@ -72,7 +72,10 @@ struct _virNWFilterHashTable { virNWFilterHashTablePtr virNWFilterParseParamAttributes(xmlNodePtr cur); int virNWFilterFormatParamAttributes(virBufferPtr buf, virNWFilterHashTablePtr table, - const char *filterref); + const char *filterref, + const unsigned char *vmuuid, + const unsigned char *mac, + const char *ifname); virNWFilterHashTablePtr virNWFilterHashTableCreate(int n); void virNWFilterHashTableFree(virNWFilterHashTablePtr table); @@ -89,12 +92,14 @@ int virNWFilterHashTablePutAll(virNWFilt abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_ # define VALID_VARVALUE \ - abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.: + abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.:, # define NWFILTER_VARNAME_IP IP # define NWFILTER_VARNAME_MAC MAC # define NWFILTER_VARNAME_IP_LEARNING ip_learning # define NWFILTER_VARNAME_DHCPSERVER DHCPSERVER +# define NWFILTER_VARNAME_IP_LEASE IP_LEASE +# define NWFILTER_VARNAME_IPV6_LEASE IPV6_LEASE /* future */ enum virNWFilterVarAccessType { VIR_NWFILTER_VAR_ACCESS_ELEMENT = 0, Index: libvirt-acl/src/conf/domain_conf.c === --- libvirt-acl.orig/src/conf/domain_conf.c +++ libvirt-acl/src/conf/domain_conf.c @@ -43,6 +43,7 @@ #include buf.h #include c-ctype.h #include logging.h +#include nwfilter_params.h #include nwfilter_conf.h #include ignore-value.h #include storage_file.h @@ -11203,7 +11204,8 @@ error: static int virDomainNetDefFormat(virBufferPtr buf, virDomainNetDefPtr def, - unsigned int flags) + unsigned int flags, + const unsigned char *vmuuid) { const char *type = virDomainNetTypeToString(def-type); @@ -11344,9 +11346,15 @@ virDomainNetDefFormat(virBufferPtr buf, } } if (def-filter) { +const char *ifname = NULL; + +if (!(flags VIR_DOMAIN_XML_INACTIVE)) +ifname = def-ifname; + virBufferAdjustIndent(buf, 6); if (virNWFilterFormatParamAttributes(buf, def-filterparams, - def-filter) 0) + def-filter, vmuuid, def-mac, + ifname) 0) return -1; virBufferAdjustIndent(buf, -6); } @@ -12630,7 +12638,7 @@ virDomainDefFormatInternal(virDomainDefP for (n = 0 ; n def-nnets ; n++) -if (virDomainNetDefFormat(buf, def-nets[n], flags) 0) +if (virDomainNetDefFormat(buf, def-nets[n], flags, def-uuid) 0) goto cleanup; for (n = 0 ; n def-nsmartcards ; n++) @@ -14883,7 +14891,7 @@ virDomainDeviceDefCopy(virCapsPtr
[libvirt] [PATCH V10 2/7] Support for atomic operations on integers
For threading support, add atomic add and sub operations working on integers. Base this on locking support provided by virMutex. --- src/util/viratomic.h | 91 +++ 1 file changed, 91 insertions(+) Index: libvirt-acl/src/util/viratomic.h === --- /dev/null +++ libvirt-acl/src/util/viratomic.h @@ -0,0 +1,91 @@ +/* + * viratomic.h: atomic integer operations + * + * Copyright (C) 2012 IBM Corporation + * + * Authors: + * Stefan Berger stef...@linux.vnet.ibm.com + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#ifndef __VIR_ATOMIC_H__ +# define __VIR_ATOMIC_H__ + +# include threads.h + +typedef struct _virAtomicInt virAtomicInt; +typedef virAtomicInt *virAtomicIntPtr; + +struct _virAtomicInt { +virMutex lock; +int value; +}; + +static inline int +virAtomicIntInit(virAtomicIntPtr vaip) +{ +vaip-value = 0; +return virMutexInit(vaip-lock); +} + +static inline void +virAtomicIntSet(virAtomicIntPtr vaip, int value) +{ + virMutexLock(vaip-lock); + + vaip-value = value; + + virMutexUnlock(vaip-lock); +} + +static inline int +virAtomicIntRead(virAtomicIntPtr vaip) +{ + return vaip-value; +} + +static inline int +virAtomicIntAdd(virAtomicIntPtr vaip, int add) +{ +int ret; + +virMutexLock(vaip-lock); + +vaip-value += add; +ret = vaip-value; + +virMutexUnlock(vaip-lock); + +return ret; +} + +static inline int +virAtomicIntSub(virAtomicIntPtr vaip, int sub) +{ +int ret; + +virMutexLock(vaip-lock); + +vaip-value -= sub; +ret = vaip-value; + +virMutexUnlock(vaip-lock); + +return ret; +} + +#endif /* __VIR_ATOMIC_H */ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V10 3/7] nwfilter: Fix support for trusted DHCP servers
Fix the support for trusted DHCP server in the ebtables code's hard-coded function applying DHCP only filtering rules: Rather than using a char * use the more flexible virNWFilterVarValuePtr that contains the trusted DHCP server(s) IP address. Process all entries. Since all callers so far provided NULL as parameter, no changes are necessary in any other code. --- src/conf/nwfilter_conf.h |2 src/nwfilter/nwfilter_ebiptables_driver.c | 72 +- 2 files changed, 44 insertions(+), 30 deletions(-) Index: libvirt-acl/src/conf/nwfilter_conf.h === --- libvirt-acl.orig/src/conf/nwfilter_conf.h +++ libvirt-acl/src/conf/nwfilter_conf.h @@ -625,7 +625,7 @@ typedef int (*virNWFilterApplyBasicRules typedef int (*virNWFilterApplyDHCPOnlyRules)(const char *ifname, const unsigned char *macaddr, - const char *dhcpserver, + virNWFilterVarValuePtr dhcpsrvs, bool leaveTemporary); typedef int (*virNWFilterRemoveBasicRules)(const char *ifname); Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c === --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -3195,7 +3195,7 @@ tear_down_tmpebchains: * @ifname: name of the backend-interface to which to apply the rules * @macaddr: MAC address the VM is using in packets sent through the *interface - * @dhcpserver: The DHCP server from which the VM may receive traffic + * @dhcpsrvrs: The DHCP server(s) from which the VM may receive traffic *from; may be NULL * @leaveTemporary: Whether to leave the table names with their temporary *names (true) or also perform the renaming to their final names as @@ -3209,14 +3209,15 @@ tear_down_tmpebchains: static int ebtablesApplyDHCPOnlyRules(const char *ifname, const unsigned char *macaddr, - const char *dhcpserver, + virNWFilterVarValuePtr dhcpsrvrs, bool leaveTemporary) { virBuffer buf = VIR_BUFFER_INITIALIZER; char chain_in [MAX_CHAINNAME_LENGTH], chain_out[MAX_CHAINNAME_LENGTH]; char macaddr_str[VIR_MAC_STRING_BUFLEN]; -char *srcIPParam = NULL; +unsigned int idx = 0; +unsigned int num_dhcpsrvrs; if (!ebtables_cmd_path) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, %s, @@ -3225,15 +3226,6 @@ ebtablesApplyDHCPOnlyRules(const char *i return -1; } -if (dhcpserver) { -virBufferAsprintf(buf, --ip-src %s, dhcpserver); -if (virBufferError(buf)) { -virBufferFreeAndReset(buf); -return -1; -} -srcIPParam = virBufferContentAndReset(buf); -} - virMacAddrFormat(macaddr, macaddr_str); ebiptablesAllTeardown(ifname); @@ -3267,20 +3259,46 @@ ebtablesApplyDHCPOnlyRules(const char *i chain_in, CMD_STOPONERR(1)); -virBufferAsprintf(buf, - CMD_DEF($EBT -t nat -A %s - -d %s - -p ipv4 --ip-protocol udp - %s - --ip-sport 67 --ip-dport 68 - -j ACCEPT) CMD_SEPARATOR - CMD_EXEC - %s, +num_dhcpsrvrs = (dhcpsrvrs != NULL) +? virNWFilterVarValueGetCardinality(dhcpsrvrs) +: 0; - chain_out, - macaddr_str, - srcIPParam != NULL ? srcIPParam : , - CMD_STOPONERR(1)); +while (true) { +char *srcIPParam = NULL; + +if (idx num_dhcpsrvrs) { +const char *dhcpserver; + +dhcpserver = virNWFilterVarValueGetNthValue(dhcpsrvrs, idx); + +if (virAsprintf(srcIPParam, --ip-src %s, dhcpserver) 0) { +virReportOOMError(); +goto tear_down_tmpebchains; +} +} + +virBufferAsprintf(buf, + CMD_DEF($EBT -t nat -A %s + -d %s + -p ipv4 --ip-protocol udp + %s + --ip-sport 67 --ip-dport 68 + -j ACCEPT) CMD_SEPARATOR + CMD_EXEC + %s, + + chain_out, + macaddr_str, + srcIPParam != NULL ? srcIPParam : , + CMD_STOPONERR(1)); + +VIR_FREE(srcIPParam);
[libvirt] [PATCH V10 1/7] Implement virHashRemoveAll function
Implement function to remove all entries of a hash table. --- src/libvirt_private.syms |1 + src/util/virhash.c | 25 + src/util/virhash.h |5 + 3 files changed, 31 insertions(+) Index: libvirt-acl/src/libvirt_private.syms === --- libvirt-acl.orig/src/libvirt_private.syms +++ libvirt-acl/src/libvirt_private.syms @@ -578,6 +578,7 @@ virHashForEach; virHashFree; virHashGetItems; virHashLookup; +virHashRemoveAll; virHashRemoveEntry; virHashRemoveSet; virHashSearch; Index: libvirt-acl/src/util/virhash.c === --- libvirt-acl.orig/src/util/virhash.c +++ libvirt-acl/src/util/virhash.c @@ -575,6 +575,31 @@ virHashRemoveSet(virHashTablePtr table, return count; } +static int +_virHashRemoveAllIter(const void *payload ATTRIBUTE_UNUSED, + const void *name ATTRIBUTE_UNUSED, + const void *data ATTRIBUTE_UNUSED) +{ +return 1; +} + +/** + * virHashRemoveAll + * @table: the hash table to clear + * + * Free the hash @table's contents. The userdata is + * deallocated with the function provided at creation time. + * + * Returns the number of items removed on success, -1 on failure + */ +ssize_t +virHashRemoveAll(virHashTablePtr table) +{ +return virHashRemoveSet(table, +_virHashRemoveAllIter, +NULL); +} + /** * virHashSearch: * @table: the hash table to search Index: libvirt-acl/src/util/virhash.h === --- libvirt-acl.orig/src/util/virhash.h +++ libvirt-acl/src/util/virhash.h @@ -127,6 +127,11 @@ int virHashRemoveEntry(virHashTablePtr t const void *name); /* + * Remove all entries from the hash table. + */ +ssize_t virHashRemoveAll(virHashTablePtr table); + +/* * Retrieve the userdata. */ void *virHashLookup(virHashTablePtr table, const void *name); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] qemuProcessStart: Switch to flags instead of bunch booleans
Currently, we have 3 boolean arguments we have to pass to qemuProcessStart(). As libvirt grows it is harder and harder to remember them and their position. Therefore we should switch to flags instead. --- diff to v1: -fix a test for START_PAUSED flag src/qemu/qemu_driver.c| 45 + src/qemu/qemu_migration.c |7 --- src/qemu/qemu_process.c | 21 + src/qemu/qemu_process.h | 12 4 files changed, 54 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 65ed290..436ef37 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1351,10 +1351,16 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, virDomainPtr dom = NULL; virDomainEventPtr event = NULL; virDomainEventPtr event2 = NULL; +unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD; virCheckFlags(VIR_DOMAIN_START_PAUSED | VIR_DOMAIN_START_AUTODESTROY, NULL); +if (flags VIR_DOMAIN_START_PAUSED) +start_flags |= VIR_QEMU_PROCESS_START_PAUSED; +if (flags VIR_DOMAIN_START_AUTODESTROY) +start_flags |= VIR_QEMU_PROCESS_START_AUTODESROY; + qemuDriverLock(driver); if (!(def = virDomainDefParseString(driver-caps, xml, QEMU_EXPECTED_VIRT_TYPES, @@ -1383,10 +1389,9 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0) goto cleanup; /* free the 'vm' we created ? */ -if (qemuProcessStart(conn, driver, vm, NULL, true, - (flags VIR_DOMAIN_START_PAUSED) != 0, - (flags VIR_DOMAIN_START_AUTODESTROY) != 0, - -1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE) 0) { +if (qemuProcessStart(conn, driver, vm, NULL, -1, NULL, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE, + start_flags) 0) { virDomainAuditStart(vm, booted, false); if (qemuDomainObjEndJob(driver, vm) 0) qemuDomainRemoveInactive(driver, vm); @@ -4138,9 +4143,9 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, } /* Set the migration source and start it up. */ -ret = qemuProcessStart(conn, driver, vm, stdio, false, true, - false, *fd, path, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_RESTORE); +ret = qemuProcessStart(conn, driver, vm, stdio, *fd, path, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, + VIR_QEMU_PROCESS_START_PAUSED); if (intermediatefd != -1) { if (ret 0) { @@ -4710,6 +4715,10 @@ qemuDomainObjStart(virConnectPtr conn, bool autodestroy = (flags VIR_DOMAIN_START_AUTODESTROY) != 0; bool bypass_cache = (flags VIR_DOMAIN_START_BYPASS_CACHE) != 0; bool force_boot = (flags VIR_DOMAIN_START_FORCE_BOOT) != 0; +unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD; + +start_flags |= start_paused ? VIR_QEMU_PROCESS_START_PAUSED : 0; +start_flags |= autodestroy ? VIR_QEMU_PROCESS_START_AUTODESROY : 0; /* * If there is a managed saved state restore it instead of starting @@ -4741,9 +4750,8 @@ qemuDomainObjStart(virConnectPtr conn, } } -ret = qemuProcessStart(conn, driver, vm, NULL, true, start_paused, - autodestroy, -1, NULL, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_CREATE); +ret = qemuProcessStart(conn, driver, vm, NULL, -1, NULL, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags); virDomainAuditStart(vm, booted, ret = 0); if (ret = 0) { virDomainEventPtr event = @@ -11027,9 +11035,10 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, if (config) virDomainObjAssignDef(vm, config, false); -rc = qemuProcessStart(snapshot-domain-conn, driver, vm, NULL, - false, true, false, -1, NULL, snap, - VIR_NETDEV_VPORT_PROFILE_OP_CREATE); +rc = qemuProcessStart(snapshot-domain-conn, + driver, vm, NULL, -1, NULL, snap, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE, + VIR_QEMU_PROCESS_START_PAUSED); virDomainAuditStart(vm, from-snapshot, rc = 0); detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT; event = virDomainEventNewFromObj(vm, @@ -4,12 +11123,16 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) { /* Flush first event, now do transition 2 or 3 */ bool paused = (flags
Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC
On 04/16/2012 10:11 AM, Stefan Bader wrote: On 16.04.2012 14:54, Stefan Bader wrote: On 16.04.2012 14:45, Cole Robinson wrote: On 04/16/2012 08:36 AM, Stefan Bader wrote: On 16.04.2012 13:58, Cole Robinson wrote: On 04/13/2012 09:14 AM, Stefan Bader wrote: I think it would be better if we just centralized this logic, as in, only set that (type ioemu) bit in conditional rather than 2. Should be pretty straightforward. Did you have something like below in mind? -Stefan From a0e214fa6dea9bd66616f37246067a2c631f4184 Mon Sep 17 00:00:00 2001 From: Stefan Bader stefan.ba...@canonical.com Date: Thu, 12 Apr 2012 15:32:41 +0200 Subject: [PATCH] libvirt: xen: do not use ioemu type for any emulated NIC When using the xm/xend stack to manage instances there is a bug that causes the emulated interfaces to be unusable when the vif config contains type=ioemu. The current code already has a special quirk to not use this keyword if no specific model is given for the emulated NIC (defaulting to rtl8139). Essentially it works because regardless of the type argument,i the Xen stack always creates emulated and paravirt interfaces and lets the guest decide which one to use. So neither xl nor xm stack actually require the type keyword for emulated NICs. [v2: move code around to have the exception in one case together] Signed-off-by: Stefan Bader stefan.ba...@canonical.com --- src/xenxs/xen_sxpr.c | 28 +++- src/xenxs/xen_xm.c | 27 ++- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index e1bbd76..3a56345 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1999,20 +1999,22 @@ xenFormatSxprNet(virConnectPtr conn, if (def-model != NULL) virBufferEscapeSexpr(buf, (model '%s'), def-model); } -else if (def-model == NULL) { -/* - * apparently (type ioemu) breaks paravirt drivers on HVM so skip - * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, (type ioemu)); -} -else if (STREQ(def-model, netfront)) { -virBufferAddLit(buf, (type netfront)); -} else { -virBufferEscapeSexpr(buf, (model '%s'), def-model); -virBufferAddLit(buf, (type ioemu)); +if (def-model != NULL STREQ(def-model, netfront)) { +virBufferAddLit(buf, (type netfront)); +} +else { +if (def-model != NULL) { +virBufferEscapeSexpr(buf, (model '%s'), def-model); +} +/* + * apparently (type ioemu) breaks paravirt drivers on HVM so skip + * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) { +virBufferAddLit(buf, (type ioemu)); +} +} } if (!isAttach) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index d65e97a..93a26f9 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1381,20 +1381,21 @@ static int xenFormatXMNet(virConnectPtr conn, if (net-model != NULL) virBufferAsprintf(buf, ,model=%s, net-model); } -else if (net-model == NULL) { -/* - * apparently type ioemu breaks paravirt drivers on HVM so skip this - * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, ,type=ioemu); -} -else if (STREQ(net-model, netfront)) { -virBufferAddLit(buf, ,type=netfront); -} else { -virBufferAsprintf(buf, ,model=%s, net-model); -virBufferAddLit(buf, ,type=ioemu); +if (net-model != NULL STREQ(net-model, netfront)) { +virBufferAddLit(buf, ,type=netfront); +} +else { +if (net-model != NULL) +virBufferAsprintf(buf, ,model=%s, net-model); + +/* + * apparently type ioemu breaks paravirt drivers on HVM so skip this + * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) +virBufferAddLit(buf, ,type=ioemu); +} } if (net-ifname) Looks good. Did you verify 'make check' passes as well? If so, ACK No :( And it fails. TEST: libvirtdconftest .!!...!!!37 FAIL FAIL: libvirtdconftest I guess this is expected as the change really does modify the generated configs. So I need to find out how to make it expect that... Make sure it's actually your patch causing those issues. I wouldn't expect your change to affect that particular
[libvirt] [libvirt-test-API PATCH 2/2] Documentation: Update to current information
Update of all the things found to be outdated. The test described in the chapter Writing a test case was changed to simpler one, with only one source file included. It shows how to use the libvirt API (no need for connectAPI etc. anymore) and mainly libvirt test API. --- .../en-US/Understanding_libvirt-test-API.xml |6 - .../en-US/Using_libvirt-test-API.xml | 26 +- .../en-US/Writing_a_test_case.xml | 325 .../libvirt-test-API_Guide/en-US/extras/log.txt| 53 ++-- 4 files changed, 164 insertions(+), 246 deletions(-) diff --git a/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml b/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml index 7fe1e97..446e9e2 100644 --- a/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml +++ b/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml @@ -186,12 +186,6 @@ repos /blockquote -bridgehead renderas=sect3/lib/bridgehead -blockquote - parafilename/lib/filename is a directory that contains the basic subroutines that call the API functions of libvirt bindings languages to form basic unit classes. The test case initiates the first action by calling these subroutines./para -/blockquote - - bridgehead renderas=sect3/utils/bridgehead blockquote parafilename/utils/filename is a directory which contains various scripts to assist in creating and verifying test cases./para diff --git a/docs/User_Guide/libvirt-test-API_Guide/en-US/Using_libvirt-test-API.xml b/docs/User_Guide/libvirt-test-API_Guide/en-US/Using_libvirt-test-API.xml index 65653e3..7fa14f5 100644 --- a/docs/User_Guide/libvirt-test-API_Guide/en-US/Using_libvirt-test-API.xml +++ b/docs/User_Guide/libvirt-test-API_Guide/en-US/Using_libvirt-test-API.xml @@ -13,21 +13,25 @@ section id=Using_libvirt-test-API-Running_a_test titleRunning a test/title - paraThe filenamemain.py/filename file, located in the root directory of the test tool, is the file that runs the test and performs other operations in libvirt-test-API./para + paraThe filenamelibvirt-test-api.py/filename file, located in the root directory of the test tool, is the file that runs the test and performs other operations in libvirt-test-API./para paraTo run a test, from the libvirt-test-API root directory enter:/para screen -# python main.py +# python libvirt-test-api.py /screen paraThe following switches can be used with the above command:/para example - titlepython main.py switches/title + titlepython libvirt-test-api.py options/title screen --C, --configfile Specify the configuration file to parse, default is 'case.conf'. --D, --delete-log Delete a log item. --H, --help Display the command usage. --L, --xmlfile Specify the name of the log file, default is 'log.xml'. +-h, --help : Display usage information +-c, --casefile: Specify configuration file +-t, --template: Print testcase config file template +-f, --logxml: Specify log file with type xml +-l, --log-level: 0 or 1 currently +-d, --delete-log: Delete log items +-m, --merge: Merge two log xmlfiles +-r, --rerun: Rerun one or more test /screen /example @@ -53,25 +57,25 @@ bridgehead renderas=sect3Deleting log information/bridgehead -paraThe switch command-D, --delete-log/command in the commandpython main.py/command command can be used to delete a test item, a test run, or all test runs in a log file./para +paraThe switch command-d, --delete-log/command in the commandpython libvirt-test-api.py/command command can be used to delete a test item, a test run, or all test runs in a log file./para paraFor example:/para itemizedlist listitem paraTo delete the test item (testid) in the test run (testrunid):/para screen -# python main.py -D log.xml testrunid testid +# python libvirt-test-api.py -d log.xml testrunid testid /screen /listitem listitem paraTo delete the test run (testrunid):/para screen -# python main.py -D log.xml testrunid +# python libvirt-test-api.py -d log.xml testrunid /screen /listitem listitem paraTo delete all test runs:/para screen -# python main.py -D log.xml all +# python libvirt-test-api.py -d log.xml all /screen /listitem /itemizedlist diff --git a/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml b/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml index 3dd714e..49ad0b5 100644 --- a/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml +++ b/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml @@ -7,7 +7,7 @@ paraTest cases form the foundation of the test tool. They are the building blocks from which test runs can be created. Test cases are written as Python scripts and define the
Re: [libvirt] [PATCH v2] qemuProcessStart: Switch to flags instead of bunch booleans
On Mon, Apr 16, 2012 at 05:09:59PM +0200, Michal Privoznik wrote: Currently, we have 3 boolean arguments we have to pass to qemuProcessStart(). As libvirt grows it is harder and harder to remember them and their position. Therefore we should switch to flags instead. --- diff to v1: -fix a test for START_PAUSED flag src/qemu/qemu_driver.c| 45 + src/qemu/qemu_migration.c |7 --- src/qemu/qemu_process.c | 21 + src/qemu/qemu_process.h | 12 4 files changed, 54 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 65ed290..436ef37 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1351,10 +1351,16 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, virDomainPtr dom = NULL; virDomainEventPtr event = NULL; virDomainEventPtr event2 = NULL; +unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD; virCheckFlags(VIR_DOMAIN_START_PAUSED | VIR_DOMAIN_START_AUTODESTROY, NULL); +if (flags VIR_DOMAIN_START_PAUSED) +start_flags |= VIR_QEMU_PROCESS_START_PAUSED; +if (flags VIR_DOMAIN_START_AUTODESTROY) +start_flags |= VIR_QEMU_PROCESS_START_AUTODESROY; + qemuDriverLock(driver); if (!(def = virDomainDefParseString(driver-caps, xml, QEMU_EXPECTED_VIRT_TYPES, @@ -1383,10 +1389,9 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0) goto cleanup; /* free the 'vm' we created ? */ -if (qemuProcessStart(conn, driver, vm, NULL, true, - (flags VIR_DOMAIN_START_PAUSED) != 0, - (flags VIR_DOMAIN_START_AUTODESTROY) != 0, - -1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE) 0) { +if (qemuProcessStart(conn, driver, vm, NULL, -1, NULL, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE, + start_flags) 0) { virDomainAuditStart(vm, booted, false); if (qemuDomainObjEndJob(driver, vm) 0) qemuDomainRemoveInactive(driver, vm); @@ -4138,9 +4143,9 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, } /* Set the migration source and start it up. */ -ret = qemuProcessStart(conn, driver, vm, stdio, false, true, - false, *fd, path, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_RESTORE); +ret = qemuProcessStart(conn, driver, vm, stdio, *fd, path, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, + VIR_QEMU_PROCESS_START_PAUSED); if (intermediatefd != -1) { if (ret 0) { @@ -4710,6 +4715,10 @@ qemuDomainObjStart(virConnectPtr conn, bool autodestroy = (flags VIR_DOMAIN_START_AUTODESTROY) != 0; bool bypass_cache = (flags VIR_DOMAIN_START_BYPASS_CACHE) != 0; bool force_boot = (flags VIR_DOMAIN_START_FORCE_BOOT) != 0; +unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD; + +start_flags |= start_paused ? VIR_QEMU_PROCESS_START_PAUSED : 0; +start_flags |= autodestroy ? VIR_QEMU_PROCESS_START_AUTODESROY : 0; /* * If there is a managed saved state restore it instead of starting @@ -4741,9 +4750,8 @@ qemuDomainObjStart(virConnectPtr conn, } } -ret = qemuProcessStart(conn, driver, vm, NULL, true, start_paused, - autodestroy, -1, NULL, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_CREATE); +ret = qemuProcessStart(conn, driver, vm, NULL, -1, NULL, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags); virDomainAuditStart(vm, booted, ret = 0); if (ret = 0) { virDomainEventPtr event = @@ -11027,9 +11035,10 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, if (config) virDomainObjAssignDef(vm, config, false); -rc = qemuProcessStart(snapshot-domain-conn, driver, vm, NULL, - false, true, false, -1, NULL, snap, - VIR_NETDEV_VPORT_PROFILE_OP_CREATE); +rc = qemuProcessStart(snapshot-domain-conn, + driver, vm, NULL, -1, NULL, snap, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE, + VIR_QEMU_PROCESS_START_PAUSED); virDomainAuditStart(vm, from-snapshot, rc = 0); detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT; event = virDomainEventNewFromObj(vm, @@ -4,12 +11123,16 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
[libvirt] [libvirt-test-API PATCH 1/2] domain:screenshot: fixes and cleanup
After the conn object was transferred into sharedmod, its import was missing. I also cleaned up and commented few lines in order to use them for documentation purposes. --- repos/domain/screenshot.py | 45 ++- 1 files changed, 35 insertions(+), 10 deletions(-) diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py index eb5d0e2..49028fe 100644 --- a/repos/domain/screenshot.py +++ b/repos/domain/screenshot.py @@ -4,42 +4,67 @@ import os import mimetypes - import libvirt +# sharedmod is needed in order to get the shared connection object +import sharedmod + +# These variables are mandatory for running the test +# The test won't be run unless both guestname and filename are specified. required_params = ('guestname', 'filename',) optional_params = ('screen',) -last_filename = None +# In this filename we store the last filename that has a screenshot +# saved inside in order to be able to clean it after +filename = None +# This is just a helper function that will be used later. def saver(stream, data, file_): return file_.write(data) +# This is the main test def screenshot(params): This method takes a screenshot of a running machine and saves it in a filename -ret = 1 + +# Shared logger to be used for messages output logger = params['logger'] +# Shared connection object to be used for messages output conn = sharedmod.libvirtobj['conn'] -dom = conn.lookupByName(params['guestname']) +guestname = params['guestname'] + +logger.info('Looking for domain %s' % guestname) +dom = conn.lookupByName(guestname) +logger.debug('Domain %s found' % guestname) st = conn.newStream(0) screen = params.get('screen', 0) +logger.info('Taking a screenshot') mime = dom.screenshot(st, int(screen), 0) +logger.debug('Screenshot taken') +# As mentioned before, the screenshot format is +# hypervisor-specific ext = mimetypes.guess_extension(mime) or '.ppm' -last_filename = params['filename'] + ext +filename = params['filename'] + ext + +# Here we create a file object used later f = file(filename, 'w') -logger.debug('Saving screenshot into %s' % filename) +logger.info('Saving screenshot into %s' % filename) +logger.info('Mimetype of the file is %s' % mime) +# Here is the use of the saver function st.recvAll(saver, f) -logger.debug('Mimetype of the file is %s' % mime) - -ret = st.finish() +logger.debug('Screenshot saved') -return ret +# The test fails (return is different from 0) if the stream is not +# properly ended +return st.finish() +# This cleanup function is called if specified by the configuration +# file. In this file we remove the saved screenshot if the filename is +# known. def cleanup(params): if last_filename: os.remove(last_filename) -- 1.7.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemuProcessStart: Switch to flags instead of bunch booleans
On 16.04.2012 17:26, Daniel P. Berrange wrote: On Mon, Apr 16, 2012 at 05:09:59PM +0200, Michal Privoznik wrote: Currently, we have 3 boolean arguments we have to pass to qemuProcessStart(). As libvirt grows it is harder and harder to remember them and their position. Therefore we should switch to flags instead. --- diff to v1: -fix a test for START_PAUSED flag src/qemu/qemu_driver.c| 45 + src/qemu/qemu_migration.c |7 --- src/qemu/qemu_process.c | 21 + src/qemu/qemu_process.h | 12 4 files changed, 54 insertions(+), 31 deletions(-) ACK, that's better now Daniel Thanks, pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-test-API PATCH 0/2] Documentation update
I updated part of the documentation that was changed in last few weeks. Screenshot test was updated again a little bit to suit the needs of a documentation and replaced the source codes of previous two tests mentioned in the documentation as it describe just what is needed and has no overhead on the reader. I'm not sure how the numbering goes in this case, but it can be updated later or I can send a v2 in case it needs to be fixed right away. Martin Kletzander (2): domain:screenshot: fixes and cleanup Documentation: Update to current information .../en-US/Understanding_libvirt-test-API.xml |6 - .../en-US/Using_libvirt-test-API.xml | 26 +- .../en-US/Writing_a_test_case.xml | 325 .../libvirt-test-API_Guide/en-US/extras/log.txt| 53 ++-- repos/domain/screenshot.py | 45 ++- 5 files changed, 199 insertions(+), 256 deletions(-) -- 1.7.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] OpenSSH authentification in Java libvirt API
Hello, It seems that today you can’t authenticate on a qemu+ssh connection via OpenSSH without having to manually type the password in CLI or a pop-up window, using the Java API for libvirt. Being able to push a String in OpenSSH user input would be an easier and faster solution, for the end-user. Sincerely, Alexandre -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] cgroup cpuset limited by default?
Hi On Mon, Apr 16, 2012 at 2:32 PM, Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com wrote: On 04/16/2012 05:34 PM, Marc-André Lureau wrote: Did you happen to perform a suspend/resume or a hibernation/restore on your computer? (Or did you do CPU hotplug manually?) If yes, you might be seeing the problem reported at: https://bugzilla.redhat.com/show_bug.cgi?id=714271 yep, thank you very much, that seems to be the reason Unfortunately, as of now the kernel still doesn't handle this properly.. IOW, we don't have a kernel fix (yet). That's sad to learn!, I am going to move all my VMs to session libvirt then :) thanks -- Marc-André Lureau -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Do not enforce source type of console[0]
On 04/16/2012 09:47 AM, Jan Kiszka wrote: On 2012-03-13 16:48, Jan Kiszka wrote: If console[0] is an alias for serial[0], do not enforce the former to have a PTY source type. This breaks serial consoles on stdio and makes no sense. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- src/qemu/qemu_process.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ef311d1..216b594 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1376,7 +1376,6 @@ qemuProcessFindCharDevicePTYs(virDomainObjPtr vm, if ((ret = virDomainChrSourceDefCopy(chr-source, ((vm-def-serials[0])-source))) != 0) return ret; -chr-source.type = VIR_DOMAIN_CHR_TYPE_PTY; } else { if (chr-source.type == VIR_DOMAIN_CHR_TYPE_PTY chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) { Michal originally made that change, I'm guessing it was an oversight though. ACK. - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] cgroup cpuset limited by default?
On Mon, Apr 16, 2012 at 06:00:22PM +0200, Marc-André Lureau wrote: Hi On Mon, Apr 16, 2012 at 2:32 PM, Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com wrote: On 04/16/2012 05:34 PM, Marc-André Lureau wrote: Did you happen to perform a suspend/resume or a hibernation/restore on your computer? (Or did you do CPU hotplug manually?) If yes, you might be seeing the problem reported at: https://bugzilla.redhat.com/show_bug.cgi?id=714271 yep, thank you very much, that seems to be the reason This problem has existed for ages now. I wonder if there's a way we get get a userspace workaround implemented. IIRC, pm-utils has the ability to run arbitrary shell scripts upon restore from suspend/hibernate. We could put a temp hack in libvirt which resets the CPU affinity in the top level libvirt cgroup. That would at least make new VMs start with good affinity. To deal with existing running VMs, we would need to record existing affinity before suspend re-store it fully afterwards which is more complicated Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] 2 trivial doc patches
Hey, I've just pushed these 2 documentation patches under the trivial rule. Christophe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] docs: add missing /span in vcpu placement doc
--- docs/formatdomain.html.in |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index bb67cd1..e08a3d2 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -359,7 +359,7 @@ 0.8.5/span, the optional attribute codecurrent/code can be used to specify whether fewer than the maximum number of virtual CPUs should be enabled. span class=sinceSince -0.9.11 (QEMU and KVM only), the optional attribute +0.9.11 (QEMU and KVM only)/span, the optional attribute codeplacement/code can be used to indicate the CPU placement mode for domain process, its value can be either static or auto, defaults to static if codecpuset/code is specified, -- 1.7.10 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] docs: fix 'omitted' typo in cputune doc
'omitted' was mispelt 'commited' twice. One of the sentences with the typo was also missing an 'is' ('each VCPU *is* pinned to all...') which I added in this commit while I was at it. --- docs/formatdomain.html.in |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e08a3d2..6f19da0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -402,8 +402,8 @@ dtcodevcpupin/code/dt dd The optional codevcpupin/code element specifies which of host -physical CPUS the domain VCPU will be pinned to. If this is ommited, -each VCPU pinned to all the physical CPUS by default. It contains two +physical CPUS the domain VCPU will be pinned to. If this is omitted, +each VCPU is pinned to all the physical CPUS by default. It contains two required attributes, the attribute codevcpu/code specifies vcpu id, and the attribute codecpuset/code is same as attribute codecpuset/code @@ -413,7 +413,7 @@ dtcodeshares/code/dt dd The optional codeshares/code element specifies the proportional -weighted share for the domain. If this is ommited, it defaults to +weighted share for the domain. If this is omitted, it defaults to the OS provided defaults. NB, There is no unit for the value, it's a relative measure based on the setting of other VM, e.g. A VM configured with value -- 1.7.10 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 15/18] blockjob: accommodate RHEL backport names
On 04/09/2012 09:52 PM, Eric Blake wrote: RHEL-only drive-mirror and drive-reopen are still under upstream qemu discussion; as a result, RHEL decided to backport things under a downstream name. Accommodate this alternate spelling. I don't think it's worth trying to support both spellings at once: if you build upstream libvirt on RHEL, you lose out on the feature, but then you are also capable of building upstream qemu for RHEL to reinstate the feature (that is, if you use RHEL, you should either stick to the distro patches, or you are assumed to be capable of building the entire virt stack yourself). * src/qemu/qemu_monitor_json.c (qemuMonitorJSONCheckCommands): Check for alternate spelling. (int qemuMonitorJSONDriveMirror, qemuMonitorJSONDriveReopen): Use that spelling. --- src/qemu/qemu_monitor_json.c |8 1 files changed, 4 insertions(+), 4 deletions(-) Just a heads-up for those following this thread: Paolo reposted a proposal for the 'drive-mirror' job that might make it into qemu 1.1: https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg01630.html In particular, the upstream proposal does _not_ allow for 'drive-mirror' as part of transaction; that would have to wait for qemu 1.2. As a result, my earlier proposal for supporting snapshot+mirror is not possible with the upstream qemu 1.1 proposal: https://www.redhat.com/archives/libvir-list/2012-March/msg01033.html However, the backport of __com.redhat_drive-mirror into RHEL 6.3 was based on the earlier versions that were proposing the blkmirror driver instead of a block job implementation, and since snapshot+mirror was first developed against a potential RHEL build, Paolo has made an effort to keep things working there, so I am still maintaining that patch series and will post a rebase of the series on top of my blockjob patches. Of course, the snapshot+mirror won't be committed upstream without upstream qemu support, but at least it will provide a comparison between the two live storage migration proposals, as well as something that could still be backported to RHEL. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V10 0/7] Add DHCP snooping support to nwfilter
This series of patches adds DHCP snooping support to libvirt's nwfilter subsystem. DHCP snooping detects DHCP leases obtained by a VM and automatically adjusts the network traffic filters to reflect the IP addresses with which a VM may send its traffic, thus for example preventing IP address spoofing. Once leases on IP addresses expire or if a VM gives up on a lease on an IP address, the filters are also adjusted. All leases are persisted and automatically applied upon a VM's restart. Leases are associated with the tuple of VM-UUID and interface MAC address. The following interface XML activates and uses the DHCP snooping: interface type='bridge' source bridge='virbr0'/ filterref filter='clean-traffic' parameter name='ip_learning' value='dhcp'/ /filterref /interface Once an IP address has been detected on an interface, 'virsh dumpxml vm' would show the IP address lease in the format IP address,lease timeout in seconds: interface type='bridge' source bridge='virbr0'/ filterref filter='clean-traffic' parameter name='ip_learning' value='dhcp'/ parameter name='IP_LEASE' value='192.168.122.210,180'/ /filterref /interface Regards, David and Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC
On 16.04.2012 17:21, Cole Robinson wrote: On 04/16/2012 10:11 AM, Stefan Bader wrote: On 16.04.2012 14:54, Stefan Bader wrote: On 16.04.2012 14:45, Cole Robinson wrote: On 04/16/2012 08:36 AM, Stefan Bader wrote: On 16.04.2012 13:58, Cole Robinson wrote: On 04/13/2012 09:14 AM, Stefan Bader wrote: I think it would be better if we just centralized this logic, as in, only set that (type ioemu) bit in conditional rather than 2. Should be pretty straightforward. Did you have something like below in mind? -Stefan From a0e214fa6dea9bd66616f37246067a2c631f4184 Mon Sep 17 00:00:00 2001 From: Stefan Bader stefan.ba...@canonical.com Date: Thu, 12 Apr 2012 15:32:41 +0200 Subject: [PATCH] libvirt: xen: do not use ioemu type for any emulated NIC When using the xm/xend stack to manage instances there is a bug that causes the emulated interfaces to be unusable when the vif config contains type=ioemu. The current code already has a special quirk to not use this keyword if no specific model is given for the emulated NIC (defaulting to rtl8139). Essentially it works because regardless of the type argument,i the Xen stack always creates emulated and paravirt interfaces and lets the guest decide which one to use. So neither xl nor xm stack actually require the type keyword for emulated NICs. [v2: move code around to have the exception in one case together] Signed-off-by: Stefan Bader stefan.ba...@canonical.com --- src/xenxs/xen_sxpr.c | 28 +++- src/xenxs/xen_xm.c | 27 ++- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index e1bbd76..3a56345 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1999,20 +1999,22 @@ xenFormatSxprNet(virConnectPtr conn, if (def-model != NULL) virBufferEscapeSexpr(buf, (model '%s'), def-model); } -else if (def-model == NULL) { -/* - * apparently (type ioemu) breaks paravirt drivers on HVM so skip - * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, (type ioemu)); -} -else if (STREQ(def-model, netfront)) { -virBufferAddLit(buf, (type netfront)); -} else { -virBufferEscapeSexpr(buf, (model '%s'), def-model); -virBufferAddLit(buf, (type ioemu)); +if (def-model != NULL STREQ(def-model, netfront)) { +virBufferAddLit(buf, (type netfront)); +} +else { +if (def-model != NULL) { +virBufferEscapeSexpr(buf, (model '%s'), def-model); +} +/* + * apparently (type ioemu) breaks paravirt drivers on HVM so skip + * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) { +virBufferAddLit(buf, (type ioemu)); +} +} } if (!isAttach) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index d65e97a..93a26f9 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1381,20 +1381,21 @@ static int xenFormatXMNet(virConnectPtr conn, if (net-model != NULL) virBufferAsprintf(buf, ,model=%s, net-model); } -else if (net-model == NULL) { -/* - * apparently type ioemu breaks paravirt drivers on HVM so skip this - * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, ,type=ioemu); -} -else if (STREQ(net-model, netfront)) { -virBufferAddLit(buf, ,type=netfront); -} else { -virBufferAsprintf(buf, ,model=%s, net-model); -virBufferAddLit(buf, ,type=ioemu); +if (net-model != NULL STREQ(net-model, netfront)) { +virBufferAddLit(buf, ,type=netfront); +} +else { +if (net-model != NULL) +virBufferAsprintf(buf, ,model=%s, net-model); + +/* + * apparently type ioemu breaks paravirt drivers on HVM so skip this + * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) +virBufferAddLit(buf, ,type=ioemu); +} } if (net-ifname) Looks good. Did you verify 'make check' passes as well? If so, ACK No :( And it fails. TEST: libvirtdconftest .!!...!!!37 FAIL FAIL: libvirtdconftest I guess this is expected as the change really does modify the generated configs. So I need to find out how to make it expect that... Make sure it's actually your patch causing those issues. I wouldn't
Re: [libvirt] [PATCH V10 0/7] Add DHCP snooping support to nwfilter
On Mon, Apr 16, 2012 at 10:08 AM, Stefan Berger stef...@linux.vnet.ibm.comwrote: This series of patches adds DHCP snooping support to libvirt's nwfilter subsystem. Stefan, David, Thank you very much for this functionality. As a side-effect, it solves a problem that I needed addressed: namely, to know via Sys-Virt, the IP address associated with a virtual machine (without having to grovel through the DHCP lease file, out of band). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V10 0/7] Add DHCP snooping support to nwfilter
On 04/16/2012 03:12 PM, dennis jenkins wrote: On Mon, Apr 16, 2012 at 10:08 AM, Stefan Berger stef...@linux.vnet.ibm.com mailto:stef...@linux.vnet.ibm.com wrote: This series of patches adds DHCP snooping support to libvirt's nwfilter subsystem. Stefan, David, Thank you very much for this functionality. As a side-effect, it solves a problem that I needed addressed: namely, to know via Sys-Virt, the IP address associated with a virtual machine (without having to grovel through the DHCP lease file, out of band). Dennis, that's great to hear. Did you test them? Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V10 0/7] Add DHCP snooping support to nwfilter
On Mon, Apr 16, 2012 at 2:22 PM, Stefan Berger stef...@linux.vnet.ibm.comwrote: ** On 04/16/2012 03:12 PM, dennis jenkins wrote: On Mon, Apr 16, 2012 at 10:08 AM, Stefan Berger stef...@linux.vnet.ibm.com wrote: This series of patches adds DHCP snooping support to libvirt's nwfilter subsystem. Stefan, David, Thank you very much for this functionality. As a side-effect, it solves a problem that I needed addressed: namely, to know via Sys-Virt, the IP address associated with a virtual machine (without having to grovel through the DHCP lease file, out of band). Dennis, that's great to hear. Did you test them? Stefan Not yet. I run Gentoo Linux, and I try to keep my system really clean by not having manually installed packages. However, if you would like some independent testing, I can tinker with it. I have to figure out the ins and outs of GIT first, though. I do my own stuff with SVN and have not learned git yet. The latest libvirt in Gentoo is version 0.9.10-r4. Ignoring the -r4, 9.10 is about 2 months old, correct? So I might be waiting a while for this patch to make it to my portage tree. Is there a specific git command line that I should use to pull a specific libvirt code set, or should I just go for the head / bleeding edge? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virsh list hangs
[please don't top-post on technical lists] On 04/15/2012 03:30 AM, Qian Zhang wrote: I think what happened to me is https://bugzilla.redhat.com/show_bug.cgi?id=757382 which is a dead lock issue between libvirtd process and its child process. It looks this issue has been fixed for Fedora 16, any plan for RHEL 6.2? If you'd like this to be backported to RHEL 6.2, perhaps the best thing to do is to open a support ticket to Red Hat specifically requesting that this Fedora BZ be cloned to RHEL. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC
On 04/16/2012 02:06 PM, Stefan Bader wrote: On 16.04.2012 17:21, Cole Robinson wrote: On 04/16/2012 10:11 AM, Stefan Bader wrote: On 16.04.2012 14:54, Stefan Bader wrote: On 16.04.2012 14:45, Cole Robinson wrote: On 04/16/2012 08:36 AM, Stefan Bader wrote: On 16.04.2012 13:58, Cole Robinson wrote: On 04/13/2012 09:14 AM, Stefan Bader wrote: I think it would be better if we just centralized this logic, as in, only set that (type ioemu) bit in conditional rather than 2. Should be pretty straightforward. Did you have something like below in mind? -Stefan From a0e214fa6dea9bd66616f37246067a2c631f4184 Mon Sep 17 00:00:00 2001 From: Stefan Bader stefan.ba...@canonical.com Date: Thu, 12 Apr 2012 15:32:41 +0200 Subject: [PATCH] libvirt: xen: do not use ioemu type for any emulated NIC When using the xm/xend stack to manage instances there is a bug that causes the emulated interfaces to be unusable when the vif config contains type=ioemu. The current code already has a special quirk to not use this keyword if no specific model is given for the emulated NIC (defaulting to rtl8139). Essentially it works because regardless of the type argument,i the Xen stack always creates emulated and paravirt interfaces and lets the guest decide which one to use. So neither xl nor xm stack actually require the type keyword for emulated NICs. [v2: move code around to have the exception in one case together] Signed-off-by: Stefan Bader stefan.ba...@canonical.com --- src/xenxs/xen_sxpr.c | 28 +++- src/xenxs/xen_xm.c | 27 ++- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index e1bbd76..3a56345 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1999,20 +1999,22 @@ xenFormatSxprNet(virConnectPtr conn, if (def-model != NULL) virBufferEscapeSexpr(buf, (model '%s'), def-model); } -else if (def-model == NULL) { -/* - * apparently (type ioemu) breaks paravirt drivers on HVM so skip - * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, (type ioemu)); -} -else if (STREQ(def-model, netfront)) { -virBufferAddLit(buf, (type netfront)); -} else { -virBufferEscapeSexpr(buf, (model '%s'), def-model); -virBufferAddLit(buf, (type ioemu)); +if (def-model != NULL STREQ(def-model, netfront)) { +virBufferAddLit(buf, (type netfront)); +} +else { +if (def-model != NULL) { +virBufferEscapeSexpr(buf, (model '%s'), def-model); +} +/* + * apparently (type ioemu) breaks paravirt drivers on HVM so skip + * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) { +virBufferAddLit(buf, (type ioemu)); +} +} } if (!isAttach) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index d65e97a..93a26f9 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1381,20 +1381,21 @@ static int xenFormatXMNet(virConnectPtr conn, if (net-model != NULL) virBufferAsprintf(buf, ,model=%s, net-model); } -else if (net-model == NULL) { -/* - * apparently type ioemu breaks paravirt drivers on HVM so skip this - * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU - */ -if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) -virBufferAddLit(buf, ,type=ioemu); -} -else if (STREQ(net-model, netfront)) { -virBufferAddLit(buf, ,type=netfront); -} else { -virBufferAsprintf(buf, ,model=%s, net-model); -virBufferAddLit(buf, ,type=ioemu); +if (net-model != NULL STREQ(net-model, netfront)) { +virBufferAddLit(buf, ,type=netfront); +} +else { +if (net-model != NULL) +virBufferAsprintf(buf, ,model=%s, net-model); + +/* + * apparently type ioemu breaks paravirt drivers on HVM so skip this + * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU + */ +if (xendConfigVersion = XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) +virBufferAddLit(buf, ,type=ioemu); +} } if (net-ifname) Looks good. Did you verify 'make check' passes as well? If so, ACK No :( And it fails. TEST: libvirtdconftest .!!...!!!37 FAIL FAIL: libvirtdconftest I guess this is expected as the change really does modify the generated configs. So I need to find out how to make it expect that... Make sure it's actually your
Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC
[adding direct cc to Osier] On 04/16/2012 01:58 PM, Cole Robinson wrote: Rebasing to HEAD made things rather worse... now 3 checks are failing (without my patch)... Might be something simple like missing a build dep. Try getting some debug output from the tests and it might be clear. If it isn't something simple, I can give your patch a spin. - Cole So the initial failure turned out to be a missing libsasl2-dev. The other two are new after the rebase: TEST: xml2vmxtest ... 31) VMware XML-2-VMX esx-in-the-wild-2 - esx-in-the-wild-2 @@ -7,7 +7,6 @@ memsize = 1000 sched.mem.max = 118 numvcpus = 4 -sched.cpu.affinity = 0,1,2,5,6,7 scsi0.present = true scsi0.virtualDev = lsilogic scsi1.present = true 32) VMware XML-2-VMX esx-in-the-wild-3 - esx-in-the-wild-3 @@ -6,7 +6,6 @@ displayName = virtDebian2 memsize = 1024 numvcpus = 2 -sched.cpu.affinity = 0,3,4,5 scsi0.present = true scsi0.virtualDev = lsilogic scsi0:0.present = true ... PASS: test_conf.sh --- exp 2012-04-16 17:27:09.672832070 + +++ out 2012-04-16 17:27:09.672832070 + @@ -1,3 +1,3 @@ error: Failed to define domain from xml-invalid -error: internal error topology cpuset syntax error +error: operation failed: domain 'test' already exists with uuid 96b2bf5e-ea49-17b7-ad2c-14308ca3ff59 FAIL: cpuset All three go away when I revert the following patch: commit 8fb2164cfff35ce0b87f1d513a0f3ca5111d7880 Author: Osier Yang jy...@redhat.com Date: Wed Apr 11 22:40:33 2012 +0800 numad: Ignore cpuset if placement is auto Yes, I independently hit the same problem and conclusion this morning. So I would think I can call my patch tested now. ;) Cool, thanks for checking. ACK to the original patch. Osier, looks like that patch caused some test fallout? Daniel and I were discussing it on IRC, although I haven't yet tried patching anything: [09:30] danpb1 eblake: (04:03:19 PM) eblake: +error: operation failed: domain 'test' already exists with uuid 88cb9633-8015-4624-a258-2a3cb9700ad0 [09:31] danpb1 eblake: the fact that we see that error message suggest a pre-existing bug in the test suite - the XML being used to define the test domain is missing a UUID [09:31] danpb1 eblake: so the 2 repeated defines get different auto-generated UUIDs thus violate the uniqueness check [09:31] danpb1 eblake: i expect if you fixed that problem, then you'd now see the 'define' operation actually suceed [09:32] eblake but since the test expects a topology syntax error, what changed to make it no longer an error? [09:33] eblake and thus exposing the latent bug in the uuid usage [09:33] danpb1 oh sure, it sounds like we still have a bug in changed semantics [09:34] danpb1 eblake: + if (def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC) { [09:34] danpb1 that line means we now don't ever parse the placement XML and so won't see the bogus data [09:35] danpb1 since the default value is PLACEMENT_MODE_DEFAULT instead of PLACEMENT_MODE_STATIC [09:35] danpb1 we need to treat DEFAULT + STATIC in the same way in the parser [09:36] eblake DEFAULT is primarily useful for where there is a difference in the XML between omitting something and explicitly requesting STATIC [09:36] eblake but if we want to always default to STATIC, maybe we could just trim the enum to get rid of DEFAULT and make STATIC == 0 [09:37] danpb1 i guess the intent was that we don't suddenly fill all existing XML with a new attribute [09:38] eblake in which case, default (for omitted) vs. static (explicit, don't lose what the user had) makes sense [09:38] eblake but then I agree that we need to handle both modes the same when deciding what else to parse -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] openvz: wire up more node information functions
On 04/16/2012 01:00 AM, Guido Günther wrote: On Mon, Apr 16, 2012 at 09:48:38AM +0800, Osier Yang wrote: On 2012年04月15日 04:20, Guido Günther wrote: in detail nodeGetCPUStats, nodeGetMemoryStats, nodeGetCellsFreeMemory and nodeGetFreeMemory --- src/openvz/openvz_driver.c |4 1 file changed, 4 insertions(+) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 3b2ffea..41fd8e4 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1704,6 +1704,10 @@ static virDriver openvzDriver = { .version = openvzGetVersion, /* 0.5.0 */ .getMaxVcpus = openvzGetMaxVCPUs, /* 0.4.6 */ .nodeGetInfo = nodeGetInfo, /* 0.3.2 */ +.nodeGetCPUStats = nodeGetCPUStats, /* 0.9.11 */ +.nodeGetMemoryStats = nodeGetMemoryStats, /* 0.9.11 */ +.nodeGetCellsFreeMemory = nodeGetCellsFreeMemory, /* 0.9.11 */ +.nodeGetFreeMemory = nodeGetFreeMemory, /* 0.9.11 */ .getCapabilities = openvzGetCapabilities, /* 0.4.6 */ .listDomains = openvzListDomains, /* 0.3.1 */ .numOfDomains = openvzNumDomains, /* 0.3.1 */ ACK Pushed. Thanks, This should be 0.9.12, not 0.9.11. Can you please push the fixup patch? -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] VN-Link vNIC memory state copying on VM Migration
Jagath, -Original Message- From: Jagath Weerasinghe [mailto:jagf...@gmail.com] Sent: Sunday, April 15, 2012 6:59 AM To: Christian Benvenuti (benve) Cc: libvir-list@redhat.com Subject: Re: [libvirt] VN-Link vNIC memory state copying on VM Migration Chris, Thanks the explanation of the algorithm. what memory state do you refer to exactly? What I meant by the memory state is the traffic being transferred through the vNIC when VM migration is triggered. Since you are worried about the traffic I assume we are talking about _live_ migration. Yes, live migration. Sorry for not mentioning it clearly. Libvirt qemu migration V3 takes place in 5 phases. (If you search the mailing list archive for migration v3 you will find a number of 3Ds) This is a simplified description of the algorithm: +--+ |VM| ++ +-+ |Src host| |dst host | ++ +-+ Phase1: BEGIN - Get XML of VM and tx it to dst Phase2: PREPARE - create (paused) VM based on XML Phase3: PERFORM - Copy RAM to dst - Pause VM .. Phase4: FINISH - Apply port profiles .. - Start VM Phase5: CONFIRM - Kill VM Libvirt on the dst host orchestrates everything using RPCs. Dst host can return error/success codes (the two libvirt also exchange data, .. see cookies) During phases 1/2/3 the VM is still running on the src host and hence able to rx/tx traffic. At the end of Phase 3, when qemu is done copying the RAM from source host to destination host (*), qemu puts the VM in pause state on the source host. OK. In phase 3, the RAM used by VM in source host is copied to the destination. Then what about the memory state of vNIC's in the M81KR VIC. Isn't there any memory for traffic passing through vNICs and also for vNIC statistics? What I am talking about is copying of this memory to the destination vNIC. No, that data (statistics, etc) are not copied to the dst host/vnic. However, you can find those statistics/counters on the switch (which are preserved across live migrations). Thanks /Chris (*) See the migration parameter 'migrate-setmaxdowntime' In phase 4 libvirt applies the port profile to the interfaces that need it and starts the VM (which had been created and put in pause state in phase 2). There is therefore a small period during which both VMs (the one on the source host and the one on the destination host) are in pause state (see dotted lines above). Phase 4 will start the VM on the dst host, Phase 5 will kill the (paused) VM on the src host. The amount of time during which both VMs are in pause state depends on a number of factors, including: - maxdowntime: the smaller you configure this value and - the longer will take the migration to complete, but - the smaller the pause duration will be - bandwidth available on the interface/s used to carry the migration traffic, and amount of RAM assigned to the VM (since you need to copy it) - time taken to complete the port profile associations on the dst host (which depends on how loaded the switches are) - ... Hope this clarifies a bit. /Chris If this memory state (or traffic) is not copied and moved to the destination vNIC how the smooth communication, which is guaranteed on VM migration, is achieved? Thanks Jagath If you refer to port profile info, then there is no copying involved: the source host disassociates the vnic port profile and the destination host re-associates the port profile on the new vnic. /Chris -Original Message- From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] On Behalf Of Jagath Weerasinghe Sent: Friday, April 13, 2012 9:16 AM To: libvir-list@redhat.com Subject: [libvirt] VN-Link vNIC memory state copying on VM Migration Hi All, I am new to libvirt. And want to know how the VM migration occurs in VN-Link (IEEE802.1Qbh). As far as I know, the memory state of vNICs in M81KR VIC has to be copied and moved to the destination vNIC on VM migration. Is that correct? If so, could you please tell me how this has been implemented in libvirt? Thanks Jagath -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 12/18] blockjob: implement block copy for qemu
On 04/13/2012 09:15 AM, Eric Blake wrote: On 04/13/2012 08:46 AM, Jiri Denemark wrote: On Mon, Apr 09, 2012 at 21:52:21 -0600, Eric Blake wrote: Minimal patch to wire up all the pieces in the previous patches to actually enable a block copy job. By minimal, I mean that qemu creates the file (that is, no REUSE_EXT flag support yet), SELinux must be disabled, a lock manager is not informed, and the audit logs aren't updated. But those will be added as improvements in future patches. +qemuDomainObjEnterMonitorWithDriver(driver, vm); +ret = qemuMonitorDriveMirror(priv-mon, NULL, device, dest, format, flags); +if (ret == 0 bandwidth != 0) +ret = qemuMonitorBlockJob(priv-mon, device, NULL, bandwidth, NULL, + BLOCK_JOB_SPEED_INTERNAL); +qemuDomainObjExitMonitorWithDriver(driver, vm); Can BLOCK_JOB_SPEED_INTERNAL fail while block copy remains running? If so, we should try to abort the job in case we fail to set the speed, otherwise we will report an error and forget about the job while qemu will keep copying data. Good catch, and virDomainBlockPull() suffers from the same ramifications. I think both code paths need the same fix. Actually, thinking about this a bit more: qemu documents that block-job-set-speed will fail if: DeviceNotActive: streaming is not active (but we already ignore this for BLOCK_JOB_SPEED_INTERNAL, thanks to commit a9d3495) NotSupported: throttling not supported It doesn't document any other errors; there are probably errors such as DeviceNotFound if an invalid device is specified, but we are less likely to hit those, since we would have hit that same error with the initial drive-mirror. Meanwhile, libvirt.c currently documents: * The maximum bandwidth (in Mbps) that will be used to do the copy can be * specified with the bandwidth parameter. If set to 0, libvirt will choose a * suitable default. Some hypervisors do not support this feature and will * return an error if bandwidth is not 0. But trying to cancel the job is risky - we've already modified the destination file; it would be nicer if we could detect the throttling limitation before even starting the job, but qemu doesn't let us do that. I will see about requesting the ability to set bandwidth as part of starting a job, rather than our current limitation of a guaranteed race, as a qemu patch. In the meantime, I think I will patch this instead by documentation: state that non-zero bandwidth in virDomainBlockRebase() may be silently ignored; if error checking is needed, the user should use a separate call to virDomainBlockJobSetSpeed(); and to see if a request was honored, use virDomainGetBlockJobInfo() (with the known race that the job might already be complete). -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC
On 2012年04月17日 04:17, Eric Blake wrote: [adding direct cc to Osier] On 04/16/2012 01:58 PM, Cole Robinson wrote: Rebasing to HEAD made things rather worse... now 3 checks are failing (without my patch)... Might be something simple like missing a build dep. Try getting some debug output from the tests and it might be clear. If it isn't something simple, I can give your patch a spin. - Cole So the initial failure turned out to be a missing libsasl2-dev. The other two are new after the rebase: TEST: xml2vmxtest ... 31) VMware XML-2-VMX esx-in-the-wild-2 - esx-in-the-wild-2 @@ -7,7 +7,6 @@ memsize = 1000 sched.mem.max = 118 numvcpus = 4 -sched.cpu.affinity = 0,1,2,5,6,7 scsi0.present = true scsi0.virtualDev = lsilogic scsi1.present = true 32) VMware XML-2-VMX esx-in-the-wild-3 - esx-in-the-wild-3 @@ -6,7 +6,6 @@ displayName = virtDebian2 memsize = 1024 numvcpus = 2 -sched.cpu.affinity = 0,3,4,5 scsi0.present = true scsi0.virtualDev = lsilogic scsi0:0.present = true ... PASS: test_conf.sh --- exp 2012-04-16 17:27:09.672832070 + +++ out 2012-04-16 17:27:09.672832070 + @@ -1,3 +1,3 @@ error: Failed to define domain from xml-invalid -error: internal error topology cpuset syntax error +error: operation failed: domain 'test' already exists with uuid 96b2bf5e-ea49-17b7-ad2c-14308ca3ff59 FAIL: cpuset All three go away when I revert the following patch: commit 8fb2164cfff35ce0b87f1d513a0f3ca5111d7880 Author: Osier Yangjy...@redhat.com Date: Wed Apr 11 22:40:33 2012 +0800 numad: Ignore cpuset if placement is auto Yes, I independently hit the same problem and conclusion this morning. So I would think I can call my patch tested now. ;) Cool, thanks for checking. ACK to the original patch. Osier, looks like that patch caused some test fallout? Oops, yes. Daniel and I were discussing it on IRC, although I haven't yet tried patching anything: [09:30] danpb1 eblake: (04:03:19 PM) eblake: +error: operation failed: domain 'test' already exists with uuid 88cb9633-8015-4624-a258-2a3cb9700ad0 [09:31] danpb1 eblake: the fact that we see that error message suggest a pre-existing bug in the test suite - the XML being used to define the test domain is missing a UUID [09:31] danpb1 eblake: so the 2 repeated defines get different auto-generated UUIDs thus violate the uniqueness check [09:31] danpb1 eblake: i expect if you fixed that problem, then you'd now see the 'define' operation actually suceed Not true, there is UUID in the dumped xml: # for i in {1..3}; do ./tools/virsh --connect test:///default \ dumpxml test | grep uuid; done uuida1b6ee1f-97de-f0ee-617a-0cdb74947df5/uuid uuidee68d7d2-3eb9-593e-2769-797ce1f4c4aa/uuid uuidfecb1d3a-918a-8412-e534-76192cf32b18/uuid It outputs different UUID each time. I'm going to fix this first. [09:32] eblake but since the test expects a topology syntax error, what changed to make it no longer an error? [09:33] eblake and thus exposing the latent bug in the uuid usage [09:33] danpb1 oh sure, it sounds like we still have a bug in changed semantics [09:34] danpb1 eblake: + if (def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC) { [09:34] danpb1 that line means we now don't ever parse the placement XML and so won't see the bogus data [09:35] danpb1 since the default value is PLACEMENT_MODE_DEFAULT instead of PLACEMENT_MODE_STATIC [09:35] danpb1 we need to treat DEFAULT + STATIC in the same way in the parser [09:36] eblake DEFAULT is primarily useful for where there is a difference in the XML between omitting something and explicitly requesting STATIC [09:36] eblake but if we want to always default to STATIC, maybe we could just trim the enum to get rid of DEFAULT and make STATIC == 0 [09:37] danpb1 i guess the intent was that we don't suddenly fill all existing XML with a new attribute Yes, that was the intent, static when cpuset is specified manually, and default for cpuset is not specified. [09:38] eblake in which case, default (for omitted) vs. static (explicit, don't lose what the user had) makes sense [09:38] eblake but then I agree that we need to handle both modes the same when deciding what else to parse I will post a patch soon after the uuid problem of test driver, to treat default and static the same (not to parse cpuset). Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] openvz: wire up more node information functions
On 2012年04月17日 05:53, Eric Blake wrote: On 04/16/2012 01:00 AM, Guido Günther wrote: On Mon, Apr 16, 2012 at 09:48:38AM +0800, Osier Yang wrote: On 2012年04月15日 04:20, Guido Günther wrote: in detail nodeGetCPUStats, nodeGetMemoryStats, nodeGetCellsFreeMemory and nodeGetFreeMemory --- src/openvz/openvz_driver.c |4 1 file changed, 4 insertions(+) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 3b2ffea..41fd8e4 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1704,6 +1704,10 @@ static virDriver openvzDriver = { .version = openvzGetVersion, /* 0.5.0 */ .getMaxVcpus = openvzGetMaxVCPUs, /* 0.4.6 */ .nodeGetInfo = nodeGetInfo, /* 0.3.2 */ +.nodeGetCPUStats = nodeGetCPUStats, /* 0.9.11 */ +.nodeGetMemoryStats = nodeGetMemoryStats, /* 0.9.11 */ +.nodeGetCellsFreeMemory = nodeGetCellsFreeMemory, /* 0.9.11 */ +.nodeGetFreeMemory = nodeGetFreeMemory, /* 0.9.11 */ .getCapabilities = openvzGetCapabilities, /* 0.4.6 */ .listDomains = openvzListDomains, /* 0.3.1 */ .numOfDomains = openvzNumDomains, /* 0.3.1 */ ACK Pushed. Thanks, This should be 0.9.12, not 0.9.11. Can you please push the fixup patch? I pushed the fix with attached patch. Regards, Osier From bf8f1b3228f01f268caae3d5dbf0ba30a02257d8 Mon Sep 17 00:00:00 2001 From: Osier Yang jy...@redhat.com Date: Tue, 17 Apr 2012 10:08:53 +0800 Subject: [PATCH] openvz: Correct the comments for new node APIs It should be 0.9.12 instead of 0.9.11 --- src/openvz/openvz_driver.c |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 9d7897f..0451c22 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1681,10 +1681,10 @@ static virDriver openvzDriver = { .version = openvzGetVersion, /* 0.5.0 */ .getMaxVcpus = openvzGetMaxVCPUs, /* 0.4.6 */ .nodeGetInfo = nodeGetInfo, /* 0.3.2 */ -.nodeGetCPUStats = nodeGetCPUStats, /* 0.9.11 */ -.nodeGetMemoryStats = nodeGetMemoryStats, /* 0.9.11 */ -.nodeGetCellsFreeMemory = nodeGetCellsFreeMemory, /* 0.9.11 */ -.nodeGetFreeMemory = nodeGetFreeMemory, /* 0.9.11 */ +.nodeGetCPUStats = nodeGetCPUStats, /* 0.9.12 */ +.nodeGetMemoryStats = nodeGetMemoryStats, /* 0.9.12 */ +.nodeGetCellsFreeMemory = nodeGetCellsFreeMemory, /* 0.9.12 */ +.nodeGetFreeMemory = nodeGetFreeMemory, /* 0.9.12 */ .getCapabilities = openvzGetCapabilities, /* 0.4.6 */ .listDomains = openvzListDomains, /* 0.3.1 */ .numOfDomains = openvzNumDomains, /* 0.3.1 */ -- 1.7.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC
On 04/16/2012 08:05 PM, Osier Yang wrote: [09:31]danpb1eblake: the fact that we see that error message suggest a pre-existing bug in the test suite - the XML being used to define the test domain is missing a UUID [09:31]danpb1eblake: so the 2 repeated defines get different auto-generated UUIDs thus violate the uniqueness check [09:31]danpb1eblake: i expect if you fixed that problem, then you'd now see the 'define' operation actually suceed Not true, there is UUID in the dumped xml: A generated UUID - that is, we have a latent bug, in that since we aren't locking down a specific UUID, the generated one is different each time, and the test failed (as expected), but for the wrong reason (mismatch in UUID, instead of the intended failure due to a cpuset syntax error). -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC
On 2012年04月17日 10:44, Eric Blake wrote: On 04/16/2012 08:05 PM, Osier Yang wrote: [09:31]danpb1eblake: the fact that we see that error message suggest a pre-existing bug in the test suite - the XML being used to define the test domain is missing a UUID [09:31]danpb1eblake: so the 2 repeated defines get different auto-generated UUIDs thus violate the uniqueness check [09:31]danpb1eblake: i expect if you fixed that problem, then you'd now see the 'define' operation actually suceed Not true, there is UUID in the dumped xml: A generated UUID - that is, we have a latent bug, in that since we aren't locking down a specific UUID, the generated one is different each time, and the test failed (as expected), but for the wrong reason (mismatch in UUID, instead of the intended failure due to a cpuset syntax error). Yeah, just because the cpuset syntax error comes up earlier before the UUID checking. I post a patch to set the fixed UUID for the objects which support UUID in test driver, see the attachment. Regards, Osier From e88a6d45bf5b14d29ce089afcb1255e24e2f222c Mon Sep 17 00:00:00 2001 From: Osier Yang jy...@redhat.com Date: Tue, 17 Apr 2012 10:52:44 +0800 Subject: [PATCH] test: Set the fixed uuid for the default XMLs The objects (domain, pool, network, etc) for testing are defined/ started each time when opening a connect to test driver, and thus the UUID for the objects will be generated each time, with different values. e.g. % for i in {1..3}; do ./tools/virsh --connect \ test:///default dumpxml test | grep uuid; done uuida1b6ee1f-97de-f0ee-617a-0cdb74947df5/uuid uuidee68d7d2-3eb9-593e-2769-797ce1f4c4aa/uuid uuidfecb1d3a-918a-8412-e534-76192cf32b18/uuid It's the potential bug which can cause operations like below to fail: $ virsh -c test:///default dumpxml test test.xml [ Some modificatons, though it's not supported, but it should work ] $ virsh -c test:///default define test.xml This patch set fixed UUID for objects which support it. (domain, pool, network). --- src/test/test_driver.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 1587958..39ac503 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -230,6 +230,7 @@ no_memory: static const char *defaultDomainXML = domain type='test' nametest/name + uuid6695eb01-f6a4-8304-79aa-97f2502e193f/uuid memory8388608/memory currentMemory2097152/currentMemory vcpu2/vcpu @@ -242,6 +243,7 @@ static const char *defaultDomainXML = static const char *defaultNetworkXML = network namedefault/name + uuiddd8fe884-6c02-601e-7551-cca97df1c5df/uuid bridge name='virbr0' / forward/ ip address='192.168.122.1' netmask='255.255.255.0' @@ -265,6 +267,7 @@ static const char *defaultInterfaceXML = static const char *defaultPoolXML = pool type='dir' namedefault-pool/name + uuiddfe224cb-28fb-8dd0-c4b2-64eb3f0f4566/uuid target path/default-pool/path /target -- 1.7.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC
On 04/16/2012 09:03 PM, Osier Yang wrote: On 2012年04月17日 10:44, Eric Blake wrote: On 04/16/2012 08:05 PM, Osier Yang wrote: [09:31]danpb1eblake: the fact that we see that error message suggest a pre-existing bug in the test suite - the XML being used to define the test domain is missing a UUID [09:31]danpb1eblake: so the 2 repeated defines get different auto-generated UUIDs thus violate the uniqueness check [09:31]danpb1eblake: i expect if you fixed that problem, then you'd now see the 'define' operation actually suceed Not true, there is UUID in the dumped xml: A generated UUID - that is, we have a latent bug, in that since we aren't locking down a specific UUID, the generated one is different each time, and the test failed (as expected), but for the wrong reason (mismatch in UUID, instead of the intended failure due to a cpuset syntax error). Yeah, just because the cpuset syntax error comes up earlier before the UUID checking. I post a patch to set the fixed UUID for the objects which support UUID in test driver, see the attachment. ACK. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [test-API] add docs files TESTCASE.py and TESTCASES.conf
*docs/TESTCASE.py: an template of testcase.py *docs/TESTCASES.conf: an template of testcase config file with simple explanation --- docs/TESTCASE.py| 45 docs/TESTCASES.conf | 70 +++ 2 files changed, 115 insertions(+), 0 deletions(-) create mode 100644 docs/TESTCASE.py create mode 100644 docs/TESTCASES.conf diff --git a/docs/TESTCASE.py b/docs/TESTCASE.py new file mode 100644 index 000..48adb56 --- /dev/null +++ b/docs/TESTCASE.py @@ -0,0 +1,45 @@ +#! /usr/bin/env python +# How to write a new testcase + +# If testcase wants to use the connection object offered +# by the framework or share variables between testcases +# import sharedmod module. +import sharedmod + +# List parameters supported by the testcase in the two +# global tuple variables with a ',' at the end of last element. +# Both of them are mandatory. +required_params = ('option1', 'option2',) +optional_params = () + +# The check function is optional. This is for some testcases that +# need to check whether specific hardware is present on box or +# testing environment is good before testing. +# Return value 0 means check pass, 1 means to skip the +# testcase during running. +def TESTCASE_check(params): +logger = params['logger'] +... +return 0 + +# This is the main testing function, The function name must be +# the same as the file name of the testcase. params['logger'] is +# provided by framework for logging. +# Return value 0 means the success of testing, 1 means testing failure. +# It is mandatory. +def TESTCASE(params): +logger = params['logger'] +... +return 0 + +# The clean function is optional. This is for testcases that dirtied +# testing environment after executed. If keyword 'clean' is set +# just below the testcase in testcase config file, the clean function +# defined in the testcase.py will be invoked by framework to do the +# cleaning job. +# Return value is optional too, 1 means clean failure, but will not +# stop the run of testing. +def TESTCASE_clean(params): +logger = params['logger'] +... +return 0 diff --git a/docs/TESTCASES.conf b/docs/TESTCASES.conf new file mode 100644 index 000..1ca351b --- /dev/null +++ b/docs/TESTCASES.conf @@ -0,0 +1,70 @@ +# single line comments looks like this + +/* + Multiline comments look like this. + The lines enclosed by the C style comments will + be skipped by parser.py +*/ + +### +# Indentation: +# An indent level is four spaces, do not use tabs to indent. +# + +# .--- testcase: The first line contains the module name and the test case name separated by a colon and is not indented. +# | +domain:undefine + +# . options: Indent options by an indent level(four spaces) +# | +guestname + +# . value: Indent Values by two indent levels(eight spaces) +# | +fedoraVM + + +# +# Keywords: 'clean', 'times', 'sleep', '{start_loop, end_loop}' +# + +# 'clean': invoke the clean function in previous testcase +clean + +# 'times': repeat testcase 'repos/domain/install_linux_cdrom.py' N times +domain:install_linux_cdrom times 2 +guestname + fedoraVM +memory + 1024 +vcpu + 1 + +# 'sleep 5': pause the run of testing for N seconds. +sleep 5 + +# The pair of 'start_loop' and 'end_loop' will +# run the testcases between them N loops +domain:start start_loop +guestname +fedoraVM + +domain:destroy end_loop 3 +guestname +fedoraVM + + +# +# Options: 'times', 'cleanup' +# always be the last line of testcase config file +# + +# .-- repeat the above testcases n more times. +# | .-- invoke the clean function in all of above testcases +# | | +options times=2 cleanup=enable + +# python libvirt-test-api.py -t repos/domain/undefine.py repos/domain/install_linux_cdrom.py \ +#repos/domain/start.py repos/domain/destroy.py +# +# The command generates a template of tescase file like above -- 1.7.7.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC
On 04/16/2012 05:58 AM, Cole Robinson wrote: On 04/13/2012 09:14 AM, Stefan Bader wrote: I think it would be better if we just centralized this logic, as in, only set that (type ioemu) bit in conditional rather than 2. Should be pretty straightforward. Did you have something like below in mind? Looks good. Did you verify 'make check' passes as well? If so, ACK Pushed. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Do not enforce source type of console[0]
On 04/16/2012 10:05 AM, Cole Robinson wrote: On 04/16/2012 09:47 AM, Jan Kiszka wrote: On 2012-03-13 16:48, Jan Kiszka wrote: If console[0] is an alias for serial[0], do not enforce the former to have a PTY source type. This breaks serial consoles on stdio and makes no sense. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- src/qemu/qemu_process.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ef311d1..216b594 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1376,7 +1376,6 @@ qemuProcessFindCharDevicePTYs(virDomainObjPtr vm, if ((ret = virDomainChrSourceDefCopy(chr-source, ((vm-def-serials[0])-source))) != 0) return ret; -chr-source.type = VIR_DOMAIN_CHR_TYPE_PTY; } else { if (chr-source.type == VIR_DOMAIN_CHR_TYPE_PTY chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) { Michal originally made that change, I'm guessing it was an oversight though. ACK. I added you to AUTHORS, and pushed. Let me know if you prefer an alternate spelling in that file. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC
On 2012年04月17日 11:38, Eric Blake wrote: On 04/16/2012 09:03 PM, Osier Yang wrote: On 2012年04月17日 10:44, Eric Blake wrote: On 04/16/2012 08:05 PM, Osier Yang wrote: [09:31]danpb1eblake: the fact that we see that error message suggest a pre-existing bug in the test suite - the XML being used to define the test domain is missing a UUID [09:31]danpb1eblake: so the 2 repeated defines get different auto-generated UUIDsthus violate the uniqueness check [09:31]danpb1eblake: i expect if you fixed that problem, then you'd now see the 'define' operation actually suceed Not true, there is UUID in the dumped xml: A generated UUID - that is, we have a latent bug, in that since we aren't locking down a specific UUID, the generated one is different each time, and the test failed (as expected), but for the wrong reason (mismatch in UUID, instead of the intended failure due to a cpuset syntax error). Yeah, just because the cpuset syntax error comes up earlier before the UUID checking. I post a patch to set the fixed UUID for the objects which support UUID in test driver, see the attachment. ACK. Two follow up patches, one is to update test read-bufsiz to delete the UUID, as domain UUID for test driver is fixed now. The other is to allow the parsing of cpuset if the placement is not specified, but cpuset is specified, and in this case the placement mode will be set as static. From 270bb38c25e4fbed32193838dc81ec52a46780c3 Mon Sep 17 00:00:00 2001 From: Osier Yang jy...@redhat.com Date: Tue, 17 Apr 2012 12:40:03 +0800 Subject: [PATCH] conf: Do not parse cpuset only if the placement is auto So that a domain xml which doesn't have placement specified, but cpuset is specified, could be parsed. And in this case, the placement mode will be set as static. --- src/conf/domain_conf.c |7 --- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b28ae5c..65a35c3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7896,11 +7896,10 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } VIR_FREE(tmp); } else { -if (def-cpumasklen) -def-placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC; +def-placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_DEFAULT; } -if (def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC) { +if (def-placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { tmp = virXPathString(string(./vcpu[1]/@cpuset), ctxt); if (tmp) { char *set = tmp; @@ -7912,6 +7911,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, def-cpumasklen) 0) goto error; VIR_FREE(tmp); +if (def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_DEFAULT) +def-placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC; } } -- 1.7.7.3 From 6ddb803f074e9d5ce59ac90444d7ebf60d997ba1 Mon Sep 17 00:00:00 2001 From: Osier Yang jy...@redhat.com Date: Tue, 17 Apr 2012 12:23:33 +0800 Subject: [PATCH] tests: Update read-bufsiz to delete the UUID of vm XML Since now we have fixed domain UUID for test driver, defining a domain with different name but same UUID doesn't work any more. This patch delete the UUID from the dumped XML so that it could be generated. --- tests/read-bufsiz |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/read-bufsiz b/tests/read-bufsiz index 2a91bcf..a4c6007 100755 --- a/tests/read-bufsiz +++ b/tests/read-bufsiz @@ -32,8 +32,10 @@ fail=0 # Output a valid definition, to be used as input. $abs_top_builddir/tools/virsh -c test:///default dumpxml 1 xml.t || fail=1 -# Change the VM name -sed -e s|nametest/name|namenewtest/name|g xml.t xml +# Change the VM name and UUID +sed -e s|nametest/name|namenewtest/name|g \ + -e \|uuid.*/uuid|d \ + xml.t xml for i in before after; do # The largest BUFSIZ I've seen is 128K. This is slightly larger. -- 1.7.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv5 04/23] blockjob: add new API flags
This patch introduces a new block job, useful for live storage migration using pre-copy streaming. Justification for including this under virDomainBlockRebase rather than adding a new command includes: 1) there are now two possible block jobs in qemu, with virDomainBlockRebase starting either type of command, and virDomainBlockJobInfo and virDomainBlockJobAbort working to end either type; 2) reusing this command allows distros to backport this feature to the libvirt 0.9.10 API without a .so bump. Note that a future patch may add a more powerful interface named virDomainBlockJobCopy, dedicated to just the block copy job, in order to expose even more options (such as setting an arbitrary format type for the destination without having to probe it from a pre-existing destination file); adding a new command for targetting just block copy would be similar to how we already have virDomainBlockPull for targetting just the block pull job. Using a live VM with the backing chain: base - snap1 - snap2 as the starting point, we have: - virDomainBlockRebase(dom, disk, /path/to/copy, 0, VIR_DOMAIN_BLOCK_REBASE_COPY) creates /path/to/copy with the same format as snap2, with no backing file, so entire chain is copied and flattened - virDomainBlockRebase(dom, disk, /path/to/copy, 0, VIR_DOMAIN_BLOCK_REBASE_COPY|VIR_DOMAIN_BLOCK_REBASE_COPY_RAW) creates /path/to/copy as a raw file, so entire chain is copied and flattened - virDomainBlockRebase(dom, disk, /path/to/copy, 0, VIR_DOMAIN_BLOCK_REBASE_COPY|VIR_DOMAIN_BLOCK_REBASE_SHALLOW) creates /path/to/copy with the same format as snap2, but with snap1 as a backing file, so only snap2 is copied. - virDomainBlockRebase(dom, disk, /path/to/copy, 0, VIR_DOMAIN_BLOCK_REBASE_COPY|VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT) reuse existing /path/to/copy (must have empty contents, and format is probed[*] from the metadata), and copy the full chain - virDomainBlockRebase(dom, disk, /path/to/copy, 0, VIR_DOMAIN_BLOCK_REBASE_COPY|VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT| VIR_DOMAIN_BLOCK_REBASE_SHALLOW) reuse existing /path/to/copy (contents must be identical to snap1, and format is probed[*] from the metadata), and copy only the contents of snap2 - virDomainBlockRebase(dom, disk, /path/to/copy, 0, VIR_DOMAIN_BLOCK_REBASE_COPY|VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT| VIR_DOMAIN_BLOCK_REBASE_SHALLOW|VIR_DOMAIN_BLOCK_REBASE_COPY_RAW) reuse existing /path/to/copy (must be raw volume with contents identical to snap1), and copy only the contents of snap2 Less useful combinations: - virDomainBlockRebase(dom, disk, /path/to/copy, 0, VIR_DOMAIN_BLOCK_REBASE_COPY|VIR_DOMAIN_BLOCK_REBASE_SHALLOW| VIR_DOMAIN_BLOCK_REBASE_COPY_RAW) fail if source is not raw, otherwise create /path/to/copy as raw and the single file is copied (no chain involved) - virDomainBlockRebase(dom, disk, /path/to/copy, 0, VIR_DOMAIN_BLOCK_REBASE_COPY|VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT| VIR_DOMAIN_BLOCK_REBASE_COPY_RAW) makes little sense: the destination must be raw but have no contents, meaning that it is an empty file, so there is nothing to reuse The other three flags are rejected without VIR_DOMAIN_BLOCK_COPY. [*] Note that probing an existing file for its format can be a security risk _if_ there is a possibility that the existing file is 'raw', in which case the guest can manipulate the file to appear like some other format. But, by virtue of the VIR_DOMAIN_BLOCK_REBASE_COPY_RAW flag, it is possible to avoid probing of raw files, at which point, probing of any remaining file type is no longer a security risk. It would be nice if we could issue an event when pivoting from phase 1 to phase 2, but qemu hasn't implemented that, and we would have to poll in order to synthesize it ourselves. Meanwhile, qemu will give us a distinct job info and completion event when we either cancel or pivot to end the job. Pivoting is accomplished via the new: virDomainBlockJobAbort(dom, disk, VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) Management applications can pre-create the copy with a relative backing file name, and use the VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT flag to have qemu reuse the metadata; if the management application also copies the backing files to a new location, this can be used to perform live storage migration of an entire backing chain. * include/libvirt/libvirt.h.in (VIR_DOMAIN_BLOCK_JOB_TYPE_COPY): New block job type. (virDomainBlockJobAbortFlags, virDomainBlockRebaseFlags): New enums. * src/libvirt.c (virDomainBlockRebase): Document the new flags, and implement general restrictions on flag combinations. (virDomainBlockJobAbort): Document the new flag. (virDomainSaveFlags, virDomainSnapshotCreateXML) (virDomainRevertToSnapshot, virDomainDetachDeviceFlags): Document restrictions. * include/libvirt/virterror.h (VIR_ERR_BLOCK_COPY_ACTIVE): New error. * src/util/virterror.c (virErrorMsg): Define it. --- was 5/18 in v4 v5: enhance commit message, fix typos in docs
[libvirt] [PATCHv5 02/23] qemu: use consistent error when qemu binary is too old
Most of our errors complaining about an inability to support a particular action due to qemu limitations used CONFIG_UNSUPPORTED, but we had a few outliers. Reported by Jiri Denemark. * src/qemu/qemu_command.c (qemuBuildDriveDevStr): Prefer CONFIG_UNSUPPORTED. * src/qemu/qemu_driver.c (qemuDomainReboot) (qemuDomainBlockJobImpl): Likewise. * src/qemu/qemu_hotplug.c (qemuDomainAttachPciControllerDevice): Likewise. * src/qemu/qemu_monitor.c (qemuMonitorTransaction) (qemuMonitorBlockJob, qemuMonitorSystemWakeup): Likewise. --- v5: new patch, factored out from comments on other patches src/qemu/qemu_command.c |2 +- src/qemu/qemu_driver.c | 13 - src/qemu/qemu_hotplug.c |2 +- src/qemu/qemu_monitor.c |6 +++--- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c82f5bc..b66ce18 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1527,7 +1527,7 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, } } else { if (info-addr.pci.function != 0) { -qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Only PCI device addresses with function=0 are supported with this QEMU binary)); return -1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 436ef37..c3555ca 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1633,12 +1633,15 @@ cleanup: return ret; } -static int qemuDomainShutdown(virDomainPtr dom) { +static int qemuDomainShutdown(virDomainPtr dom) +{ return qemuDomainShutdownFlags(dom, 0); } -static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) { +static int +qemuDomainReboot(virDomainPtr dom, unsigned int flags) +{ struct qemud_driver *driver = dom-conn-privateData; virDomainObjPtr vm; int ret = -1; @@ -1682,7 +1685,7 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) { #if HAVE_YAJL if (qemuCapsGet(priv-qemuCaps, QEMU_CAPS_MONITOR_JSON)) { if (!qemuCapsGet(priv-qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) { -qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Reboot is not supported with this QEMU binary)); goto cleanup; } @@ -11643,11 +11646,11 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base, if (qemuCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) { async = true; } else if (!qemuCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCKJOB_SYNC)) { -qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(block jobs not supported with this QEMU binary)); goto cleanup; } else if (base) { -qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(partial block pull not supported with this QEMU binary)); goto cleanup; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 857b980..7cf7b90 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -335,7 +335,7 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver, if (controller-type == VIR_DOMAIN_CONTROLLER_TYPE_USB controller-model == -1 !qemuCapsGet(priv-qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI)) { -qemuReportError(VIR_ERR_OPERATION_FAILED, +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(USB controller hotplug unsupported in this QEMU binary)); goto cleanup; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 36f3832..2f66c46 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2696,7 +2696,7 @@ qemuMonitorTransaction(qemuMonitorPtr mon, virJSONValuePtr actions) if (mon-json) ret = qemuMonitorJSONTransaction(mon, actions); else -qemuReportError(VIR_ERR_INVALID_ARG, %s, +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(transaction requires JSON monitor)); return ret; } @@ -2786,7 +2786,7 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, ret = qemuMonitorJSONBlockJob(mon, device, base, bandwidth, info, mode, async); else -qemuReportError(VIR_ERR_INVALID_ARG, %s, +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(block jobs require JSON monitor)); return ret; } @@ -2918,7 +2918,7 @@ int qemuMonitorSystemWakeup(qemuMonitorPtr mon) } if (!mon-json) { -qemuReportError(VIR_ERR_NO_SUPPORT, %s, +
[libvirt] [PATCHv5 00/23] live block migration
This series shows how to do live block migration using both streaming mirror (as proposed for upstream qemu 1.1) and snapshot+mirror (as proposed for RHEL 6.3, although not as actively tested, since streaming mirror seems so much nicer). v4 (blockjob): https://www.redhat.com/archives/libvir-list/2012-April/msg00330.html v1 (snapshot): https://www.redhat.com/archives/libvir-list/2012-March/msg00578.html I've hopefully resolved the review comments Jiri gave against v4, and have tested this against Paolo's proposed builds of qemu for RHEL 6.3. Patches 1-14 are ready to apply if qemu commits to the 'drive-mirror' interface for qemu 1.1. Patches 15-17 need a bit more thought to decide if we also want to support partial streaming support (similar to the optional 'base' argument to 'block-stream') in 'drive-mirror' in qemu 1.2; it might be wiser to defer these until we know for sure what happens with qemu. Patches 18-23 are only valid for the RHEL 6.3 build, although patches 19-23 would be useful if qemu 1.2 enhances 'drive-mirror' to support snapshot+mirror. I've tried to give more details in the various commits up to 14, which are the most important to review for purposes of backporting; other changes in later patches include fixing a use-after-free bug in the snapshot+mirror code, and altering patch 18 to match the difference in what RHEL 6.3 will probably be providing. Available here as well: git fetch git://repo.or.cz/libvirt/ericb.git snapshot http://repo.or.cz/w/libvirt/ericb.git/shortlog/refs/heads/snapshot I think 1-3 can be applied without controversy, 4-7 can be applied _if_ we think that the libvirt API is reasonable no matter what qemu throws at us (and I'm pretty positive that it is, as I have already been through several rounds of design changes on the qemu side where the libvirt API still held up without problem), and 8 onwards should be delayed until we have a firm commitment on the actual qemu 1.1 implementation. Eric Blake (23): virsh: minor syntactic cleanups qemu: use consistent error when qemu binary is too old blockjob: add virsh blockpull --wait blockjob: add new API flags blockjob: add 'blockcopy' to virsh blockjob: enhance xml to track mirrors across libvirtd restart blockjob: react to active block copy blockjob: add qemu capabilities related to block jobs blockjob: return appropriate event and info blockjob: support pivot operation on cancel blockjob: implement block copy for qemu blockjob: relax block job behavior when setting speed up front blockjob: allow for existing files blockjob: allow mirroring under SELinux blockjob: add virDomainBlockCopy blockjob: enhance virsh 'blockcopy' blockjob: wire up qemu and RPC for block copy blockjob: accommodate RHEL backport names snapshot: allow for creation of mirrored snapshots snapshot: add new snapshot delete flags snapshot: make it possible to check for mirrored snapshot snapshot: implement new snapshot delete flags in qemu snapshot: enable mirrored snapshots on transient vm docs/apibuild.py |1 + docs/formatdomain.html.in | 13 + docs/formatsnapshot.html.in| 31 + docs/schemas/domaincommon.rng | 24 +- docs/schemas/domainsnapshot.rng|8 + include/libvirt/libvirt.h.in | 50 ++- include/libvirt/virterror.h|1 + src/conf/domain_conf.c | 125 - src/conf/domain_conf.h |8 + src/driver.h |5 + src/libvirt.c | 268 - src/libvirt_private.syms |2 + src/libvirt_public.syms|5 + src/qemu/qemu_capabilities.c |2 + src/qemu/qemu_capabilities.h |2 + src/qemu/qemu_command.c|2 +- src/qemu/qemu_driver.c | 624 +++- src/qemu/qemu_hotplug.c|9 +- src/qemu/qemu_monitor.c| 56 ++- src/qemu/qemu_monitor.h| 12 + src/qemu/qemu_monitor_json.c | 94 +++ src/qemu/qemu_monitor_json.h | 19 +- src/remote/remote_driver.c |1 + src/remote/remote_protocol.x | 12 +- src/remote_protocol-structs|9 + src/rpc/gendispatch.pl |1 + src/util/virterror.c |6 + .../disk_snapshot_mirror.xml | 13 + .../disk_snapshot_mirror.xml | 49 ++ tests/domainsnapshotxml2xmltest.c |4 +- .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 42 ++
[libvirt] [PATCHv5 01/23] virsh: minor syntactic cleanups
No semantic change. * tools/virsh.c: Fix some spacing issues, {} usage, long lines, and redundant (). --- v5: new patch, factored out from comments on other patches tools/virsh.c | 197 + 1 files changed, 100 insertions(+), 97 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 8ef57e0..5400487 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -402,9 +402,9 @@ static virTypedParameterPtr vshFindTypedParamByName(const char *name, static char *vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -static char *editWriteToTempFile (vshControl *ctl, const char *doc); -static int editFile (vshControl *ctl, const char *filename); -static char *editReadBackFile (vshControl *ctl, const char *filename); +static char *editWriteToTempFile(vshControl *ctl, const char *doc); +static int editFile(vshControl *ctl, const char *filename); +static char *editReadBackFile(vshControl *ctl, const char *filename); /* Typedefs, function prototypes for job progress reporting. * There are used by some long lingering commands like @@ -604,7 +604,7 @@ static int disconnected = 0; /* we may have been disconnected */ */ static void vshCatchDisconnect(int sig, siginfo_t *siginfo, void *context ATTRIBUTE_UNUSED) { -if ((sig == SIGPIPE) || +if (sig == SIGPIPE || (SA_SIGINFO siginfo-si_signo == SIGPIPE)) disconnected++; } @@ -1044,8 +1044,7 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) goto cleanup; } -if ((persistent !optPersistent) || -(!persistent !optTransient)) { +if (!(persistent ? optPersistent : optTransient)) { virDomainFree(dom); dom = NULL; continue; @@ -3269,7 +3268,7 @@ cmdSave(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptString(cmd, file, to) = 0) goto cleanup; -if (vshCommandOptBool (cmd, verbose)) +if (vshCommandOptBool(cmd, verbose)) verbose = true; if (pipe(p) 0) @@ -3576,7 +3575,7 @@ cmdManagedSave(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, name))) return false; -if (vshCommandOptBool (cmd, verbose)) +if (vshCommandOptBool(cmd, verbose)) verbose = true; if (pipe(p) 0) @@ -4013,9 +4012,9 @@ doDump(void *opaque) if (!(dom = vshCommandOptDomain(ctl, cmd, name))) goto out; -if (vshCommandOptBool (cmd, live)) +if (vshCommandOptBool(cmd, live)) flags |= VIR_DUMP_LIVE; -if (vshCommandOptBool (cmd, crash)) +if (vshCommandOptBool(cmd, crash)) flags |= VIR_DUMP_CRASH; if (vshCommandOptBool(cmd, bypass-cache)) flags |= VIR_DUMP_BYPASS_CACHE; @@ -4054,7 +4053,7 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptString(cmd, file, to) = 0) return false; -if (vshCommandOptBool (cmd, verbose)) +if (vshCommandOptBool(cmd, verbose)) verbose = true; if (pipe(p) 0) @@ -5394,7 +5393,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) } virSkipSpaces(cur); -if ((*cur == ',') || (*cur == 0)) { +if (*cur == ',' || *cur == 0) { if (unuse) { VIR_UNUSE_CPU(cpumap, cpu); } else { @@ -5995,9 +5994,9 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd) */ static const vshCmdInfo info_blkiotune[] = { {help, N_(Get or set blkio parameters)}, -{desc, N_(Get or set the current blkio parameters for a guest \ - domain.\n \ -To get the blkio parameters use following command: \n\n \ +{desc, N_(Get or set the current blkio parameters for a guest + domain.\n +To get the blkio parameters use following command: \n\n virsh # blkiotune domain)}, {NULL, NULL} }; @@ -6143,9 +6142,9 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) */ static const vshCmdInfo info_memtune[] = { {help, N_(Get or set memory parameters)}, -{desc, N_(Get or set the current memory parameters for a guest \ - domain.\n \ -To get the memory parameters use following command: \n\n \ +{desc, N_(Get or set the current memory parameters for a guest + domain.\n +To get the memory parameters use following command: \n\n virsh # memtune domain)}, {NULL, NULL} }; @@ -6335,9 +6334,9 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd) */ static const vshCmdInfo info_numatune[] = { {help, N_(Get or set numa parameters)}, -{desc, N_(Get or set the current numa parameters for a guest \ - domain.\n \ -To get the numa parameters use following command: \n\n \ +{desc, N_(Get or
[libvirt] [PATCHv5 05/23] blockjob: add 'blockcopy' to virsh
Rather than further overloading 'blockpull', I decided to create a new virsh command to expose the new flags of virDomainBlockRebase. Blocking until the command completes naturally is pointless, since the block copy job is intended to run indefinitely. Instead, I made the command support three --wait modes: by default, it runs until mirroring is started; with --pivot, it pivots as soon as mirroring is started; and with --finish, it aborts (for a clean copy) as soon as mirroring is started. * tools/virsh.c (VSH_CMD_BLOCK_JOB_COPY): New mode. (blockJobImpl): Support new flags. (cmdBlockCopy): New command. (cmdBlockJob): Support new job info, new abort flag. * tools/virsh.pod (blockcopy, blockjob): Document the new command and flags. --- was 6/18 in v4 v5: add --wait flag, consistent capitalization tools/virsh.c | 212 +++ tools/virsh.pod | 49 - 2 files changed, 243 insertions(+), 18 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 95ed7bc..0f79022 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -7517,7 +7517,8 @@ typedef enum { VSH_CMD_BLOCK_JOB_INFO = 1, VSH_CMD_BLOCK_JOB_SPEED = 2, VSH_CMD_BLOCK_JOB_PULL = 3, -} VSH_CMD_BLOCK_JOB_MODE; +VSH_CMD_BLOCK_JOB_COPY = 4, +} vshCmdBlockJobMode; static int blockJobImpl(vshControl *ctl, const vshCmd *cmd, @@ -7528,6 +7529,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, const char *name, *path; unsigned long bandwidth = 0; int ret = -1; +const char *base = NULL; unsigned int flags = 0; if (!vshConnectionUsability(ctl, ctl-conn)) @@ -7544,22 +7546,39 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, goto cleanup; } -if (mode == VSH_CMD_BLOCK_JOB_ABORT) { +switch ((vshCmdBlockJobMode) mode) { +case VSH_CMD_BLOCK_JOB_ABORT: if (vshCommandOptBool(cmd, async)) flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC; +if (vshCommandOptBool(cmd, pivot)) +flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT; ret = virDomainBlockJobAbort(dom, path, flags); -} else if (mode == VSH_CMD_BLOCK_JOB_INFO) { +break; +case VSH_CMD_BLOCK_JOB_INFO: ret = virDomainGetBlockJobInfo(dom, path, info, 0); -} else if (mode == VSH_CMD_BLOCK_JOB_SPEED) { +break; +case VSH_CMD_BLOCK_JOB_SPEED: ret = virDomainBlockJobSetSpeed(dom, path, bandwidth, 0); -} else if (mode == VSH_CMD_BLOCK_JOB_PULL) { -const char *base = NULL; +break; +case VSH_CMD_BLOCK_JOB_PULL: if (vshCommandOptString(cmd, base, base) 0) goto cleanup; if (base) ret = virDomainBlockRebase(dom, path, base, bandwidth, 0); else ret = virDomainBlockPull(dom, path, bandwidth, 0); +break; +case VSH_CMD_BLOCK_JOB_COPY: +flags |= VIR_DOMAIN_BLOCK_REBASE_COPY; +if (vshCommandOptBool(cmd, shallow)) +flags |= VIR_DOMAIN_BLOCK_REBASE_SHALLOW; +if (vshCommandOptBool(cmd, reuse-external)) +flags |= VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT; +if (vshCommandOptBool(cmd, raw)) +flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_RAW; +if (vshCommandOptString(cmd, dest, base) 0) +goto cleanup; +ret = virDomainBlockRebase(dom, path, base, bandwidth, flags); } cleanup: @@ -7571,6 +7590,158 @@ cleanup: } /* + * blockcopy command + */ +static const vshCmdInfo info_block_copy[] = { +{help, N_(Start a block copy operation.)}, +{desc, N_(Populate a disk from its backing image.)}, +{NULL, NULL} +}; + +static const vshCmdOptDef opts_block_copy[] = { +{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)}, +{path, VSH_OT_DATA, VSH_OFLAG_REQ, N_(fully-qualified path of disk)}, +{dest, VSH_OT_DATA, VSH_OFLAG_REQ, N_(path of the copy to create)}, +{bandwidth, VSH_OT_DATA, VSH_OFLAG_NONE, N_(bandwidth limit in MB/s)}, +{shallow, VSH_OT_BOOL, 0, N_(make the copy share a backing chain)}, +{reuse-external, VSH_OT_BOOL, 0, N_(reuse existing destination)}, +{raw, VSH_OT_BOOL, 0, N_(use raw destination file)}, +{wait, VSH_OT_BOOL, 0, N_(wait for job to reach mirroring phase)}, +{verbose, VSH_OT_BOOL, 0, N_(with --wait, display the progress)}, +{timeout, VSH_OT_INT, VSH_OFLAG_NONE, + N_(with --wait, abort if copy exceeds timeout (in seconds))}, +{pivot, VSH_OT_BOOL, 0, N_(with --wait, pivot when mirroring starts)}, +{finish, VSH_OT_BOOL, 0, N_(with --wait, quit when mirroring starts)}, +{async, VSH_OT_BOOL, 0, + N_(with --wait, don't wait for cancel to finish)}, +{NULL, 0, 0, NULL} +}; + +static bool +cmdBlockCopy(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom = NULL; +bool ret = false; +bool blocking = vshCommandOptBool(cmd, wait); +bool verbose = vshCommandOptBool(cmd, verbose); +bool pivot = vshCommandOptBool(cmd,
[libvirt] [PATCHv5 12/23] blockjob: relax block job behavior when setting speed up front
In qemu, it is possible to call 'migrate_set_speed' prior to 'migrate', and therefore ensure a constant throttling through the entire migration. However, this is not possible with 'block-job-set-speed', which fails if a job is not already active. This means that you can't detect a device that doesn't support throttling until after you have already started a block job and let an unknown amount of unthrottled data through. Aborting the job upon failure to set the speed seems a bit harsh, since it would have been nicer to prevent the job from starting in the first place, rather than letting an unknown amount of data be processed before detecting the speed failure. So I propose relaxing the documentation, and explicitly mentioning that setting the speed is a best-effort attempt that might be ignored. On the other hand, I've also requested that qemu consider adding an optional parameter to allow setting the speed at the creation of a block job. * src/libvirt.c (virDomainBlockPull, virDomainBlockRebase): Update the documentation. * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl) (qemuDomainBlockCopy): Log but don't fail when speed change fails. --- v5: new patch; see also this qemu request: https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg02185.html src/libvirt.c | 12 ++-- src/qemu/qemu_driver.c | 18 -- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index af42d3b..17c7576 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18145,7 +18145,11 @@ error: * The maximum bandwidth (in Mbps) that will be used to do the copy can be * specified with the bandwidth parameter. If set to 0, libvirt will choose a * suitable default. Some hypervisors do not support this feature and will - * return an error if bandwidth is not 0. + * return an error if bandwidth is not 0; others will make a best effort + * attempt, but silently ignore failure to set the speed. The actual speed + * can be determined with virDomainGetBlockJobInfo(), and setting speed via + * virDomainBlockJobSetSpeed() allows more control, at the expense of a race + * condition where the job may end before the speed can be set or queried. * * This is shorthand for virDomainBlockRebase() with a NULL base. * @@ -18263,7 +18267,11 @@ error: * The maximum bandwidth (in Mbps) that will be used to do the copy can be * specified with the bandwidth parameter. If set to 0, libvirt will choose a * suitable default. Some hypervisors do not support this feature and will - * return an error if bandwidth is not 0. + * return an error if bandwidth is not 0; others will make a best effort + * attempt, but silently ignore failure to set the speed. The actual speed + * can be determined with virDomainGetBlockJobInfo(), and setting speed via + * virDomainBlockJobSetSpeed() allows more control, at the expense of a race + * condition where the job may end before the speed can be set or queried. * * When @base is NULL and @flags is 0, this is identical to * virDomainBlockPull(). diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5c3cea8..aec5c4c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11799,9 +11799,12 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base, * relying on qemu to do this. */ ret = qemuMonitorBlockJob(priv-mon, device, base, bandwidth, info, mode, async); -if (ret == 0 mode == BLOCK_JOB_PULL bandwidth) -ret = qemuMonitorBlockJob(priv-mon, device, NULL, bandwidth, NULL, - BLOCK_JOB_SPEED_INTERNAL, async); +if (ret == 0 mode == BLOCK_JOB_PULL bandwidth) { +/* Setting speed now is best-effort. */ +if (qemuMonitorBlockJob(priv-mon, device, NULL, bandwidth, NULL, +BLOCK_JOB_SPEED_INTERNAL, async) 0) +VIR_WARN(failed to set bandwidth for disk %s, disk-dst); +} qemuDomainObjExitMonitorWithDriver(driver, vm); if (ret 0) goto endjob; @@ -11990,9 +11993,12 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path, /* Actually start the mirroring */ qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorDriveMirror(priv-mon, device, dest, format, flags); -if (ret == 0 bandwidth != 0) -ret = qemuMonitorBlockJob(priv-mon, device, NULL, bandwidth, NULL, - BLOCK_JOB_SPEED_INTERNAL, true); +if (ret == 0 bandwidth != 0) { +/* Setting speed now is best-effort. */ +if (qemuMonitorBlockJob(priv-mon, device, NULL, bandwidth, NULL, +BLOCK_JOB_SPEED_INTERNAL, true) 0) +VIR_WARN(failed to set bandwidth for disk %s, disk-dst); +} qemuDomainObjExitMonitorWithDriver(driver, vm); endjob: -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com