On 06/07/2012 04:46 AM, Onkar N Mahajan wrote: > Hello All, > > With this patch , you will be able to run tests that use multiple VMs > with libvirt. One such > test is VLAN test ; Which libvirt autotest currently is not capable of > running. > Capability to run multiple VMs simultaneoulsy and 'explicit' > sharing/restricting of resources with > permissions important for my forthcoming security patches which will test > libvirt security infrastructure > (namely svirt). This patch only attempts segregate VM disk resources (CDROM, > VM images) under a VM > directory which is created per VM at the time of installation. Forthcoming > patches will address > *explicit* sharing/restricting of resources with configuration options.
Cool! I can't wait to see them! > My intention here was to 'do more with less code' , i.e., minimally > disturb existing autotest code. > For unattended_install , all you need to do is : Great, this is a good mentality/approach. As-is, most of the libvirt Autotest code was 'organically' developed/evolved. We'll be the first to admit, some if it is pretty scary. Since it's still very immature, please don't be afraid of helping (or asking us to) fix broken areas. It's much better for the long run if we spend a little extra time to get the fundamentals proper, before more and more tests are added. > (1) create a VM directory under /tmp/libvirt_autotest_root/ whose name must > exactly match > param 'main_vm' ( in base.cfg ) for e.g., > /tmp/libvirt_autotest_root/vm1/ , and > (2) create iso directory under VM directory > (/tmp/libvirt_autotest_root/vm1/)- namely > 'linux/iso' (or simply 'iso' - recommended , but requires small change > in guest-os.cfg for > your guest at param 'cdrom_cd1' , for instance originally your > 'cdrom_cd1' would look like > 'cdrom_cd1 = isos/linux/RHELX.iso' , but now it can just be 'cdrom_cd1 > = isos/RHELX.iso' ) > (3) Place your guest CDROM in the driectory created in (1) and (2) > i.e., /tmp/libvirt_autotest_root/vm1/linux/iso ( or simply > /tmp/libvirt_autotest_root/vm1/iso > , see recomendation in (2)) > > Signed-off-by: Onkar N Mahajan<[email protected]> > > --- > client/tests/libvirt/tests-shared.cfg.sample | 12 ++-- > client/virt/libvirt_vm.py | 69 > +++++++++++++++++++++++++- > client/virt/virt_utils.py | 3 + > 3 files changed, 77 insertions(+), 7 deletions(-) > > diff --git a/client/tests/libvirt/tests-shared.cfg.sample > b/client/tests/libvirt/tests-shared.cfg.sample > index 1fe0894..64cf516 100644 > --- a/client/tests/libvirt/tests-shared.cfg.sample > +++ b/client/tests/libvirt/tests-shared.cfg.sample > @@ -28,13 +28,13 @@ include virtio-win.cfg > # * The parameters cdrom_unattended, floppy, kernel and initrd are generated > # by LIBVIRT autotest, so remember to put them under a writable location > # (for example, the cdrom share can be read only) > -image_name(_.*)? ?<= /tmp/libvirt_autotest_root/images/ > -cdrom(_.*)? ?<= /tmp/libvirt_autotest_root/ > -floppy ?<= /tmp/libvirt_autotest_root/ > -image_dir = /tmp/libvirt_autotest_root/ > +image_name(_.*)? ?<= /tmp/libvirt_autotest_root/$main_vm/ > +cdrom(_.*)? ?<= /tmp/libvirt_autotest_root/$main_vm/ > +floppy ?<= /tmp/libvirt_autotest_root/$main_vm/ > +image_dir = /tmp/libvirt_autotest_root/$main_vm/ > Linux..unattended_install: > - kernel ?<= /tmp/libvirt_autotest_root/ > - initrd ?<= /tmp/libvirt_autotest_root/ > + kernel ?<= /tmp/libvirt_autotest_root/$main_vm/ > + initrd ?<= /tmp/libvirt_autotest_root/$main_vm/ > > # Uncomment the following lines to enable abort-on-error mode: > #abort_on_error = yes I don't quite follow the need for vm-specific directories. Are you intending to use them for explicit selinux contexts and permissions testing? If so, this is probably okay, but: My concern for svirt testing this way (and our using /tmp/... in general) is just making things more difficult and further from the intended end-user use-case. In both Fedora and RHEL, default SELinux policy and file contexts are already setup for using /var/lib/libvirt/... under many different scenarios. I'm wondering if there's a good reason for libvirt autotest to even use /tmp at all, or if it's just done to mirror kvm autotest. I've always thought we should roll with the defaults more, with most of this stuff under /var/lib/libvirt/... where it's expected anyway. i.e. I think there's more value from a testing perspective in exercising it the way it was intended to be used first, and secondarily supporting special-case setups as needed. IIRC, there's nothing stopping setting non-standard contexts/permissions on stuff under /var/lib/libvirt/... Can anyone think of a reason libvirt autotest should not default to the defaults??? N.B. 3-4 months ago, I was successful in testing a move of everything under /var/lib/libvirt and running with selinux enabled/enforcing w/o complaint. So it's possible to do. > diff --git a/client/virt/libvirt_vm.py b/client/virt/libvirt_vm.py > index cbed8aa..035cedb 100644 > --- a/client/virt/libvirt_vm.py > +++ b/client/virt/libvirt_vm.py > @@ -457,6 +457,34 @@ def virsh_detach_device(name, xml_file, extra = "", uri > = ""): > logging.error("Detaching device from VM %s failed." % name) > return False > > +def virsh_list(option="--all", uri = ""): > + """ > + - list domains - > + @param option: Can take various values see 'virsh help list' > + for more details. > + @param uri : connect uri > + > + Returns a list : > + (<rc of virsh list command> ,<dictionary 'dom-name': > + [<dom-id> ,<dom-status> ]) This return seems a bit convoluted since only a True/False rc is returned anyway. Why not just return the list of dictionaries, and throwing exceptions to signal problems (see below also)? > + > + """ > + cmd = "list %s" % option > + try: > + cmd_res = virsh_cmd(cmd, uri) > + d = {} > + for line in cmd_res.split("\n"): > + l = [] > + m = re.search('^\s([0-9]*\-?)\s+(\w+)\s+(.*)', line) Would '^\s*([0-9]*\-?)\s+(\w+)\s+(.*)' be safer in case starting whitespace gets lost for whatever reason? > + if m: > + l.append(m.group(1)) > + l.append(m.group(3)) > + d[m.group(2)] = l > + return (True, d) > + except error.CmdError, detail: If we catch exceptions at this low level, it will negate 'error testing' possibilities - granted testing 'virsh list' with invalid options is of arguable value. However IMHO we should leave options open. > + logging.error("Failed listing domains : %s" % (detail)) > + return (False, None) > + > > class VM(virt_vm.BaseVM): > """ > @@ -529,6 +557,43 @@ class VM(virt_vm.BaseVM): > """ > return virsh_is_alive(self.name, self.connect_uri) > > + def list_peer_domain(self, option="--all"): Can we call this list_peer_domains (plural)? I think it will 'read' better in test code. > + """ > + Returns list of domains for the uri > + """ > + (rc, val) = virsh_list(option, self.connect_uri) > + return (rc, val) This is fine. > + > + > + def nxt_free_vnc_port(self): > + """ > + Returns max VNC port in use for a particular uri > + """ > + (rc, val) = self.list_peer_domain() > + if not rc: > + logging.error("Unable to get peer domain information") > + sys.exit(1) > + else: > + port = 0 > + for key in val.keys(): > + """ > + This currently , only works with VMs installed with > + valid VNC port , and doesn't work with domains installed > + with<graphics type='vnc' port='-1' autoport='yes'/>. So > + domains installed with autotest will be installed with valid > + port numbers , externally installed VMs (installed with for > + instance virt-manager, virt-install - must be started before > this > + test is performed.To get valid results and for libvirt > + autotest to work successfully. > + """ > + cmd_res = virsh_cmd("vncdisplay %s" % key, self.connect_uri) > + m = re.search('(.*)\:(\d+)', cmd_res) > + if m.group(2)>= port: > + port = m.group(2) > + p = int(port) > + if p<= self.vnc_port: > + p += self.vnc_port > + return p+1 This is a VERY specific/limited use method that doesn't throw exceptions when expectations aren't met. However, libvirt autotest's handling of vnc/spice/etc graphics is fairly 'wonky' and I've been meaning to clean it up for a while now. What are your thoughts (and testing ideas) on if we got rid of all the ancillary port finding code and just rely on libvirt to auto-assign by default? Maybe that would make this method unnecessary? N/B: runtime auto-assign is what KVM autotest does. > def is_dead(self): > """ > @@ -1161,7 +1226,9 @@ class VM(virt_vm.BaseVM): > > # Find available VNC port, if needed > if params.get("display") == "vnc": > - self.vnc_port = virt_utils.find_free_port(5900, 6100) > + p = self.nxt_free_vnc_port() > + self.vnc_port = virt_utils.find_free_port(p, 6100) > + > > # Find available spice port, if needed > if params.get("spice"): > diff --git a/client/virt/virt_utils.py b/client/virt/virt_utils.py > index 4bbfc7b..e9ce755 100644 > --- a/client/virt/virt_utils.py > +++ b/client/virt/virt_utils.py > @@ -678,6 +678,9 @@ def create_image(params, root_dir): > """ > format = params.get("image_format", "qcow2") > image_filename = get_image_filename(params, root_dir) > + > + if not os.path.exists(os.path.dirname(image_filename)): > + os.mkdir(os.path.dirname(image_filename)) > size = params.get("image_size", "10G") > if params.get("create_with_dd") == "yes" and format == "raw": > # maps K,M,G,T => (count, bs) > -- > 1.7.4.4 > > -- Chris Evich, RHCA, RHCE, RHCDS, RHCSS Quality Assurance Engineer e-mail: cevich + `@' + redhat.com o: 1-888-RED-HAT1 x44214 _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
