- Devel - libvirt List Archives

[libvirt] rbd storage pool support for libvirt
by Sage Weil 19 Nov '10

19 Nov '10

Hi, We've been working on RBD, a distributed block device backed by the Ceph distributed object store. (Ceph is a highly scalable, fault tolerant distributed storage and file system; see http://ceph.newdream.net.) Although the Ceph file system client has been in Linux since 2.6.34, the RBD block device was just merged for 2.6.37. We also have patches pending for Qemu that use librados to natively talk to the Ceph storage backend, avoiding any kernel dependency. To support disks backed by RBD in libvirt, we originally proposed a 'virtual' type that simply passed the configuration information through to qemu, but that idea was shot down for a variety of reasons: http://www.redhat.com/archives/libvir-list/2010-June/thread.html#00257 It sounds like the "right" approach is to create a storage pool type. Ceph also has a 'pool' concept that contains some number of RBD images and a command line tool to manipulate (create, destroy, resize, rename, snapshot, etc.) those images, which seems to map nicely onto the storage pool abstraction. For example, $ rbd create foo -s 1000 rbd image 'foo': size 1000 MB in 250 objects order 22 (4096 KB objects) adding rbd image to directory... creating rbd image... done. $ rbd create bar -s 10000 [...] $ rbd list bar foo Something along the lines of <pool type="rbd"> <name>virtimages</name> <source mode="kernel"> <host monitor="ceph-mon1.domain.com:6789"/> <host monitor="ceph-mon2.domain.com:6789"/> <host monitor="ceph-mon3.domain.com:6789"/> <pool name="rbd"/> </source> </pool> or whatever (I'm not too familiar with the libvirt schema)? One difference between the existing pool types listed at libvirt.org/storage.html is that RBD does not necessarily associate itself with a path in the local file system. If the native qemu driver is used, there is no path involved, just a magic string passed to qemu (rbd:poolname/imagename). If the kernel RBD driver is used, it gets mapped to a /dev/rbd/$n (or similar, depending on the udev rule), but $n is not static across reboots. In any case, before someone goes off and implements something, does this look like the right general approach to adding rbd support to libvirt? Thanks! sage

4 11

[libvirt] [PATCH] doc: Add doc for missed options of migrate
by Osier Yang 19 Nov '10

19 Nov '10

* tools/virsh.pod (add doc for options like "--p2p", "--direct", "--copy-storage-all", "dname", etc.) --- tools/virsh.pod | 23 ++++++++++++++++------- 1 files changed, 16 insertions(+), 7 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index c4a5803..036848d 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -456,13 +456,22 @@ Provide the maximum number of virtual CPUs supported for a guest VM on this connection. If provided, the I<type> parameter must be a valid type attribute for the <domain> element of XML. -=item B<migrate> optional I<--live> I<--suspend> I<domain-id> I<desturi> -I<migrateuri> - -Migrate domain to another host. Add --live for live migration; --suspend -leaves the domain paused on the destination host. The I<desturi> is the -connection URI of the destination host, and I<migrateuri> is the -migration URI, which usually can be omitted. +=item B<migrate> optional I<--live> I<--p2p> I<--direct> I<--tunnelled> +I<--persistent> I<--undefinesource> I<--suspend> I<--copy-storage-all> +I<--copy-storage-inc> I<domain-id> I<desturi> I<migrateuri> I<dname> + +Migrate domain to another host. Add I<--live> for live migration; I<--p2p> +for peer-2-peer migration; I<--direct> for direct migration; I<--tunnelled> +for tunnelled migration; I<--persistent> leaves the domain persistent on +destination host, I<--undefinesource> undefine the domain on source host, +I<--suspend> leaves the domain paused on the destination host. +I<--copy-storage-all> indicates migration with non-shared storage with full +disk copy, I<--copy-storage-inc> indicates migration with non-shared storage +with incremental copy (same base image shared between source and destination). +The I<desturi> is the connection URI of the destination host, and +I<migrateuri> is the migration URI, which usually can be omitted. +I<dname> is used for renaming the domain to new name during migration, which +also usually can be ommited. =item B<migrate-setmaxdowntime> I<domain-id> I<downtime> -- 1.7.3.2

2 2

[libvirt] [PATCH 0/2] Support of auto-dump on watchdog event in libvirtd
by Hu Tao 19 Nov '10

19 Nov '10

This patch series adds support of auto-dump on watchdog event in libvirtd. If a qemu guest is configured with a watchdog device and libvirtd receives a watchdog event from the guest, it can do something on the guest, currently `dump' action is defined. This patch series adds a new watchdog action `managed', it has a subaction wich indicates the action libvirtd will take on a watchdog event. The meaning of subaction_arg depends on subaction. If subaction is `dump', then subaction_arg defines a folder path into which libvirtd saves dumpfile. An example: <watchdog model='i6300esb' action='managed' subaction='dump' subaction_arg='/mnt/data/dumps'> In order to make the function work, there must be a watchdog device added to guest, and guest must have a watchdog daemon running, for example, /etc/init.d/watchdog start or auto-started on boot. Suggestions are welcome! Hu Tao (2): Add a thread pool implementation Add a watchdog action `managed'. src/Makefile.am | 3 +- src/conf/domain_conf.c | 27 ++++++++- src/conf/domain_conf.h | 11 ++++- src/qemu/qemu_conf.c | 2 +- src/qemu/qemu_conf.h | 6 ++ src/qemu/qemu_driver.c | 138 ++++++++++++++++++++++++++++++++++++++++++----- src/util/threadpool.c | 119 +++++++++++++++++++++++++++++++++++++++++ src/util/threadpool.h | 34 ++++++++++++ 8 files changed, 319 insertions(+), 21 deletions(-) create mode 100644 src/util/threadpool.c create mode 100644 src/util/threadpool.h -- 1.7.3 -- Thanks, Hu Tao

2 6

[libvirt] [PATCH] doc: add doc for missed parameters of attach-disk
by Osier Yang 18 Nov '10

18 Nov '10

* tools/virsh.pod (add docs for --persistent and --sourcetype of attach-disk, break the long lines in the meantime) --- tools/virsh.pod | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index b4644d8..c4a5803 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -645,7 +645,9 @@ See the documentation to learn about libvirt XML format for a device. For cdrom and floppy devices, this command only replaces the media within the single existing device; consider using B<update-device> for this usage. -=item B<attach-disk> I<domain-id> I<source> I<target> optional I<--driver driver> I<--subdriver subdriver> I<--type type> I<--mode mode> +=item B<attach-disk> I<domain-id> I<source> I<target> optional +I<--driver driver> I<--subdriver subdriver> I<--type type> +I<--mode mode> I<--persistent> I<--sourcetype soucetype> Attach a new disk device to the domain. I<source> and I<target> are paths for the files and devices. @@ -654,8 +656,12 @@ I<type> can indicate I<cdrom> or I<floppy> as alternative to the disk default, although this use only replaces the media within the existing virtual cdrom or floppy device; consider using B<update-device> for this usage instead. I<mode> can specify the two specific mode I<readonly> or I<shareable>. +I<persistent> indicates the changes will affect the next boot of the domain. +I<sourcetype> can indicate the type of source (block|file) -=item B<attach-interface> I<domain-id> I<type> I<source> optional I<--target target> I<--mac mac> I<--script script> I<--model model> I<--persistent> +=item B<attach-interface> I<domain-id> I<type> I<source> optional +I<--target target> I<--mac mac> I<--script script> I<--model model> +I<--persistent> Attach a new network interface to the domain. I<type> can be either I<network> to indicate a physical network device or I<bridge> to indicate a bridge to a device. -- 1.7.3.2

2 1

[libvirt] Fix a failure to restore SELinux label for character devices
by Daniel Veillard 18 Nov '10

18 Nov '10

The code in SELinuxRestoreSecurityChardevLabel() was trying to use SELinuxSetFilecon directly for devices or file types while it should really use SELinuxRestoreSecurityFileLabel encapsulating routine, which avoid various problems like resolving symlinks, making sure he file exists and work around NFS problems Daniel diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 936a1a6..996177a 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -694,9 +694,10 @@ SELinuxRestoreSecurityChardevLabel(virDomainObjPtr vm, switch (dev->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: - ret = SELinuxSetFilecon(dev->data.file.path, secdef->imagelabel); + if (SELinuxRestoreSecurityFileLabel(dev->data.file.path) < 0) + goto done; + ret = 0; break; - case VIR_DOMAIN_CHR_TYPE_PIPE: if ((virAsprintf(&out, "%s.out", dev->data.file.path) < 0) || (virAsprintf(&in, "%s.in", dev->data.file.path) < 0)) { -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

3 3

[libvirt] [PATCH 0/2] Support the QED disk image format
by Adam Litke 18 Nov '10

18 Nov '10

Qemu is about to gain support for a new disk image format: QED. Details on this format (including specification) can be found here: http://wiki.qemu.org/Features/QED. This short series of patches allows QED images to be used with libvirt. Adam Litke (2): QED: Basic support for QED images Support for probing qed image metadata src/util/storage_file.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++- src/util/storage_file.h | 1 + 2 files changed, 85 insertions(+), 1 deletions(-) -- 1.7.3.2.164.g6f10c

2 4

[libvirt] Interesting "Mobile apps" food for thought
by Justin Clift 18 Nov '10

18 Nov '10

Just came across this. There's a Linux distribution based off of Centos, called ClearOS, that's targeted at a router/gateway/vpn/etc. They've designed their own API for managing things (nice looking site), and people are creating nifty looking visual mobile apps for it. http://www.clearfoundation.com/docs/developer/api/start Here's screenshot of an Android app, monitoring bandwidth on a remote host (worth looking at): http://www.clearfoundation.com/docs/_detail/developer/api/android_monitor02… This is the kind of thing I can see people making virt apps for, as we get our doc coverage done properly, mobile friendly API's well documented, and our Community really built. :) Regards and best wishes, Justin Clift

2 2

[libvirt] [PATCH] virsh: tell if domain is updated in dominfo
by Osier Yang 18 Nov '10

18 Nov '10

* tools/virsh.c (introduce new information 'Updated' for 'dominfo', standsfor if a domain is updated via operations like 'update-device', 'update-disk', etc.) --- tools/virsh.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index ae88cc0..d260443 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1992,6 +1992,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) virSecurityModel secmodel; virSecurityLabel seclabel; int persistent = 0; + int updated = -1; int ret = TRUE, autostart; unsigned int id; char *str, uuid[VIR_UUID_STRING_BUFLEN]; @@ -2053,6 +2054,12 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) else vshPrint(ctl, "%-15s %s\n", _("Persistent:"), persistent ? _("yes") : _("no")); + updated = virDomainIsUpdated(dom); + if (updated < 0) + vshPrint(ctl, "%-15s %s\n", _("Updated:"), _("unknown")); + else + vshPrint(ctl, "%-15s %s\n", _("Updated:"), updated ? _("yes") : _("no")); + /* Check and display whether the domain autostarts or not */ if (!virDomainGetAutostart(dom, &autostart)) { vshPrint(ctl, "%-15s %s\n", _("Autostart:"), -- 1.7.3.2

2 2

[libvirt] polkit auth issue
by Jim Fehlig 18 Nov '10

18 Nov '10

I'm trying to debug a PolicyKit auth issue in libvirt and looking for some suggestions. Server has the following policy for user ski52 in /etc/PolicyKit/PolicyKit.conf : <match action="org.libvirt.unix.manage"> <match user="ski52"> <return result="auth_self_keep_always"/> </match> </match> I can authenticate via polkit when logged directly into server via ssh as ski52 ski52@vhost52:~> virsh -c qemu:///system list Attempting to obtain authorization for org.libvirt.unix.manage. Authentication is required. Password: Successfully obtained the authorization for org.libvirt.unix.manage. Id Name State ---------------------------------- 33 vm1 running But when using qemu+ssh remotely ski53@vhost53:~> virsh -c qemu+ssh://ski52@vhost52/system list Attempting to obtain authorization for org.libvirt.unix.manage. Authentication as an administrative user is required. Password: polkit-grant-helper-pam: pam_authenticated failed: Authentication failure Failed to obtain authorization for org.libvirt.unix.manage. error: authentication failed error: failed to connect to the hypervisor AFAICT by tracing with gdb, the client calls polkit-auth *locally* when authentication is needed, instead of invoking polkit-auth on the server. This backtrace from gdb on the client machine shows 'polkit-auth --obtain' being called locally from virConnectAuthGainPolkit() #0 virConnectAuthGainPolkit (privilege=0x7ffff7b8b3ba "org.libvirt.unix.manage") at libvirt.c:111 #1 0x00007ffff7a912a3 in virConnectAuthCallbackDefault (cred=0x7fffffffdd20, ncred=1, cbdata=0x0) at libvirt.c:149 #2 0x00007ffff7ac367f in remoteAuthPolkit (conn=0x63ec10, priv=0x7ffff7e25010, in_open=1, auth=0x7ffff7dc9bc0) at remote/remote_driver.c:7431 #3 0x00007ffff7ac1d8d in remoteAuthenticate (conn=0x63ec10, priv=0x7ffff7e25010, in_open=1, auth=0x7ffff7dc9bc0, authtype=0x0) at remote/remote_driver.c:6864 #4 0x00007ffff7ab5936 in doRemoteOpen (conn=0x63ec10, priv=0x7ffff7e25010, auth=0x7ffff7dc9bc0, flags=0) at remote/remote_driver.c:854 .. Has anyone else observed such behavior? Any hints on how to forward the polkit-auth call to the server? Both client and server are libvirt 0.8.5 btw. Regards, Jim

2 1

[libvirt] VMware Workstation/Player support
by Jean-Baptiste Rouault 17 Nov '10

17 Nov '10

Hi all, I'm working on the hynesim project (http://www.hynesim.org http://blog.hynesim.org) We are going to add VMware Workstation support to hynesim, so we would like to contribute to the development of a libvirt driver. There probably are multiple ways of doing this, one of them could be to use the VIX C API provided by VMware : http://www.vmware.com/support/developer/vix-api/ However, the VIX license seems to be quite restrictive : http://www.vmware.com/download/eula/vixapi_eula.html I'm not a license expert so I don't know if this license forbids using the API in software like libvirt. Another possibility could be to run VMware command line tools from libvirt to control guests. As Daniel P. Berrange stated on IRC yesterday, the vmx <-> XML conversion code can be reused from the ESX driver. I'd like to have your opinion about the "best" to implement this VMware Workstation driver. Regards, Jean-Baptiste Rouault

4 7

[libvirt] [PATCH] virt-aa-helper: Fix several compile errors
by Matthias Bolte 17 Nov '10

17 Nov '10

Include locale.h for setlocale(). Revert the usage string back to it's original form. Use puts() instead of fputs(), as fputs() expects a FILE*. Add closing parenthesis to some vah_error() calls. Use argv[0] instead of an undefined argv0. --- src/security/virt-aa-helper.c | 39 ++++++++++++++++++++------------------- 1 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 12adbbb..41bc598 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -26,6 +26,7 @@ #include <getopt.h> #include <stdbool.h> #include <sys/utsname.h> +#include <locale.h> #include "internal.h" #include "buf.h" @@ -84,21 +85,21 @@ vahDeinit(vahControl * ctl) static void vah_usage(void) { - fprintf(stdout, _("\n%s [options] [< def.xml]\n\n - Options:\n - -a | --add load profile\n - -c | --create create profile from template\n - -D | --delete unload and delete profile\n - -f | --add-file <file> add file to profile\n - -F | --append-file <file> append file to profile\n - -r | --replace reload profile\n - -R | --remove unload profile\n - -h | --help this help\n - -u | --uuid <uuid> uuid (profile name)\n -\n", progname); - - fputs(_("This command is intended to be used by libvirtd " - "and not used directly.\n")); + printf(_("\n%s [options] [< def.xml]\n\n" + " Options:\n" + " -a | --add load profile\n" + " -c | --create create profile from template\n" + " -D | --delete unload and delete profile\n" + " -f | --add-file <file> add file to profile\n" + " -F | --append-file <file> append file to profile\n" + " -r | --replace reload profile\n" + " -R | --remove unload profile\n" + " -h | --help this help\n" + " -u | --uuid <uuid> uuid (profile name)\n" + "\n"), progname); + + puts(_("This command is intended to be used by libvirtd " + "and not used directly.\n")); return; } @@ -289,7 +290,7 @@ update_include_file(const char *include_file, const char *included_files, } if (VIR_CLOSE(fd) != 0) { - vah_error(NULL, 0, _("failed to close or write to profile")_; + vah_error(NULL, 0, _("failed to close or write to profile")); goto clean; } rc = 0; @@ -396,7 +397,7 @@ create_profile(const char *profile, const char *profile_name, } if (VIR_CLOSE(fd) != 0) { - vah_error(NULL, 0, _("failed to close or write to profile")_; + vah_error(NULL, 0, _("failed to close or write to profile")); goto clean_all; } rc = 0; @@ -1061,7 +1062,7 @@ vahParseArgv(vahControl * ctl, int argc, char **argv) case 'f': case 'F': if ((ctl->newfile = strdup(optarg)) == NULL) - vah_error(ctl, 1, _("could not allocate memory for disk")_; + vah_error(ctl, 1, _("could not allocate memory for disk")); ctl->append = arg == 'F'; break; case 'h': @@ -1140,7 +1141,7 @@ main(int argc, char **argv) if (setlocale(LC_ALL, "") == NULL || bindtextdomain(PACKAGE, LOCALEDIR) == NULL || textdomain(PACKAGE) == NULL) { - fprintf(stderr, _("%s: initialization failed\n"), argv0); + fprintf(stderr, _("%s: initialization failed\n"), argv[0]); exit(EXIT_FAILURE); } -- 1.7.0.4

2 2

[libvirt] [PATCH 0/6] [RFC] Add disk streaming support to libvirt
by Adam Litke 17 Nov '10

17 Nov '10

( Those on Cc: sorry for any duplicated messages. git was configured to use the wrong smtp relay ) I've been working with Anthony Liguori and Stefan Hajnoczi to enable data streaming to copy-on-read disk images in qemu. This work is working its way through review and I expect it to be upstream soon as part of the support for the new QED disk image format. Disk streaming is extremely useful when provisioning domains from a central repository of template images. Currently the domain must be provisioned by either: 1) copying the template image to local storage before the VM can be started or, 2) creating a qcow2 image that backs to a base image in the remote repository. Option 1 can introduce a significant delay when provisioning large disks. Option 2 introduces a permanent dependency on a remote service, increased network load to satisfy disk reads. Device streaming provides the "instant-on" benefits of option 2 without introducing a permanent dependency to the image repository. Once the VM is started, the contents of the disk can be streamed to the local image in parallel. Once streaming is finished, the domain has a complete and coherent copy of the image and no longer depends on the central image repository. Qemu will support two streaming modes: full device and single sector. Full device streaming is the easiest to use because one command will cause the whole device to be streamed as fast as possible. Single sector mode can be used if one wants to throttle streaming to reduce I/O pressure. In this mode, a management tool issues individual commands to stream single sectors. To enable this support in libvirt, I propose the following API... virDomainStreamDisk() will start or stop a full device stream or stream a single sector of a device. The behavior is controlled by setting virDomainStreamDiskFlags. When either starting or stopping a full device stream, the return value is either 0 or -1 to indicate whether the operation succeeded. For a single sector stream, a device offset is returned (or -1 on failure). This value can be used to continue streaming with a subsequent call to virDomainStreamDisk(). virDomainStreamDiskInfo() returns information about active full device streams (the device alias, current streaming position, and total size). While streaming can be used to enable live block migration, this API addresses the direct streaming use case. If we can agree that the above use case is compelling on its own, I would prefer to work on the block migration workflow immediately after this series. In that case, we just need to be careful not to paint ourselves into a corner with respect to migration, but specifics about that feature can be discussed later. Does this seem reasonable? Adam Litke (6): Add new API virDomainStreamDisk[Info] to header and drivers virDomainStreamDisk: Add public symbols to libvirt API Implement disk streaming in the qemu driver Add disk streaming support to the remote driver Add new disk streaming commands to virsh python: Add python bindings for virDomainStreamDisk[Info] daemon/remote.c | 96 +++++++++++++++++++++ daemon/remote_dispatch_args.h | 2 + daemon/remote_dispatch_prototypes.h | 16 ++++ daemon/remote_dispatch_ret.h | 2 + daemon/remote_dispatch_table.h | 10 ++ include/libvirt/libvirt.h.in | 34 ++++++++ python/generator.py | 1 + python/libvirt-override-api.xml | 5 + python/libvirt-override.c | 46 ++++++++++ src/driver.h | 11 +++ src/esx/esx_driver.c | 2 + src/libvirt.c | 121 +++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ src/lxc/lxc_driver.c | 2 + src/opennebula/one_driver.c | 2 + src/openvz/openvz_driver.c | 2 + src/phyp/phyp_driver.c | 2 + src/qemu/qemu_driver.c | 75 +++++++++++++++++ src/qemu/qemu_monitor.c | 41 +++++++++ src/qemu/qemu_monitor.h | 6 ++ src/qemu/qemu_monitor_json.c | 104 +++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 7 ++ src/qemu/qemu_monitor_text.c | 156 ++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 8 ++ src/remote/remote_driver.c | 85 +++++++++++++++++++ src/remote/remote_protocol.c | 63 ++++++++++++++ src/remote/remote_protocol.h | 51 +++++++++++ src/remote/remote_protocol.x | 37 ++++++++- src/test/test_driver.c | 2 + src/uml/uml_driver.c | 2 + src/vbox/vbox_tmpl.c | 2 + src/xen/xen_driver.c | 2 + tools/virsh.c | 157 +++++++++++++++++++++++++++++++++++ 33 files changed, 1157 insertions(+), 1 deletions(-) -- 1.7.3.2.164.g6f10c

1 6

[libvirt] [PATCH] build: fix autobuild failures on gcov upgrade
by Eric Blake 17 Nov '10

17 Nov '10

* autobuild.sh: Avoid syntax error on failed test. * tools/Makefile.am (CLEANFILES): Clean coverage files. --- Pushing under the build-breaker rule. Last time I ran ./autobuild.sh was on F13; and upgrading to F14 exposed these leftovers due to a newer gcov than what was in the stale files, in the form of spurious messages that break 'make check': +profiling:/home/remote/eblake/libvirt-tmp/tools/virsh-console.gcda:Version mismatch - expected 405R got 404R and concluding with a bug in the autobuild.sh script itself: ./autobuild.sh: line 44: test: =: unary operator expected autobuild.sh | 2 +- tools/Makefile.am | 1 + 2 files changed, 2 insertions(+), 1 deletions(-) diff --git a/autobuild.sh b/autobuild.sh index 130f72d..8248a59 100755 --- a/autobuild.sh +++ b/autobuild.sh @@ -41,7 +41,7 @@ st=$( { make check syntax-check 2>&1; echo $? >&4; } | tee "$RESULTS" ) exec 3>&- -test $st = 0 +test "$st" = 0 test -x /usr/bin/lcov && make cov rm -f *.tar.gz diff --git a/tools/Makefile.am b/tools/Makefile.am index 376ffa8..8a5fb52 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -160,5 +160,6 @@ endif # LIBVIRT_INIT_SCRIPT_RED_HAT CLEANFILES = $(bin_SCRIPTS) $(man1_MANS) +CLEANFILES += *.gcov .libs/*.gcda .libs/*.gcno *.gcno *.gcda *.i *.s DISTCLEANFILES = $(BUILT_SOURCES) -- 1.7.3.2

1 0

[libvirt] [PATCH] docs: added compiling page and significantly expanded windows page
by Justin Clift 17 Nov '10

17 Nov '10

Also added an additional menu placement for the windows page, in order to attract further potential testers. --- docs/compiling.html.in | 48 ++++++++++++++++++++++++ docs/downloads.html.in | 41 +-------------------- docs/sitemap.html.in | 10 +++++ docs/windows.html.in | 95 +++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 142 insertions(+), 52 deletions(-) create mode 100644 docs/compiling.html.in diff --git a/docs/compiling.html.in b/docs/compiling.html.in new file mode 100644 index 0000000..471f52d --- /dev/null +++ b/docs/compiling.html.in @@ -0,0 +1,48 @@ +<?xml version="1.0"?> +<html> + <body> + <h1><a name="installation">libvirt Installation</a></h1> + + <ul id="toc"></ul> + + <h2><a name="Compilatio">Compiling a release tarball</a></h2> + + <p> + libvirt uses the standard configure/make/install steps: + </p> + + <pre> + gunzip -c libvirt-xxx.tar.gz | tar xvf - + cd libvirt-xxxx + ./configure --help</pre> + + <p> + To see the options, then the compilation/installation proper: + </p> + + <pre> + ./configure [possible options] + make + make install</pre> + + <p> + At that point you may have to rerun ldconfig or a similar utility to + update your list of installed shared libs. + </p> + + <h2><a name="build">Building from a GIT checkout</a></h2> + + <p> + The libvirt build process uses GNU autotools, so after obtaining a + checkout it is necessary to generate the configure script and Makefile.in + templates using the <code>autogen.sh</code> command, passing the extra + arguments as for configure. As an example, to do a complete build and + install it into your home directory run: + </p> + + <pre> + ./autogen.sh --prefix=$HOME/usr --enable-compile-warnings=error + make + make install</pre> + </body> +</html> diff --git a/docs/downloads.html.in b/docs/downloads.html.in index 709bee8..64a16c9 100644 --- a/docs/downloads.html.in +++ b/docs/downloads.html.in @@ -91,46 +91,9 @@ <br /> - <h1><a name="installation">libvirt Installation</a></h1> - - <h2><a name="Compilatio">Compiling a release tarball</a></h2> - - <p> - libvirt uses the standard configure/make/install steps: - </p> - - <pre> - gunzip -c libvirt-xxx.tar.gz | tar xvf - - cd libvirt-xxxx - ./configure --help</pre> - - <p> - To see the options, then the compilation/installation proper: - </p> - - <pre> - ./configure [possible options] - make - make install</pre> - <p> - At that point you may have to rerun ldconfig or a similar utility to - update your list of installed shared libs. + Once you've have obtained the libvirt source code, you can compile it + using the <a href="compiling.html">instructions here</a>. </p> - - <h2><a name="build">Building from a GIT checkout</a></h2> - - <p> - The libvirt build process uses GNU autotools, so after obtaining a - checkout it is necessary to generate the configure script and Makefile.in - templates using the <code>autogen.sh</code> command, passing the extra - arguments as for configure. As an example, to do a complete build and - install it into your home directory run: - </p> - - <pre> - ./autogen.sh --prefix=$HOME/usr --enable-compile-warnings=error - make - make install</pre> </body> </html> diff --git a/docs/sitemap.html.in b/docs/sitemap.html.in index 692da29..bd39e10 100644 --- a/docs/sitemap.html.in +++ b/docs/sitemap.html.in @@ -21,12 +21,22 @@ <li> <a href="downloads.html">Downloads</a> <span>Get the latest source releases, binary builds and get access to the source repository</span> + <ul> + <li> + <a href="windows.html">Windows</a> + <span>Downloads for Windows</span> + </li> + </ul> </li> <li> <a href="docs.html">Documentation</a> <span>Information for users, administrators and developers</span> <ul> <li> + <a href="compiling.html">Compiling</a> + <span>How to compile libvirt</span> + </li> + <li> <a href="deployment.html">Deployment</a> <span>Information about deploying and using libvirt</span> <ul> diff --git a/docs/windows.html.in b/docs/windows.html.in index 8ca6b0d..f973d76 100644 --- a/docs/windows.html.in +++ b/docs/windows.html.in @@ -3,20 +3,93 @@ <body> <h1 >Windows support</h1> + <ul id="toc"></ul> + + <p> + Libvirt is known to work as a client (not server) on Windows XP + (32-bit), Windows 7 (64-bit), and other Windows variants. + </p> + + <h2><a name="installer">Experimental installation package</a></h2> + + <p> + A windows installation package is in development. An experimental + first version is available here: + </p> + + <a href="http://libvirt.org/sources/win32_experimental/Libvirt-0.8.5-0.exe">http://libvirt.org/sources/win32_experimental/Libvirt-0.8.5-0.exe</a> + <p> - Libvirt can be compiled on Windows - using the free <a href="http://www.mingw.org/">MinGW compiler</a>. - You can also cross-compile to a Windows target - from a Fedora machine using the packages available - <a href="http://hg.et.redhat.com/misc/fedora-mingw--devel/">from - the Fedora MinGW project</a> + <b>It is not production ready.</b> + </p> + + <p> + This version includes the libvirt development headers and libraries + for compiling against, the virsh shell with it's needed dependencies, + and untested Python bindings. + </p> + + <h3><a name="caveats">Caveats</h3> + + <ul> + <li> + This installer just repackages the files compiled using Matthias + Bolte's msys_setup scripting (described below). + </li> + <li> + This is a .exe installer, created using NSIS. We're looking into + something to create .msi installers as well. + </li> + <li> + The working connection types at the moment are very limited. Only + <b>qemu+tcp://</b> is known to work for sure. Anything using SSH, + such as <b>qemu+ssh://</b>, definitely doesn't work. Connecting + to ESX servers doesn't yet work either, due to a bug involving + GnuTLS, which should be fixed in the next release. + </li> + <li> + The script for the NSIS installer is available online + <a href="https://github.com/justinclift/nsis_libvirt_installer">here</a>. + </li> + </ul> + + <h3><a name="feedback">Feedback</h3> + + <p> + Feedback and suggestions on changes to make and what else to include + <a href="contact.html">are desired</a>. + + <h2><a name="compiling">Compiling yourself</a></h2> + + <p> + Libvirt can be compiled on Windows using the free + <a href="http://www.mingw.org/">MinGW compiler</a>. + </p> + + <h3><a name="msys_setup">MSYS Build script</a></h3> + + <p> + The easiest way is to use the <b>msys_setup</b> script, developed by + Matthias Bolte. This is actively developed and kept current with + libvirt releases: + </p> + + <a href="https://github.com/photron/msys_setup">https://github.com/photron/msys_setup</a> + + <h3><a name="cross-compile">Cross compiling</a></h3> + + <p> + You can also cross-compile to a Windows target from a Fedora machine + using the packages available + <a href="http://hg.et.redhat.com/misc/fedora-mingw--devel/">from the Fedora MinGW project</a> (which includes a working libvirt specfile). </p> + <h3><a name="configure">By hand</a></h3> + <p> - Libvirt can only be built as a client on Windows - allowing remote access to systems libvirt servers. - Configure libvirt like this: + Use these options when following the instructions on the + <a href="compiling.html">Compiling</a> page. </p> <pre> @@ -32,9 +105,5 @@ --without-libvirtd </pre> - <p> - We intend to supply Windows binaries on this page - later, but at the moment you have to compile from source. - </p> </body> </html> -- 1.7.3

4 9

[libvirt] [PATCH] maint: use gnulib configmake rather than open-coding things
by Eric Blake 17 Nov '10

17 Nov '10

In some cases, we were just plain confusing (BINDIR expanded to $(libexecdir), GETTEXT_PACKAGE redefined PACKAGE for no apparant reason); in other cases, we might as well benefit from gnulib doing the work for us (PKGDATADIR), sometimes with different spellings (LOCAL_STATE_DIR vs. LOCALSTATEDIR). * bootstrap.conf (gnulib_modules): Add configmake. * daemon/Makefile.am (libvirtd_CFLAGS): Drop defines provided by gnulib. * src/Makefile.am (INCLUDES): Likewise. * tests/Makefile.am (INCLUDES): Likewise. * tools/Makefile.am (virsh_CFLAGS): Likewise. * daemon/libvirtd.c (qemudInitPaths, usage, main): Update clients. * src/cpu/cpu_map.c (CPUMAPFILE): Likewise. * src/driver.c (DEFAULT_DRIVER_DIR): Likewise. * src/internal.h (_): Likewise. * src/libvirt.c (virInitialize): Likewise. * src/lxc/lxc_conf.h (LXC_CONFIG_DIR, LXC_STATE_DIR, LXC_LOG_DIR): Likewise. * src/lxc/lxc_conf.c (lxcCapsInit, lxcLoadDriverConfig): Likewise. * src/network/bridge_driver.c (NETWORK_PID_DIR) (NETWORK_STATE_DIR, DNSMASQ_STATE_DIR, networkStartup): Likewise. * src/nwfilter/nwfilter_driver.c (nwfilterDriverStartup): Likewise. * src/qemu/qemu_conf.c (qemudLoadDriverConfig): Likewise. * src/qemu/qemu_driver.c (qemudStartup): Likewise. * src/remote/remote_driver.h (LIBVIRTD_PRIV_UNIX_SOCKET) (LIBVIRTD_PRIV_UNIX_SOCKET_RO, LIBVIRTD_CONFIGURATION_FILE) (LIBVIRT_PKI_DIR): Likewise. * src/secret/secret_driver.c (secretDriverStartup): Likewise. * src/security/security_apparmor.c (VIRT_AA_HELPER): Likewise. * src/security/virt-aa-helper.c (main): Likewise. * src/storage/storage_backend_disk.c (PARTHELPER): Likewise. * src/storage/storage_driver.c (storageDriverStartup): Likewise. * src/uml/uml_driver.c (TEMPDIR, umlStartup): Likewise. * src/util/hooks.c (LIBVIRT_HOOK_DIR): Likewise. * tools/virsh.c (main): Likewise. * docs/hooks.html.in: Likewise. --- bootstrap.conf | 1 + daemon/Makefile.am | 6 ++---- daemon/libvirtd.c | 14 ++++++++------ docs/hooks.html.in | 2 +- src/Makefile.am | 8 -------- src/cpu/cpu_map.c | 4 ++-- src/driver.c | 3 ++- src/internal.h | 2 +- src/libvirt.c | 3 ++- src/lxc/lxc_conf.c | 7 ++++--- src/lxc/lxc_conf.h | 7 ++++--- src/network/bridge_driver.c | 11 ++++++----- src/nwfilter/nwfilter_driver.c | 6 +++--- src/qemu/qemu_conf.c | 6 ++++-- src/qemu/qemu_driver.c | 16 ++++++++-------- src/remote/remote_driver.h | 12 +++++++----- src/secret/secret_driver.c | 3 ++- src/security/security_apparmor.c | 5 +++-- src/security/virt-aa-helper.c | 7 ++++--- src/storage/storage_backend_disk.c | 3 ++- src/storage/storage_driver.c | 5 +++-- src/uml/uml_driver.c | 9 +++++---- src/util/hooks.c | 3 ++- tests/Makefile.am | 1 - tools/Makefile.am | 2 -- tools/virsh.c | 5 +++-- 26 files changed, 79 insertions(+), 72 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 12f64c8..2ad1957 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -25,6 +25,7 @@ c-ctype canonicalize-lgpl close connect +configmake count-one-bits crypto/md5 dirname-lgpl diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 8808226..72778e5 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -85,12 +85,10 @@ libvirtd_CFLAGS = \ -I$(top_srcdir)/src/remote \ $(LIBXML_CFLAGS) $(GNUTLS_CFLAGS) $(SASL_CFLAGS) \ $(POLKIT_CFLAGS) \ - $(WARN_CFLAGS) -DLOCAL_STATE_DIR="\"$(localstatedir)\"" \ + $(WARN_CFLAGS) \ $(COVERAGE_CFLAGS) \ - -DSYSCONF_DIR="\"$(sysconfdir)\"" \ -DQEMUD_PID_FILE="\"$(QEMUD_PID_FILE)\"" \ - -DREMOTE_PID_FILE="\"$(REMOTE_PID_FILE)\"" \ - -DGETTEXT_PACKAGE=\"$(PACKAGE)\" + -DREMOTE_PID_FILE="\"$(REMOTE_PID_FILE)\"" libvirtd_LDFLAGS = \ $(WARN_CFLAGS) \ diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 9446638..aab7667 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -137,6 +137,8 @@ static int unix_sock_ro_mask = 0777; /* Allow world */ #endif /* __sun */ +#include "configmake.h" + static int godaemon = 0; /* -d: Be a daemon */ static int verbose = 0; /* -v: Verbose mode */ static int timeout = -1; /* -t: Shutdown timeout */ @@ -745,7 +747,7 @@ static int qemudInitPaths(struct qemud_server *server, /* The base_dir_prefix is the base under which all libvirtd * files live */ if (server->privileged) { - if (!(base_dir_prefix = strdup (LOCAL_STATE_DIR))) + if (!(base_dir_prefix = strdup (LOCALSTATEDIR))) goto no_memory; } else { uid_t uid = geteuid(); @@ -3038,9 +3040,9 @@ libvirt management daemon:\n\ %s\n\ \n"), argv0, - SYSCONF_DIR, - LOCAL_STATE_DIR, - LOCAL_STATE_DIR, + SYSCONFDIR, + LOCALSTATEDIR, + LOCALSTATEDIR, LIBVIRT_CACERT, LIBVIRT_SERVERCERT, LIBVIRT_SERVERKEY, @@ -3137,7 +3139,7 @@ int main(int argc, char **argv) { if (remote_config_file == NULL) { static const char *default_config_file - = SYSCONF_DIR "/libvirt/libvirtd.conf"; + = SYSCONFDIR "/libvirt/libvirtd.conf"; remote_config_file = (access(default_config_file, R_OK) == 0 ? default_config_file @@ -3169,7 +3171,7 @@ int main(int argc, char **argv) { /* Ensure the rundir exists (on tmpfs on some systems) */ if (geteuid() == 0) { - const char *rundir = LOCAL_STATE_DIR "/run/libvirt"; + const char *rundir = LOCALSTATEDIR "/run/libvirt"; if (mkdir (rundir, 0755)) { if (errno != EEXIST) { diff --git a/docs/hooks.html.in b/docs/hooks.html.in index 2ce785f..3503f8c 100644 --- a/docs/hooks.html.in +++ b/docs/hooks.html.in @@ -19,7 +19,7 @@ <h2><a name="location">Script location</a></h2> <p>The libvirt hook scripts are located in the directory - <code>$SYSCONF_DIR/libvirt/hooks/</code>.</p> + <code>$SYSCONFDIR/libvirt/hooks/</code>.</p> <ul> <li>In Linux distributions such as Fedora and RHEL, this is <code>/etc/libvirt/hooks/</code>. Other Linux distributions may do diff --git a/src/Makefile.am b/src/Makefile.am index 20c0c9f..a9a1986 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -11,14 +11,6 @@ INCLUDES = \ -I@top_srcdir@/include \ $(DRIVER_MODULE_CFLAGS) \ $(LIBXML_CFLAGS) \ - -DLIBDIR=\""$(libdir)"\" \ - -DBINDIR=\""$(libexecdir)"\" \ - -DSBINDIR=\""$(sbindir)"\" \ - -DSYSCONF_DIR="\"$(sysconfdir)\"" \ - -DLOCALEBASEDIR=\""$(datadir)/locale"\" \ - -DPKGDATADIR=\""$(pkgdatadir)"\" \ - -DLOCAL_STATE_DIR=\""$(localstatedir)"\" \ - -DGETTEXT_PACKAGE=\"$(PACKAGE)\" \ $(WARN_CFLAGS) \ $(LOCK_CHECKING_CFLAGS) \ -DIN_LIBVIRT \ diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c index 263bb9e..0c5ff8a 100644 --- a/src/cpu/cpu_map.c +++ b/src/cpu/cpu_map.c @@ -1,7 +1,7 @@ /* * cpu_map.c: internal functions for handling CPU mapping configuration * - * Copyright (C) 2009 Red Hat, Inc. + * Copyright (C) 2009-2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -26,7 +26,7 @@ #include "memory.h" #include "cpu.h" #include "cpu_map.h" - +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_CPU diff --git a/src/driver.c b/src/driver.c index a6f5558..d83b1fd 100644 --- a/src/driver.c +++ b/src/driver.c @@ -1,7 +1,7 @@ /* * driver.c: Helpers for loading drivers * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -28,6 +28,7 @@ #include "memory.h" #include "logging.h" #include "util.h" +#include "configmake.h" #define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/drivers" diff --git a/src/internal.h b/src/internal.h index fab3e11..a98daa3 100644 --- a/src/internal.h +++ b/src/internal.h @@ -52,7 +52,7 @@ # define INET_ADDRSTRLEN 16 # endif -# define _(str) dgettext(GETTEXT_PACKAGE, (str)) +# define _(str) dgettext(PACKAGE, (str)) # define N_(str) str /* String equality tests, suggested by Jim Meyering. */ diff --git a/src/libvirt.c b/src/libvirt.c index 3c8bf30..eba0dee 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -39,6 +39,7 @@ #include "uuid.h" #include "util.h" #include "memory.h" +#include "configmake.h" #ifndef WITH_DRIVER_MODULES # ifdef WITH_TEST @@ -342,7 +343,7 @@ virInitialize(void) if (winsock_init () == -1) return -1; #endif - if (!bindtextdomain(GETTEXT_PACKAGE, LOCALEBASEDIR)) + if (!bindtextdomain(PACKAGE, LOCALEDIR)) return (-1); /* diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c index f4479e6..59d1161 100644 --- a/src/lxc/lxc_conf.c +++ b/src/lxc/lxc_conf.c @@ -1,4 +1,5 @@ /* + * Copyright (C) 2010 Red Hat, Inc. * Copyright IBM Corp. 2008 * * lxc_conf.c: config functions for managing linux containers @@ -34,7 +35,7 @@ #include "memory.h" #include "logging.h" #include "uuid.h" - +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_LXC @@ -73,7 +74,7 @@ virCapsPtr lxcCapsInit(void) "exe", utsname.machine, sizeof(int) == 4 ? 32 : 8, - BINDIR "/libvirt_lxc", + LIBEXECDIR "/libvirt_lxc", NULL, 0, NULL)) == NULL) @@ -114,7 +115,7 @@ int lxcLoadDriverConfig(lxc_driver_t *driver) goto no_memory; - if ((filename = strdup(SYSCONF_DIR "/libvirt/lxc.conf")) == NULL) + if ((filename = strdup(SYSCONFDIR "/libvirt/lxc.conf")) == NULL) goto no_memory; /* Avoid error from non-existant or unreadable file. */ diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h index e4c2c52..f820d6d 100644 --- a/src/lxc/lxc_conf.h +++ b/src/lxc/lxc_conf.h @@ -33,10 +33,11 @@ # include "capabilities.h" # include "threads.h" # include "cgroup.h" +# include "configmake.h" -# define LXC_CONFIG_DIR SYSCONF_DIR "/libvirt/lxc" -# define LXC_STATE_DIR LOCAL_STATE_DIR "/run/libvirt/lxc" -# define LXC_LOG_DIR LOCAL_STATE_DIR "/log/libvirt/lxc" +# define LXC_CONFIG_DIR SYSCONFDIR "/libvirt/lxc" +# define LXC_STATE_DIR LOCALSTATEDIR "/run/libvirt/lxc" +# define LXC_LOG_DIR LOCALSTATEDIR "/log/libvirt/lxc" # define LXC_AUTOSTART_DIR LXC_CONFIG_DIR "/autostart" typedef struct __lxc_driver lxc_driver_t; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 631fbf1..54890f9 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -58,11 +58,12 @@ #include "logging.h" #include "dnsmasq.h" #include "util/network.h" +#include "configmake.h" -#define NETWORK_PID_DIR LOCAL_STATE_DIR "/run/libvirt/network" -#define NETWORK_STATE_DIR LOCAL_STATE_DIR "/lib/libvirt/network" +#define NETWORK_PID_DIR LOCALSTATEDIR "/run/libvirt/network" +#define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network" -#define DNSMASQ_STATE_DIR LOCAL_STATE_DIR "/lib/libvirt/dnsmasq" +#define DNSMASQ_STATE_DIR LOCALSTATEDIR "/lib/libvirt/dnsmasq" #define VIR_FROM_THIS VIR_FROM_NETWORK @@ -209,10 +210,10 @@ networkStartup(int privileged) { if (privileged) { if (virAsprintf(&driverState->logDir, - "%s/log/libvirt/qemu", LOCAL_STATE_DIR) == -1) + "%s/log/libvirt/qemu", LOCALSTATEDIR) == -1) goto out_of_memory; - if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) + if ((base = strdup (SYSCONFDIR "/libvirt")) == NULL) goto out_of_memory; } else { char *userdir = virGetUserDirectory(uid); diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 4efeb3a..a305de6 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -2,7 +2,7 @@ * nwfilter_driver.c: core driver for network filter APIs * (based on storage_driver.c) * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * Copyright (C) 2010 IBM Corporation * Copyright (C) 2010 Stefan Berger @@ -37,7 +37,7 @@ #include "nwfilter_conf.h" #include "nwfilter_driver.h" #include "nwfilter_gentech_driver.h" - +#include "configmake.h" #include "nwfilter_learnipaddr.h" @@ -83,7 +83,7 @@ nwfilterDriverStartup(int privileged) { nwfilterDriverLock(driverState); if (privileged) { - if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) + if ((base = strdup (SYSCONFDIR "/libvirt")) == NULL) goto out_of_memory; } else { uid_t uid = geteuid(); diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 83a117a..35caccc 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -56,6 +56,7 @@ #include "cpu/cpu.h" #include "domain_nwfilter.h" #include "files.h" +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -112,7 +113,7 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, virReportOOMError(); return -1; } - if (!(driver->vncTLSx509certdir = strdup(SYSCONF_DIR "/pki/libvirt-vnc"))) { + if (!(driver->vncTLSx509certdir = strdup(SYSCONFDIR "/pki/libvirt-vnc"))) { virReportOOMError(); return -1; } @@ -121,7 +122,8 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, virReportOOMError(); return -1; } - if (!(driver->spiceTLSx509certdir = strdup(SYSCONF_DIR "/pki/libvirt-spice"))) { + if (!(driver->spiceTLSx509certdir + = strdup(SYSCONFDIR "/pki/libvirt-spice"))) { virReportOOMError(); return -1; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 772fd50..f3b158a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -84,7 +84,7 @@ #include "virtaudit.h" #include "files.h" #include "fdstream.h" - +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1762,28 +1762,28 @@ qemudStartup(int privileged) { if (privileged) { if (virAsprintf(&qemu_driver->logDir, - "%s/log/libvirt/qemu", LOCAL_STATE_DIR) == -1) + "%s/log/libvirt/qemu", LOCALSTATEDIR) == -1) goto out_of_memory; - if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) + if ((base = strdup (SYSCONFDIR "/libvirt")) == NULL) goto out_of_memory; if (virAsprintf(&qemu_driver->stateDir, - "%s/run/libvirt/qemu", LOCAL_STATE_DIR) == -1) + "%s/run/libvirt/qemu", LOCALSTATEDIR) == -1) goto out_of_memory; if (virAsprintf(&qemu_driver->libDir, - "%s/lib/libvirt/qemu", LOCAL_STATE_DIR) == -1) + "%s/lib/libvirt/qemu", LOCALSTATEDIR) == -1) goto out_of_memory; if (virAsprintf(&qemu_driver->cacheDir, - "%s/cache/libvirt/qemu", LOCAL_STATE_DIR) == -1) + "%s/cache/libvirt/qemu", LOCALSTATEDIR) == -1) goto out_of_memory; if (virAsprintf(&qemu_driver->saveDir, - "%s/lib/libvirt/qemu/save", LOCAL_STATE_DIR) == -1) + "%s/lib/libvirt/qemu/save", LOCALSTATEDIR) == -1) goto out_of_memory; if (virAsprintf(&qemu_driver->snapshotDir, - "%s/lib/libvirt/qemu/snapshot", LOCAL_STATE_DIR) == -1) + "%s/lib/libvirt/qemu/snapshot", LOCALSTATEDIR) == -1) goto out_of_memory; } else { uid_t uid = geteuid(); diff --git a/src/remote/remote_driver.h b/src/remote/remote_driver.h index 49a63bd..1504eec 100644 --- a/src/remote/remote_driver.h +++ b/src/remote/remote_driver.h @@ -2,7 +2,7 @@ * remote_internal.h: driver to provide access to libvirtd running * on a remote machine * - * Copyright (C) 2006-2007 Red Hat, Inc. + * Copyright (C) 2006-2007, 2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -26,6 +26,8 @@ # include "libvirt/virterror.h" +# include "configmake.h" + int remoteRegister (void); unsigned long remoteVersion(void); @@ -33,13 +35,13 @@ unsigned long remoteVersion(void); # define LIBVIRTD_LISTEN_ADDR NULL # define LIBVIRTD_TLS_PORT "16514" # define LIBVIRTD_TCP_PORT "16509" -# define LIBVIRTD_PRIV_UNIX_SOCKET LOCAL_STATE_DIR "/run/libvirt/libvirt-sock" -# define LIBVIRTD_PRIV_UNIX_SOCKET_RO LOCAL_STATE_DIR "/run/libvirt/libvirt-sock-ro" +# define LIBVIRTD_PRIV_UNIX_SOCKET LOCALSTATEDIR "/run/libvirt/libvirt-sock" +# define LIBVIRTD_PRIV_UNIX_SOCKET_RO LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro" # define LIBVIRTD_USER_UNIX_SOCKET "/.libvirt/libvirt-sock" -# define LIBVIRTD_CONFIGURATION_FILE SYSCONF_DIR "/libvirtd.conf" +# define LIBVIRTD_CONFIGURATION_FILE SYSCONFDIR "/libvirtd.conf" /* Defaults for PKI directory. */ -# define LIBVIRT_PKI_DIR SYSCONF_DIR "/pki" +# define LIBVIRT_PKI_DIR SYSCONFDIR "/pki" # define LIBVIRT_CACERT LIBVIRT_PKI_DIR "/CA/cacert.pem" # define LIBVIRT_CLIENTKEY LIBVIRT_PKI_DIR "/libvirt/private/clientkey.pem" # define LIBVIRT_CLIENTCERT LIBVIRT_PKI_DIR "/libvirt/clientcert.pem" diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 2b5a7c6..c5a876b 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -42,6 +42,7 @@ #include "uuid.h" #include "virterror_internal.h" #include "files.h" +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_SECRET @@ -994,7 +995,7 @@ secretDriverStartup(int privileged) secretDriverLock(driverState); if (privileged) { - base = strdup(SYSCONF_DIR "/libvirt"); + base = strdup(SYSCONFDIR "/libvirt"); if (base == NULL) goto out_of_memory; } else { diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index be39f75..b43c4ac 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -38,11 +38,12 @@ #include "pci.h" #include "hostusb.h" #include "files.h" +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_SECURITY #define SECURITY_APPARMOR_VOID_DOI "0" #define SECURITY_APPARMOR_NAME "apparmor" -#define VIRT_AA_HELPER BINDIR "/virt-aa-helper" +#define VIRT_AA_HELPER LIBEXECDIR "/virt-aa-helper" /* Data structure to pass to *FileIterate so we have everything we need */ struct SDPDOP { @@ -561,7 +562,7 @@ AppArmorRestoreSecurityAllLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, } /* Called via virExecWithHook. Output goes to - * LOCAL_STATE_DIR/log/libvirt/qemu/<vm name>.log + * LOCALSTATEDIR/log/libvirt/qemu/<vm name>.log */ static int AppArmorSetSecurityProcessLabel(virSecurityDriverPtr drv, virDomainObjPtr vm) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 5708cd8..0f94fe4 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -38,6 +38,7 @@ #include "hostusb.h" #include "pci.h" #include "files.h" +#include "configmake.h" static char *progname; @@ -1179,11 +1180,11 @@ main(int argc, char **argv) goto clean; } else { virBufferVSprintf(&buf, " \"%s/log/libvirt/**/%s.log\" w,\n", - LOCAL_STATE_DIR, ctl->def->name); + LOCALSTATEDIR, ctl->def->name); virBufferVSprintf(&buf, " \"%s/lib/libvirt/**/%s.monitor\" rw,\n", - LOCAL_STATE_DIR, ctl->def->name); + LOCALSTATEDIR, ctl->def->name); virBufferVSprintf(&buf, " \"%s/run/libvirt/**/%s.pid\" rwk,\n", - LOCAL_STATE_DIR, ctl->def->name); + LOCALSTATEDIR, ctl->def->name); if (ctl->files) virBufferVSprintf(&buf, "%s", ctl->files); } diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index a8864dd..c7ade6b 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -31,10 +31,11 @@ #include "storage_backend_disk.h" #include "util.h" #include "memory.h" +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_STORAGE -#define PARTHELPER BINDIR "/libvirt_parthelper" +#define PARTHELPER LIBEXECDIR "/libvirt_parthelper" #define SECTOR_SIZE 512 diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index f6672d9..6df706b 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1,7 +1,7 @@ /* * storage_driver.c: core driver for storage APIs * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -46,6 +46,7 @@ #include "storage_backend.h" #include "logging.h" #include "files.h" +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -125,7 +126,7 @@ storageDriverStartup(int privileged) { storageDriverLock(driverState); if (privileged) { - if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) + if ((base = strdup (SYSCONFDIR "/libvirt")) == NULL) goto out_of_memory; } else { uid_t uid = geteuid(); diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 3588894..77610d4 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -61,11 +61,12 @@ #include "domain_nwfilter.h" #include "files.h" #include "fdstream.h" +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_UML /* For storing short-lived temporary files. */ -#define TEMPDIR LOCAL_STATE_DIR "/cache/libvirt" +#define TEMPDIR LOCALSTATEDIR "/cache/libvirt" typedef struct _umlDomainObjPrivate umlDomainObjPrivate; typedef umlDomainObjPrivate *umlDomainObjPrivatePtr; @@ -371,14 +372,14 @@ umlStartup(int privileged) { if (privileged) { if (virAsprintf(&uml_driver->logDir, - "%s/log/libvirt/uml", LOCAL_STATE_DIR) == -1) + "%s/log/libvirt/uml", LOCALSTATEDIR) == -1) goto out_of_memory; - if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) + if ((base = strdup (SYSCONFDIR "/libvirt")) == NULL) goto out_of_memory; if (virAsprintf(&uml_driver->monitorDir, - "%s/run/libvirt/uml-guest", LOCAL_STATE_DIR) == -1) + "%s/run/libvirt/uml-guest", LOCALSTATEDIR) == -1) goto out_of_memory; } else { diff --git a/src/util/hooks.c b/src/util/hooks.c index 6e4e2b9..8e24564 100644 --- a/src/util/hooks.c +++ b/src/util/hooks.c @@ -37,6 +37,7 @@ #include "logging.h" #include "memory.h" #include "files.h" +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_HOOK @@ -44,7 +45,7 @@ virReportErrorHelper(NULL, VIR_FROM_HOOK, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) -#define LIBVIRT_HOOK_DIR SYSCONF_DIR "/libvirt/hooks" +#define LIBVIRT_HOOK_DIR SYSCONFDIR "/libvirt/hooks" VIR_ENUM_DECL(virHookDriver) VIR_ENUM_DECL(virHookDaemonOp) diff --git a/tests/Makefile.am b/tests/Makefile.am index 20b6805..77b6fb9 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -17,7 +17,6 @@ INCLUDES = \ $(SASL_CFLAGS) \ $(SELINUX_CFLAGS) \ $(APPARMOR_CFLAGS) \ - -DGETTEXT_PACKAGE=\"$(PACKAGE)\" \ $(COVERAGE_CFLAGS) \ $(WARN_CFLAGS) diff --git a/tools/Makefile.am b/tools/Makefile.am index 921d621..376ffa8 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -55,8 +55,6 @@ virsh_CFLAGS = \ -I$(top_srcdir)/src \ -I$(top_srcdir)/src/util \ -I$(top_srcdir) \ - -DGETTEXT_PACKAGE=\"$(PACKAGE)\" \ - -DLOCALEBASEDIR=\""$(datadir)/locale"\" \ $(WARN_CFLAGS) \ $(COVERAGE_CFLAGS) \ $(LIBXML_CFLAGS) \ diff --git a/tools/virsh.c b/tools/virsh.c index 3a74053..743d5a1 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -53,6 +53,7 @@ #include "libvirt/libvirt-qemu.h" #include "files.h" #include "../daemon/event.h" +#include "configmake.h" static char *progname; @@ -11786,11 +11787,11 @@ main(int argc, char **argv) perror("setlocale"); /* failure to setup locale is not fatal */ } - if (!bindtextdomain(GETTEXT_PACKAGE, LOCALEBASEDIR)) { + if (!bindtextdomain(PACKAGE, LOCALEDIR)) { perror("bindtextdomain"); return -1; } - if (!textdomain(GETTEXT_PACKAGE)) { + if (!textdomain(PACKAGE)) { perror("textdomain"); return -1; } -- 1.7.3.2

2 3

[libvirt] [PATCH] doc: update virsh manual
by Osier Yang 17 Nov '10

17 Nov '10

* tools/virsh.pod (change things like "edit domain.xml" into "vi domain.xml", so that it's more clear for user) --- tools/virsh.pod | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 8032256..ca8ba74 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -306,7 +306,7 @@ L<http://libvirt.org/formatdomain.html#elementsCPU> The following commands manipulate domains directly, as stated previously most commands take domain-id as the first parameter. The -I<domain-id> can be specified as an short integer, a name or a full UUID. +I<domain-id> can be specified as a short integer, a name or a full UUID. =over 4 @@ -334,7 +334,7 @@ If I<--console> is requested, attach to the console after creation. B<Example> virsh dumpxml <domain-id> > domain.xml - edit domain.xml + vi domain.xml (or make changes with your other text editor) virsh create < domain.xml =item B<define> I<FILE> @@ -431,7 +431,7 @@ Edit the XML configuration file for a domain. This is equivalent to: virsh dumpxml domain > domain.xml - edit domain.xml + vi domain.xml (or make changes with your other text editor) virsh define domain.xml except that it does some error checking. @@ -631,7 +631,7 @@ is not available the processes will provide an exit code of 1. =head1 DEVICE COMMANDS The following commands manipulate devices associated to domains. -The domain-id can be specified as an short integer, a name or a full UUID. +The domain-id can be specified as a short integer, a name or a full UUID. To better understand the values allowed as options for the command reading the documentation at L<http://libvirt.org/formatdomain.html> on the format of the device sections to get the most accurate set of accepted values. @@ -735,7 +735,7 @@ Edit the XML configuration file for a network. This is equivalent to: virsh net-dumpxml network > network.xml - edit network.xml + vi network.xml (or make changes with your other text editor) virsh net-define network.xml except that it does some error checking. @@ -851,7 +851,7 @@ Edit the XML configuration file for a storage pool. This is equivalent to: virsh pool-dumpxml pool > pool.xml - edit pool.xml + vi pool.xml (or make changes with your other text editor) virsh pool-define pool.xml except that it does some error checking. @@ -908,7 +908,7 @@ pre-existing volume. B<Example> virsh vol-dumpxml --pool storagepool1 appvolume1 > newvolume.xml - edit newvolume.xml + vi newvolume.xml (or make changes with your other text editor) virsh vol-create differentstoragepool newvolume.xml =item B<vol-create-from> I<pool-or-uuid> I<FILE> [optional I<--inputpool> @@ -1138,7 +1138,7 @@ Edit the XML of a network filter. This is equivalent to: virsh nwfilter-dumpxml myfilter > myfilter.xml - edit myfilter.xml + vi myfilter.xml (or make changes with your other text editor) virsh nwfilter-define myfilter.xml except that it does some error checking. -- 1.7.3.2

2 1

[libvirt] [PATCH 0/3] improve i18n
by Eric Blake 17 Nov '10

17 Nov '10

This patch series depends on these two previous patches (not reviewed yet): https://www.redhat.com/archives/libvir-list/2010-November/msg00733.html https://www.redhat.com/archives/libvir-list/2010-November/msg00626.html It also replaces my earlier patch here: https://www.redhat.com/archives/libvir-list/2010-September/msg00583.html It also makes it possible to boostrap on FreeBSD, as a side-effect of updating bootstrap. Eric Blake (3): maint: improve i18n on non-Linux maint: update to latest gnulib virt-aa-helper: translate error messages .gitignore | 65 +++++++------ .gnulib | 2 +- .x-sc_bindtextdomain | 2 + Makefile.am | 1 + bootstrap | 44 ++++++--- build-aux/.gitignore | 16 --- cfg.mk | 2 + daemon/libvirtd.c | 10 ++- m4/.gitignore | 37 ------- po/.gitignore | 17 ---- po/POTFILES.in | 1 + src/internal.h | 11 ++- src/lxc/lxc_controller.c | 9 ++ src/security/security_apparmor.c | 1 + src/security/virt-aa-helper.c | 194 ++++++++++++++++++++------------------ src/storage/parthelper.c | 15 +++- tools/virsh.c | 4 +- 17 files changed, 215 insertions(+), 216 deletions(-) create mode 100644 .x-sc_bindtextdomain delete mode 100644 build-aux/.gitignore delete mode 100644 m4/.gitignore delete mode 100644 po/.gitignore -- 1.7.3.2

2 7

[libvirt] [PATCH] maint: avoid remaining sprintf uses
by Eric Blake 17 Nov '10

17 Nov '10

* cfg.mk (sc_prohibit_sprintf): New rule. (sc_prohibit_asprintf): Avoid false positives. * docs/hacking.html.in (Printf-style functions): Document the policy. * .x-sc_prohibit_sprintf: New exemptions. * src/vbox/vbox_tmpl.c (vboxStartMachine, vboxAttachUSB): Use virAsprintf instead. * src/uml/uml_driver.c (umlOpenMonitor): Use snprintf instead. * tools/virsh.c (cmdDetachInterface): Likewise. * src/security/security_selinux.c (SELinuxGenSecurityLabel): Likewise. * src/openvz/openvz_driver.c (openvzDomainDefineCmd): Likewise, and ensure large enough buffer. --- .x-sc_prohibit_sprintf | 3 +++ cfg.mk | 13 ++++++++++--- docs/hacking.html.in | 9 +++++++++ src/openvz/openvz_driver.c | 5 +++-- src/security/security_selinux.c | 6 +++--- src/uml/uml_driver.c | 3 ++- tools/virsh.c | 2 +- 7 files changed, 31 insertions(+), 10 deletions(-) create mode 100644 .x-sc_prohibit_sprintf diff --git a/.x-sc_prohibit_sprintf b/.x-sc_prohibit_sprintf new file mode 100644 index 0000000..0a1f448 --- /dev/null +++ b/.x-sc_prohibit_sprintf @@ -0,0 +1,3 @@ +^docs/ +^po/ +ChangeLog diff --git a/cfg.mk b/cfg.mk index 16c2ae3..01cada8 100644 --- a/cfg.mk +++ b/cfg.mk @@ -238,10 +238,17 @@ sc_prohibit_strcmp_and_strncmp: halt='use STREQ() in place of the above uses of str[n]cmp' \ $(_sc_search_regexp) -# Use virAsprintf rather than a'sprintf since *strp is undefined on error. +# Use virAsprintf rather than as'printf since *strp is undefined on error. sc_prohibit_asprintf: - @prohibit='\<[a]sprintf\>' \ - halt='use virAsprintf, not a'sprintf \ + @prohibit='\<a[s]printf\>' \ + halt='use virAsprintf, not as'printf \ + $(_sc_search_regexp) + +# Use snprintf rather than s'printf, even if buffer is provably large enough, +# since gnulib has more guarantees for snprintf portability +sc_prohibit_sprintf: + @prohibit='\<[s]printf\>' \ + halt='use snprintf, not s'printf \ $(_sc_search_regexp) sc_prohibit_strncpy: diff --git a/docs/hacking.html.in b/docs/hacking.html.in index bd8b443..a79250e 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -602,6 +602,15 @@ of arguments. </p> + <p> + When printing to a string, consider using virBuffer for + incremental allocations, virAsprintf for a one-shot allocation, + and snprintf for fixed-width buffers. Do not use sprintf, even + if you can prove the buffer won't overflow, since gnulib does + not provide the same portability guarantees for sprintf as it + does for snprintf. + </p> + <h2><a name="goto">Use of goto</a></h2> <p> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 2893f69..f799691 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -58,6 +58,7 @@ #include "memory.h" #include "bridge.h" #include "files.h" +#include "intprops.h" #define VIR_FROM_THIS VIR_FROM_OPENVZ @@ -104,7 +105,7 @@ openvzDomainDefineCmd(const char *args[], int narg; int veid; int max_veid; - char str_id[10]; + char str_id[INT_BUFSIZE_BOUND(max_veid)]; FILE *fp; for (narg = 0; narg < maxarg; narg++) @@ -162,7 +163,7 @@ openvzDomainDefineCmd(const char *args[], max_veid++; } - sprintf(str_id, "%d", max_veid); + snprintf(str_id, sizeof(str_id), "%d", max_veid); ADD_ARG_LIT(str_id); ADD_ARG_LIT("--name"); diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 7dd9b14..5e0e7bb 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -182,12 +182,12 @@ SELinuxGenSecurityLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, c2 = virRandom(1024); if ( c1 == c2 ) { - sprintf(mcs, "s0:c%d", c1); + snprintf(mcs, sizeof(mcs), "s0:c%d", c1); } else { if ( c1 < c2 ) - sprintf(mcs, "s0:c%d,c%d", c1, c2); + snprintf(mcs, sizeof(mcs), "s0:c%d,c%d", c1, c2); else - sprintf(mcs, "s0:c%d,c%d", c2, c1); + snprintf(mcs, sizeof(mcs), "s0:c%d,c%d", c2, c1); } } while(mcsAdd(mcs) == -1); diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 348f299..5b2a553 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -655,7 +655,8 @@ restat: } memset(addr.sun_path, 0, sizeof addr.sun_path); - sprintf(addr.sun_path + 1, "libvirt-uml-%u", vm->pid); + snprintf(addr.sun_path + 1, sizeof(addr.sun_path) - 1, + "libvirt-uml-%u", vm->pid); VIR_DEBUG("Reply address for monitor is '%s'", addr.sun_path+1); if (bind(priv->monitor, (struct sockaddr *)&addr, sizeof addr) < 0) { virReportSystemError(errno, diff --git a/tools/virsh.c b/tools/virsh.c index cd20d34..053aee7 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -8450,7 +8450,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - sprintf(buf, "/domain/devices/interface[@type='%s']", type); + snprintf(buf, sizeof(buf), "/domain/devices/interface[@type='%s']", type); obj = xmlXPathEval(BAD_CAST buf, ctxt); if ((obj == NULL) || (obj->type != XPATH_NODESET) || (obj->nodesetval == NULL) || (obj->nodesetval->nodeNr == 0)) { -- 1.7.3.2

2 4

[libvirt] [PATCH v2] replace last instances of close()
by Stefan Berger 17 Nov '10

17 Nov '10

v2: - added comments about the monitor lock being used to protect the file descriptor 'fd' - eofcb is not set anywhere, so also removing it; the if statement checking on it seems dead code I am replacing the last instances of close() I found with VIR_CLOSE() / VIR_FORCE_CLOSE respectively. The first part patches virsh, which I missed out on previously. The 2nd patch I had left out intentionally to look at it more carefully: The 'closed' variable could be easily removed since it wasn't used anywhere else. The possible race condition that could result from the filedescriptor being closed and not set to -1 (and possibly let us write into 'something' totally different if the fd was allocated by another thread) seems to be prevented by the qemuMonitorLock() already placed around the code that reads from or writes to the fd. So the change of this code as shown in the patch should not have any side-effects. Signed-off-by: Stefan Berger<stefanb(a)us.ibm.com> --- src/qemu/qemu_monitor.c | 32 +++++++++++--------------------- tools/virsh.c | 4 ++-- 2 files changed, 13 insertions(+), 23 deletions(-) Index: libvirt-acl/tools/virsh.c =================================================================== --- libvirt-acl.orig/tools/virsh.c +++ libvirt-acl/tools/virsh.c @@ -8994,12 +8994,12 @@ editWriteToTempFile (vshControl *ctl, co if (safewrite (fd, doc, strlen (doc)) == -1) { vshError(ctl, _("write: %s: failed to write to temporary file: %s"), ret, strerror(errno)); - close (fd); + VIR_FORCE_CLOSE(fd); unlink (ret); VIR_FREE(ret); return NULL; } - if (close (fd) == -1) { + if (VIR_CLOSE(fd)< 0) { vshError(ctl, _("close: %s: failed to write or close temporary file: %s"), ret, strerror(errno)); unlink (ret); Index: libvirt-acl/src/qemu/qemu_monitor.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_monitor.c +++ libvirt-acl/src/qemu/qemu_monitor.c @@ -44,7 +44,7 @@ #define DEBUG_RAW_IO 0 struct _qemuMonitor { - virMutex lock; + virMutex lock; /* also used to protect fd */ virCond notify; int refs; @@ -71,11 +71,6 @@ struct _qemuMonitor { * the next monitor msg */ int lastErrno; - /* If the monitor EOF callback is currently active (stops more commands being run) */ - unsigned eofcb: 1; - /* If the monitor is in process of shutting down */ - unsigned closed: 1; - unsigned json: 1; }; @@ -356,6 +351,7 @@ qemuMonitorIOProcess(qemuMonitorPtr mon) } +/* Call this function while holding the monitor lock. */ static int qemuMonitorIOWriteWithFD(qemuMonitorPtr mon, const char *data, @@ -400,7 +396,10 @@ qemuMonitorIOWriteWithFD(qemuMonitorPtr return ret; } -/* Called when the monitor is able to write data */ +/* + * Called when the monitor is able to write data + * Call this function while holding the monitor lock. + */ static int qemuMonitorIOWrite(qemuMonitorPtr mon) { @@ -433,6 +432,7 @@ qemuMonitorIOWrite(qemuMonitorPtr mon) /* * Called when the monitor has incoming data to read + * Call this function while holding the monitor lock. * * Returns -1 on error, or number of bytes read */ @@ -505,6 +505,7 @@ qemuMonitorIO(int watch, int fd, int eve qemuMonitorPtr mon = opaque; int quit = 0, failed = 0; + /* lock access to the monitor and protect fd */ qemuMonitorLock(mon); qemuMonitorRef(mon); #if DEBUG_IO @@ -692,17 +693,11 @@ void qemuMonitorClose(qemuMonitorPtr mon VIR_DEBUG("mon=%p", mon); qemuMonitorLock(mon); - if (!mon->closed) { + + if (mon->fd>= 0) { if (mon->watch) virEventRemoveHandle(mon->watch); - if (mon->fd != -1) - close(mon->fd); - /* NB: ordinarily one might immediately set mon->watch to -1 - * and mon->fd to -1, but there may be a callback active - * that is still relying on these fields being valid. So - * we merely close them, but not clear their values and - * use this explicit 'closed' flag to track this state */ - mon->closed = 1; + VIR_FORCE_CLOSE(mon->fd); } if (qemuMonitorUnref(mon)> 0) @@ -715,11 +710,6 @@ int qemuMonitorSend(qemuMonitorPtr mon, { int ret = -1; - if (mon->eofcb) { - msg->lastErrno = EIO; - return -1; - } - mon->msg = msg; qemuMonitorUpdateWatch(mon);

2 2

[libvirt] net-info doc for "virsh commands references"
by Osier Yang 17 Nov '10

17 Nov '10

Hi Justin It's a rough doc, I'm not good at it. :-) any problem please let me known. Regards - Osier

2 2

[libvirt] [PATCH] [RESEND] To list basic information about the network.
by Osier Yang 17 Nov '10

17 Nov '10

* tools/virsh.c --- tools/virsh.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 59 insertions(+), 0 deletions(-) mode change 100644 => 100755 tools/virsh.c diff --git a/tools/virsh.c b/tools/virsh.c old mode 100644 new mode 100755 index d15a8df..3a74053 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3678,6 +3678,64 @@ cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd) return ret; } +/* + * "net-info" command + */ +static const vshCmdInfo info_network_info[] = { + {"help", N_("network information")}, + {"desc", "Returns basic information about the network"}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_network_info[] = { + {"network", VSH_OT_DATA, VSH_OFLAG_REQ, N_("network name")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdNetworkInfo(vshControl *ctl, const vshCmd *cmd) +{ + virNetworkPtr network; + char uuid[VIR_UUID_STRING_BUFLEN]; + int autostart; + int persistent = -1; + int active = -1; + char *bridge = NULL; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return FALSE; + + if (!(network = vshCommandOptNetworkBy(ctl, cmd, NULL, + VSH_BYNAME))) + return FALSE; + + vshPrint(ctl, "%-15s %s\n", _("Name"), virNetworkGetName(network)); + + if (virNetworkGetUUIDString(network, uuid) == 0) + vshPrint(ctl, "%-15s %s\n", _("UUID"), uuid); + + active = virNetworkIsActive(network); + if (active >= 0) + vshPrint(ctl, "%-15s %s\n", _("Active:"), active? _("yes") : _("no")); + + persistent = virNetworkIsPersistent(network); + if (persistent < 0) + vshPrint(ctl, "%-15s %s\n", _("Persistent:"), _("unknown")); + else + vshPrint(ctl, "%-15s %s\n", _("Persistent:"), persistent ? _("yes") : _("no")); + + if (virNetworkGetAutostart(network, &autostart) < 0) + vshPrint(ctl, "%-15s %s\n", _("Autostart:"), _("no autostart")); + else + vshPrint(ctl, "%-15s %s\n", _("Autostart:"), autostart ? _("yes") : _("no")); + + bridge = virNetworkGetBridgeName(network); + if (bridge) + vshPrint(ctl, "%-15s %s\n", _("Bridge:"), bridge); + + virNetworkFree(network); + return TRUE; +} /* * "iface-edit" command @@ -9879,6 +9937,7 @@ static const vshCmdDef commands[] = { {"net-destroy", cmdNetworkDestroy, opts_network_destroy, info_network_destroy}, {"net-dumpxml", cmdNetworkDumpXML, opts_network_dumpxml, info_network_dumpxml}, {"net-edit", cmdNetworkEdit, opts_network_edit, info_network_edit}, + {"net-info", cmdNetworkInfo, opts_network_info, info_network_info}, {"net-list", cmdNetworkList, opts_network_list, info_network_list}, {"net-name", cmdNetworkName, opts_network_name, info_network_name}, {"net-start", cmdNetworkStart, opts_network_start, info_network_start}, -- 1.7.3.2

4 5

[libvirt] [PATCH v3] deprecate fclose() and introduce VIR_{FORCE_}FCLOSE()
by Stefan Berger 17 Nov '10

17 Nov '10

V3: - fixes from V2 review + one lost hunk -> diff(tree+V2, tree+v3) at bottom of email V2: - following Eric's suggestion, deprecating fdclose(), introducing VIR_FDCLOSE() - following some other nits that Eric pointed out - replaced some more fclose () I previously had missed (last 2 files in patch) Similarly to deprecating close(), I am now deprecating fclose() and introduce VIR_FORCE_FCLOSE() and VIR_FCLOSE(). Also, fdopen() is replaced with VIR_FDOPEN(). Most of the files are opened in read-only mode, so usage of VIR_FORCE_CLOSE() seemed appropriate. Others that are opened in write mode already had the fclose()< 0 check and I converted those to VIR_FCLOSE()< 0. I did not find occurrences of possible double-closed files on the way. Signed-off-by: Stefan Berger<stefanb(a)us.ibm.com> --- HACKING | 33 +++++++++++++++++++++++++++------ daemon/libvirtd.c | 6 +++--- docs/hacking.html.in | 36 ++++++++++++++++++++++++++++-------- src/libvirt_private.syms | 2 ++ src/nodeinfo.c | 8 ++++---- src/openvz/openvz_conf.c | 16 +++++++++------- src/openvz/openvz_driver.c | 4 ++-- src/qemu/qemu_driver.c | 4 ++-- src/storage/storage_backend.c | 18 +++++++----------- src/storage/storage_backend_fs.c | 4 ++-- src/storage/storage_backend_iscsi.c | 9 +++------ src/storage/storage_backend_scsi.c | 2 +- src/uml/uml_driver.c | 8 ++++---- src/util/cgroup.c | 10 +++++----- src/util/dnsmasq.c | 5 +++-- src/util/files.c | 34 ++++++++++++++++++++++++++++++++++ src/util/files.h | 10 +++++++++- src/util/macvtap.c | 4 ++-- src/util/stats_linux.c | 5 +++-- src/util/util.c | 10 ++++------ src/xen/block_stats.c | 3 ++- src/xen/xen_driver.c | 7 ++++--- src/xen/xen_hypervisor.c | 8 +++----- tests/nodeinfotest.c | 5 +++-- tests/testutils.c | 8 ++++---- tests/xencapstest.c | 7 +++---- 26 files changed, 173 insertions(+), 93 deletions(-) Index: libvirt-acl/src/util/files.c =================================================================== --- libvirt-acl.orig/src/util/files.c +++ libvirt-acl/src/util/files.c @@ -44,3 +44,37 @@ int virClose(int *fdptr, bool preserve_e return rc; } + + +int virFclose(FILE **file, bool preserve_errno) +{ + int saved_errno; + int rc = 0; + + if (*file) { + if (preserve_errno) + saved_errno = errno; + rc = fclose(*file); + *file = NULL; + if (preserve_errno) + errno = saved_errno; + } + + return rc; +} + + +FILE *virFdopen(int *fdptr, const char *mode) +{ + FILE *file = NULL; + + if (*fdptr>= 0) { + file = fdopen(*fdptr, mode); + if (file) + *fdptr = -1; + } else { + errno = EBADF; + } + + return file; +} Index: libvirt-acl/src/util/files.h =================================================================== --- libvirt-acl.orig/src/util/files.h +++ libvirt-acl/src/util/files.h @@ -27,20 +27,28 @@ # define __VIR_FILES_H_ # include<stdbool.h> +# include<stdio.h> # include "internal.h" # include "ignore-value.h" -/* Don't call this directly - use the macros below */ +/* Don't call these directly - use the macros below */ int virClose(int *fdptr, bool preserve_errno) ATTRIBUTE_RETURN_CHECK; +int virFclose(FILE **file, bool preserve_errno) ATTRIBUTE_RETURN_CHECK; +FILE *virFdopen(int *fdptr, const char *mode) ATTRIBUTE_RETURN_CHECK; /* For use on normal paths; caller must check return value, and failure sets errno per close(). */ # define VIR_CLOSE(FD) virClose(&(FD), false) +# define VIR_FCLOSE(FILE) virFclose(&(FILE), false) + +/* Wrapper around fdopen that consumes fd on success. */ +# define VIR_FDOPEN(FD, MODE) virFdopen(&(FD), MODE) /* For use on cleanup paths; errno is unaffected by close, and no return value to worry about. */ # define VIR_FORCE_CLOSE(FD) ignore_value(virClose(&(FD), true)) +# define VIR_FORCE_FCLOSE(FILE) ignore_value(virFclose(&(FILE), true)) #endif /* __VIR_FILES_H */ Index: libvirt-acl/src/libvirt_private.syms =================================================================== --- libvirt-acl.orig/src/libvirt_private.syms +++ libvirt-acl/src/libvirt_private.syms @@ -344,6 +344,8 @@ virFDStreamCreateFile; # files.h virClose; +virFclose; +virFdopen; # hash.h Index: libvirt-acl/daemon/libvirtd.c =================================================================== --- libvirt-acl.orig/daemon/libvirtd.c +++ libvirt-acl/daemon/libvirtd.c @@ -512,7 +512,7 @@ static int qemudWritePidFile(const char return -1; } - if (!(fh = fdopen(fd, "w"))) { + if (!(fh = VIR_FDOPEN(fd, "w"))) { VIR_ERROR(_("Failed to fdopen pid file '%s' : %s"), pidFile, virStrerror(errno, ebuf, sizeof ebuf)); VIR_FORCE_CLOSE(fd); @@ -522,11 +522,11 @@ static int qemudWritePidFile(const char if (fprintf(fh, "%lu\n", (unsigned long)getpid())< 0) { VIR_ERROR(_("%s: Failed to write to pid file '%s' : %s"), argv0, pidFile, virStrerror(errno, ebuf, sizeof ebuf)); - fclose(fh); + VIR_FORCE_FCLOSE(fh); return -1; } - if (fclose(fh) == EOF) { + if (VIR_FCLOSE(fh) == EOF) { VIR_ERROR(_("%s: Failed to close pid file '%s' : %s"), argv0, pidFile, virStrerror(errno, ebuf, sizeof ebuf)); return -1; Index: libvirt-acl/src/nodeinfo.c =================================================================== --- libvirt-acl.orig/src/nodeinfo.c +++ libvirt-acl/src/nodeinfo.c @@ -46,6 +46,7 @@ #include "virterror_internal.h" #include "count-one-bits.h" #include "intprops.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -102,8 +103,7 @@ get_cpu_value(unsigned int cpu, const ch } cleanup: - if (pathfp) - fclose(pathfp); + VIR_FORCE_FCLOSE(pathfp); VIR_FREE(path); return value; @@ -155,7 +155,7 @@ static unsigned long count_thread_siblin } cleanup: - fclose(pathfp); + VIR_FORCE_FCLOSE(pathfp); VIR_FREE(path); return ret; @@ -329,7 +329,7 @@ int nodeGetInfo(virConnectPtr conn ATTRI return -1; } ret = linuxNodeInfoCPUPopulate(cpuinfo, nodeinfo); - fclose(cpuinfo); + VIR_FORCE_FCLOSE(cpuinfo); if (ret< 0) return -1; Index: libvirt-acl/src/openvz/openvz_conf.c =================================================================== --- libvirt-acl.orig/src/openvz/openvz_conf.c +++ libvirt-acl/src/openvz/openvz_conf.c @@ -528,7 +528,7 @@ int openvzLoadDomains(struct openvz_driv dom = NULL; } - fclose(fp); + VIR_FORCE_FCLOSE(fp); return 0; @@ -536,7 +536,7 @@ int openvzLoadDomains(struct openvz_driv virReportOOMError(); cleanup: - fclose(fp); + VIR_FORCE_FCLOSE(fp); if (dom) virDomainObjUnref(dom); return -1; @@ -888,6 +888,7 @@ openvzSetDefinedUUID(int vpsid, unsigned { char *conf_file; char uuidstr[VIR_UUID_STRING_BUFLEN]; + FILE *fp = NULL; int ret = -1; if (uuid == NULL) @@ -900,21 +901,22 @@ openvzSetDefinedUUID(int vpsid, unsigned goto cleanup; if (uuidstr[0] == 0) { - FILE *fp = fopen(conf_file, "a"); /* append */ + fp = fopen(conf_file, "a"); /* append */ if (fp == NULL) goto cleanup; virUUIDFormat(uuid, uuidstr); - /* Record failure if fprintf or fclose fails, + /* Record failure if fprintf or VIR_FCLOSE fails, and be careful always to close the stream. */ - if ((fprintf(fp, "\n#UUID: %s\n", uuidstr)< 0) - + (fclose(fp) == EOF)) + if ((fprintf(fp, "\n#UUID: %s\n", uuidstr)< 0) || + (VIR_FCLOSE(fp) == EOF)) goto cleanup; } ret = 0; cleanup: + VIR_FORCE_FCLOSE(fp); VIR_FREE(conf_file); return ret; } @@ -996,7 +998,7 @@ int openvzGetVEID(const char *name) { } ok = fscanf(fp, "%d\n",&veid ) == 1; - fclose(fp); + VIR_FORCE_FCLOSE(fp); if (ok&& veid>= 0) return veid; Index: libvirt-acl/src/openvz/openvz_driver.c =================================================================== --- libvirt-acl.orig/src/openvz/openvz_driver.c +++ libvirt-acl/src/openvz/openvz_driver.c @@ -154,7 +154,7 @@ openvzDomainDefineCmd(const char *args[] max_veid = veid; } } - fclose(fp); + VIR_FORCE_FCLOSE(fp); if (max_veid == 0) { max_veid = 100; @@ -189,7 +189,7 @@ no_memory: return -1; cleanup: - fclose(fp); + VIR_FORCE_FCLOSE(fp); return -1; #undef ADD_ARG Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -4588,7 +4588,7 @@ static int qemudGetProcessInfo(unsigned /* startstack -> processor */ "%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d", &usertime,&systime,&cpu) != 3) { - fclose(pidinfo); + VIR_FORCE_FCLOSE(pidinfo); VIR_WARN0("cannot parse process status data"); errno = -EINVAL; return -1; @@ -4608,7 +4608,7 @@ static int qemudGetProcessInfo(unsigned VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d", pid, tid, usertime, systime, cpu); - fclose(pidinfo); + VIR_FORCE_FCLOSE(pidinfo); return 0; } Index: libvirt-acl/src/storage/storage_backend.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend.c +++ libvirt-acl/src/storage/storage_backend.c @@ -1398,7 +1398,7 @@ virStorageBackendRunProgRegex(virStorage goto cleanup; } - if ((list = fdopen(fd, "r")) == NULL) { + if ((list = VIR_FDOPEN(fd, "r")) == NULL) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot read fd")); goto cleanup; @@ -1458,10 +1458,8 @@ virStorageBackendRunProgRegex(virStorage VIR_FREE(reg); - if (list) - fclose(list); - else - VIR_FORCE_CLOSE(fd); + VIR_FORCE_FCLOSE(list); + VIR_FORCE_CLOSE(fd); while ((err = waitpid(child,&exitstatus, 0) == -1)&& errno == EINTR); @@ -1531,9 +1529,9 @@ virStorageBackendRunProgNul(virStoragePo goto cleanup; } - if ((fp = fdopen(fd, "r")) == NULL) { + if ((fp = VIR_FDOPEN(fd, "r")) == NULL) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot read fd")); + "%s", _("cannot open file using fd")); goto cleanup; } @@ -1573,10 +1571,8 @@ virStorageBackendRunProgNul(virStoragePo VIR_FREE(v[i]); VIR_FREE(v); - if (fp) - fclose (fp); - else - VIR_FORCE_CLOSE(fd); + VIR_FORCE_FCLOSE(fp); + VIR_FORCE_CLOSE(fd); while ((w_err = waitpid (child,&exitstatus, 0) == -1)&& errno == EINTR) /* empty */ ; Index: libvirt-acl/src/storage/storage_backend_fs.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend_fs.c +++ libvirt-acl/src/storage/storage_backend_fs.c @@ -284,12 +284,12 @@ virStorageBackendFileSystemIsMounted(vir while ((getmntent_r(mtab,&ent, buf, sizeof(buf))) != NULL) { if (STREQ(ent.mnt_dir, pool->def->target.path)) { - fclose(mtab); + VIR_FORCE_FCLOSE(mtab); return 1; } } - fclose(mtab); + VIR_FORCE_FCLOSE(mtab); return 0; } Index: libvirt-acl/src/storage/storage_backend_iscsi.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend_iscsi.c +++ libvirt-acl/src/storage/storage_backend_iscsi.c @@ -188,7 +188,7 @@ virStorageBackendIQNFound(virStoragePool goto out; } - if ((fp = fdopen(fd, "r")) == NULL) { + if ((fp = VIR_FDOPEN(fd, "r")) == NULL) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to open stream for file descriptor " "when reading output from '%s': '%s'"), @@ -235,11 +235,8 @@ out: } VIR_FREE(line); - if (fp != NULL) { - fclose(fp); - } else { - VIR_FORCE_CLOSE(fd); - } + VIR_FORCE_FCLOSE(fp); + VIR_FORCE_CLOSE(fd); return ret; } Index: libvirt-acl/src/storage/storage_backend_scsi.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend_scsi.c +++ libvirt-acl/src/storage/storage_backend_scsi.c @@ -70,7 +70,7 @@ getDeviceType(uint32_t host, } gottype = fgets(typestr, 3, typefile); - fclose(typefile); + VIR_FORCE_FCLOSE(typefile); if (gottype == NULL) { virReportSystemError(errno, Index: libvirt-acl/src/uml/uml_driver.c =================================================================== --- libvirt-acl.orig/src/uml/uml_driver.c +++ libvirt-acl/src/uml/uml_driver.c @@ -587,11 +587,11 @@ reopen: if (fscanf(file, "%d",&vm->pid) != 1) { errno = EINVAL; - fclose(file); + VIR_FORCE_FCLOSE(file); goto cleanup; } - if (fclose(file)< 0) + if (VIR_FCLOSE(file)< 0) goto cleanup; rc = 0; @@ -1096,7 +1096,7 @@ static int umlGetProcessInfo(unsigned lo if (fscanf(pidinfo, "%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu %llu",&usertime,&systime) != 2) { umlDebug("not enough arg"); - fclose(pidinfo); + VIR_FORCE_FCLOSE(pidinfo); return -1; } @@ -1109,7 +1109,7 @@ static int umlGetProcessInfo(unsigned lo umlDebug("Got %llu %llu %llu", usertime, systime, *cpuTime); - fclose(pidinfo); + VIR_FORCE_FCLOSE(pidinfo); return 0; } Index: libvirt-acl/src/util/cgroup.c =================================================================== --- libvirt-acl.orig/src/util/cgroup.c +++ libvirt-acl/src/util/cgroup.c @@ -31,6 +31,7 @@ #include "memory.h" #include "cgroup.h" #include "logging.h" +#include "files.h" #define CGROUP_MAX_VAL 512 @@ -127,13 +128,12 @@ static int virCgroupDetectMounts(virCgro } } - fclose(mounts); + VIR_FORCE_FCLOSE(mounts); return 0; no_memory: - if (mounts) - fclose(mounts); + VIR_FORCE_FCLOSE(mounts); return -ENOMEM; } @@ -192,12 +192,12 @@ static int virCgroupDetectPlacement(virC } } - fclose(mapping); + VIR_FORCE_FCLOSE(mapping); return 0; no_memory: - fclose(mapping); + VIR_FORCE_FCLOSE(mapping); return -ENOMEM; } Index: libvirt-acl/src/util/dnsmasq.c =================================================================== --- libvirt-acl.orig/src/util/dnsmasq.c +++ libvirt-acl/src/util/dnsmasq.c @@ -44,6 +44,7 @@ #include "memory.h" #include "virterror_internal.h" #include "logging.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_NETWORK #define DNSMASQ_HOSTSFILE_SUFFIX "hostsfile" @@ -171,7 +172,7 @@ hostsfileWrite(const char *path, for (i = 0; i< nhosts; i++) { if (fputs(hosts[i].host, f) == EOF || fputc('\n', f) == EOF) { rc = errno; - fclose(f); + VIR_FORCE_FCLOSE(f); if (istmp) unlink(tmp); @@ -180,7 +181,7 @@ hostsfileWrite(const char *path, } } - if (fclose(f) == EOF) { + if (VIR_FCLOSE(f) == EOF) { rc = errno; goto cleanup; } Index: libvirt-acl/src/util/macvtap.c =================================================================== --- libvirt-acl.orig/src/util/macvtap.c +++ libvirt-acl/src/util/macvtap.c @@ -414,11 +414,11 @@ int openTap(const char *ifname, virReportSystemError(errno, "%s",_("cannot determine macvtap's tap device " "interface index")); - fclose(file); + VIR_FORCE_FCLOSE(file); return -1; } - fclose(file); + VIR_FORCE_FCLOSE(file); if (snprintf(tapname, sizeof(tapname), "/dev/tap%d", ifindex)>= sizeof(tapname)) { Index: libvirt-acl/src/util/util.c =================================================================== --- libvirt-acl.orig/src/util/util.c +++ libvirt-acl/src/util/util.c @@ -1816,7 +1816,7 @@ int virFileWritePidPath(const char *pidf goto cleanup; } - if (!(file = fdopen(fd, "w"))) { + if (!(file = VIR_FDOPEN(fd, "w"))) { rc = errno; VIR_FORCE_CLOSE(fd); goto cleanup; @@ -1830,10 +1830,8 @@ int virFileWritePidPath(const char *pidf rc = 0; cleanup: - if (file&& - fclose(file)< 0) { + if (VIR_FCLOSE(file)< 0) rc = errno; - } return rc; } @@ -1864,11 +1862,11 @@ int virFileReadPid(const char *dir, if (fscanf(file, "%d", pid) != 1) { rc = EINVAL; - fclose(file); + VIR_FORCE_FCLOSE(file); goto cleanup; } - if (fclose(file)< 0) { + if (VIR_FCLOSE(file)< 0) { rc = errno; goto cleanup; } Index: libvirt-acl/src/xen/xen_driver.c =================================================================== --- libvirt-acl.orig/src/xen/xen_driver.c +++ libvirt-acl/src/xen/xen_driver.c @@ -47,6 +47,7 @@ #include "pci.h" #include "uuid.h" #include "fdstream.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_XEN @@ -216,10 +217,10 @@ xenUnifiedProbe (void) return 1; #endif #ifdef __sun - FILE *fh; + int fd; - if (fh = fopen("/dev/xen/domcaps", "r")) { - fclose(fh); + if ((fd = open("/dev/xen/domcaps", O_RDONLY))>= 0) { + VIR_FORCE_CLOSE(fd); return 1; } #endif Index: libvirt-acl/src/xen/xen_hypervisor.c =================================================================== --- libvirt-acl.orig/src/xen/xen_hypervisor.c +++ libvirt-acl/src/xen/xen_hypervisor.c @@ -2641,7 +2641,7 @@ xenHypervisorMakeCapabilities(virConnect capabilities = fopen ("/sys/hypervisor/properties/capabilities", "r"); if (capabilities == NULL) { if (errno != ENOENT) { - fclose(cpuinfo); + VIR_FORCE_FCLOSE(cpuinfo); virReportSystemError(errno, _("cannot read file %s"), "/sys/hypervisor/properties/capabilities"); @@ -2654,10 +2654,8 @@ xenHypervisorMakeCapabilities(virConnect cpuinfo, capabilities); - if (cpuinfo) - fclose(cpuinfo); - if (capabilities) - fclose(capabilities); + VIR_FORCE_FCLOSE(cpuinfo); + VIR_FORCE_FCLOSE(capabilities); return caps; #endif /* __sun */ Index: libvirt-acl/tests/nodeinfotest.c =================================================================== --- libvirt-acl.orig/tests/nodeinfotest.c +++ libvirt-acl/tests/nodeinfotest.c @@ -9,6 +9,7 @@ #include "internal.h" #include "nodeinfo.h" #include "util.h" +#include "files.h" #ifndef __linux__ @@ -49,10 +50,10 @@ static int linuxTestCompareFiles(const c fprintf(stderr, "\n%s\n", error->message); virFreeError(error); } - fclose(cpuinfo); + VIR_FORCE_FCLOSE(cpuinfo); return -1; } - fclose(cpuinfo); + VIR_FORCE_FCLOSE(cpuinfo); /* 'nodes' is filled using libnuma.so from current machine * topology, which makes it unsuitable for the test suite Index: libvirt-acl/tests/testutils.c =================================================================== --- libvirt-acl.orig/tests/testutils.c +++ libvirt-acl/tests/testutils.c @@ -180,26 +180,26 @@ int virtTestLoadFile(const char *file, if (fstat(fileno(fp),&st)< 0) { fprintf (stderr, "%s: failed to fstat: %s\n", file, strerror(errno)); - fclose(fp); + VIR_FORCE_FCLOSE(fp); return -1; } if (st.st_size> (buflen-1)) { fprintf (stderr, "%s: larger than buffer (> %d)\n", file, buflen-1); - fclose(fp); + VIR_FORCE_FCLOSE(fp); return -1; } if (st.st_size) { if (fread(*buf, st.st_size, 1, fp) != 1) { fprintf (stderr, "%s: read failed: %s\n", file, strerror(errno)); - fclose(fp); + VIR_FORCE_FCLOSE(fp); return -1; } } (*buf)[st.st_size] = '\0'; - fclose(fp); + VIR_FORCE_FCLOSE(fp); return st.st_size; } Index: libvirt-acl/tests/xencapstest.c =================================================================== --- libvirt-acl.orig/tests/xencapstest.c +++ libvirt-acl/tests/xencapstest.c @@ -9,6 +9,7 @@ #include "xml.h" #include "testutils.h" #include "xen/xen_hypervisor.h" +#include "files.h" static char *progname; static char *abs_srcdir; @@ -63,10 +64,8 @@ static int testCompareFiles(const char * fail: free(actualxml); - if (fp1) - fclose(fp1); - if (fp2) - fclose(fp2); + VIR_FORCE_FCLOSE(fp1); + VIR_FORCE_FCLOSE(fp2); virCapabilitiesFree(caps); return ret; Index: libvirt-acl/docs/hacking.html.in =================================================================== --- libvirt-acl.orig/docs/hacking.html.in +++ libvirt-acl/docs/hacking.html.in @@ -414,25 +414,45 @@ <h2><a name="file_handling">File handling</a></h2> <p> - Use of the close() API is deprecated in libvirt code base to help - avoiding double-closing of a file descriptor. Instead of this API, - use the macro from files.h + Usage of the<code>fdopen()</code>,<code>close()</code>,<code>fclose()</code> + APIs is deprecated in libvirt code base to help avoiding double-closing of files + or file descriptors, which is particulary dangerous in a multi-threaded + applications. Instead of these APIs, use the macros from files.h </p> -<ul> +<ul> +<li><p>eg opening a file from a file descriptor</p> + +<pre> + if ((file = VIR_FDOPEN(fd, "r")) == NULL) { + virReportSystemError(errno, "%s", + _("failed to open file from file descriptor")); + return -1; + } + /* fd is now invalid; only access the file using file variable */ +</pre></li> + <li><p>e.g. close a file descriptor</p> <pre> if (VIR_CLOSE(fd)< 0) { - virReportSystemError(errno, _("failed to close file")); + virReportSystemError(errno, "%s", _("failed to close file")); } -</pre> -</li> +</pre></li> + +<li><p>eg close a file</p> + +<pre> + if (VIR_FCLOSE(file)< 0) { + virReportSystemError(errno, "%s", _("failed to close file")); + } +</pre></li> -<li><p>eg close a file descriptor in an error path, without losing +<li><p>eg close a file or file descriptor in an error path, without losing the previous<code>errno</code> value</p> <pre> VIR_FORCE_CLOSE(fd); + VIR_FORCE_FCLOSE(file); </pre> </li> </ul> Index: libvirt-acl/HACKING =================================================================== --- libvirt-acl.orig/HACKING +++ libvirt-acl/HACKING @@ -339,22 +339,43 @@ routines, use the macros from memory.h File handling ============= -Use of the close() API is deprecated in libvirt code base to help avoiding -double-closing of a file descriptor. Instead of this API, use the macro from -files.h +Usage of the "fdopen()", "close()", "fclose()" APIs is deprecated in libvirt +code base to help avoiding double-closing of files or file descriptors, which +is particulary dangerous in a multi-threaded applications. Instead of these +APIs, use the macros from files.h + +- eg opening a file from a file descriptor + + if ((file = VIR_FDOPEN(fd, "r")) == NULL) { + virReportSystemError(errno, "%s", + _("failed to open file from file descriptor")); + return -1; + } + /* fd is now invalid; only access the file using file variable */ + + - e.g. close a file descriptor if (VIR_CLOSE(fd)< 0) { - virReportSystemError(errno, _("failed to close file")); + virReportSystemError(errno, "%s", _("failed to close file")); + } + + + +- eg close a file + + if (VIR_FCLOSE(file)< 0) { + virReportSystemError(errno, "%s", _("failed to close file")); } -- eg close a file descriptor in an error path, without losing the previous -"errno" value +- eg close a file or file descriptor in an error path, without losing the +previous "errno" value VIR_FORCE_CLOSE(fd); + VIR_FORCE_FCLOSE(file); Index: libvirt-acl/src/util/stats_linux.c =================================================================== --- libvirt-acl.orig/src/util/stats_linux.c +++ libvirt-acl/src/util/stats_linux.c @@ -25,6 +25,7 @@ # include "util.h" # include "stats_linux.h" # include "memory.h" +# include "files.h" # define VIR_FROM_THIS VIR_FROM_STATS_LINUX @@ -98,12 +99,12 @@ linuxDomainInterfaceStats(const char *pa stats->tx_packets = tx_packets; stats->tx_errs = tx_errs; stats->tx_drop = tx_drop; - fclose (fp); + VIR_FORCE_FCLOSE (fp); return 0; } } - fclose (fp); + VIR_FORCE_FCLOSE(fp); virStatsError(VIR_ERR_INTERNAL_ERROR, "/proc/net/dev: Interface not found"); Index: libvirt-acl/src/xen/block_stats.c =================================================================== --- libvirt-acl.orig/src/xen/block_stats.c +++ libvirt-acl/src/xen/block_stats.c @@ -27,6 +27,7 @@ # include "util.h" # include "block_stats.h" # include "memory.h" +# include "files.h" # define VIR_FROM_THIS VIR_FROM_STATS_LINUX @@ -100,7 +101,7 @@ read_stat (const char *path) /* read, but don't bail out before closing */ i = fread (str, 1, sizeof str - 1, fp); - if (fclose (fp) != 0 /* disk error */ + if (VIR_FCLOSE(fp) != 0 /* disk error */ || i< 1) /* ensure we read at least one byte */ return -1; ------------------------------------------------------------- --- /root/tmp/bak/libvirt-acl/src/openvz/openvz_conf.c 2010-11-15 17:03:15.962422965 -0500 +++ src/openvz/openvz_conf.c 2010-11-16 06:31:49.716482090 -0500 @@ -887,8 +887,9 @@ openvzSetDefinedUUID(int vpsid, unsigned char *uuid) { char *conf_file; char uuidstr[VIR_UUID_STRING_BUFLEN]; + FILE *fp = NULL; int ret = -1; if (uuid == NULL) return -1; @@ -899,9 +900,9 @@ if (openvzGetVPSUUID(vpsid, uuidstr, sizeof(uuidstr))) goto cleanup; if (uuidstr[0] == 0) { - FILE *fp = fopen(conf_file, "a"); /* append */ + fp = fopen(conf_file, "a"); /* append */ if (fp == NULL) goto cleanup; virUUIDFormat(uuid, uuidstr); @@ -914,8 +915,9 @@ } ret = 0; cleanup: + VIR_FORCE_FCLOSE(fp); VIR_FREE(conf_file); return ret; } --- /root/tmp/bak/libvirt-acl/src/util/files.c 2010-11-15 17:03:15.949423495 -0500 +++ src/util/files.c 2010-11-16 06:31:49.714482170 -0500 @@ -71,8 +71,10 @@ if (*fdptr>= 0) { file = fdopen(*fdptr, mode); if (file) *fdptr = -1; + } else { + errno = EBADF; } return file; } --- /root/tmp/bak/libvirt-acl/src/util/files.h 2010-11-15 17:03:15.956423210 -0500 +++ src/util/files.h 2010-11-16 06:31:49.714482170 -0500 @@ -32,17 +32,19 @@ # include "internal.h" # include "ignore-value.h" -/* Don't call this directly - use the macros below */ +/* Don't call these directly - use the macros below */ int virClose(int *fdptr, bool preserve_errno) ATTRIBUTE_RETURN_CHECK; int virFclose(FILE **file, bool preserve_errno) ATTRIBUTE_RETURN_CHECK; -FILE *virFdopen(int *fdptr, const char *mode); +FILE *virFdopen(int *fdptr, const char *mode) ATTRIBUTE_RETURN_CHECK; /* For use on normal paths; caller must check return value, and failure sets errno per close(). */ # define VIR_CLOSE(FD) virClose(&(FD), false) # define VIR_FCLOSE(FILE) virFclose(&(FILE), false) + +/* Wrapper around fdopen that consumes fd on success. */ # define VIR_FDOPEN(FD, MODE) virFdopen(&(FD), MODE) /* For use on cleanup paths; errno is unaffected by close, and no return value to worry about. */ --- /root/tmp/bak/libvirt-acl/src/util/stats_linux.c 2010-11-15 17:03:15.994421659 -0500 +++ src/util/stats_linux.c 2010-11-16 06:31:49.724481766 -0500 @@ -98,9 +98,9 @@ stats->tx_bytes = tx_bytes; stats->tx_packets = tx_packets; stats->tx_errs = tx_errs; stats->tx_drop = tx_drop; - fclose (fp); + VIR_FORCE_FCLOSE (fp); return 0; } } --- /root/tmp/bak/libvirt-acl/src/xen/xen_driver.c 2010-11-15 17:03:15.972422557 -0500 +++ src/xen/xen_driver.c 2010-11-16 06:31:49.722481846 -0500 @@ -218,9 +218,9 @@ #endif #ifdef __sun int fd; - if (fd = open("/dev/xen/domcaps", O_RDONLY)) { + if ((fd = open("/dev/xen/domcaps", O_RDONLY))>= 0) { VIR_FORCE_CLOSE(fd); return 1; } #endif

2 2

[libvirt] [PATCH] nwfilter: also purge ip(6)tables rules before detecting IP address
by Stefan Berger 17 Nov '10

17 Nov '10

Rather than only cleaning any remaining ebtables rules, also clean those applied to iptables and ip6tables when detecting the IP address of an interface. Previous applied iptables rules may hinder DHCP packets. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 @@ -117,6 +117,7 @@ static int ebtablesRemoveBasicRules(cons static int ebiptablesDriverInit(void); static void ebiptablesDriverShutdown(void); static int ebtablesCleanAll(const char *ifname); +static int ebiptablesAllTeardown(const char *ifname); static virMutex execCLIMutex; @@ -2942,7 +2943,7 @@ ebtablesApplyBasicRules(const char *ifna virFormatMacAddr(macaddr, macaddr_str); - ebtablesCleanAll(ifname); + ebiptablesAllTeardown(ifname); ebtablesCreateTmpRootChain(&buf, 1, ifname, 1); @@ -3041,7 +3042,7 @@ ebtablesApplyDHCPOnlyRules(const char *i virFormatMacAddr(macaddr, macaddr_str); - ebtablesCleanAll(ifname); + ebiptablesAllTeardown(ifname); ebtablesCreateTmpRootChain(&buf, 1, ifname, 1); ebtablesCreateTmpRootChain(&buf, 0, ifname, 1); @@ -3143,7 +3144,7 @@ ebtablesApplyDropAllRules(const char *if return 1; } - ebtablesCleanAll(ifname); + ebiptablesAllTeardown(ifname); ebtablesCreateTmpRootChain(&buf, 1, ifname, 1); ebtablesCreateTmpRootChain(&buf, 0, ifname, 1);

2 2

[libvirt] Experimental 32-bit windows libvirt installer
by Justin Clift 16 Nov '10

16 Nov '10

Hi all, If people have time to try out a tentative first "Libvirt Windows Installer", it's online here: http://libvirt.org/sources/win32_experimental/Libvirt-0.8.5-0.exe At the moment it includes the libvirt development headers and libraries for compiling against, plus the virsh shell with it's needed dependencies, and that's about it. It's been compiled on Windows 7 x86_64, but (in theory) might also work on earlier windows versions like Vista and XP. (not yet tested) A couple of notes: + This installer is _not_ production ready. More like early, early stage alpha. ;) + This installer just repackages the files compiled using Matthias Bolte's msys_setup scripting: https://github.com/photron/msys_setup + This is a .exe installer, created using NSIS. Mathias Bolte mentioned, it might be a better idea to use something that creates .msi files, which I kind of agree with. So, next versions will probably not be NSIS based, but no idea what they will be based upon yet. + The only connection type I've tried, and that's known to work for sure, is qemu+tcp://. qemu+ssh:// doesn't work on windows (at this stage), and qemu+tls:// (on windows) doesn't work due to a problem with GnuTLS. If anyone has time to look into the GnuTLS problem, that would be really helpful. :) + This version includes Python bindings, but I haven't tried/tested them. :) + Suggestions for what else to include are welcome. ie. other bindings (C#, what else?), and so on + The NSIS installer scripting is on github if anyone's interested: https://github.com/justinclift/nsis_libvirt_installer Regards and best wishes, Justin Clift

6 21

[libvirt] [PATCH] daemon: fix indention problem in daemon/libvirt.c
by Osier Yang 16 Nov '10

16 Nov '10

* daemon/libvirtd.c --- daemon/libvirtd.c | 40 ++++++++++++++++++++-------------------- 1 files changed, 20 insertions(+), 20 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index ab2b579..9446638 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1077,33 +1077,33 @@ static int qemudNetworkEnable(struct qemud_server *server) { static gnutls_session_t remoteInitializeTLSSession (void) { - gnutls_session_t session; - int err; + gnutls_session_t session; + int err; - err = gnutls_init (&session, GNUTLS_SERVER); - if (err != 0) goto failed; + err = gnutls_init (&session, GNUTLS_SERVER); + if (err != 0) goto failed; - /* avoid calling all the priority functions, since the defaults - * are adequate. - */ - err = gnutls_set_default_priority (session); - if (err != 0) goto failed; + /* avoid calling all the priority functions, since the defaults + * are adequate. + */ + err = gnutls_set_default_priority (session); + if (err != 0) goto failed; - err = gnutls_credentials_set (session, GNUTLS_CRD_CERTIFICATE, x509_cred); - if (err != 0) goto failed; + err = gnutls_credentials_set (session, GNUTLS_CRD_CERTIFICATE, x509_cred); + if (err != 0) goto failed; - /* request client certificate if any. - */ - gnutls_certificate_server_set_request (session, GNUTLS_CERT_REQUEST); + /* request client certificate if any. + */ + gnutls_certificate_server_set_request (session, GNUTLS_CERT_REQUEST); - gnutls_dh_set_prime_bits (session, DH_BITS); + gnutls_dh_set_prime_bits (session, DH_BITS); - return session; + return session; - failed: - VIR_ERROR(_("remoteInitializeTLSSession: %s"), - gnutls_strerror (err)); - return NULL; +failed: + VIR_ERROR(_("remoteInitializeTLSSession: %s"), + gnutls_strerror (err)); + return NULL; } /* Check DN is on tls_allowed_dn_list. */ -- 1.7.3.2

2 1

[libvirt] [PATCH] qemu: fix typos in qemu_monitor_text.c
by Osier Yang 16 Nov '10

16 Nov '10

From: root <root(a)Osier.(none)> * src/qemu/qemu_monitor_text.c (qemuMonitorTextChangeMedia) --- src/qemu/qemu_monitor_text.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 2552111..64ec57b 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -906,7 +906,7 @@ int qemuMonitorTextChangeMedia(qemuMonitorPtr mon, if (qemuMonitorCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("could not eject media on %s"), devname); + _("could not change media on %s"), devname); goto cleanup; } @@ -915,7 +915,7 @@ int qemuMonitorTextChangeMedia(qemuMonitorPtr mon, * No message is printed on success it seems */ if (strstr(reply, "device ")) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("could not eject media on %s: %s"), devname, reply); + _("could not change media on %s: %s"), devname, reply); goto cleanup; } -- 1.7.3.2

2 2

[libvirt] Schedule for next libvirt releases
by Daniel Veillard 16 Nov '10

16 Nov '10

Let's try to keep with end of month based release cycles, so assuming 0.8.6 gets pushed on the 30th, that means 2 weeks from today, and hence we would enter the freeze on Tuesday 23 next week. This means one more week to push patches for 0.8.6. For the following release, I will try to get something out at the end of December, I guess it won't be too hard to do the freeze in the last week, and I will try to make a release for the new year (0.8.7 unless something crazy lands in git in the meantime :-) Happy hacking ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

1 0

[libvirt] [PATCH] [RESEND] qemu: record timestamp in qemu domain log
by Osier Yang 16 Nov '10

16 Nov '10

Currently only support domain start and shutdown, for domain start, record timestamp before the qemu command line, and for domain shutdown, just say it's shutting down with timestamp. * src/qemu/qemu_driver.c --- src/qemu/qemu_driver.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fd61864..423befb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3877,6 +3877,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, char ebuf[1024]; char *pidfile = NULL; int logfile = -1; + char *timestamp; qemuDomainObjPrivatePtr priv = vm->privateData; struct qemudHookData hookData; @@ -4084,7 +4085,17 @@ static int qemudStartVMDaemon(virConnectPtr conn, goto cleanup; } + if ((timestamp = virTimestamp()) == NULL) { + virReportOOMError(); + goto cleanup; + } else if (safewrite(logfile, timestamp, strlen(timestamp)) < 0) { + VIR_WARN("Unable to write timestamp to logfile: %s", + virStrerror(errno, ebuf, sizeof ebuf)); + VIR_FREE(timestamp); + } + tmp = progenv; + while (*tmp) { if (safewrite(logfile, *tmp, strlen(*tmp)) < 0) VIR_WARN("Unable to write envv to logfile: %s", @@ -4233,7 +4244,6 @@ cleanup: return -1; } - static void qemudShutdownVMDaemon(struct qemud_driver *driver, virDomainObjPtr vm, int migrated) { @@ -4243,10 +4253,44 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver, virErrorPtr orig_err; virDomainDefPtr def; int i; + int logfile = -1; + char *timestamp; + char ebuf[1024]; VIR_DEBUG("Shutting down VM '%s' pid=%d migrated=%d", vm->def->name, vm->pid, migrated); + if ((logfile = qemudLogFD(driver, vm->def->name)) < 0) { + /* To not break the normal domain shutdown process, skip the + * timestamp log writing if failed on opening log file. */ + VIR_WARN("Unable to open logfile: %s", + virStrerror(errno, ebuf, sizeof ebuf)); + } else { + if ((timestamp = virTimestamp()) == NULL) { + virReportOOMError(); + } else { + char *shutdownMsg; + + if ((virAsprintf(&shutdownMsg, "%s: shutting down\n", + timestamp)) < 0) { + virReportOOMError(); + } else { + if (safewrite(logfile, shutdownMsg, strlen(shutdownMsg)) < 0) { + VIR_WARN("Unable to write shutdownMsg to logfile: %s", + virStrerror(errno, ebuf, sizeof ebuf)); + } + + VIR_FREE(shutdownMsg); + } + + VIR_FREE(timestamp); + } + + if (VIR_CLOSE(logfile) < 0) + VIR_WARN("Unable to close logfile: %s", + virStrerror(errno, ebuf, sizeof ebuf)); + } + /* This method is routinely used in clean up paths. Disable error * reporting so we don't squash a legit error. */ orig_err = virSaveLastError(); @@ -4382,7 +4426,6 @@ retry: } } - static virDrvOpenStatus qemudOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { -- 1.7.3.2

3 7

[libvirt] [PATCH] replace last instances of close()
by Stefan Berger 16 Nov '10

16 Nov '10

I am replacing the last instances of close() I found with VIR_CLOSE() / VIR_FORCE_CLOSE respectively. The first patch of virsh I missed out on previously. The 2nd patch I had left out intentionally to look at it more carefully: The 'closed' variable could be easily removed since it wasn't used anywhere else. The possible race condition that could result from the filedescriptor being closed and not set to -1 (and possibly let us write into 'something' totally different if the fd was allocated by another thread) seems to be prevented by the qemuMonitorLock() already placed around the code that reads from or writes to the fd. So the change of this code as shown in the patch should not have any side-effects. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- src/qemu/qemu_monitor.c | 14 +++----------- tools/virsh.c | 4 ++-- 2 files changed, 5 insertions(+), 13 deletions(-) Index: libvirt-acl/tools/virsh.c =================================================================== --- libvirt-acl.orig/tools/virsh.c +++ libvirt-acl/tools/virsh.c @@ -8977,12 +8977,12 @@ editWriteToTempFile (vshControl *ctl, co if (safewrite (fd, doc, strlen (doc)) == -1) { vshError(ctl, _("write: %s: failed to write to temporary file: %s"), ret, strerror(errno)); - close (fd); + VIR_FORCE_CLOSE(fd); unlink (ret); VIR_FREE(ret); return NULL; } - if (close (fd) == -1) { + if (VIR_CLOSE(fd) < 0) { vshError(ctl, _("close: %s: failed to write or close temporary file: %s"), ret, strerror(errno)); unlink (ret); Index: libvirt-acl/src/qemu/qemu_monitor.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_monitor.c +++ libvirt-acl/src/qemu/qemu_monitor.c @@ -73,8 +73,6 @@ struct _qemuMonitor { /* If the monitor EOF callback is currently active (stops more commands being run) */ unsigned eofcb: 1; - /* If the monitor is in process of shutting down */ - unsigned closed: 1; unsigned json: 1; }; @@ -692,17 +690,11 @@ void qemuMonitorClose(qemuMonitorPtr mon VIR_DEBUG("mon=%p", mon); qemuMonitorLock(mon); - if (!mon->closed) { + + if (mon->fd >= 0) { if (mon->watch) virEventRemoveHandle(mon->watch); - if (mon->fd != -1) - close(mon->fd); - /* NB: ordinarily one might immediately set mon->watch to -1 - * and mon->fd to -1, but there may be a callback active - * that is still relying on these fields being valid. So - * we merely close them, but not clear their values and - * use this explicit 'closed' flag to track this state */ - mon->closed = 1; + VIR_FORCE_CLOSE(mon->fd); } if (qemuMonitorUnref(mon) > 0)

3 6

[libvirt] process= support for 'qemu-kvm -name' [Bug 576950]
by John Morrissey 16 Nov '10

16 Nov '10

I wrote (attached here, and to the bug) a quick patch that sets the process name to the same value as the window title. I'm unsure where to go from here. Should I add support for converting "native" QEMU command lines to libvirt XML? What would that look like, since I'm not modifying the libvirt format? Should it just drop any ,process= from the QEMU command line it's parsing? I also imagine the test cases will need updating. If someone could give me some high-level guidance, I'd be happy to keep going on this. john -- John Morrissey _o /\ ---- __o jwm(a)horde.net _-< \_ / \ ---- < \, www.horde.net/ __(_)/_(_)________/ \_______(_) /_(_)__

4 11

[libvirt] [PATCH] nwfilter: use /bin/sh rather than requiring bash
by Eric Blake 16 Nov '10

16 Nov '10

* src/nwfilter/nwfilter_ebiptables_driver.c (ebiptablesWriteToTempFile): Use /bin/sh. (bash_cmd_path): Delete. (ebiptablesDriverInit, ebiptablesDriverShutdown): No need to search for bash. (CMD_EXEC): Prefer $() over ``, since we can assume POSIX. (iptablesSetupVirtInPost): Use portable 'test' syntax. (iptablesLinkIPTablesBaseChain): Use POSIX $(()) syntax. --- It turns out that the current use of test x == x in nwfilter was technically safe after all, since we were requiring the existence of bash (and that was also relatively reasonable, since nwfilter is only compiled for Linux). But that still feels awkward, when it is easier to just fix things to work with /bin/sh. Here, I went the easy route, and assumed that since things are Linux, we can use shell constructs that aren't strictly portable to all /bin/sh implementations, so long as they are portable to POSIX (and will thus work with dash or busybox sh). src/nwfilter/nwfilter_ebiptables_driver.c | 25 +++++++++++++------------ 1 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 20d1ba3..c3a0d3e 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -52,12 +52,16 @@ #define CHAINPREFIX_HOST_IN_TEMP 'J' #define CHAINPREFIX_HOST_OUT_TEMP 'P' +/* This file generates a temporary shell script. Since ebiptables is + Linux-specific, we can be reasonably certain that /bin/sh is more + or less POSIX-compliant, so we can use $() and $(()). However, we + cannot assume that /bin/sh is bash, so stick to POSIX syntax. */ #define CMD_SEPARATOR "\n" #define CMD_DEF_PRE "cmd='" #define CMD_DEF_POST "'" #define CMD_DEF(X) CMD_DEF_PRE X CMD_DEF_POST -#define CMD_EXEC "eval res=\\`\"${cmd}\"\\`" CMD_SEPARATOR +#define CMD_EXEC "eval res=\\$(\"${cmd}\")" CMD_SEPARATOR #define CMD_STOPONERR(X) \ X ? "if [ $? -ne 0 ]; then" \ " echo \"Failure to execute command '${cmd}'.\";" \ @@ -76,7 +80,6 @@ static char *ebtables_cmd_path; static char *iptables_cmd_path; static char *ip6tables_cmd_path; -static char *bash_cmd_path; static char *grep_cmd_path; static char *gawk_cmd_path; @@ -427,7 +430,7 @@ static int iptablesLinkIPTablesBaseChain(const char *iptables_cmd, " " CMD_DEF("%s -I %s %d -j %s") CMD_SEPARATOR " " CMD_EXEC " %s" - " let r=r+1\n" + " r=$(( $r + 1 ))\n" " " CMD_DEF("%s -D %s ${r}") CMD_SEPARATOR " " CMD_EXEC " %s" @@ -650,7 +653,7 @@ iptablesSetupVirtInPost(const char *iptables_cmd, virBufferVSprintf(buf, "res=$(%s -n -L " VIRT_IN_POST_CHAIN " | grep \"\\%s %s\")\n" - "if [ \"${res}\" == \"\" ]; then " + "if [ \"${res}\" = \"\" ]; then " CMD_DEF("%s" " -A " VIRT_IN_POST_CHAIN " %s %s -j ACCEPT") CMD_SEPARATOR @@ -2431,7 +2434,7 @@ ebiptablesDisplayRuleInstance(virConnectPtr conn ATTRIBUTE_UNUSED, * * Write the string into a temporary file and return the name of * the temporary file. The string is assumed to contain executable - * commands. A line '#!/bin/bash' will automatically be written + * commands. A line '#!/bin/sh' will automatically be written * as the first line in the file. The permissions of the file are * set so that the file can be run as an executable script. */ @@ -2444,7 +2447,7 @@ ebiptablesWriteToTempFile(const char *string) { char *header; size_t written; - virBufferVSprintf(&buf, "#!%s\n", bash_cmd_path); + virBufferAddLit(&buf, "#!/bin/sh\n"); if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); @@ -2513,10 +2516,10 @@ err_exit: * commands executed via the script the was run. * * Returns 0 in case of success, != 0 in case of an error. The returned - * value is NOT the result of running the commands inside the bash + * value is NOT the result of running the commands inside the shell * script. * - * Execute a sequence of commands (held in the given buffer) as a bash + * Execute a sequence of commands (held in the given buffer) as a /bin/sh * script and return the status of the execution. */ static int @@ -3657,7 +3660,6 @@ ebiptablesDriverInit(void) if (virMutexInit(&execCLIMutex)) return EINVAL; - bash_cmd_path = virFindFileInPath("bash"); gawk_cmd_path = virFindFileInPath("gawk"); grep_cmd_path = virFindFileInPath("grep"); @@ -3701,9 +3703,9 @@ ebiptablesDriverInit(void) VIR_FREE(ip6tables_cmd_path); } - /* ip(6)tables support needs bash, gawk & grep, ebtables doesn't */ + /* ip(6)tables support needs gawk & grep, ebtables doesn't */ if ((iptables_cmd_path != NULL || ip6tables_cmd_path != NULL) && - (!grep_cmd_path || !bash_cmd_path || !gawk_cmd_path)) { + (!grep_cmd_path || !gawk_cmd_path)) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("essential tools to support ip(6)tables " "firewalls could not be located")); @@ -3730,7 +3732,6 @@ static void ebiptablesDriverShutdown() { VIR_FREE(gawk_cmd_path); - VIR_FREE(bash_cmd_path); VIR_FREE(grep_cmd_path); VIR_FREE(ebtables_cmd_path); VIR_FREE(iptables_cmd_path); -- 1.7.3.2

2 7

[libvirt] [PATCH] [TCK] nwfilter: add a test case using concurrency
by Stefan Berger 16 Nov '10

16 Nov '10

Now that the existing scripts are (hopefully) cleaned up and my POSIX compliancy shell-scripting skills have temporarily :-) improved, I am now adding a test case that exercises concurrency. The test case creates and destroys 2 VMs concurrently as well as changes their referenced filters in a tight loop. This kind of test previously uncovered - deadlocks in libvirt due to lock-ordering in the nwfilter subsystem - libvirt termination bug in libaugeas due to doubly closed file descriptors and the side effects All of these have been fixed recently. The test script is known to run in bash, dash and ksh shells. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- scripts/nwfilter/060-concurrency.t | 5 scripts/nwfilter/concurrency/chg-vm-filter.sh | 19 + scripts/nwfilter/concurrency/start-destroy-vm.sh | 19 + scripts/nwfilter/concurrency/tck-vm1-filter1.xml | 26 + scripts/nwfilter/concurrency/tck-vm1-filter2.xml | 26 + scripts/nwfilter/concurrency/tck-vm1.xml | 30 + scripts/nwfilter/concurrency/tck-vm2-filter1.xml | 26 + scripts/nwfilter/concurrency/tck-vm2-filter2.xml | 26 + scripts/nwfilter/concurrency/tck-vm2.xml | 30 + scripts/nwfilter/nwfilter_concurrent.sh | 371 +++++++++++++++++++++++ 10 files changed, 578 insertions(+) Index: libvirt-tck/scripts/nwfilter/060-concurrency.t =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/060-concurrency.t @@ -0,0 +1,5 @@ +#!/bin/sh + +pwd=$(dirname -- "$0") + +(cd -- "${pwd}"; sh ./nwfilter_concurrent.sh --tap-test) Index: libvirt-tck/scripts/nwfilter/nwfilter_concurrent.sh =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/nwfilter_concurrent.sh @@ -0,0 +1,371 @@ +#!/bin/sh + +VIRSH=virsh + +# For each line starting with uri=, remove the prefix and set the hold +# space to the rest of the line. Then at file end, print the hold +# space, which is effectively the last uri= line encountered. +uri=$(sed -n '/^uri[ ]*=[ ]*/ { + s/// + h +} +$ { + x + p +}' < "$LIBVIRT_TCK_CONFIG") +: "${uri:=qemu:///system}" + +LIBVIRT_URI=${uri} + +FLAG_WAIT="$((1<<0))" +FLAG_ATTACH="$((1<<1))" +FLAG_VERBOSE="$((1<<2))" +FLAG_LIBVIRT_TEST="$((1<<3))" +FLAG_TAP_TEST="$((1<<4))" +FLAG_FORCE_CLEAN="$((1<<5))" + +failctr=0 +passctr=0 +attachfailctr=0 +attachctr=0 + +TAP_FAIL_LIST="" +TAP_FAIL_CTR=0 +TAP_TOT_CTR=0 + +usage() { + cmd="$0" +cat <<EOF +Usage: ${cmd} [--help|-h|-?] [--noattach] [--wait] [--verbose] + [--libvirt-test] [--tap-test] + +Options: + --help,-h,-? : Display this help screen. + --noattach : Skip tests that attach and detach a network interface + --wait : Wait for the user to press the enter key once an error + was detected + --verbose : Verbose output + --libvirt-test : Use the libvirt test output format + --tap-test : TAP format output + --force : Allow the automatic cleaning of VMs and nwfilters + previously created by the TCK test suite + +This test will create two virtual machines. The one virtual machine +will use a filter called '${TESTFILTERNAME}', and reference the filter +'clean-traffic' which should be available by default with every install. +The other virtual machine will reference the filter 'tck-testcase' and will +have its filter permanently updated. +EOF +} + + +tap_fail() { + txt=$(echo "$2" | gawk '{print substr($0,1,66)}') + echo "not ok $1 - ${txt}" + TAP_FAIL_LIST="$TAP_FAIL_LIST $1 " + TAP_FAIL_CTR=$(($TAP_FAIL_CTR + 1)) + TAP_TOT_CTR=$(($TAP_TOT_CTR + 1)) +} + +tap_pass() { + txt=$(echo "$2" | gawk '{print substr($0,1,70)}') + echo "ok $1 - ${txt}" + TAP_TOT_CTR=$(($TAP_TOT_CTR + 1)) +} + +tap_final() { + [ -n "${TAP_FAIL_LIST}" ] && echo "FAILED tests ${TAP_FAIL_LIST}" + + okay=`echo "($TAP_TOT_CTR-$TAP_FAIL_CTR)*100/$TAP_TOT_CTR" | bc -l` + txt=$(echo $okay | gawk '{print substr($0,1,5)}') + echo "Failed ${TAP_FAIL_CTR}/${TAP_TOT_CTR} tests, ${txt}% okay" +} + +# A wrapper for mktemp in case it does not exist +# Echos the name of a temporary file. +mktmpdir() { + { + tmp=$( (umask 077 && mktemp -d ./nwfvmtest.XXXXXX) 2>/dev/null) && + test -n "$tmp" && test -d "$tmp" + } || + { + tmp=./nwfvmtest$$-$RANDOM + (umask 077 && mkdir "$tmp") + } || { echo "failed to create secure temporary directory" >&2; exit 1; } + echo "${tmp}" + return 0 +} + + +PRGDIR="./concurrency" +CREATE_DES_VM1="${PRGDIR}/start-destroy-vm.sh 1" +CREATE_DES_VM2="${PRGDIR}/start-destroy-vm.sh 2" +CHG_FILTER_VM1="${PRGDIR}/chg-vm-filter.sh 1" +CHG_FILTER_VM2="${PRGDIR}/chg-vm-filter.sh 2" + + +startPrgs() +{ + flags="$1" + + sh ${CHG_FILTER_VM1} "$4" 2>&1 >/dev/null & + [ $? -ne 0 ] && \ + ( killPrgs "${flags}" "Could not start program 3" ; return 1; ) + CHG_FILTER_VM1_THR=$! + + sh ${CHG_FILTER_VM2} "$5" 2>&1 >/dev/null & + [ $? -ne 0 ] && \ + ( killPrgs "${flags}" "Could not start program 4" ; return 1; ) + CHG_FILTER_VM2_THR=$! + + # Give some time for the filters to be created + sleep 2 + + sh ${CREATE_DES_VM1} "$2" 2>&1 >/dev/null & + [ $? -ne 0 ] && \ + ( killPrgs "${flags}" "Could not start program 1" ; return 1; ) + CREATE_DES_VM1_THR=$! + + sh ${CREATE_DES_VM2} "$3" 2>&1 >/dev/null & + [ $? -ne 0 ] && \ + ( killPrgs "${flags}" "Could not start program 2" ; return 1; ) + CREATE_DES_VM2_THR=$! + +} + + +killPrgs() +{ + msg="$1" + + # terminate all process + [ "x${CREATE_DES_VM1_THR}x" != "xx" ] && \ + kill -9 ${CREATE_DES_VM1_THR} + [ "x${CREATE_DES_VM2_THR}x" != "xx" ] && \ + kill -9 ${CREATE_DES_VM2_THR} + [ "x${CHG_FILTER_VM1_THR}x" != "xx" ] && \ + kill -9 ${CHG_FILTER_VM1_THR} + [ "x${CHG_FILTER_VM2_THR}x" != "xx" ] && \ + kill -9 ${CHG_FILTER_VM2_THR} +} + + +testFail() +{ + flags="$1" + msg="$2" + + failctr=$(($failctr + 1)) + if [ $(($flags & $FLAG_VERBOSE)) -ne 0 ]; then + echo "FAIL : ${msg}" + fi + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then + echo "Press enter" + read enter + fi + [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ] && \ + test_result $(($passctr + $failctr)) "" 1 + [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ] && \ + tap_fail $(($passctr + $failctr)) "${msg}" +} + + +testPass() +{ + flags="$1" + msg="$2" + + passctr=$(($passctr + 1)) + if [ $(($flags & $FLAG_VERBOSE)) -ne 0 ]; then + echo "PASS : ${msg}" + fi + [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ] && \ + test_result $(($passctr + $failctr)) "" 0 + [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ] && \ + tap_pass $(($passctr + $failctr)) "${msg}" +} + +cleanup() +{ + rm -rf "$1" + killPrgs + exit $2 +} + +runTest() +{ + flags="$1" + + passctr=0 + failctr=0 + + tmpdir=`mktmpdir` + failctr=0 + passctr=0 + logvm1="${PWD}/${tmpdir}/logvm1" + logvm2="${PWD}/${tmpdir}/logvm2" + logfivm1="${PWD}/${tmpdir}/logfivm1" + logfivm2="${PWD}/${tmpdir}/logfivm2" + + loops=15 + + trap "cleanup ${tmpdir} 1" INT + + if [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then + # Need to display the number of total tests + tap_total=$((4 * $loops)) + echo "1..${tap_total}" + fi + + startPrgs "${flags}" "${logvm1}" "${logvm2}" \ + "${logfivm1}" "${logfivm2}" + + [ $? -ne 0 ] && rm -rf "${tmpdir}" && return 1; + + sleep 8 + + expect=1 + + while :; + do + # VM start/destroy cycles expected to be slow + tmp=$(($expect / 3)) + # The logs give us the number of cycles the VMs were created + # and destroyed. + val=$(cat "${logvm1}" 2>/dev/null | tail -n 1) + ( [ "x${val}x" = "xx" ] || [ ${val} -lt ${tmp} ] ) \ + && testFail "${flags}" \ + "VM1 log - step ${expect} ($val < $tmp)" \ + || testPass "${flags}" \ + "VM1 log - step ${expect} ($val >= $tmp)" + + val=$(cat "${logvm2}" 2>/dev/null | tail -n 1) + ( [ "x${val}x" = "xx" ] || [ ${val} -lt ${tmp} ] ) \ + && testFail "${flags}" \ + "VM2 log - step ${expect} ($val < $tmp)" \ + || testPass "${flags}" \ + "VM2 log - step ${expect} ($val >= $tmp)" + + # The changing of the filters is expected to work much faster + tmp=$(($expect * 5)) + # Also here the logs give us the number of filter change cycles + val=$(cat "${logfivm1}" 2>/dev/null | tail -n 1) + ( [ "x${val}x" = "xx" ] || [ ${val} -lt ${tmp} ] ) \ + && testFail "${flags}" \ + "VM1 filter log - step ${expect} ($val < $tmp)" \ + || testPass "${flags}" \ + "VM1 filter log - step ${expect} ($val >= $tmp)" + + val=$(cat "${logfivm2}" 2>/dev/null | tail -n 1) + ( [ "x${val}x" = "xx" ] || [ ${val} -lt ${tmp} ] ) \ + && testFail "${flags}" \ + "VM2 filter log - step ${expect} ($val < $tmp)" \ + || testPass "${flags}" \ + "VM2 filter log - step ${expect} ($val >= $tmp)" + + expect=$(($expect + 1)) + [ ${expect} -gt ${loops} ] && break; + + sleep 4 + done + + cleanup "${tmpdir}" 0 +} + + +main() { + prgname="$0" + xmldir="nwfilterxml2xmlin" + fwalldir="nwfilterxml2fwallout" + found=0 + filtername="tck-testcase" + libvirtdpid=-1 + + flags=${FLAG_ATTACH} + + while [ $# -ne 0 ]; do + case "$1" in + --help|-h|-\?) usage ${prgname}; exit 0;; + --noattach) flags=$(($flags & ~$FLAG_ATTACH));; + --wait) flags=$(($flags | $FLAG_WAIT ));; + --verbose) flags=$(($flags | $FLAG_VERBOSE ));; + --libvirt-test) flags=$(($flags | $FLAG_LIBVIRT_TEST ));; + --tap-test) flags=$(($flags | $FLAG_TAP_TEST ));; + --force) flags=$(($flags | $FLAG_FORCE_CLEAN ));; + *) usage ${prgname}; exit 1;; + esac + shift 1 + done + + if [ `uname` != "Linux" ]; then + if [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then + echo "1..0 # Skipped: Only valid on Linux hosts" + else + echo "This script will only run on Linux." + fi + exit 1; + fi + + if [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then + if [ "${LIBVIRT_URI}" != "qemu:///system" ]; then + echo "1..0 # Skipped: Only valid for Qemu system driver" + exit 0 + fi + + for name in `virsh list --all | awk '{print $2}'` + do + case ${name} in + tck*) + if [ "x${LIBVIRT_TCK_AUTOCLEAN}" = "x1" ] || \ + [ $(($flags & $FLAG_FORCE_CLEAN)) -ne 0 ]; then + res=$(virsh destroy ${name} 2>&1) + rc1=$? + res=$(virsh undefine ${name} 2>&1) + rc2=$? + if [ $rc1 -ne 0 ] && [ $rc2 -ne 0 ]; then + echo "Bail out! Could not destroy VM ${name}: ${res}" + exit 0 + fi + else + echo "Bail out! VM ${name} already exists, use --force to clean" + exit 1 + fi + esac + done + + for name in `virsh nwfilter-list | awk '{print $2}'` + do + case ${name} in + tck*) + if [ "x${LIBVIRT_TCK_AUTOCLEAN}" = "x1" ] || \ + [ $(($flags & $FLAG_FORCE_CLEAN)) -ne 0 ]; then + res=$(virsh nwfilter-undefine ${name} 2>&1) + if [ $? -ne 0 ]; then + echo "Bail out! Could not undefine filter ${name}: ${res}" + exit 1 + fi + else + echo "Bail out! Filter ${name} already exists, use --force to clean" + exit 1 + fi + esac + done + fi + + if [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ]; then + curdir="${PWD}" + . test-lib.sh + if [ $? -ne 0 ]; then + exit 1 + fi + test_intro $this_test + cd "${curdir}" + [ $? -ne 0 ] && echo "cd failed" && exit 1 + fi + + runTest "${flags}" + + return 0 +} + +main "$@" Index: libvirt-tck/scripts/nwfilter/concurrency/chg-vm-filter.sh =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/chg-vm-filter.sh @@ -0,0 +1,19 @@ +#!/bin/sh +cd $(dirname "$0") +ctr=0 +[ -z "$2" ] && exit 1 +idx="$1" +logfile="$2" +rm -f "${logfile}" +touch "${logfile}" +while :; +do + virsh nwfilter-define tck-vm${idx}-filter1.xml + [ $? -ne 0 ] && break + [ ! -w "${logfile}" ] && break + virsh nwfilter-define tck-vm${idx}-filter2.xml + [ $? -ne 0 ] && break + ctr=$(($ctr + 1)) + [ ! -w "${logfile}" ] && break + echo "${ctr}" >> "${logfile}" +done Index: libvirt-tck/scripts/nwfilter/concurrency/start-destroy-vm.sh =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/start-destroy-vm.sh @@ -0,0 +1,19 @@ +#!/bin/sh +cd $(dirname "$0") +ctr=0 +[ -z "$2" ] && exit 1 +idx="$1" +logfile="$2" +rm -f "${logfile}" +touch "${logfile}" +while :; +do + virsh create tck-vm${idx}.xml + [ $? -ne 0 ] && break + sleep 2 + virsh destroy tck-vm${idx} + [ $? -ne 0 ] && break + ctr=$(($ctr + 1)) + [ ! -w "${logfile}" ] && break + echo "${ctr}" >> "${logfile}" +done Index: libvirt-tck/scripts/nwfilter/concurrency/tck-vm1-filter1.xml =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/tck-vm1-filter1.xml @@ -0,0 +1,26 @@ +<filter name='tck-vm1-filter' chain='root'> + <uuid>464d2617-43d0-7694-b479-320b72dac187</uuid> + <filterref filter='clean-traffic'/> + <rule action='accept' direction='in' priority='500'> + <all comment='test'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <all/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='21' dstportend='22'/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='80'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <icmp/> + </rule> + <rule action='accept' direction='out' priority='500'> + <udp dstportstart='53'/> + </rule> + <rule action='drop' direction='inout' priority='500'> + <all/> + </rule> +</filter> + Index: libvirt-tck/scripts/nwfilter/concurrency/tck-vm1-filter2.xml =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/tck-vm1-filter2.xml @@ -0,0 +1,26 @@ +<filter name='tck-vm1-filter' chain='root'> + <uuid>464d2617-43d0-7694-b479-320b72dac187</uuid> + <filterref filter='clean-traffic'/> + <rule action='accept' direction='in' priority='500'> + <all comment='test again'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <all/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='21' dstportend='22'/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='80'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <icmp/> + </rule> + <rule action='accept' direction='out' priority='500'> + <udp dstportstart='53'/> + </rule> + <rule action='drop' direction='inout' priority='500'> + <all/> + </rule> +</filter> + Index: libvirt-tck/scripts/nwfilter/concurrency/tck-vm1.xml =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/tck-vm1.xml @@ -0,0 +1,30 @@ + <domain type='kvm'> + <name>tck-vm1</name> + <memory>32768</memory> + <currentMemory>32768</currentMemory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <interface type='bridge'> + <source bridge='virbr0'/> + <filterref filter='tck-vm1-filter'> + </filterref> + <target dev='tck-vm1-if0'/> + </interface> + <console type='pty'> + </console> + <input type='mouse' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes'/> + </devices> + </domain> Index: libvirt-tck/scripts/nwfilter/concurrency/tck-vm2-filter1.xml =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/tck-vm2-filter1.xml @@ -0,0 +1,26 @@ +<filter name='tck-vm2-filter' chain='root'> + <uuid>364d2617-43d0-7694-b479-320b72dac187</uuid> + <filterref filter='clean-traffic'/> + <rule action='accept' direction='in' priority='500'> + <all comment='another test'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <all/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='21' dstportend='22'/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='80'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <icmp/> + </rule> + <rule action='accept' direction='out' priority='500'> + <udp dstportstart='53'/> + </rule> + <rule action='drop' direction='inout' priority='500'> + <all/> + </rule> +</filter> + Index: libvirt-tck/scripts/nwfilter/concurrency/tck-vm2-filter2.xml =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/tck-vm2-filter2.xml @@ -0,0 +1,26 @@ +<filter name='tck-vm2-filter' chain='root'> + <uuid>364d2617-43d0-7694-b479-320b72dac187</uuid> + <filterref filter='clean-traffic'/> + <rule action='accept' direction='in' priority='500'> + <all comment='another test again'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <all/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='21' dstportend='22'/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp dstportstart='80'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <icmp/> + </rule> + <rule action='accept' direction='out' priority='500'> + <udp dstportstart='53'/> + </rule> + <rule action='drop' direction='inout' priority='500'> + <all/> + </rule> +</filter> + Index: libvirt-tck/scripts/nwfilter/concurrency/tck-vm2.xml =================================================================== --- /dev/null +++ libvirt-tck/scripts/nwfilter/concurrency/tck-vm2.xml @@ -0,0 +1,30 @@ + <domain type='kvm'> + <name>tck-vm2</name> + <memory>32768</memory> + <currentMemory>32768</currentMemory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <interface type='bridge'> + <source bridge='virbr0'/> + <filterref filter='tck-vm2-filter'> + </filterref> + <target dev='tck-vm2-if0'/> + </interface> + <console type='pty'> + </console> + <input type='mouse' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes'/> + </devices> + </domain>

2 2

[libvirt] [PATCH v2] [TCK] Convert scripts to also run with dash
by Stefan Berger 16 Nov '10

16 Nov '10

v2: - following Eric's suggestions from review of v1 - runs with ksh shell as well I am converting the shell-scripted tap tests to also run using the dash and ksh shell, fixing some other problems on the way as well. Two of the tests' data files contained '\\1' in an sed command. This created char(1) with the dash, '\1' with the bash. I found it easier to replace the sed command with a different one than figuring out how to solve this escape problem so it works for both shells. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- scripts/networks/100-apply-verify-host.t | 11 scripts/networks/networkApplyTest.sh | 189 +++--- scripts/networks/networkxml2hostout/tck-testnet-1.dat | 10 scripts/networks/networkxml2hostout/tck-testnet-1.post.dat | 10 scripts/networks/networkxml2hostout/tck-testnet-2.dat | 10 scripts/networks/networkxml2hostout/tck-testnet-2.post.dat | 10 scripts/nwfilter/050-apply-verify-host.t | 11 scripts/nwfilter/nwfilter2vmtest.sh | 375 ++++++------- scripts/nwfilter/nwfilterxml2fwallout/comment-test.fwall | 6 9 files changed, 310 insertions(+), 322 deletions(-) Index: libvirt-tck/scripts/networks/networkApplyTest.sh =================================================================== --- libvirt-tck.orig/scripts/networks/networkApplyTest.sh +++ libvirt-tck/scripts/networks/networkApplyTest.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh VIRSH=virsh @@ -33,8 +33,8 @@ TAP_FAIL_LIST="" TAP_FAIL_CTR=0 TAP_TOT_CTR=0 -function usage() { - local cmd="$0" +usage() { + cmd="$0" cat <<EOF Usage: ${cmd} [--help|-h|-?] [--noattach] [--wait] [--verbose] [--libvirt-test] [--tap-test] @@ -56,37 +56,37 @@ EOF } -function tap_fail() { - echo "not ok $1 - ${2:0:66}" - TAP_FAIL_LIST+="$1 " - ((TAP_FAIL_CTR++)) - ((TAP_TOT_CTR++)) +tap_fail() { + txt=$(echo "$2" | gawk '{print substr($0,1,66)}') + echo "not ok $1 - ${txt}" + TAP_FAIL_LIST="$TAP_FAIL_LIST $1 " + TAP_FAIL_CTR=$(($TAP_FAIL_CTR + 1)) + TAP_TOT_CTR=$(($TAP_TOT_CTR + 1)) } -function tap_pass() { - echo "ok $1 - ${2:0:70}" - ((TAP_TOT_CTR++)) +tap_pass() { + txt=$(echo "$2" | gawk '{print substr($0,1,66)}') + echo "ok $1 - ${txt}" + TAP_TOT_CTR=$(($TAP_TOT_CTR + 1)) } -function tap_final() { - local okay - +tap_final() { [ -n "${TAP_FAIL_LIST}" ] && echo "FAILED tests ${TAP_FAIL_LIST}" okay=$(echo "($TAP_TOT_CTR-$TAP_FAIL_CTR)*100/$TAP_TOT_CTR" | bc -l) - echo "Failed ${TAP_FAIL_CTR}/${TAP_TOT_CTR} tests, ${okay:0:5}% okay" + txt=$(echo $okay | gawk '{print substr($0,1,5)}') + echo "Failed ${TAP_FAIL_CTR}/${TAP_TOT_CTR} tests, ${txt}% okay" } # A wrapper for mktemp in case it does not exist # Echos the name of a secure temporary directory. -function mktmpdir() { - local tmp +mktmpdir() { { - tmp=$( (umask 077 && mktemp -d ./nwfvmtest.XXXXXX) 2>/dev/null) && + tmp=$( (umask 077 && mktemp -d ./nwtst.XXXXXX) 2>/dev/null) && test -n "$tmp" && test -d "$tmp" } || { - tmp=./nwfvmtest$$-$RANDOM + tmp=./nwtst$$-$RANDOM (umask 077 && mkdir "$tmp") } || { echo "failed to create secure temporary directory" >&2; exit 1; } echo "${tmp}" @@ -94,16 +94,16 @@ function mktmpdir() { } -function checkExpectedOutput() { - local xmlfile="$1" - local datafile="$2" - local flags="$3" - local skipregex="$4" - local cmd line tmpdir tmpfile tmpfile2 skip +checkExpectedOutput() { + xmlfile="$1" + datafile="$2" + flags="$3" + skipregex="$4" tmpdir=$(mktmpdir) tmpfile=$tmpdir/file tmpfile2=$tmpdir/file2 + OIFS="${IFS}" if echo a | grep -E '(a|b)' >/dev/null 2>&1 then EGREP='grep -E' @@ -112,11 +112,12 @@ function checkExpectedOutput() { exec 4<"${datafile}" - read <&4 - line="${REPLY}" + IFS="" + + read -r line <&4 while [ "x${line}x" != "xx" ]; do - cmd=$(echo "${line##\#}") + cmd=$(printf %s\\n "${line##\#}") skip=0 if [ "x${skipregex}x" != "xx" ]; then @@ -128,10 +129,14 @@ function checkExpectedOutput() { : >"${tmpfile2}" while :; do - read <&4 - line="${REPLY}" + read -r line <&4 + + case "${line}" in + '#'*) letter="#";; + *) letter="";; + esac - if [ "${line:0:1}" == "#" ] || [ "x${line}x" == "xx" ]; then + if [ "x${letter}x" = "x#x" ] || [ "x${line}x" = "xx" ]; then if [ ${skip} -ne 0 ]; then break @@ -140,49 +145,50 @@ function checkExpectedOutput() { diff "${tmpfile}" "${tmpfile2}" >/dev/null if [ $? -ne 0 ]; then - if [ $((flags & FLAG_VERBOSE)) -ne 0 ]; then + if [ $(($flags & $FLAG_VERBOSE)) -ne 0 ]; then echo "FAIL ${xmlfile} : ${cmd}" diff "${tmpfile}" "${tmpfile2}" fi - ((failctr++)) - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then - echo "tmp files: $tmpfile, $tmpfile2" - echo "Press enter" - read + failctr=$(($failctr + 1)) + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then + echo "tmp files: $tmpfile, $tmpfile2" + echo "Press enter" + read enter fi - [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \ - test_result $((passctr+failctr)) "" 1 - [ $((flags & FLAG_TAP_TEST)) -ne 0 ] && \ - tap_fail $((passctr+failctr)) "${xmlfile} : ${cmd}" + [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ] && \ + test_result $(($passctr + $failctr)) "" 1 + [ $(($flags & FLAG_TAP_TEST)) -ne 0 ] && \ + tap_fail $(($passctr + $failctr)) "${xmlfile} : ${cmd}" else - ((passctr++)) - [ $((flags & FLAG_VERBOSE)) -ne 0 ] && \ - echo "PASS ${xmlfile} : ${cmd}" - [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \ - test_result $((passctr+failctr)) "" 0 - [ $((flags & FLAG_TAP_TEST)) -ne 0 ] && \ - tap_pass $((passctr+failctr)) "${xmlfile} : ${cmd}" + passctr=$(($passctr + 1)) + [ $(($flags & $FLAG_VERBOSE)) -ne 0 ] && \ + echo "PASS ${xmlfile} : ${cmd}" + [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ] && \ + test_result $(($passctr + $failctr)) "" 0 + [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ] && \ + tap_pass $(($passctr + $failctr)) "${xmlfile} : ${cmd}" fi break fi - echo "${line}" >> "${tmpfile2}" + printf %s\\n "${line}" >> "${tmpfile2}" done done exec 4>&- rm -rf "${tmpdir}" + + IFS="${OIFS}" } -function doTest() { - local xmlfile="$1" - local datafile="$2" - local postdatafile="$3" - local flags="$4" - local netname +doTest() { + xmlfile="$1" + datafile="$2" + postdatafile="$3" + flags="$4" if [ ! -r "${xmlfile}" ]; then echo "FAIL : Cannot access filter XML file ${xmlfile}." @@ -205,24 +211,20 @@ function doTest() { } -function runTests() { - local xmldir="$1" - local hostoutdir="$2" - local flags="$3" - local datafiles f c - local tap_total=0 ctr=0 - - pushd "${PWD}" > /dev/null - cd "${hostoutdir}" - datafiles=$(ls *.dat) - popd > /dev/null +runTests() { + xmldir="$1" + hostoutdir="$2" + flags="$3" + tap_total=0 ctr=0 + + datafiles=$(cd "${hostoutdir}";ls *.dat) - if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then + if [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then # Need to count the number of total tests for fil in ${datafiles}; do c=$(grep -c "^#" "${hostoutdir}/${fil}") - ((tap_total+=c)) - ((ctr++)) + tap_total=$(($tap_total + $c)) + ctr=$(($ctr + 1)) done echo "1..${tap_total}" fi @@ -236,9 +238,9 @@ function runTests() { "${hostoutdir}/${f}.post.dat" "${flags}" done - if [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ]; then - test_final $((passctr+failctr)) $failctr - elif [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then + if [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ]; then + test_final $(($passctr + $failctr)) $failctr + elif [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then tap_final else echo "" @@ -250,29 +252,27 @@ function runTests() { } -function main() { - local prgname="$0" - local vm1 vm2 - local xmldir="networkxml2xmlin" - local hostoutdir="networkxml2hostout" - local res rc - local flags +main() { + prgname="$0" + xmldir="networkxml2xmlin" + hostoutdir="networkxml2hostout" + flags=0 while [ $# -ne 0 ]; do case "$1" in --help|-h|-\?) usage ${prgname}; exit 0;; - --wait) ((flags |= FLAG_WAIT ));; - --verbose) ((flags |= FLAG_VERBOSE ));; - --libvirt-test) ((flags |= FLAG_LIBVIRT_TEST ));; - --tap-test) ((flags |= FLAG_TAP_TEST ));; - --force) ((flags |= FLAG_FORCE_CLEAN ));; + --wait) flags=$(($flags | $FLAG_WAIT ));; + --verbose) flags=$(($flags | $FLAG_VERBOSE ));; + --libvirt-test) flags=$(($flags | $FLAG_LIBVIRT_TEST ));; + --tap-test) flags=$(($flags | $FLAG_TAP_TEST ));; + --force) flags=$(($flags | $FLAG_FORCE_CLEAN ));; *) usage ${prgname}; exit 1;; esac shift 1 done if [ $(uname) != "Linux" ]; then - if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then + if [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then echo "1..0 # Skipped: Only valid on Linux hosts" else echo "This script will only run on Linux." @@ -280,7 +280,7 @@ function main() { exit 1; fi - if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then + if [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then if [ "${LIBVIRT_URI}" != "qemu:///system" ]; then echo "1..0 # Skipped: Only valid for Qemu system driver" exit 0 @@ -290,8 +290,8 @@ function main() { do case ${name} in tck*) - if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" -o \ - $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then + if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" ] || \ + [ $(($flags & $FLAG_FORCE_CLEAN)) -ne 0 ]; then res=$(virsh destroy ${name} 2>&1) res=$(virsh undefine ${name} 2>&1) if [ $? -ne 0 ]; then @@ -309,8 +309,8 @@ function main() { do case ${name} in tck*) - if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" -o \ - $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then + if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" ] || \ + [ $(($flags & $FLAG_FORCE_CLEAN)) -ne 0 ]; then res=$(virsh net-destroy ${name} 2>&1) rc=$? res=$(virsh net-undefine ${name} 2>&1) @@ -326,18 +326,17 @@ function main() { done fi - if [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ]; then - pushd "${PWD}" > /dev/null + if [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ]; then + curdir="${PWD}" . ./test-lib.sh if [ $? -ne 0 ]; then exit 1 fi test_intro $this_test - popd > /dev/null + cd "${curdir}" + [ $? -ne 0 ] && echo "cd failed" && exit 1 fi - res=$(${VIRSH} capabilities 2>&1) - runTests "${xmldir}" "${hostoutdir}" "${flags}" return 0 Index: libvirt-tck/scripts/nwfilter/nwfilter2vmtest.sh =================================================================== --- libvirt-tck.orig/scripts/nwfilter/nwfilter2vmtest.sh +++ libvirt-tck/scripts/nwfilter/nwfilter2vmtest.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh ORIG_IFNAME="vnet0" ATTACH_IFNAME="attach0" @@ -6,17 +6,20 @@ TESTFILTERNAME="nwfiltertestfilter" TESTVM2FWALLDATA="nwfilterxml2fwallout/testvm.fwall.dat" VIRSH=virsh -uri= -if [ "x${LIBVIRT_TCK_CONFIG}x" != "xx" ]; then - uri_exp=`cat ${LIBVIRT_TCK_CONFIG} | grep "^uri\s*=" | sed -e 's/uri\s*=\s*//' | tail -n 1` - if [ "x${uri_exp}x" != "xx" ]; then - eval "uri=${uri_exp}" - fi -else - uri="qemu:///system" -fi -LIBVIRT_URI=${uri} +# For each line starting with uri=, remove the prefix and set the hold +# space to the rest of the line. Then at file end, print the hold +# space, which is effectively the last uri= line encountered. +uri=$(sed -n '/^uri[ ]*=[ ]*/ { + s/// + h +} +$ { + x + p +}' < "$LIBVIRT_TCK_CONFIG") +: "${uri:=qemu:///system}" +LIBVIRT_URI=${uri} FLAG_WAIT="$((1<<0))" FLAG_ATTACH="$((1<<1))" @@ -34,8 +37,8 @@ TAP_FAIL_LIST="" TAP_FAIL_CTR=0 TAP_TOT_CTR=0 -function usage() { - local cmd="$0" +usage() { + cmd="$0" cat <<EOF Usage: ${cmd} [--help|-h|-?] [--noattach] [--wait] [--verbose] [--libvirt-test] [--tap-test] @@ -60,73 +63,69 @@ EOF } -function tap_fail() { - echo "not ok $1 - ${2:0:66}" - TAP_FAIL_LIST+="$1 " - ((TAP_FAIL_CTR++)) - ((TAP_TOT_CTR++)) +tap_fail() { + txt=$(echo "$2" | gawk '{print substr($0,1,66)}') + echo "not ok $1 - ${txt}" + TAP_FAIL_LIST="$TAP_FAIL_LIST $1 " + TAP_FAIL_CTR=$(($TAP_FAIL_CTR + 1)) + TAP_TOT_CTR=$(($TAP_TOT_CTR + 1)) } -function tap_pass() { - echo "ok $1 - ${2:0:70}" - ((TAP_TOT_CTR++)) +tap_pass() { + txt=$(echo "$2" | gawk '{print substr($0,1,70)}') + echo "ok $1 - ${txt}" + TAP_TOT_CTR=$(($TAP_TOT_CTR + 1)) } -function tap_final() { - local okay - +tap_final() { [ -n "${TAP_FAIL_LIST}" ] && echo "FAILED tests ${TAP_FAIL_LIST}" okay=`echo "($TAP_TOT_CTR-$TAP_FAIL_CTR)*100/$TAP_TOT_CTR" | bc -l` - echo "Failed ${TAP_FAIL_CTR}/${TAP_TOT_CTR} tests, ${okay:0:5}% okay" + txt=$(echo $okay | gawk '{print substr($0,1,5)}') + echo "Failed ${TAP_FAIL_CTR}/${TAP_TOT_CTR} tests, ${txt}% okay" } # A wrapper for mktemp in case it does not exist # Echos the name of a temporary file. -function mktmpfile() { - local tmp - type -P mktemp > /dev/null - if [ $? -eq 0 ]; then - tmp=$(mktemp -t nwfvmtest.XXXXXX) - echo ${tmp} - else - while :; do - tmp="/tmp/nwfvmtest.${RANDOM}" - if [ ! -f ${tmp} ]; then - touch ${tmp} - chmod 666 ${tmp} - echo ${tmp} - break - fi - done - fi +mktmpdir() { + { + tmp=$( (umask 077 && mktemp -d ./nwfvmtest.XXXXXX) 2>/dev/null) && + test -n "$tmp" && test -d "$tmp" + } || + { + tmp=./nwfvmtest$$-$RANDOM + (umask 077 && mkdir "$tmp") + } || { echo "failed to create secure temporary directory" >&2; exit 1; } + echo "${tmp}" return 0 } -function checkExpectedOutput() { - local xmlfile="$1" - local fwallfile="$2" - local ifname="$3" - local flags="$4" - local skipregex="$5" - local regex="s/${ORIG_IFNAME}/${ifname}/g" - local cmd line tmpfile tmpfile2 skip - - tmpfile=`mktmpfile` - tmpfile2=`mktmpfile` +checkExpectedOutput() { + xmlfile="$1" + fwallfile="$2" + ifname="$3" + flags="$4" + skipregex="$5" + regex="s/${ORIG_IFNAME}/${ifname}/g" + + tmpdir=$(mktmpdir) + tmpfile=$tmpdir/file + tmpfile2=$tmpdir/file2 + OIFS="${IFS}" exec 4<${fwallfile} - read <&4 - line="${REPLY}" + IFS="" + + read -r line <&4 while [ "x${line}x" != "xx" ]; do - cmd=`echo ${line##\#} | sed ${regex}` + cmd=$(printf %s\\n ${line##\#} | sed ${regex}) skip=0 if [ "x${skipregex}x" != "xx" ]; then - skip=`echo ${cmd} | grep -c -E ${skipregex}` + skip=`printf %s\\n ${cmd} | grep -c -E ${skipregex}` fi eval ${cmd} 2>&1 | tee ${tmpfile} 1>/dev/null @@ -135,10 +134,14 @@ function checkExpectedOutput() { touch ${tmpfile2} while [ 1 ]; do - read <&4 - line="${REPLY}" + read -r line <&4 - if [ "${line:0:1}" == "#" ] || [ "x${line}x" == "xx" ]; then + case "${line}" in + '#'*) letter="#";; + *) letter="";; + esac + + if [ "x${letter}x" = "x#x" ] || [ "x${line}x" = "xx" ]; then if [ ${skip} -ne 0 ]; then break @@ -147,52 +150,53 @@ function checkExpectedOutput() { diff ${tmpfile} ${tmpfile2} >/dev/null if [ $? -ne 0 ]; then - if [ $((flags & FLAG_VERBOSE)) -ne 0 ]; then + if [ $(($flags & $FLAG_VERBOSE)) -ne 0 ]; then echo "FAIL ${xmlfile} : ${cmd}" diff ${tmpfile} ${tmpfile2} fi - ((failctr++)) - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then + failctr=$(($failctr + 1)) + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then echo "tmp files: $tmpfile, $tmpfile2" echo "Press enter" - read + read enter fi - [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \ - test_result $((passctr+failctr)) "" 1 - [ $((flags & FLAG_TAP_TEST)) -ne 0 ] && \ - tap_fail $((passctr+failctr)) "${xmlfile} : ${cmd}" + [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ] && \ + test_result $(($passctr + $failctr)) "" 1 + [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ] && \ + tap_fail $(($passctr + $failctr)) "${xmlfile} : ${cmd}" else - ((passctr++)) - [ $((flags & FLAG_VERBOSE)) -ne 0 ] && \ + passctr=$(($passctr + 1)) + [ $(($flags & $FLAG_VERBOSE)) -ne 0 ] && \ echo "PASS ${xmlfile} : ${cmd}" - [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \ - test_result $((passctr+failctr)) "" 0 - [ $((flags & FLAG_TAP_TEST)) -ne 0 ] && \ - tap_pass $((passctr+failctr)) "${xmlfile} : ${cmd}" + [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ] && \ + test_result $(($passctr + $failctr)) "" 0 + [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ] && \ + tap_pass $(($passctr + $failctr)) "${xmlfile} : ${cmd}" fi break fi - echo "${line}" | sed ${regex} >> ${tmpfile2} + printf %s\\n "${line}" | sed ${regex} >> ${tmpfile2} done done exec 4>&- - rm -rf "${tmpfile}" "${tmpfile2}" 2>/dev/null + rm -rf "${tmpdir}" + + IFS="${OIFS}" } -function doTest() { - local xmlfile="$1" - local fwallfile="$2" - local vm1name="$3" - local vm2name="$4" - local flags="$5" - local testnum="$6" - local linenums ctr=0 - local tmpfile b msg rc +doTest() { + xmlfile="$1" + fwallfile="$2" + vm1name="$3" + vm2name="$4" + flags="$5" + testnum="$6" + ctr=0 if [ ! -r "${xmlfile}" ]; then echo "FAIL : Cannot access filter XML file ${xmlfile}." @@ -207,9 +211,10 @@ function doTest() { checkExpectedOutput "${TESTFILTERNAME}" "${TESTVM2FWALLDATA}" \ "${vm2name}" "${flags}" "" - if [ $((flags & FLAG_ATTACH)) -ne 0 ]; then + if [ $(($flags & $FLAG_ATTACH)) -ne 0 ]; then - tmpfile=`mktmpfile` + tmpdir=$(mktmpdir) + tmpfile=$tmpdir/tmpfile b=`{ ${VIRSH} dumpxml ${vm1name} | tr -d "\n"; echo; } | \ sed "s/.*\<interface.*source bridge='$[a-zA-Z0-9_]\+$'.*<\/interface>.*/\1/"` @@ -225,7 +230,7 @@ EOF msg=`${VIRSH} attach-device "${vm1name}" "${tmpfile}" > /dev/null` rc=$? - ((attachctr++)) + attachctr=$(($attachctr + 1)) if [ $rc -eq 0 ]; then checkExpectedOutput "${xmlfile}" "${fwallfile}" "${ATTACH_IFNAME}" \ @@ -237,7 +242,7 @@ EOF echo "FAIL: Detach of interface failed." fi else - if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then + if [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then # In case of TAP, run the test anyway so we get to the full number # of tests checkExpectedOutput "${xmlfile}" "${fwallfile}" "${ATTACH_IFNAME}" \ @@ -245,48 +250,45 @@ EOF checkExpectedOutput "${TESTFILTERNAME}" "${TESTVM2FWALLDATA}" \ "${vm2name}" "${flags}" #"(PRE|POST)ROUTING" fi - - ((attachfailctr++)) - if [ $((flags & FLAG_VERBOSE)) -ne 0 ]; then + + attachfailctr=$(($attachfailctr + 1)) + if [ $(($flags & $FLAG_VERBOSE)) -ne 0 ]; then echo "FAIL: Could not attach interface to vm ${vm1name}." - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then echo "Press enter" - read + read enter fi fi fi - rm -rf ${tmpfile} + rm -rf ${tmpdir} fi return 0 } -function runTests() { - local vm1name="$1" - local vm2name="$2" - local xmldir="$3" - local fwalldir="$4" - local flags="$5" - local fwallfiles f c - local tap_total=0 ctr=0 - - pushd ${PWD} > /dev/null - cd ${fwalldir} - fwallfiles=`ls *.fwall` - popd > /dev/null +runTests() { + vm1name="$1" + vm2name="$2" + xmldir="$3" + fwalldir="$4" + flags="$5" + tap_total=0 + ctr=0 - if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then + fwallfiles=$(cd ${fwalldir}; ls *.fwall) + + if [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then # Need to count the number of total tests for fil in ${fwallfiles}; do c=$(grep -c "^#" ${fwalldir}/${fil}) - ((tap_total+=c)) - ((ctr++)) + tap_total=$(($tap_total + $c)) + ctr=$(($ctr + 1)) done c=$(grep -c "^#" "${TESTVM2FWALLDATA}") - ((tap_total+=c*ctr)) - [ $((flags & FLAG_ATTACH)) -ne 0 ] && ((tap_total*=2)) + tap_total=$(($tap_total + $c * $ctr)) + [ $(($flags & $FLAG_ATTACH)) -ne 0 ] && tap_total=$(($tap_total * 2)) echo "1..${tap_total}" fi @@ -296,9 +298,9 @@ function runTests() { "${vm2name}" "${flags}" done - if [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ]; then - test_final $((passctr+failctr)) $failctr - elif [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then + if [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ]; then + test_final $(($passctr + $failctr)) $failctr + elif [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then tap_final else echo "" @@ -310,14 +312,14 @@ function runTests() { } -function createVM() { - local vmname="$1" - local filtername="$2" - local ipaddr="$3" - local macaddr="$4" - local flags="$5" - local res - local tmpfile='mktmpfile' +createVM() { + vmname="$1" + filtername="$2" + ipaddr="$3" + macaddr="$4" + flags="$5" + tmpdir=$(mktmpdir) + tmpfile=$tmpdir/tmpfile cat > ${tmpfile} << EOF <domain type='kvm'> @@ -363,33 +365,32 @@ EOF res=$(${VIRSH} start ${vmname}) if [ $? -ne 0 ]; then echo "Could not start VM ${vmname} : ${res}" - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi $(${VIRSH} undefine ${vmname}) return 1 fi - [ $((flags & FLAG_VERBOSE)) -ne 0 ] && echo "Created VM ${vmname}." + [ $(($flags & $FLAG_VERBOSE)) -ne 0 ] && echo "Created VM ${vmname}." - rm -rf ${tmpfile} + rm -rf ${tmpdir} return 0 } -function destroyVM() { - local vmname="$1" - local flags="$2" - local res +destroyVM() { + vmname="$1" + flags="$2" res=$(${VIRSH} destroy ${vmname}) if [ $? -ne 0 ]; then echo "Could not destroy VM ${vmname} : ${res}" - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi @@ -397,23 +398,23 @@ function destroyVM() { res=$(${VIRSH} undefine ${vmname}) if [ $? -ne 0 ]; then echo "Could not undefine VM ${vmname} : ${res}" - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi - [ $((flags & FLAG_VERBOSE)) -ne 0 ] && echo "Destroyed VM ${vmname}." + [ $(($flags & $FLAG_VERBOSE)) -ne 0 ] && echo "Destroyed VM ${vmname}." return 0 } -function createTestFilters() { - local flags="$1" - local tmpfile=`mktmpfile` - local res +createTestFilters() { + flags="$1" + tmpdir=$(mktmpdir) + tmpfile=$tmpdir/tmpfile cat >${tmpfile} << EOF <filter name="${TESTFILTERNAME}"> @@ -431,11 +432,11 @@ EOF res=$(${VIRSH} nwfilter-define ${tmpfile}) if [ $? -ne 0 ]; then echo "Could not define filter : ${res}" - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi - rm -rf ${tmpfile} + rm -rf ${tmpdir} return 1 fi @@ -446,39 +447,38 @@ EOF res=$(${VIRSH} nwfilter-define ${tmpfile}) if [ $? -ne 0 ]; then echo "Could not define filter : ${res}" - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi - rm -rf ${tmpfile} + rm -rf ${tmpdir} return 1 fi - rm -rf ${tmpfile} + rm -rf ${tmpdir} return 0 } -function deleteTestFilter() { - local flags="$1" - local res +deleteTestFilter() { + flags="$1" res=$(${VIRSH} nwfilter-undefine ${TESTFILTERNAME} 2>&1) if [ $? -ne 0 ]; then echo "Could not undefine filter : ${res}" - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi res=$(${VIRSH} nwfilter-undefine tck-testcase 2>&1) if [ $? -ne 0 ]; then echo "Could not undefine filter : ${res}" - if [ $((flags & FLAG_WAIT)) -ne 0 ]; then + if [ $(($flags & $FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi @@ -486,34 +486,32 @@ function deleteTestFilter() { } -function main() { - local prgname="$0" - local vm1 vm2 - local xmldir="nwfilterxml2xmlin" - local fwalldir="nwfilterxml2fwallout" - local found=0 vms res - local filtername="tck-testcase" - local libvirtdpid=-1 - local flags OPWD +main() { + prgname="$0" + xmldir="nwfilterxml2xmlin" + fwalldir="nwfilterxml2fwallout" + found=0 + filtername="tck-testcase" + libvirtdpid=-1 - ((flags=${FLAG_ATTACH})) + flags=${FLAG_ATTACH} while [ $# -ne 0 ]; do case "$1" in --help|-h|-\?) usage ${prgname}; exit 0;; - --noattach) ((flags &= ~FLAG_ATTACH ));; - --wait) ((flags |= FLAG_WAIT ));; - --verbose) ((flags |= FLAG_VERBOSE ));; - --libvirt-test) ((flags |= FLAG_LIBVIRT_TEST ));; - --tap-test) ((flags |= FLAG_TAP_TEST ));; - --force) ((flags |= FLAG_FORCE_CLEAN ));; + --noattach) flags=$(($flags & ~$FLAG_ATTACH));; + --wait) flags=$(($flags | $FLAG_WAIT ));; + --verbose) flags=$(($flags | $FLAG_VERBOSE ));; + --libvirt-test) flags=$(($flags | $FLAG_LIBVIRT_TEST ));; + --tap-test) flags=$(($flags | $FLAG_TAP_TEST ));; + --force) flags=$(($flags | $FLAG_FORCE_CLEAN ));; *) usage ${prgname}; exit 1;; esac shift 1 done if [ `uname` != "Linux" ]; then - if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then + if [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then echo "1..0 # Skipped: Only valid on Linux hosts" else echo "This script will only run on Linux." @@ -521,21 +519,23 @@ function main() { exit 1; fi - if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then + if [ $(($flags & $FLAG_TAP_TEST)) -ne 0 ]; then if [ "${LIBVIRT_URI}" != "qemu:///system" ]; then echo "1..0 # Skipped: Only valid for Qemu system driver" exit 0 fi - for name in `virsh list | awk '{print $2}'` + for name in `virsh list --all | awk '{print $2}'` do case ${name} in tck*) - if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" -o \ - $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then + if [ "x${LIBVIRT_TCK_AUTOCLEAN}" = "x1" ] || \ + [ $(($flags & $FLAG_FORCE_CLEAN)) -ne 0 ]; then res=$(virsh destroy ${name} 2>&1) + rc1=$? res=$(virsh undefine ${name} 2>&1) - if [ $? -ne 0 ]; then + rc2=$? + if [ $rc1 -ne 0 ] && [ $rc2 -ne 0 ]; then echo "Bail out! Could not undefine nwfiler ${name}: ${res}" exit 0 fi @@ -550,35 +550,34 @@ function main() { do case ${name} in tck*) - if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" -o \ - $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then + if [ "x${LIBVIRT_TCK_AUTOCLEAN}" = "x1" ] || \ + [ $(($flags & $FLAG_FORCE_CLEAN)) -ne 0 ]; then res=$(virsh nwfilter-undefine ${name} 2>&1) if [ $? -ne 0 ]; then - echo "Bail out! Could not undefine domain ${name}: ${res}" + echo "Bail out! Could not undefine filter ${name}: ${res}" exit 1 fi else - echo "Bail out! Domain ${name} already exists, use --force to clean" + echo "Bail out! Filter ${name} already exists, use --force to clean" exit 1 fi esac done fi - if [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ]; then - pushd ${PWD} > /dev/null + if [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ]; then + curdir="${PWD}" . test-lib.sh if [ $? -ne 0 ]; then exit 1 fi test_intro $this_test - popd > /dev/null + cd "${curdir}" + [ $? -ne 0 ] && echo "cd failed" && exit 1 fi - res=$(${VIRSH} capabilities 2>&1) - - vm1="tck-testvm${RANDOM}" - vm2="tck-testvm${RANDOM}" + vm1="tck-test$$1" + vm2="tck-test$$2" createTestFilters "${flags}" if [ $? -ne 0 ]; then Index: libvirt-tck/scripts/networks/100-apply-verify-host.t =================================================================== --- libvirt-tck.orig/scripts/networks/100-apply-verify-host.t +++ libvirt-tck/scripts/networks/100-apply-verify-host.t @@ -1,10 +1,5 @@ -#!/bin/bash +#!/bin/sh -pwd=$(dirname $0) +pwd=$(dirname -- "$0") -pushd ${PWD} > /dev/null - -cd ${pwd} -bash ./networkApplyTest.sh --tap-test - -popd > /dev/null +(cd -- "${pwd}"; sh ./networkApplyTest.sh --tap-test) Index: libvirt-tck/scripts/nwfilter/050-apply-verify-host.t =================================================================== --- libvirt-tck.orig/scripts/nwfilter/050-apply-verify-host.t +++ libvirt-tck/scripts/nwfilter/050-apply-verify-host.t @@ -1,10 +1,5 @@ -#!/bin/bash +#!/bin/sh -pwd=$(dirname $0) +pwd=$(dirname -- "$0") -pushd ${PWD} > /dev/null - -cd ${pwd} -bash ./nwfilter2vmtest.sh --tap-test --noattach - -popd > /dev/null \ No newline at end of file +(cd -- "${pwd}"; sh ./nwfilter2vmtest.sh --tap-test --noattach) Index: libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.dat =================================================================== --- libvirt-tck.orig/scripts/networks/networkxml2hostout/tck-testnet-1.dat +++ libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.dat @@ -1,17 +1,17 @@ -#iptables -t nat -L -n | grep ' 10\\.1\\.2\\.' +#iptables -t nat -L -n | grep ' 10\.1\.2\.' MASQUERADE tcp -- 10.1.2.0/24 !10.1.2.0/24 masq ports: 1024-65535 MASQUERADE udp -- 10.1.2.0/24 !10.1.2.0/24 masq ports: 1024-65535 MASQUERADE all -- 10.1.2.0/24 !10.1.2.0/24 -#iptables -n -L FORWARD | grep ' 10\\.1\\.2\\.' +#iptables -n -L FORWARD | grep ' 10\.1\.2\.' ACCEPT all -- 0.0.0.0/0 10.1.2.0/24 state RELATED,ESTABLISHED ACCEPT all -- 10.1.2.0/24 0.0.0.0/0 -#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*\$dnsmasq[[:print:]*]\$|\\1|p' +#ps aux | sed -n '/dnsmasq .*10\.1\.2\./ s|.*$dnsmasq[[:print:]*]$|\1|p' dnsmasq --strict-order --bind-interfaces --pid-file=/var/run/libvirt/network/tck-testnet.pid --conf-file= --listen-address 10.1.2.1 --except-interface lo --dhcp-range 10.1.2.2,10.1.2.254 --dhcp-lease-max=253 --dhcp-no-override -#route -n | grep '10\\.1\\.2\\.' +#route -n | grep '10\.1\.2\.' 10.1.2.0 0.0.0.0 255.255.255.0 U 0 0 0 tck-testbr #brctl show | grep tck-testbr tck-testbr 8000.000000000000 yes -#ifconfig tck-testbr | grep ':10\\.1\\.2\\.' +#ifconfig tck-testbr | grep ':10\.1\.2\.' inet addr:10.1.2.1 Bcast:10.1.2.255 Mask:255.255.255.0 #virsh net-list | grep tck-testnet tck-testnet active no Index: libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-2.dat =================================================================== --- libvirt-tck.orig/scripts/networks/networkxml2hostout/tck-testnet-2.dat +++ libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-2.dat @@ -1,14 +1,14 @@ -#iptables -L FORWARD -n | grep ' 10\\.1\\.2\\.' +#iptables -L FORWARD -n | grep ' 10\.1\.2\.' ACCEPT all -- 0.0.0.0/0 10.1.2.0/24 ACCEPT all -- 10.1.2.0/24 0.0.0.0/0 -#iptables -t nat -L -n | grep ' 10\\.1\\.2\\.' -#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*\$dnsmasq[[:print:]*]\$|\\1|p' +#iptables -t nat -L -n | grep ' 10\.1\.2\.' +#ps aux | sed -n '/dnsmasq .*10\.1\.2\./ s|.*$dnsmasq[[:print:]*]$|\1|p' dnsmasq --strict-order --bind-interfaces --pid-file=/var/run/libvirt/network/tck-testnet.pid --conf-file= --listen-address 10.1.2.1 --except-interface lo --dhcp-range 10.1.2.2,10.1.2.254 --dhcp-lease-max=253 --dhcp-no-override -#route -n | grep '10\\.1\\.2\\.' +#route -n | grep '10\.1\.2\.' 10.1.2.0 0.0.0.0 255.255.255.0 U 0 0 0 tck-testbr #brctl show | grep tck-testbr tck-testbr 8000.000000000000 yes -#ifconfig tck-testbr | grep ':10\\.1\\.2\\.' +#ifconfig tck-testbr | grep ':10\.1\.2\.' inet addr:10.1.2.1 Bcast:10.1.2.255 Mask:255.255.255.0 #virsh net-list | grep tck-testnet tck-testnet active no Index: libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.post.dat =================================================================== --- libvirt-tck.orig/scripts/networks/networkxml2hostout/tck-testnet-1.post.dat +++ libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.post.dat @@ -1,7 +1,7 @@ -#iptables -t nat -L -n | grep ' 10\\.1\\.2\\.' -#iptables -n -L FORWARD | grep ' 10\\.1\\.2\\.' -#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*\$dnsmasq[[:print:]*]\$|\\1|p' -#route -n | grep '10\\.1\\.2\\.' +#iptables -t nat -L -n | grep ' 10\.1\.2\.' +#iptables -n -L FORWARD | grep ' 10\.1\.2\.' +#ps aux | sed -n '/dnsmasq .*10\.1\.2\./ s|.*$dnsmasq[[:print:]*]$|\1|p' +#route -n | grep '10\.1\.2\.' #brctl show | grep tck-testbr -#ifconfig tck-testbr 2>/dev/null| grep ':10\\.1\\.2\\.' +#ifconfig tck-testbr 2>/dev/null| grep ':10\.1\.2\.' #virsh net-list | grep tck-testnet Index: libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-2.post.dat =================================================================== --- libvirt-tck.orig/scripts/networks/networkxml2hostout/tck-testnet-2.post.dat +++ libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-2.post.dat @@ -1,7 +1,7 @@ -#iptables -t nat -L -n | grep ' 10\\.1\\.2' -#iptables -n -L FORWARD | grep ' 10\\.1\\.2' -#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*\$dnsmasq[[:print:]*]\$|\\1|p' -#route -n | grep '10\\.1\\.2\\.' +#iptables -t nat -L -n | grep ' 10\.1\.2' +#iptables -n -L FORWARD | grep ' 10\.1\.2' +#ps aux | sed -n '/dnsmasq .*10\.1\.2\./ s|.*$dnsmasq[[:print:]*]$|\1|p' +#route -n | grep '10\.1\.2\.' #brctl show | grep tck-testbr -#ifconfig tck-testbr 2>/dev/null | grep ':10\\.1\\.2\\.' +#ifconfig tck-testbr 2>/dev/null | grep ':10\.1\.2\.' #virsh net-list | grep tck-testnet Index: libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/comment-test.fwall =================================================================== --- libvirt-tck.orig/scripts/nwfilter/nwfilterxml2fwallout/comment-test.fwall +++ libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/comment-test.fwall @@ -33,21 +33,21 @@ Chain FI-vnet0 (1 references) target prot opt source destination RETURN tcp ::/0 a:b:c::/128 /* tcp/ipv6 rule */ tcp spts:256:4369 dpts:32:33 state ESTABLISHED ctdir ORIGINAL RETURN udp ::/0 ::/0 /* `ls`;${COLUMNS};$(ls);"test";&'3 spaces' */ state ESTABLISHED ctdir ORIGINAL -RETURN sctp ::/0 ::/0 /* comment with lone ', `, ", `, \\, $x, and two spaces */ state ESTABLISHED ctdir ORIGINAL +RETURN sctp ::/0 ::/0 /* comment with lone ', `, ", `, \, $x, and two spaces */ state ESTABLISHED ctdir ORIGINAL RETURN ah ::/0 ::/0 /* tmp=`mktemp`; echo ${RANDOM} > ${tmp} ; cat < ${tmp}; rm -f ${tmp} */ state ESTABLISHED ctdir ORIGINAL #ip6tables -L FO-vnet0 -n Chain FO-vnet0 (1 references) target prot opt source destination ACCEPT tcp a:b:c::/128 ::/0 MAC 01:02:03:04:05:06 /* tcp/ipv6 rule */ tcp spts:32:33 dpts:256:4369 state NEW,ESTABLISHED ctdir REPLY ACCEPT udp ::/0 ::/0 /* `ls`;${COLUMNS};$(ls);"test";&'3 spaces' */ state NEW,ESTABLISHED ctdir REPLY -ACCEPT sctp ::/0 ::/0 /* comment with lone ', `, ", `, \\, $x, and two spaces */ state NEW,ESTABLISHED ctdir REPLY +ACCEPT sctp ::/0 ::/0 /* comment with lone ', `, ", `, \, $x, and two spaces */ state NEW,ESTABLISHED ctdir REPLY ACCEPT ah ::/0 ::/0 /* tmp=`mktemp`; echo ${RANDOM} > ${tmp} ; cat < ${tmp}; rm -f ${tmp} */ state NEW,ESTABLISHED ctdir REPLY #ip6tables -L HI-vnet0 -n Chain HI-vnet0 (1 references) target prot opt source destination RETURN tcp ::/0 a:b:c::/128 /* tcp/ipv6 rule */ tcp spts:256:4369 dpts:32:33 state ESTABLISHED ctdir ORIGINAL RETURN udp ::/0 ::/0 /* `ls`;${COLUMNS};$(ls);"test";&'3 spaces' */ state ESTABLISHED ctdir ORIGINAL -RETURN sctp ::/0 ::/0 /* comment with lone ', `, ", `, \\, $x, and two spaces */ state ESTABLISHED ctdir ORIGINAL +RETURN sctp ::/0 ::/0 /* comment with lone ', `, ", `, \, $x, and two spaces */ state ESTABLISHED ctdir ORIGINAL RETURN ah ::/0 ::/0 /* tmp=`mktemp`; echo ${RANDOM} > ${tmp} ; cat < ${tmp}; rm -f ${tmp} */ state ESTABLISHED ctdir ORIGINAL #ip6tables -L libvirt-host-in -n | grep vnet0 | tr -s " " HI-vnet0 all ::/0 ::/0 [goto] PHYSDEV match --physdev-in vnet0

2 2

[libvirt] [PATCH v2] deprecate fclose() and introduce VIR_{FORCE_}FCLOSE()
by Stefan Berger 15 Nov '10

15 Nov '10

V2: - following Eric's suggestion, deprecating fdclose(), introducing VIR_FDCLOSE() - following some other nits that Eric pointed out - replaced some more fclose () I previously had missed (last 2 files in patch) Similarly to deprecating close(), I am now deprecating fclose() and introduce VIR_FORCE_FCLOSE() and VIR_FCLOSE(). Also, fdopen() is replaced with VIR_FDOPEN(). Most of the files are opened in read-only mode, so usage of VIR_FORCE_CLOSE() seemed appropriate. Others that are opened in write mode already had the fclose() < 0 check and I converted those to VIR_FCLOSE() < 0. I did not find occurrences of possible double-closed files on the way. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- HACKING | 33 +++++++++++++++++++++++++++------ daemon/libvirtd.c | 6 +++--- docs/hacking.html.in | 36 ++++++++++++++++++++++++++++-------- src/libvirt_private.syms | 2 ++ src/nodeinfo.c | 8 ++++---- src/openvz/openvz_conf.c | 12 ++++++------ src/openvz/openvz_driver.c | 4 ++-- src/qemu/qemu_driver.c | 4 ++-- src/storage/storage_backend.c | 18 +++++++----------- src/storage/storage_backend_fs.c | 4 ++-- src/storage/storage_backend_iscsi.c | 9 +++------ src/storage/storage_backend_scsi.c | 2 +- src/uml/uml_driver.c | 8 ++++---- src/util/cgroup.c | 10 +++++----- src/util/dnsmasq.c | 5 +++-- src/util/files.c | 32 ++++++++++++++++++++++++++++++++ src/util/files.h | 6 ++++++ src/util/macvtap.c | 4 ++-- src/util/stats_linux.c | 3 ++- src/util/util.c | 10 ++++------ src/xen/block_stats.c | 3 ++- src/xen/xen_driver.c | 7 ++++--- src/xen/xen_hypervisor.c | 8 +++----- tests/nodeinfotest.c | 5 +++-- tests/testutils.c | 8 ++++---- tests/xencapstest.c | 7 +++---- 26 files changed, 164 insertions(+), 90 deletions(-) Index: libvirt-acl/src/util/files.c =================================================================== --- libvirt-acl.orig/src/util/files.c +++ libvirt-acl/src/util/files.c @@ -44,3 +44,35 @@ int virClose(int *fdptr, bool preserve_e return rc; } + + +int virFclose(FILE **file, bool preserve_errno) +{ + int saved_errno; + int rc = 0; + + if (*file) { + if (preserve_errno) + saved_errno = errno; + rc = fclose(*file); + *file = NULL; + if (preserve_errno) + errno = saved_errno; + } + + return rc; +} + + +FILE *virFdopen(int *fdptr, const char *mode) +{ + FILE *file = NULL; + + if (*fdptr >= 0) { + file = fdopen(*fdptr, mode); + if (file) + *fdptr = -1; + } + + return file; +} Index: libvirt-acl/src/util/files.h =================================================================== --- libvirt-acl.orig/src/util/files.h +++ libvirt-acl/src/util/files.h @@ -27,6 +27,7 @@ # define __VIR_FILES_H_ # include <stdbool.h> +# include <stdio.h> # include "internal.h" # include "ignore-value.h" @@ -34,13 +35,18 @@ /* Don't call this directly - use the macros below */ int virClose(int *fdptr, bool preserve_errno) ATTRIBUTE_RETURN_CHECK; +int virFclose(FILE **file, bool preserve_errno) ATTRIBUTE_RETURN_CHECK; +FILE *virFdopen(int *fdptr, const char *mode); /* For use on normal paths; caller must check return value, and failure sets errno per close(). */ # define VIR_CLOSE(FD) virClose(&(FD), false) +# define VIR_FCLOSE(FILE) virFclose(&(FILE), false) +# define VIR_FDOPEN(FD, MODE) virFdopen(&(FD), MODE) /* For use on cleanup paths; errno is unaffected by close, and no return value to worry about. */ # define VIR_FORCE_CLOSE(FD) ignore_value(virClose(&(FD), true)) +# define VIR_FORCE_FCLOSE(FILE) ignore_value(virFclose(&(FILE), true)) #endif /* __VIR_FILES_H */ Index: libvirt-acl/src/libvirt_private.syms =================================================================== --- libvirt-acl.orig/src/libvirt_private.syms +++ libvirt-acl/src/libvirt_private.syms @@ -344,6 +344,8 @@ virFDStreamCreateFile; # files.h virClose; +virFclose; +virFdopen; # hash.h Index: libvirt-acl/daemon/libvirtd.c =================================================================== --- libvirt-acl.orig/daemon/libvirtd.c +++ libvirt-acl/daemon/libvirtd.c @@ -512,7 +512,7 @@ static int qemudWritePidFile(const char return -1; } - if (!(fh = fdopen(fd, "w"))) { + if (!(fh = VIR_FDOPEN(fd, "w"))) { VIR_ERROR(_("Failed to fdopen pid file '%s' : %s"), pidFile, virStrerror(errno, ebuf, sizeof ebuf)); VIR_FORCE_CLOSE(fd); @@ -522,11 +522,11 @@ static int qemudWritePidFile(const char if (fprintf(fh, "%lu\n", (unsigned long)getpid()) < 0) { VIR_ERROR(_("%s: Failed to write to pid file '%s' : %s"), argv0, pidFile, virStrerror(errno, ebuf, sizeof ebuf)); - fclose(fh); + VIR_FORCE_FCLOSE(fh); return -1; } - if (fclose(fh) == EOF) { + if (VIR_FCLOSE(fh) == EOF) { VIR_ERROR(_("%s: Failed to close pid file '%s' : %s"), argv0, pidFile, virStrerror(errno, ebuf, sizeof ebuf)); return -1; Index: libvirt-acl/src/nodeinfo.c =================================================================== --- libvirt-acl.orig/src/nodeinfo.c +++ libvirt-acl/src/nodeinfo.c @@ -46,6 +46,7 @@ #include "virterror_internal.h" #include "count-one-bits.h" #include "intprops.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -102,8 +103,7 @@ get_cpu_value(unsigned int cpu, const ch } cleanup: - if (pathfp) - fclose(pathfp); + VIR_FORCE_FCLOSE(pathfp); VIR_FREE(path); return value; @@ -155,7 +155,7 @@ static unsigned long count_thread_siblin } cleanup: - fclose(pathfp); + VIR_FORCE_FCLOSE(pathfp); VIR_FREE(path); return ret; @@ -329,7 +329,7 @@ int nodeGetInfo(virConnectPtr conn ATTRI return -1; } ret = linuxNodeInfoCPUPopulate(cpuinfo, nodeinfo); - fclose(cpuinfo); + VIR_FORCE_FCLOSE(cpuinfo); if (ret < 0) return -1; Index: libvirt-acl/src/openvz/openvz_conf.c =================================================================== --- libvirt-acl.orig/src/openvz/openvz_conf.c +++ libvirt-acl/src/openvz/openvz_conf.c @@ -528,7 +528,7 @@ int openvzLoadDomains(struct openvz_driv dom = NULL; } - fclose(fp); + VIR_FORCE_FCLOSE(fp); return 0; @@ -536,7 +536,7 @@ int openvzLoadDomains(struct openvz_driv virReportOOMError(); cleanup: - fclose(fp); + VIR_FORCE_FCLOSE(fp); if (dom) virDomainObjUnref(dom); return -1; @@ -906,10 +906,10 @@ openvzSetDefinedUUID(int vpsid, unsigned virUUIDFormat(uuid, uuidstr); - /* Record failure if fprintf or fclose fails, + /* Record failure if fprintf or VIR_FCLOSE fails, and be careful always to close the stream. */ - if ((fprintf(fp, "\n#UUID: %s\n", uuidstr) < 0) - + (fclose(fp) == EOF)) + if ((fprintf(fp, "\n#UUID: %s\n", uuidstr) < 0) || + (VIR_FCLOSE(fp) == EOF)) goto cleanup; } @@ -996,7 +996,7 @@ int openvzGetVEID(const char *name) { } ok = fscanf(fp, "%d\n", &veid ) == 1; - fclose(fp); + VIR_FORCE_FCLOSE(fp); if (ok && veid >= 0) return veid; Index: libvirt-acl/src/openvz/openvz_driver.c =================================================================== --- libvirt-acl.orig/src/openvz/openvz_driver.c +++ libvirt-acl/src/openvz/openvz_driver.c @@ -154,7 +154,7 @@ openvzDomainDefineCmd(const char *args[] max_veid = veid; } } - fclose(fp); + VIR_FORCE_FCLOSE(fp); if (max_veid == 0) { max_veid = 100; @@ -189,7 +189,7 @@ no_memory: return -1; cleanup: - fclose(fp); + VIR_FORCE_FCLOSE(fp); return -1; #undef ADD_ARG Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -4588,7 +4588,7 @@ static int qemudGetProcessInfo(unsigned /* startstack -> processor */ "%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d", &usertime, &systime, &cpu) != 3) { - fclose(pidinfo); + VIR_FORCE_FCLOSE(pidinfo); VIR_WARN0("cannot parse process status data"); errno = -EINVAL; return -1; @@ -4608,7 +4608,7 @@ static int qemudGetProcessInfo(unsigned VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d", pid, tid, usertime, systime, cpu); - fclose(pidinfo); + VIR_FORCE_FCLOSE(pidinfo); return 0; } Index: libvirt-acl/src/storage/storage_backend.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend.c +++ libvirt-acl/src/storage/storage_backend.c @@ -1398,7 +1398,7 @@ virStorageBackendRunProgRegex(virStorage goto cleanup; } - if ((list = fdopen(fd, "r")) == NULL) { + if ((list = VIR_FDOPEN(fd, "r")) == NULL) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot read fd")); goto cleanup; @@ -1458,10 +1458,8 @@ virStorageBackendRunProgRegex(virStorage VIR_FREE(reg); - if (list) - fclose(list); - else - VIR_FORCE_CLOSE(fd); + VIR_FORCE_FCLOSE(list); + VIR_FORCE_CLOSE(fd); while ((err = waitpid(child, &exitstatus, 0) == -1) && errno == EINTR); @@ -1531,9 +1529,9 @@ virStorageBackendRunProgNul(virStoragePo goto cleanup; } - if ((fp = fdopen(fd, "r")) == NULL) { + if ((fp = VIR_FDOPEN(fd, "r")) == NULL) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot read fd")); + "%s", _("cannot open file using fd")); goto cleanup; } @@ -1573,10 +1571,8 @@ virStorageBackendRunProgNul(virStoragePo VIR_FREE(v[i]); VIR_FREE(v); - if (fp) - fclose (fp); - else - VIR_FORCE_CLOSE(fd); + VIR_FORCE_FCLOSE(fp); + VIR_FORCE_CLOSE(fd); while ((w_err = waitpid (child, &exitstatus, 0) == -1) && errno == EINTR) /* empty */ ; Index: libvirt-acl/src/storage/storage_backend_fs.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend_fs.c +++ libvirt-acl/src/storage/storage_backend_fs.c @@ -284,12 +284,12 @@ virStorageBackendFileSystemIsMounted(vir while ((getmntent_r(mtab, &ent, buf, sizeof(buf))) != NULL) { if (STREQ(ent.mnt_dir, pool->def->target.path)) { - fclose(mtab); + VIR_FORCE_FCLOSE(mtab); return 1; } } - fclose(mtab); + VIR_FORCE_FCLOSE(mtab); return 0; } Index: libvirt-acl/src/storage/storage_backend_iscsi.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend_iscsi.c +++ libvirt-acl/src/storage/storage_backend_iscsi.c @@ -188,7 +188,7 @@ virStorageBackendIQNFound(virStoragePool goto out; } - if ((fp = fdopen(fd, "r")) == NULL) { + if ((fp = VIR_FDOPEN(fd, "r")) == NULL) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to open stream for file descriptor " "when reading output from '%s': '%s'"), @@ -235,11 +235,8 @@ out: } VIR_FREE(line); - if (fp != NULL) { - fclose(fp); - } else { - VIR_FORCE_CLOSE(fd); - } + VIR_FORCE_FCLOSE(fp); + VIR_FORCE_CLOSE(fd); return ret; } Index: libvirt-acl/src/storage/storage_backend_scsi.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend_scsi.c +++ libvirt-acl/src/storage/storage_backend_scsi.c @@ -70,7 +70,7 @@ getDeviceType(uint32_t host, } gottype = fgets(typestr, 3, typefile); - fclose(typefile); + VIR_FORCE_FCLOSE(typefile); if (gottype == NULL) { virReportSystemError(errno, Index: libvirt-acl/src/uml/uml_driver.c =================================================================== --- libvirt-acl.orig/src/uml/uml_driver.c +++ libvirt-acl/src/uml/uml_driver.c @@ -587,11 +587,11 @@ reopen: if (fscanf(file, "%d", &vm->pid) != 1) { errno = EINVAL; - fclose(file); + VIR_FORCE_FCLOSE(file); goto cleanup; } - if (fclose(file) < 0) + if (VIR_FCLOSE(file) < 0) goto cleanup; rc = 0; @@ -1096,7 +1096,7 @@ static int umlGetProcessInfo(unsigned lo if (fscanf(pidinfo, "%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu %llu", &usertime, &systime) != 2) { umlDebug("not enough arg"); - fclose(pidinfo); + VIR_FORCE_FCLOSE(pidinfo); return -1; } @@ -1109,7 +1109,7 @@ static int umlGetProcessInfo(unsigned lo umlDebug("Got %llu %llu %llu", usertime, systime, *cpuTime); - fclose(pidinfo); + VIR_FORCE_FCLOSE(pidinfo); return 0; } Index: libvirt-acl/src/util/cgroup.c =================================================================== --- libvirt-acl.orig/src/util/cgroup.c +++ libvirt-acl/src/util/cgroup.c @@ -31,6 +31,7 @@ #include "memory.h" #include "cgroup.h" #include "logging.h" +#include "files.h" #define CGROUP_MAX_VAL 512 @@ -127,13 +128,12 @@ static int virCgroupDetectMounts(virCgro } } - fclose(mounts); + VIR_FORCE_FCLOSE(mounts); return 0; no_memory: - if (mounts) - fclose(mounts); + VIR_FORCE_FCLOSE(mounts); return -ENOMEM; } @@ -192,12 +192,12 @@ static int virCgroupDetectPlacement(virC } } - fclose(mapping); + VIR_FORCE_FCLOSE(mapping); return 0; no_memory: - fclose(mapping); + VIR_FORCE_FCLOSE(mapping); return -ENOMEM; } Index: libvirt-acl/src/util/dnsmasq.c =================================================================== --- libvirt-acl.orig/src/util/dnsmasq.c +++ libvirt-acl/src/util/dnsmasq.c @@ -44,6 +44,7 @@ #include "memory.h" #include "virterror_internal.h" #include "logging.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_NETWORK #define DNSMASQ_HOSTSFILE_SUFFIX "hostsfile" @@ -171,7 +172,7 @@ hostsfileWrite(const char *path, for (i = 0; i < nhosts; i++) { if (fputs(hosts[i].host, f) == EOF || fputc('\n', f) == EOF) { rc = errno; - fclose(f); + VIR_FORCE_FCLOSE(f); if (istmp) unlink(tmp); @@ -180,7 +181,7 @@ hostsfileWrite(const char *path, } } - if (fclose(f) == EOF) { + if (VIR_FCLOSE(f) == EOF) { rc = errno; goto cleanup; } Index: libvirt-acl/src/util/macvtap.c =================================================================== --- libvirt-acl.orig/src/util/macvtap.c +++ libvirt-acl/src/util/macvtap.c @@ -436,11 +436,11 @@ int openTap(const char *ifname, virReportSystemError(errno, "%s",_("cannot determine macvtap's tap device " "interface index")); - fclose(file); + VIR_FORCE_FCLOSE(file); return -1; } - fclose(file); + VIR_FORCE_FCLOSE(file); if (snprintf(tapname, sizeof(tapname), "/dev/tap%d", ifindex) >= sizeof(tapname)) { Index: libvirt-acl/src/util/util.c =================================================================== --- libvirt-acl.orig/src/util/util.c +++ libvirt-acl/src/util/util.c @@ -1816,7 +1816,7 @@ int virFileWritePidPath(const char *pidf goto cleanup; } - if (!(file = fdopen(fd, "w"))) { + if (!(file = VIR_FDOPEN(fd, "w"))) { rc = errno; VIR_FORCE_CLOSE(fd); goto cleanup; @@ -1830,10 +1830,8 @@ int virFileWritePidPath(const char *pidf rc = 0; cleanup: - if (file && - fclose(file) < 0) { + if (VIR_FCLOSE(file) < 0) rc = errno; - } return rc; } @@ -1864,11 +1862,11 @@ int virFileReadPid(const char *dir, if (fscanf(file, "%d", pid) != 1) { rc = EINVAL; - fclose(file); + VIR_FORCE_FCLOSE(file); goto cleanup; } - if (fclose(file) < 0) { + if (VIR_FCLOSE(file) < 0) { rc = errno; goto cleanup; } Index: libvirt-acl/src/xen/xen_driver.c =================================================================== --- libvirt-acl.orig/src/xen/xen_driver.c +++ libvirt-acl/src/xen/xen_driver.c @@ -47,6 +47,7 @@ #include "pci.h" #include "uuid.h" #include "fdstream.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_XEN @@ -216,10 +217,10 @@ xenUnifiedProbe (void) return 1; #endif #ifdef __sun - FILE *fh; + int fd; - if (fh = fopen("/dev/xen/domcaps", "r")) { - fclose(fh); + if (fd = open("/dev/xen/domcaps", O_RDONLY)) { + VIR_FORCE_CLOSE(fd); return 1; } #endif Index: libvirt-acl/src/xen/xen_hypervisor.c =================================================================== --- libvirt-acl.orig/src/xen/xen_hypervisor.c +++ libvirt-acl/src/xen/xen_hypervisor.c @@ -2641,7 +2641,7 @@ xenHypervisorMakeCapabilities(virConnect capabilities = fopen ("/sys/hypervisor/properties/capabilities", "r"); if (capabilities == NULL) { if (errno != ENOENT) { - fclose(cpuinfo); + VIR_FORCE_FCLOSE(cpuinfo); virReportSystemError(errno, _("cannot read file %s"), "/sys/hypervisor/properties/capabilities"); @@ -2654,10 +2654,8 @@ xenHypervisorMakeCapabilities(virConnect cpuinfo, capabilities); - if (cpuinfo) - fclose(cpuinfo); - if (capabilities) - fclose(capabilities); + VIR_FORCE_FCLOSE(cpuinfo); + VIR_FORCE_FCLOSE(capabilities); return caps; #endif /* __sun */ Index: libvirt-acl/tests/nodeinfotest.c =================================================================== --- libvirt-acl.orig/tests/nodeinfotest.c +++ libvirt-acl/tests/nodeinfotest.c @@ -9,6 +9,7 @@ #include "internal.h" #include "nodeinfo.h" #include "util.h" +#include "files.h" #ifndef __linux__ @@ -49,10 +50,10 @@ static int linuxTestCompareFiles(const c fprintf(stderr, "\n%s\n", error->message); virFreeError(error); } - fclose(cpuinfo); + VIR_FORCE_FCLOSE(cpuinfo); return -1; } - fclose(cpuinfo); + VIR_FORCE_FCLOSE(cpuinfo); /* 'nodes' is filled using libnuma.so from current machine * topology, which makes it unsuitable for the test suite Index: libvirt-acl/tests/testutils.c =================================================================== --- libvirt-acl.orig/tests/testutils.c +++ libvirt-acl/tests/testutils.c @@ -180,26 +180,26 @@ int virtTestLoadFile(const char *file, if (fstat(fileno(fp), &st) < 0) { fprintf (stderr, "%s: failed to fstat: %s\n", file, strerror(errno)); - fclose(fp); + VIR_FORCE_FCLOSE(fp); return -1; } if (st.st_size > (buflen-1)) { fprintf (stderr, "%s: larger than buffer (> %d)\n", file, buflen-1); - fclose(fp); + VIR_FORCE_FCLOSE(fp); return -1; } if (st.st_size) { if (fread(*buf, st.st_size, 1, fp) != 1) { fprintf (stderr, "%s: read failed: %s\n", file, strerror(errno)); - fclose(fp); + VIR_FORCE_FCLOSE(fp); return -1; } } (*buf)[st.st_size] = '\0'; - fclose(fp); + VIR_FORCE_FCLOSE(fp); return st.st_size; } Index: libvirt-acl/tests/xencapstest.c =================================================================== --- libvirt-acl.orig/tests/xencapstest.c +++ libvirt-acl/tests/xencapstest.c @@ -9,6 +9,7 @@ #include "xml.h" #include "testutils.h" #include "xen/xen_hypervisor.h" +#include "files.h" static char *progname; static char *abs_srcdir; @@ -63,10 +64,8 @@ static int testCompareFiles(const char * fail: free(actualxml); - if (fp1) - fclose(fp1); - if (fp2) - fclose(fp2); + VIR_FORCE_FCLOSE(fp1); + VIR_FORCE_FCLOSE(fp2); virCapabilitiesFree(caps); return ret; Index: libvirt-acl/docs/hacking.html.in =================================================================== --- libvirt-acl.orig/docs/hacking.html.in +++ libvirt-acl/docs/hacking.html.in @@ -414,25 +414,45 @@ <h2><a name="file_handling">File handling</a></h2> <p> - Use of the close() API is deprecated in libvirt code base to help - avoiding double-closing of a file descriptor. Instead of this API, - use the macro from files.h + Usage of the <code>fdopen()</code>, <code>close()</code>, <code>fclose()</code> + APIs is deprecated in libvirt code base to help avoiding double-closing of files + or file descriptors, which is particulary dangerous in a multi-threaded + applications. Instead of these APIs, use the macros from files.h </p> - <ul> + <ul> + <li><p>eg opening a file from a file descriptor</p> + +<pre> + if ((file = VIR_FDOPEN(fd, "r")) == NULL) { + virReportSystemError(errno, "%s", + _("failed to open file from file descriptor")); + return -1; + } + /* fd is now invalid; only access the file using file variable */ +</pre></li> + <li><p>e.g. close a file descriptor</p> <pre> if (VIR_CLOSE(fd) < 0) { - virReportSystemError(errno, _("failed to close file")); + virReportSystemError(errno, "%s", _("failed to close file")); } -</pre> - </li> +</pre></li> + + <li><p>eg close a file</p> + +<pre> + if (VIR_FCLOSE(file) < 0) { + virReportSystemError(errno, "%s", _("failed to close file")); + } +</pre></li> - <li><p>eg close a file descriptor in an error path, without losing + <li><p>eg close a file or file descriptor in an error path, without losing the previous <code>errno</code> value</p> <pre> VIR_FORCE_CLOSE(fd); + VIR_FORCE_FCLOSE(file); </pre> </li> </ul> Index: libvirt-acl/HACKING =================================================================== --- libvirt-acl.orig/HACKING +++ libvirt-acl/HACKING @@ -339,22 +339,43 @@ routines, use the macros from memory.h File handling ============= -Use of the close() API is deprecated in libvirt code base to help avoiding -double-closing of a file descriptor. Instead of this API, use the macro from -files.h +Usage of the "fdopen()", "close()", "fclose()" APIs is deprecated in libvirt +code base to help avoiding double-closing of files or file descriptors, which +is particulary dangerous in a multi-threaded applications. Instead of these +APIs, use the macros from files.h + +- eg opening a file from a file descriptor + + if ((file = VIR_FDOPEN(fd, "r")) == NULL) { + virReportSystemError(errno, "%s", + _("failed to open file from file descriptor")); + return -1; + } + /* fd is now invalid; only access the file using file variable */ + + - e.g. close a file descriptor if (VIR_CLOSE(fd) < 0) { - virReportSystemError(errno, _("failed to close file")); + virReportSystemError(errno, "%s", _("failed to close file")); + } + + + +- eg close a file + + if (VIR_FCLOSE(file) < 0) { + virReportSystemError(errno, "%s", _("failed to close file")); } -- eg close a file descriptor in an error path, without losing the previous -"errno" value +- eg close a file or file descriptor in an error path, without losing the +previous "errno" value VIR_FORCE_CLOSE(fd); + VIR_FORCE_FCLOSE(file); Index: libvirt-acl/src/util/stats_linux.c =================================================================== --- libvirt-acl.orig/src/util/stats_linux.c +++ libvirt-acl/src/util/stats_linux.c @@ -25,6 +25,7 @@ # include "util.h" # include "stats_linux.h" # include "memory.h" +# include "files.h" # define VIR_FROM_THIS VIR_FROM_STATS_LINUX @@ -103,7 +104,7 @@ linuxDomainInterfaceStats(const char *pa return 0; } } - fclose (fp); + VIR_FORCE_FCLOSE(fp); virStatsError(VIR_ERR_INTERNAL_ERROR, "/proc/net/dev: Interface not found"); Index: libvirt-acl/src/xen/block_stats.c =================================================================== --- libvirt-acl.orig/src/xen/block_stats.c +++ libvirt-acl/src/xen/block_stats.c @@ -27,6 +27,7 @@ # include "util.h" # include "block_stats.h" # include "memory.h" +# include "files.h" # define VIR_FROM_THIS VIR_FROM_STATS_LINUX @@ -100,7 +101,7 @@ read_stat (const char *path) /* read, but don't bail out before closing */ i = fread (str, 1, sizeof str - 1, fp); - if (fclose (fp) != 0 /* disk error */ + if (VIR_FCLOSE(fp) != 0 /* disk error */ || i < 1) /* ensure we read at least one byte */ return -1;

2 1

[libvirt] [PATCH] Fix ipv4-addr definition in interface.rng
by Laine Stump 15 Nov '10

15 Nov '10

Eric Blake pointed out a deficiency in this regex when it was copy-pasted into network.rng. This is just propogating the fix back to its origin. --- docs/schemas/interface.rng | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/docs/schemas/interface.rng b/docs/schemas/interface.rng index a0df9ba..75893b5 100644 --- a/docs/schemas/interface.rng +++ b/docs/schemas/interface.rng @@ -439,7 +439,7 @@ <define name='ipv4-addr'> <data type='string'> - <param name="pattern">(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))\.){3}((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))</param> + <param name="pattern">(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9]))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9]))</param> </data> </define> -- 1.7.2.3

2 2

[libvirt] [PATCH] interface.rng: Make miimon and arpmon optional for bond interfaces
by Laine Stump 15 Nov '10

15 Nov '10

This has been optional in netcf for awhile, but the change hadn't been propogated to the libvirt copy of the RNG. --- I noticed this was out of date while updating the netcf RNG with the new regex for ipv4-address. Note that, although the diff is long, it's just adding <optional>...</optional> around the section, and re-indenting. Everything in the middle is only whitespace changes. docs/schemas/interface.rng | 77 +++++++++++++++++++++++--------------------- 1 files changed, 40 insertions(+), 37 deletions(-) diff --git a/docs/schemas/interface.rng b/docs/schemas/interface.rng index 75893b5..4dd1bb4 100644 --- a/docs/schemas/interface.rng +++ b/docs/schemas/interface.rng @@ -177,43 +177,46 @@ xmit_hash_policy (since 2.6.3/3.2.2) --> - <choice> - <element name="miimon"> -  - <attribute name="freq"><ref name="uint"/></attribute> - <optional> - <attribute name="downdelay"><ref name="uint"/></attribute> - </optional> - <optional> - <attribute name="updelay"><ref name="uint"/></attribute> - </optional> - <optional> -  - <attribute name="carrier"> - <choice> -  - <value>ioctl</value> -  - <value>netif</value> - </choice> - </attribute> - </optional> - </element> - <element name="arpmon"> - <attribute name="interval"><ref name="uint"/></attribute> - <attribute name="target"><ref name="ipv4-addr"/></attribute> - <optional> - <attribute name="validate"> - <choice> - <value>none</value> - <value>active</value> - <value>backup</value> - <value>all</value> - </choice> - </attribute> - </optional> - </element> - </choice> + <optional> + <choice> + <element name="miimon"> +  + <attribute name="freq"><ref name="uint"/></attribute> + <optional> + <attribute name="downdelay"><ref name="uint"/></attribute> + </optional> + <optional> + <attribute name="updelay"><ref name="uint"/></attribute> + </optional> + <optional> +  + <attribute name="carrier"> + <choice> +  + <value>ioctl</value> +  + <value>netif</value> + </choice> + </attribute> + </optional> + </element> + <element name="arpmon"> + <attribute name="interval"><ref name="uint"/></attribute> + <attribute name="target"><ref name="ipv4-addr"/></attribute> + <optional> + <attribute name="validate"> + <choice> + <value>none</value> + <value>active</value> + <value>backup</value> + <value>all</value> + </choice> + </attribute> + </optional> + </element> + </choice> + </optional> + <oneOrMore>  <ref name="bare-ethernet-interface"/> -- 1.7.2.3

2 2

[libvirt] [PATCH] [TCK] Convert scripts to also run with dash
by Stefan Berger 15 Nov '10

15 Nov '10

I am converting the shell-scripted tap tests to also run using the dash shell, fixing some other problems on the way as well. Two of the tests' data files contained '\\1' in an sed command. This created char(1) with the dash, '\1' with the bash. I found it easier to replace the sed command with a different one than figuring out how to solve this escape problem so it works for both shells. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- scripts/networks/100-apply-verify-host.t | 9 scripts/networks/networkApplyTest.sh | 133 +++----- scripts/networks/networkxml2hostout/tck-testnet-1.dat | 2 scripts/networks/networkxml2hostout/tck-testnet-2.dat | 2 scripts/nwfilter/050-apply-verify-host.t | 9 scripts/nwfilter/nwfilter2vmtest.sh | 298 ++++++++---------- 6 files changed, 215 insertions(+), 238 deletions(-) Index: libvirt-tck/scripts/networks/networkApplyTest.sh =================================================================== --- libvirt-tck.orig/scripts/networks/networkApplyTest.sh +++ libvirt-tck/scripts/networks/networkApplyTest.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh VIRSH=virsh @@ -33,8 +33,8 @@ TAP_FAIL_LIST="" TAP_FAIL_CTR=0 TAP_TOT_CTR=0 -function usage() { - local cmd="$0" +usage() { + cmd="$0" cat <<EOF Usage: ${cmd} [--help|-h|-?] [--noattach] [--wait] [--verbose] [--libvirt-test] [--tap-test] @@ -56,37 +56,37 @@ EOF } -function tap_fail() { - echo "not ok $1 - ${2:0:66}" - TAP_FAIL_LIST+="$1 " - ((TAP_FAIL_CTR++)) - ((TAP_TOT_CTR++)) +tap_fail() { + txt=$(echo "$2" | gawk '{print substr($0,1,66)}') + echo "not ok $1 - ${txt}" + TAP_FAIL_LIST="$TAP_FAIL_LIST $1 " + TAP_FAIL_CTR=$((TAP_FAIL_CTR+1)) + TAP_TOT_CTR=$((TAP_TOT_CTR+1)) } -function tap_pass() { - echo "ok $1 - ${2:0:70}" - ((TAP_TOT_CTR++)) +tap_pass() { + txt=$(echo "$2" | gawk '{print substr($0,1,66)}') + echo "ok $1 - ${txt}" + TAP_TOT_CTR=$((TAP_TOT_CTR+1)) } -function tap_final() { - local okay - +tap_final() { [ -n "${TAP_FAIL_LIST}" ] && echo "FAILED tests ${TAP_FAIL_LIST}" okay=$(echo "($TAP_TOT_CTR-$TAP_FAIL_CTR)*100/$TAP_TOT_CTR" | bc -l) - echo "Failed ${TAP_FAIL_CTR}/${TAP_TOT_CTR} tests, ${okay:0:5}% okay" + txt=$(echo $okay | gawk '{print substr($0,1,5)}') + echo "Failed ${TAP_FAIL_CTR}/${TAP_TOT_CTR} tests, ${txt}% okay" } # A wrapper for mktemp in case it does not exist # Echos the name of a secure temporary directory. -function mktmpdir() { - local tmp +mktmpdir() { { - tmp=$( (umask 077 && mktemp -d ./nwfvmtest.XXXXXX) 2>/dev/null) && + tmp=$( (umask 077 && mktemp -d ./nwtst.XXXXXX) 2>/dev/null) && test -n "$tmp" && test -d "$tmp" } || { - tmp=./nwfvmtest$$-$RANDOM + tmp=./nwtst$$-$RANDOM (umask 077 && mkdir "$tmp") } || { echo "failed to create secure temporary directory" >&2; exit 1; } echo "${tmp}" @@ -94,12 +94,11 @@ function mktmpdir() { } -function checkExpectedOutput() { - local xmlfile="$1" - local datafile="$2" - local flags="$3" - local skipregex="$4" - local cmd line tmpdir tmpfile tmpfile2 skip +checkExpectedOutput() { + xmlfile="$1" + datafile="$2" + flags="$3" + skipregex="$4" tmpdir=$(mktmpdir) tmpfile=$tmpdir/file @@ -112,8 +111,8 @@ function checkExpectedOutput() { exec 4<"${datafile}" - read <&4 - line="${REPLY}" + IFS="" + read line <&4 while [ "x${line}x" != "xx" ]; do cmd=$(echo "${line##\#}") @@ -128,10 +127,11 @@ function checkExpectedOutput() { : >"${tmpfile2}" while :; do - read <&4 - line="${REPLY}" + read line <&4 + + letter=$(echo ${line} | gawk '{print substr($1,1,1)}') - if [ "${line:0:1}" == "#" ] || [ "x${line}x" == "xx" ]; then + if [ "x${letter}x" = "x#x" ] || [ "x${line}x" = "xx" ]; then if [ ${skip} -ne 0 ]; then break @@ -144,18 +144,18 @@ function checkExpectedOutput() { echo "FAIL ${xmlfile} : ${cmd}" diff "${tmpfile}" "${tmpfile2}" fi - ((failctr++)) + failctr=$((failctr+1)) if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "tmp files: $tmpfile, $tmpfile2" echo "Press enter" - read + read enter fi [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \ test_result $((passctr+failctr)) "" 1 [ $((flags & FLAG_TAP_TEST)) -ne 0 ] && \ tap_fail $((passctr+failctr)) "${xmlfile} : ${cmd}" else - ((passctr++)) + passctr=$((passctr+1)) [ $((flags & FLAG_VERBOSE)) -ne 0 ] && \ echo "PASS ${xmlfile} : ${cmd}" [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \ @@ -177,12 +177,11 @@ function checkExpectedOutput() { } -function doTest() { - local xmlfile="$1" - local datafile="$2" - local postdatafile="$3" - local flags="$4" - local netname +doTest() { + xmlfile="$1" + datafile="$2" + postdatafile="$3" + flags="$4" if [ ! -r "${xmlfile}" ]; then echo "FAIL : Cannot access filter XML file ${xmlfile}." @@ -205,24 +204,20 @@ function doTest() { } -function runTests() { - local xmldir="$1" - local hostoutdir="$2" - local flags="$3" - local datafiles f c - local tap_total=0 ctr=0 - - pushd "${PWD}" > /dev/null - cd "${hostoutdir}" - datafiles=$(ls *.dat) - popd > /dev/null +runTests() { + xmldir="$1" + hostoutdir="$2" + flags="$3" + tap_total=0 ctr=0 + + datafiles=$(cd "${hostoutdir}";ls *.dat) if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then # Need to count the number of total tests for fil in ${datafiles}; do c=$(grep -c "^#" "${hostoutdir}/${fil}") - ((tap_total+=c)) - ((ctr++)) + tap_total=$((tap_total+c)) + ctr=$((ctr+1)) done echo "1..${tap_total}" fi @@ -250,22 +245,20 @@ function runTests() { } -function main() { - local prgname="$0" - local vm1 vm2 - local xmldir="networkxml2xmlin" - local hostoutdir="networkxml2hostout" - local res rc - local flags +main() { + prgname="$0" + xmldir="networkxml2xmlin" + hostoutdir="networkxml2hostout" + flags=0 while [ $# -ne 0 ]; do case "$1" in --help|-h|-\?) usage ${prgname}; exit 0;; - --wait) ((flags |= FLAG_WAIT ));; - --verbose) ((flags |= FLAG_VERBOSE ));; - --libvirt-test) ((flags |= FLAG_LIBVIRT_TEST ));; - --tap-test) ((flags |= FLAG_TAP_TEST ));; - --force) ((flags |= FLAG_FORCE_CLEAN ));; + --wait) flags=$((flags | FLAG_WAIT ));; + --verbose) flags=$((flags | FLAG_VERBOSE ));; + --libvirt-test) flags=$((flags | FLAG_LIBVIRT_TEST ));; + --tap-test) flags=$((flags | FLAG_TAP_TEST ));; + --force) flags=$((flags | FLAG_FORCE_CLEAN ));; *) usage ${prgname}; exit 1;; esac shift 1 @@ -290,8 +283,8 @@ function main() { do case ${name} in tck*) - if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" -o \ - $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then + if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" ] || \ + [ $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then res=$(virsh destroy ${name} 2>&1) res=$(virsh undefine ${name} 2>&1) if [ $? -ne 0 ]; then @@ -309,8 +302,8 @@ function main() { do case ${name} in tck*) - if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" -o \ - $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then + if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" ] || \ + [ $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then res=$(virsh net-destroy ${name} 2>&1) rc=$? res=$(virsh net-undefine ${name} 2>&1) @@ -327,13 +320,13 @@ function main() { fi if [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ]; then - pushd "${PWD}" > /dev/null + curdir="${PWD}" . ./test-lib.sh if [ $? -ne 0 ]; then exit 1 fi test_intro $this_test - popd > /dev/null + cd "${curdir}" fi res=$(${VIRSH} capabilities 2>&1) Index: libvirt-tck/scripts/nwfilter/nwfilter2vmtest.sh =================================================================== --- libvirt-tck.orig/scripts/nwfilter/nwfilter2vmtest.sh +++ libvirt-tck/scripts/nwfilter/nwfilter2vmtest.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh ORIG_IFNAME="vnet0" ATTACH_IFNAME="attach0" @@ -6,17 +6,20 @@ TESTFILTERNAME="nwfiltertestfilter" TESTVM2FWALLDATA="nwfilterxml2fwallout/testvm.fwall.dat" VIRSH=virsh -uri= -if [ "x${LIBVIRT_TCK_CONFIG}x" != "xx" ]; then - uri_exp=`cat ${LIBVIRT_TCK_CONFIG} | grep "^uri\s*=" | sed -e 's/uri\s*=\s*//' | tail -n 1` - if [ "x${uri_exp}x" != "xx" ]; then - eval "uri=${uri_exp}" - fi -else - uri="qemu:///system" -fi -LIBVIRT_URI=${uri} +# For each line starting with uri=, remove the prefix and set the hold +# space to the rest of the line. Then at file end, print the hold +# space, which is effectively the last uri= line encountered. +uri=$(sed -n '/^uri[ ]*=[ ]*/ { + s/// + h +} +$ { + x + p +}' < "$LIBVIRT_TCK_CONFIG") +: "${uri:=qemu:///system}" +LIBVIRT_URI=${uri} FLAG_WAIT="$((1<<0))" FLAG_ATTACH="$((1<<1))" @@ -34,8 +37,8 @@ TAP_FAIL_LIST="" TAP_FAIL_CTR=0 TAP_TOT_CTR=0 -function usage() { - local cmd="$0" +usage() { + cmd="$0" cat <<EOF Usage: ${cmd} [--help|-h|-?] [--noattach] [--wait] [--verbose] [--libvirt-test] [--tap-test] @@ -60,66 +63,61 @@ EOF } -function tap_fail() { - echo "not ok $1 - ${2:0:66}" - TAP_FAIL_LIST+="$1 " - ((TAP_FAIL_CTR++)) - ((TAP_TOT_CTR++)) +tap_fail() { + txt=$(echo "$2" | gawk '{print substr($0,1,66)}') + echo "not ok $1 - ${txt}" + TAP_FAIL_LIST="$TAP_FAIL_LIST $1 " + TAP_FAIL_CTR=$((TAP_FAIL_CTR+1)) + TAP_TOT_CTR=$((TAP_TOT_CTR+1)) } -function tap_pass() { - echo "ok $1 - ${2:0:70}" - ((TAP_TOT_CTR++)) +tap_pass() { + txt=$(echo "$2" | gawk '{print substr($0,1,70)}') + echo "ok $1 - ${txt}" + TAP_TOT_CTR=$((TAP_TOT_CTR+1)) } -function tap_final() { - local okay - +tap_final() { [ -n "${TAP_FAIL_LIST}" ] && echo "FAILED tests ${TAP_FAIL_LIST}" okay=`echo "($TAP_TOT_CTR-$TAP_FAIL_CTR)*100/$TAP_TOT_CTR" | bc -l` - echo "Failed ${TAP_FAIL_CTR}/${TAP_TOT_CTR} tests, ${okay:0:5}% okay" + txt=$(echo $okay | gawk '{print substr($0,1,5)}') + echo "Failed ${TAP_FAIL_CTR}/${TAP_TOT_CTR} tests, ${txt}% okay" } # A wrapper for mktemp in case it does not exist # Echos the name of a temporary file. -function mktmpfile() { - local tmp - type -P mktemp > /dev/null - if [ $? -eq 0 ]; then - tmp=$(mktemp -t nwfvmtest.XXXXXX) - echo ${tmp} - else - while :; do - tmp="/tmp/nwfvmtest.${RANDOM}" - if [ ! -f ${tmp} ]; then - touch ${tmp} - chmod 666 ${tmp} - echo ${tmp} - break - fi - done - fi +mktmpdir() { + { + tmp=$( (umask 077 && mktemp -d ./nwfvmtest.XXXXXX) 2>/dev/null) && + test -n "$tmp" && test -d "$tmp" + } || + { + tmp=./nwfvmtest$$-$RANDOM + (umask 077 && mkdir "$tmp") + } || { echo "failed to create secure temporary directory" >&2; exit 1; } + echo "${tmp}" return 0 } -function checkExpectedOutput() { - local xmlfile="$1" - local fwallfile="$2" - local ifname="$3" - local flags="$4" - local skipregex="$5" - local regex="s/${ORIG_IFNAME}/${ifname}/g" - local cmd line tmpfile tmpfile2 skip - - tmpfile=`mktmpfile` - tmpfile2=`mktmpfile` +checkExpectedOutput() { + xmlfile="$1" + fwallfile="$2" + ifname="$3" + flags="$4" + skipregex="$5" + regex="s/${ORIG_IFNAME}/${ifname}/g" + + tmpdir=$(mktmpdir) + tmpfile=$tmpdir/file + tmpfile2=$tmpdir/file2 exec 4<${fwallfile} - read <&4 - line="${REPLY}" + IFS="" + + read line <&4 while [ "x${line}x" != "xx" ]; do cmd=`echo ${line##\#} | sed ${regex}` @@ -135,10 +133,11 @@ function checkExpectedOutput() { touch ${tmpfile2} while [ 1 ]; do - read <&4 - line="${REPLY}" + read line <&4 - if [ "${line:0:1}" == "#" ] || [ "x${line}x" == "xx" ]; then + letter=$(echo $line | gawk '{print substr($1,1,1)}') + + if [ "x${letter}x" = "x#x" ] || [ "x${line}x" = "xx" ]; then if [ ${skip} -ne 0 ]; then break @@ -151,18 +150,18 @@ function checkExpectedOutput() { echo "FAIL ${xmlfile} : ${cmd}" diff ${tmpfile} ${tmpfile2} fi - ((failctr++)) + failctr=$((failctr+1)) if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "tmp files: $tmpfile, $tmpfile2" echo "Press enter" - read + read enter fi [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \ test_result $((passctr+failctr)) "" 1 [ $((flags & FLAG_TAP_TEST)) -ne 0 ] && \ tap_fail $((passctr+failctr)) "${xmlfile} : ${cmd}" else - ((passctr++)) + passctr=$((passctr+1)) [ $((flags & FLAG_VERBOSE)) -ne 0 ] && \ echo "PASS ${xmlfile} : ${cmd}" [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \ @@ -180,19 +179,18 @@ function checkExpectedOutput() { exec 4>&- - rm -rf "${tmpfile}" "${tmpfile2}" 2>/dev/null + rm -rf "${tmpdir}" } -function doTest() { - local xmlfile="$1" - local fwallfile="$2" - local vm1name="$3" - local vm2name="$4" - local flags="$5" - local testnum="$6" - local linenums ctr=0 - local tmpfile b msg rc +doTest() { + xmlfile="$1" + fwallfile="$2" + vm1name="$3" + vm2name="$4" + flags="$5" + testnum="$6" + ctr=0 if [ ! -r "${xmlfile}" ]; then echo "FAIL : Cannot access filter XML file ${xmlfile}." @@ -209,7 +207,8 @@ function doTest() { if [ $((flags & FLAG_ATTACH)) -ne 0 ]; then - tmpfile=`mktmpfile` + tmpdir=$(mktmpdir) + tmpfile=$tmpdir/tmpfile b=`{ ${VIRSH} dumpxml ${vm1name} | tr -d "\n"; echo; } | \ sed "s/.*\<interface.*source bridge='$[a-zA-Z0-9_]\+$'.*<\/interface>.*/\1/"` @@ -225,7 +224,7 @@ EOF msg=`${VIRSH} attach-device "${vm1name}" "${tmpfile}" > /dev/null` rc=$? - ((attachctr++)) + attachctr=$((attachctr+1)) if [ $rc -eq 0 ]; then checkExpectedOutput "${xmlfile}" "${fwallfile}" "${ATTACH_IFNAME}" \ @@ -245,48 +244,45 @@ EOF checkExpectedOutput "${TESTFILTERNAME}" "${TESTVM2FWALLDATA}" \ "${vm2name}" "${flags}" #"(PRE|POST)ROUTING" fi - - ((attachfailctr++)) + + attachfailctr=$((attachfailctr+1)) if [ $((flags & FLAG_VERBOSE)) -ne 0 ]; then echo "FAIL: Could not attach interface to vm ${vm1name}." if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter" - read + read enter fi fi fi - rm -rf ${tmpfile} + rm -rf ${tmpdir} fi return 0 } -function runTests() { - local vm1name="$1" - local vm2name="$2" - local xmldir="$3" - local fwalldir="$4" - local flags="$5" - local fwallfiles f c - local tap_total=0 ctr=0 - - pushd ${PWD} > /dev/null - cd ${fwalldir} - fwallfiles=`ls *.fwall` - popd > /dev/null +runTests() { + vm1name="$1" + vm2name="$2" + xmldir="$3" + fwalldir="$4" + flags="$5" + tap_total=0 + ctr=0 + + fwallfiles=$(cd ${fwalldir}; ls *.fwall) if [ $((flags & FLAG_TAP_TEST)) -ne 0 ]; then # Need to count the number of total tests for fil in ${fwallfiles}; do c=$(grep -c "^#" ${fwalldir}/${fil}) - ((tap_total+=c)) - ((ctr++)) + tap_total=$((tap_total+c)) + ctr=$((ctr+1)) done c=$(grep -c "^#" "${TESTVM2FWALLDATA}") - ((tap_total+=c*ctr)) - [ $((flags & FLAG_ATTACH)) -ne 0 ] && ((tap_total*=2)) + tap_total=$((tap_total+c*ctr)) + [ $((flags & FLAG_ATTACH)) -ne 0 ] && tap_total=$((tap_total*2)) echo "1..${tap_total}" fi @@ -310,14 +306,14 @@ function runTests() { } -function createVM() { - local vmname="$1" - local filtername="$2" - local ipaddr="$3" - local macaddr="$4" - local flags="$5" - local res - local tmpfile='mktmpfile' +createVM() { + vmname="$1" + filtername="$2" + ipaddr="$3" + macaddr="$4" + flags="$5" + tmpdir=$(mktmpdir) + tmpfile=$tmpdir/tmpfile cat > ${tmpfile} << EOF <domain type='kvm'> @@ -365,7 +361,7 @@ EOF echo "Could not start VM ${vmname} : ${res}" if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi $(${VIRSH} undefine ${vmname}) return 1 @@ -373,23 +369,22 @@ EOF [ $((flags & FLAG_VERBOSE)) -ne 0 ] && echo "Created VM ${vmname}." - rm -rf ${tmpfile} + rm -rf ${tmpdir} return 0 } -function destroyVM() { - local vmname="$1" - local flags="$2" - local res +destroyVM() { + vmname="$1" + flags="$2" res=$(${VIRSH} destroy ${vmname}) if [ $? -ne 0 ]; then echo "Could not destroy VM ${vmname} : ${res}" if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi @@ -399,7 +394,7 @@ function destroyVM() { echo "Could not undefine VM ${vmname} : ${res}" if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi @@ -410,10 +405,10 @@ function destroyVM() { } -function createTestFilters() { - local flags="$1" - local tmpfile=`mktmpfile` - local res +createTestFilters() { + flags="$1" + tmpdir=$(mktmpdir) + tmpfile=$tmpdir/tmpfile cat >${tmpfile} << EOF <filter name="${TESTFILTERNAME}"> @@ -433,9 +428,9 @@ EOF echo "Could not define filter : ${res}" if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi - rm -rf ${tmpfile} + rm -rf ${tmpdir} return 1 fi @@ -448,28 +443,27 @@ EOF echo "Could not define filter : ${res}" if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi - rm -rf ${tmpfile} + rm -rf ${tmpdir} return 1 fi - rm -rf ${tmpfile} + rm -rf ${tmpdir} return 0 } -function deleteTestFilter() { - local flags="$1" - local res +deleteTestFilter() { + flags="$1" res=$(${VIRSH} nwfilter-undefine ${TESTFILTERNAME} 2>&1) if [ $? -ne 0 ]; then echo "Could not undefine filter : ${res}" if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi @@ -478,7 +472,7 @@ function deleteTestFilter() { echo "Could not undefine filter : ${res}" if [ $((flags & FLAG_WAIT)) -ne 0 ]; then echo "Press enter." - read + read enter fi return 1 fi @@ -486,27 +480,25 @@ function deleteTestFilter() { } -function main() { - local prgname="$0" - local vm1 vm2 - local xmldir="nwfilterxml2xmlin" - local fwalldir="nwfilterxml2fwallout" - local found=0 vms res - local filtername="tck-testcase" - local libvirtdpid=-1 - local flags OPWD +main() { + prgname="$0" + xmldir="nwfilterxml2xmlin" + fwalldir="nwfilterxml2fwallout" + found=0 + filtername="tck-testcase" + libvirtdpid=-1 - ((flags=${FLAG_ATTACH})) + flags=${FLAG_ATTACH} while [ $# -ne 0 ]; do case "$1" in --help|-h|-\?) usage ${prgname}; exit 0;; - --noattach) ((flags &= ~FLAG_ATTACH ));; - --wait) ((flags |= FLAG_WAIT ));; - --verbose) ((flags |= FLAG_VERBOSE ));; - --libvirt-test) ((flags |= FLAG_LIBVIRT_TEST ));; - --tap-test) ((flags |= FLAG_TAP_TEST ));; - --force) ((flags |= FLAG_FORCE_CLEAN ));; + --noattach) flags=$((flags & ~FLAG_ATTACH));; + --wait) flags=$((flags | FLAG_WAIT ));; + --verbose) flags=$((flags | FLAG_VERBOSE ));; + --libvirt-test) flags=$((flags | FLAG_LIBVIRT_TEST ));; + --tap-test) flags=$((flags | FLAG_TAP_TEST ));; + --force) flags=$((flags | FLAG_FORCE_CLEAN ));; *) usage ${prgname}; exit 1;; esac shift 1 @@ -527,15 +519,17 @@ function main() { exit 0 fi - for name in `virsh list | awk '{print $2}'` + for name in `virsh list --all | awk '{print $2}'` do case ${name} in tck*) - if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" -o \ - $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then + if [ "x${LIBVIRT_TCK_AUTOCLEAN}" = "x1" ] || \ + [ $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then res=$(virsh destroy ${name} 2>&1) + rc1=$? res=$(virsh undefine ${name} 2>&1) - if [ $? -ne 0 ]; then + rc2=$? + if [ $rc1 -ne 0 ] && [ $rc2 -ne 0 ]; then echo "Bail out! Could not undefine nwfiler ${name}: ${res}" exit 0 fi @@ -550,15 +544,15 @@ function main() { do case ${name} in tck*) - if [ "x${LIBVIRT_TCK_AUTOCLEAN}" == "x1" -o \ - $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then + if [ "x${LIBVIRT_TCK_AUTOCLEAN}" = "x1" ] || \ + [ $((flags & FLAG_FORCE_CLEAN)) -ne 0 ]; then res=$(virsh nwfilter-undefine ${name} 2>&1) if [ $? -ne 0 ]; then - echo "Bail out! Could not undefine domain ${name}: ${res}" + echo "Bail out! Could not undefine filter ${name}: ${res}" exit 1 fi else - echo "Bail out! Domain ${name} already exists, use --force to clean" + echo "Bail out! Filter ${name} already exists, use --force to clean" exit 1 fi esac @@ -566,19 +560,19 @@ function main() { fi if [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ]; then - pushd ${PWD} > /dev/null + curdir="${PWD}" . test-lib.sh if [ $? -ne 0 ]; then exit 1 fi test_intro $this_test - popd > /dev/null + cd "${curdir}" fi res=$(${VIRSH} capabilities 2>&1) - vm1="tck-testvm${RANDOM}" - vm2="tck-testvm${RANDOM}" + vm1="tck-test$$1" + vm2="tck-test$$2" createTestFilters "${flags}" if [ $? -ne 0 ]; then Index: libvirt-tck/scripts/networks/100-apply-verify-host.t =================================================================== --- libvirt-tck.orig/scripts/networks/100-apply-verify-host.t +++ libvirt-tck/scripts/networks/100-apply-verify-host.t @@ -1,10 +1,5 @@ -#!/bin/bash +#!/bin/sh pwd=$(dirname $0) -pushd ${PWD} > /dev/null - -cd ${pwd} -bash ./networkApplyTest.sh --tap-test - -popd > /dev/null +(cd ${pwd}; sh ./networkApplyTest.sh --tap-test) Index: libvirt-tck/scripts/nwfilter/050-apply-verify-host.t =================================================================== --- libvirt-tck.orig/scripts/nwfilter/050-apply-verify-host.t +++ libvirt-tck/scripts/nwfilter/050-apply-verify-host.t @@ -1,10 +1,5 @@ -#!/bin/bash +#!/bin/sh pwd=$(dirname $0) -pushd ${PWD} > /dev/null - -cd ${pwd} -bash ./nwfilter2vmtest.sh --tap-test --noattach - -popd > /dev/null \ No newline at end of file +(cd ${pwd}; sh ./nwfilter2vmtest.sh --tap-test --noattach) Index: libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.dat =================================================================== --- libvirt-tck.orig/scripts/networks/networkxml2hostout/tck-testnet-1.dat +++ libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.dat @@ -5,7 +5,7 @@ MASQUERADE all -- 10.1.2.0/24 #iptables -n -L FORWARD | grep ' 10\\.1\\.2\\.' ACCEPT all -- 0.0.0.0/0 10.1.2.0/24 state RELATED,ESTABLISHED ACCEPT all -- 10.1.2.0/24 0.0.0.0/0 -#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*\$dnsmasq[[:print:]*]\$|\\1|p' +#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*dnsmasq|dnsmasq|p' dnsmasq --strict-order --bind-interfaces --pid-file=/var/run/libvirt/network/tck-testnet.pid --conf-file= --listen-address 10.1.2.1 --except-interface lo --dhcp-range 10.1.2.2,10.1.2.254 --dhcp-lease-max=253 --dhcp-no-override #route -n | grep '10\\.1\\.2\\.' 10.1.2.0 0.0.0.0 255.255.255.0 U 0 0 0 tck-testbr Index: libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-2.dat =================================================================== --- libvirt-tck.orig/scripts/networks/networkxml2hostout/tck-testnet-2.dat +++ libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-2.dat @@ -2,7 +2,7 @@ ACCEPT all -- 0.0.0.0/0 10.1.2.0/24 ACCEPT all -- 10.1.2.0/24 0.0.0.0/0 #iptables -t nat -L -n | grep ' 10\\.1\\.2\\.' -#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*\$dnsmasq[[:print:]*]\$|\\1|p' +#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*dnsmasq|dnsmasq|p' dnsmasq --strict-order --bind-interfaces --pid-file=/var/run/libvirt/network/tck-testnet.pid --conf-file= --listen-address 10.1.2.1 --except-interface lo --dhcp-range 10.1.2.2,10.1.2.254 --dhcp-lease-max=253 --dhcp-no-override #route -n | grep '10\\.1\\.2\\.' 10.1.2.0 0.0.0.0 255.255.255.0 U 0 0 0 tck-testbr

3 8

[libvirt] [PATCH v2] macvtap: convert send / recv function to use libnl
by Stefan Berger 15 Nov '10

15 Nov '10

-V2: - adapting patch due to changes in the same file by another patch In a second step I am converting the netlink send/receive functions to use libnl. I tested this with 802.1Qbg profiles and my test server and did not see a regression. Caveat: The online documentation of libnl talks about nl_socket_alloc() but the header file provides nl_handle_alloc() -- this could be a hint to a possible problem between libnl versions... http://www.infradead.org/~tgr/libnl/doc/group__socket.html versus http://libnl.sourcearchive.com/documentation/1.1/group__socket_gf903c9ea089… Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- src/util/macvtap.c | 82 +++++++++++++++++++---------------------------------- 1 file changed, 30 insertions(+), 52 deletions(-) Index: libvirt-acl/src/util/macvtap.c =================================================================== --- libvirt-acl.orig/src/util/macvtap.c +++ libvirt-acl/src/util/macvtap.c @@ -83,16 +83,6 @@ enum virVirtualPortOp { }; -static int nlOpen(void) -{ - int fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE); - if (fd < 0) - virReportSystemError(errno, - "%s",_("cannot open netlink socket")); - return fd; -} - - /** * nlComm: * @nlmsg: pointer to netlink message @@ -106,8 +96,8 @@ static int nlOpen(void) * buffer will be returned. */ static -int nlComm(struct nlmsghdr *nlmsg, - char **respbuf, unsigned int *respbuflen, +int nlComm(struct nl_msg *nl_msg, + unsigned char **respbuf, unsigned int *respbuflen, int nl_pid) { int rc = 0; @@ -116,24 +106,29 @@ int nlComm(struct nlmsghdr *nlmsg, .nl_pid = nl_pid, .nl_groups = 0, }; - int rcvChunkSize = 1024; // expecting less than that - int rcvoffset = 0; ssize_t nbytes; struct timeval tv = { .tv_sec = NETLINK_ACK_TIMEOUT_S, }; fd_set readfds; - int fd = nlOpen(); + int fd; int n; + struct nl_handle *nlhandle = nl_handle_alloc(); + struct nlmsghdr *nlmsg = nlmsg_hdr(nl_msg); - if (fd < 0) + if (!nlhandle) return -1; + if (nl_connect(nlhandle, NETLINK_ROUTE) < 0) { + rc = -1; + goto err_exit; + } + + nlmsg_set_dst(nl_msg, &nladdr); + nlmsg->nlmsg_pid = getpid(); - nlmsg->nlmsg_flags |= NLM_F_ACK; - nbytes = sendto(fd, (void *)nlmsg, nlmsg->nlmsg_len, 0, - (struct sockaddr *)&nladdr, sizeof(nladdr)); + nbytes = nl_send_auto_complete(nlhandle, nl_msg); if (nbytes < 0) { virReportSystemError(errno, "%s", _("cannot send to netlink socket")); @@ -141,6 +136,8 @@ int nlComm(struct nlmsghdr *nlmsg, goto err_exit; } + fd = nl_socket_get_fd(nlhandle); + FD_ZERO(&readfds); FD_SET(fd, &readfds); @@ -156,28 +153,9 @@ int nlComm(struct nlmsghdr *nlmsg, goto err_exit; } - while (1) { - if (VIR_REALLOC_N(*respbuf, rcvoffset+rcvChunkSize) < 0) { - virReportOOMError(); - rc = -1; - goto err_exit; - } - - socklen_t addrlen = sizeof(nladdr); - nbytes = recvfrom(fd, &((*respbuf)[rcvoffset]), rcvChunkSize, 0, - (struct sockaddr *)&nladdr, &addrlen); - if (nbytes < 0) { - if (errno == EAGAIN || errno == EINTR) - continue; - virReportSystemError(errno, "%s", - _("error receiving from netlink socket")); - rc = -1; - goto err_exit; - } - rcvoffset += nbytes; - break; - } - *respbuflen = rcvoffset; + *respbuflen = nl_recv(nlhandle, &nladdr, respbuf, NULL); + if (*respbuflen <= 0) + rc = -1; err_exit: if (rc == -1) { @@ -186,7 +164,7 @@ err_exit: *respbuflen = 0; } - VIR_FORCE_CLOSE(fd); + nl_handle_destroy(nlhandle); return rc; } @@ -206,7 +184,7 @@ link_add(const char *type, struct nlmsgerr *err; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; int ifindex; - char *recvbuf = NULL; + unsigned char *recvbuf = NULL; unsigned int recvbuflen; struct nl_msg *nl_msg; struct nlattr *linkinfo, *info_data; @@ -255,7 +233,7 @@ link_add(const char *type, nla_nest_end(nl_msg, linkinfo); - if (nlComm(nlmsg_hdr(nl_msg), &recvbuf, &recvbuflen, 0) < 0) { + if (nlComm(nl_msg, &recvbuf, &recvbuflen, 0) < 0) { rc = -1; goto err_exit; } @@ -327,7 +305,7 @@ link_del(const char *ifname) struct nlmsghdr *resp; struct nlmsgerr *err; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; - char *recvbuf = NULL; + unsigned char *recvbuf = NULL; unsigned int recvbuflen; struct nl_msg *nl_msg; @@ -344,7 +322,7 @@ link_del(const char *ifname) if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0) goto buffer_too_small; - if (nlComm(nlmsg_hdr(nl_msg), &recvbuf, &recvbuflen, 0) < 0) { + if (nlComm(nl_msg, &recvbuf, &recvbuflen, 0) < 0) { rc = -1; goto err_exit; } @@ -739,7 +717,7 @@ getLldpadPid(void) { static int link_dump(bool nltarget_kernel, const char *ifname, int ifindex, - struct nlattr **tb, char **recvbuf) + struct nlattr **tb, unsigned char **recvbuf) { int rc = 0; struct nlmsghdr *resp; @@ -776,7 +754,7 @@ link_dump(bool nltarget_kernel, const ch } } - if (nlComm(nlmsg_hdr(nl_msg), recvbuf, &recvbuflen, pid) < 0) { + if (nlComm(nl_msg, recvbuf, &recvbuflen, pid) < 0) { rc = -1; goto err_exit; } @@ -862,7 +840,7 @@ ifaceGetNthParent(int ifindex, const cha { int rc; struct nlattr *tb[IFLA_MAX + 1] = { NULL, }; - char *recvbuf = NULL; + unsigned char *recvbuf = NULL; bool end = false; unsigned int i = 0; @@ -1022,7 +1000,7 @@ doPortProfileOpSetLink(bool nltarget_ker .ifi_family = AF_UNSPEC, .ifi_index = ifindex, }; - char *recvbuf = NULL; + unsigned char *recvbuf = NULL; unsigned int recvbuflen = 0; uint32_t pid = 0; struct nl_msg *nl_msg; @@ -1133,7 +1111,7 @@ doPortProfileOpSetLink(bool nltarget_ker } } - if (nlComm(nlmsg_hdr(nl_msg), &recvbuf, &recvbuflen, pid) < 0) { + if (nlComm(nl_msg, &recvbuf, &recvbuflen, pid) < 0) { rc = -1; goto err_exit; } @@ -1201,7 +1179,7 @@ doPortProfileOpCommon(bool nltarget_kern uint8_t op) { int rc; - char *recvbuf = NULL; + unsigned char *recvbuf = NULL; struct nlattr *tb[IFLA_MAX + 1] = { NULL , }; int repeats = STATUS_POLL_TIMEOUT_USEC / STATUS_POLL_INTERVL_USEC; uint16_t status = 0;

2 2

[libvirt] [PATCH] RFC: Remove all object hashtable caches from virConnectPtr
by Daniel P. Berrange 15 Nov '10

15 Nov '10

The virConnectPtr struct will cache instances of all other objects. APIs like virDomainLookupByUUID will return a cached object, so if you do virDomainLookupByUUID twice in a row, you'll get the same exact virDomainPtr instance. This does not have any performance benefit, since the actual logic in virDomainLookupByUUID (and other APIs returning virDomainPtr, etc instances) is not short-circuited. All it does is to ensure there is only one single virDomainPtr in existance for any given UUID. The caching has a number of downsides though, all relating to stale data. If APIs aren't careful to always overwrite the 'id' field in virDomainPtr it may become out of data. Likewise for the name field, if a guest is renamed, or if a guest is deleted, and then a new one created with the same UUID but different name. This has been an ongoing, endless source of bugs for all applications using libvirt from languages with garbage collection, causing them to get virDomainPtr instances from long ago with stale data. The caching is also a waste of memory resources, since both applications, and language bindings often maintain their own hashtable caches of object instances. This patch removes all the hash table caching, so all APIs return brand new virDomainPtr (etc) object instances. Other options include - Explicitly overrite the 'name' field in the cached objects every time they are looked up, to ensure fully up2date objects - Add a flag to virConnectOpen to allow apps to turn off caching themselves. * src/datatypes.h: Delete all hash tables. * src/datatypes.c: Remove all object caching code --- src/datatypes.c | 657 +++++++++++++------------------------------------------ src/datatypes.h | 10 - 2 files changed, 149 insertions(+), 518 deletions(-) diff --git a/src/datatypes.c b/src/datatypes.c index 46009ae..66dbc99 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -41,120 +41,6 @@ * * ************************************************************************/ -/** - * virDomainFreeName: - * @domain: a domain object - * - * Destroy the domain object, this is just used by the domain hash callback. - * - * Returns 0 in case of success and -1 in case of failure. - */ -static int -virDomainFreeName(virDomainPtr domain, const char *name ATTRIBUTE_UNUSED) -{ - return (virUnrefDomain(domain)); -} - -/** - * virNetworkFreeName: - * @network: a network object - * - * Destroy the network object, this is just used by the network hash callback. - * - * Returns 0 in case of success and -1 in case of failure. - */ -static int -virNetworkFreeName(virNetworkPtr network, const char *name ATTRIBUTE_UNUSED) -{ - return (virUnrefNetwork(network)); -} - -/** - * virInterfaceFreeName: - * @interface: an interface object - * - * Destroy the interface object, this is just used by the interface hash callback. - * - * Returns 0 in case of success and -1 in case of failure. - */ -static int -virInterfaceFreeName(virInterfacePtr iface, const char *name ATTRIBUTE_UNUSED) -{ - return (virUnrefInterface(iface)); -} - -/** - * virStoragePoolFreeName: - * @pool: a pool object - * - * Destroy the pool object, this is just used by the pool hash callback. - * - * Returns 0 in case of success and -1 in case of failure. - */ -static int -virStoragePoolFreeName(virStoragePoolPtr pool, const char *name ATTRIBUTE_UNUSED) -{ - return (virUnrefStoragePool(pool)); -} - -/** - * virStorageVolFreeName: - * @vol: a vol object - * - * Destroy the vol object, this is just used by the vol hash callback. - * - * Returns 0 in case of success and -1 in case of failure. - */ -static int -virStorageVolFreeName(virStorageVolPtr vol, const char *name ATTRIBUTE_UNUSED) -{ - return (virUnrefStorageVol(vol)); -} - -/** - * virSecretFreeName: - * @secret_: a secret object - * - * Destroy the secret object, this is just used by the secret hash callback. - * - * Returns 0 in case of success and -1 in case of failure. - */ -static void -virSecretFreeName(void *secret_, const char *name ATTRIBUTE_UNUSED) -{ - virSecretPtr secret; - - secret = secret_; - virUnrefSecret(secret); -} - -/** - * virNWFilterPoolFreeName: - * @pool: a nwfilter pool object - * - * Destroy the nwfilter pool object, this is just used by the nwfilter pool hash callback. - * - * Returns 0 in case of success and -1 in case of failure. - */ -static int -virNWFilterPoolFreeName(virNWFilterPtr pool, const char *name ATTRIBUTE_UNUSED) -{ - return (virUnrefNWFilter(pool)); -} - -/** - * virDomainSnapshotFreeName: - * @snapshot: a domain snapshotobject - * - * Destroy the domain snapshot object, this is just used by the domain hash callback. - * - * Returns 0 in case of success and -1 in case of failure. - */ -static int -virDomainSnapshotFreeName(virDomainSnapshotPtr snapshot, const char *name ATTRIBUTE_UNUSED) -{ - return (virUnrefDomainSnapshot(snapshot)); -} /** * virGetConnect: @@ -182,53 +68,12 @@ virGetConnect(void) { ret->privateData = NULL; ret->networkPrivateData = NULL; ret->interfacePrivateData = NULL; - ret->domains = virHashCreate(20); - if (ret->domains == NULL) - goto failed; - ret->networks = virHashCreate(20); - if (ret->networks == NULL) - goto failed; - ret->interfaces = virHashCreate(20); - if (ret->interfaces == NULL) - goto failed; - ret->storagePools = virHashCreate(20); - if (ret->storagePools == NULL) - goto failed; - ret->storageVols = virHashCreate(20); - if (ret->storageVols == NULL) - goto failed; - ret->nodeDevices = virHashCreate(256); - if (ret->nodeDevices == NULL) - goto failed; - ret->secrets = virHashCreate(20); - if (ret->secrets == NULL) - goto failed; - ret->nwfilterPools = virHashCreate(20); - if (ret->nwfilterPools == NULL) - goto failed; ret->refs = 1; return(ret); failed: if (ret != NULL) { - if (ret->domains != NULL) - virHashFree(ret->domains, (virHashDeallocator) virDomainFreeName); - if (ret->networks != NULL) - virHashFree(ret->networks, (virHashDeallocator) virNetworkFreeName); - if (ret->interfaces != NULL) - virHashFree(ret->interfaces, (virHashDeallocator) virInterfaceFreeName); - if (ret->storagePools != NULL) - virHashFree(ret->storagePools, (virHashDeallocator) virStoragePoolFreeName); - if (ret->storageVols != NULL) - virHashFree(ret->storageVols, (virHashDeallocator) virStorageVolFreeName); - if (ret->nodeDevices != NULL) - virHashFree(ret->nodeDevices, (virHashDeallocator) virNodeDeviceFree); - if (ret->secrets != NULL) - virHashFree(ret->secrets, virSecretFreeName); - if (ret->nwfilterPools != NULL) - virHashFree(ret->nwfilterPools, (virHashDeallocator) virNWFilterPoolFreeName); - virMutexDestroy(&ret->lock); VIR_FREE(ret); } @@ -247,22 +92,6 @@ failed: static void virReleaseConnect(virConnectPtr conn) { DEBUG("release connection %p", conn); - if (conn->domains != NULL) - virHashFree(conn->domains, (virHashDeallocator) virDomainFreeName); - if (conn->networks != NULL) - virHashFree(conn->networks, (virHashDeallocator) virNetworkFreeName); - if (conn->interfaces != NULL) - virHashFree(conn->interfaces, (virHashDeallocator) virInterfaceFreeName); - if (conn->storagePools != NULL) - virHashFree(conn->storagePools, (virHashDeallocator) virStoragePoolFreeName); - if (conn->storageVols != NULL) - virHashFree(conn->storageVols, (virHashDeallocator) virStorageVolFreeName); - if (conn->nodeDevices != NULL) - virHashFree(conn->nodeDevices, (virHashDeallocator) virNodeDeviceFree); - if (conn->secrets != NULL) - virHashFree(conn->secrets, virSecretFreeName); - if (conn->nwfilterPools != NULL) - virHashFree(conn->nwfilterPools, (virHashDeallocator) virNWFilterPoolFreeName); virResetError(&conn->err); @@ -353,36 +182,23 @@ virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) { virUUIDFormat(uuid, uuidstr); - ret = (virDomainPtr) virHashLookup(conn->domains, uuidstr); - if (ret == NULL) { - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->magic = VIR_DOMAIN_MAGIC; - ret->conn = conn; - ret->id = -1; - memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); - ret->snapshots = virHashCreate(20); - - if (virHashAddEntry(conn->domains, uuidstr, ret) < 0) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to add domain to connection hash table")); - goto error; - } - conn->refs++; - DEBUG("New hash entry %p", ret); - } else { - DEBUG("Existing hash entry %p: refs now %d", ret, ret->refs+1); + if (VIR_ALLOC(ret) < 0) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; + } + ret->name = strdup(name); + if (ret->name == NULL) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; } + ret->magic = VIR_DOMAIN_MAGIC; + ret->conn = conn; + ret->id = -1; + memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); + + conn->refs++; ret->refs++; virMutexUnlock(&conn->lock); return(ret); @@ -415,18 +231,9 @@ virReleaseDomain(virDomainPtr domain) { virUUIDFormat(domain->uuid, uuidstr); DEBUG("release domain %p %s %s", domain, domain->name, uuidstr); - if (virHashRemoveEntry(conn->domains, uuidstr, NULL) < 0) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s", - _("domain missing from connection hash table")); - conn = NULL; - } - domain->magic = -1; domain->id = -1; VIR_FREE(domain->name); - if (domain->snapshots != NULL) - virHashFree(domain->snapshots, (virHashDeallocator) virDomainSnapshotFreeName); VIR_FREE(domain); if (conn) { @@ -499,31 +306,22 @@ virGetNetwork(virConnectPtr conn, const char *name, const unsigned char *uuid) { virUUIDFormat(uuid, uuidstr); - ret = (virNetworkPtr) virHashLookup(conn->networks, uuidstr); - if (ret == NULL) { - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->magic = VIR_NETWORK_MAGIC; - ret->conn = conn; - memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); - - if (virHashAddEntry(conn->networks, uuidstr, ret) < 0) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to add network to connection hash table")); - goto error; - } - conn->refs++; + if (VIR_ALLOC(ret) < 0) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; } + ret->name = strdup(name); + if (ret->name == NULL) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; + } + ret->magic = VIR_NETWORK_MAGIC; + ret->conn = conn; + memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); + + conn->refs++; ret->refs++; virMutexUnlock(&conn->lock); return(ret); @@ -556,13 +354,6 @@ virReleaseNetwork(virNetworkPtr network) { virUUIDFormat(network->uuid, uuidstr); DEBUG("release network %p %s %s", network, network->name, uuidstr); - if (virHashRemoveEntry(conn->networks, uuidstr, NULL) < 0) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s", - _("network missing from connection hash table")); - conn = NULL; - } - network->magic = -1; VIR_FREE(network->name); VIR_FREE(network); @@ -641,70 +432,28 @@ virGetInterface(virConnectPtr conn, const char *name, const char *mac) { virMutexLock(&conn->lock); - ret = (virInterfacePtr) virHashLookup(conn->interfaces, name); - - if (ret != NULL) { - if (STRCASENEQ(ret->mac, mac)) { - /* - * If the mac address has changed, try to modify it in - * place, which will only work if the new mac is the - * same length as, or shorter than, the old mac. - */ - size_t newmaclen = strlen(mac); - size_t oldmaclen = strlen(ret->mac); - if (newmaclen <= oldmaclen) { - strcpy(ret->mac, mac); - } else { - /* - * If it's longer, we're kind of screwed, because we - * can't add a new hashtable entry (it would clash - * with the existing entry of same name), and we can't - * free/re-alloc the existing entry's mac, as some - * other thread may already be using the existing mac - * pointer. Fortunately, this should never happen, - * since the length of the mac address for any - * interface is determined by the type of the - * interface, and that is unlikely to change. - */ - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, - _("Failed to change interface mac address " - "from %s to %s due to differing lengths."), - ret->mac, mac); - ret = NULL; - goto error; - } - } - } else { - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->mac = strdup(mac); - if (ret->mac == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } + if (VIR_ALLOC(ret) < 0) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; + } + ret->name = strdup(name); + if (ret->name == NULL) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; + } + ret->mac = strdup(mac); + if (ret->mac == NULL) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; + } - ret->magic = VIR_INTERFACE_MAGIC; - ret->conn = conn; + ret->magic = VIR_INTERFACE_MAGIC; + ret->conn = conn; - if (virHashAddEntry(conn->interfaces, name, ret) < 0) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to add interface to connection hash table")); - goto error; - } - conn->refs++; - } + conn->refs++; ret->refs++; virMutexUnlock(&conn->lock); return(ret); @@ -735,15 +484,6 @@ virReleaseInterface(virInterfacePtr iface) { virConnectPtr conn = iface->conn; DEBUG("release interface %p %s", iface, iface->name); - if (virHashRemoveEntry(conn->interfaces, iface->name, NULL) < 0) { - /* unlock before reporting error because error report grabs lock */ - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s", - _("interface missing from connection hash table")); - /* don't decr the conn refct if we weren't connected to it */ - conn = NULL; - } - iface->magic = -1; VIR_FREE(iface->name); VIR_FREE(iface->mac); @@ -820,31 +560,22 @@ virGetStoragePool(virConnectPtr conn, const char *name, const unsigned char *uui virUUIDFormat(uuid, uuidstr); - ret = (virStoragePoolPtr) virHashLookup(conn->storagePools, uuidstr); - if (ret == NULL) { - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->magic = VIR_STORAGE_POOL_MAGIC; - ret->conn = conn; - memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); - - if (virHashAddEntry(conn->storagePools, uuidstr, ret) < 0) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, - "%s", _("failed to add storage pool to connection hash table")); - goto error; - } - conn->refs++; + if (VIR_ALLOC(ret) < 0) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; + } + ret->name = strdup(name); + if (ret->name == NULL) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; } + ret->magic = VIR_STORAGE_POOL_MAGIC; + ret->conn = conn; + memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); + + conn->refs++; ret->refs++; virMutexUnlock(&conn->lock); return(ret); @@ -878,13 +609,6 @@ virReleaseStoragePool(virStoragePoolPtr pool) { virUUIDFormat(pool->uuid, uuidstr); DEBUG("release pool %p %s %s", pool, pool->name, uuidstr); - if (virHashRemoveEntry(conn->storagePools, uuidstr, NULL) < 0) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s", - _("pool missing from connection hash table")); - conn = NULL; - } - pool->magic = -1; VIR_FREE(pool->name); VIR_FREE(pool); @@ -958,42 +682,33 @@ virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, const c } virMutexLock(&conn->lock); - ret = (virStorageVolPtr) virHashLookup(conn->storageVols, key); - if (ret == NULL) { - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->pool = strdup(pool); - if (ret->pool == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - if (virStrcpyStatic(ret->key, key) == NULL) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, - _("Volume key %s too large for destination"), key); - goto error; - } - ret->magic = VIR_STORAGE_VOL_MAGIC; - ret->conn = conn; - - if (virHashAddEntry(conn->storageVols, key, ret) < 0) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to add storage vol to connection hash table")); - goto error; - } - conn->refs++; + if (VIR_ALLOC(ret) < 0) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; + } + ret->pool = strdup(pool); + if (ret->pool == NULL) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; + } + ret->name = strdup(name); + if (ret->name == NULL) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; + } + if (virStrcpyStatic(ret->key, key) == NULL) { + virMutexUnlock(&conn->lock); + virLibConnError(VIR_ERR_INTERNAL_ERROR, + _("Volume key %s too large for destination"), key); + goto error; } + ret->magic = VIR_STORAGE_VOL_MAGIC; + ret->conn = conn; + + conn->refs++; ret->refs++; virMutexUnlock(&conn->lock); return(ret); @@ -1025,13 +740,6 @@ virReleaseStorageVol(virStorageVolPtr vol) { virConnectPtr conn = vol->conn; DEBUG("release vol %p %s", vol, vol->name); - if (virHashRemoveEntry(conn->storageVols, vol->key, NULL) < 0) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s", - _("vol missing from connection hash table")); - conn = NULL; - } - vol->magic = -1; VIR_FREE(vol->name); VIR_FREE(vol->pool); @@ -1105,30 +813,21 @@ virGetNodeDevice(virConnectPtr conn, const char *name) } virMutexLock(&conn->lock); - ret = (virNodeDevicePtr) virHashLookup(conn->nodeDevices, name); - if (ret == NULL) { - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->magic = VIR_NODE_DEVICE_MAGIC; - ret->conn = conn; - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - - if (virHashAddEntry(conn->nodeDevices, name, ret) < 0) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, - "%s", _("failed to add node dev to conn hash table")); - goto error; - } - conn->refs++; + if (VIR_ALLOC(ret) < 0) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; } + ret->magic = VIR_NODE_DEVICE_MAGIC; + ret->conn = conn; + ret->name = strdup(name); + if (ret->name == NULL) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; + } + + conn->refs++; ret->refs++; virMutexUnlock(&conn->lock); return(ret); @@ -1159,13 +858,6 @@ virReleaseNodeDevice(virNodeDevicePtr dev) { virConnectPtr conn = dev->conn; DEBUG("release dev %p %s", dev, dev->name); - if (virHashRemoveEntry(conn->nodeDevices, dev->name, NULL) < 0) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, - "%s", _("dev missing from connection hash table")); - conn = NULL; - } - dev->magic = -1; VIR_FREE(dev->name); VIR_FREE(dev->parent); @@ -1239,30 +931,21 @@ virGetSecret(virConnectPtr conn, const unsigned char *uuid, virUUIDFormat(uuid, uuidstr); - ret = virHashLookup(conn->secrets, uuidstr); - if (ret == NULL) { - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->magic = VIR_SECRET_MAGIC; - ret->conn = conn; - memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); - ret->usageType = usageType; - if (!(ret->usageID = strdup(usageID))) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - if (virHashAddEntry(conn->secrets, uuidstr, ret) < 0) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to add secret to conn hash table")); - goto error; - } - conn->refs++; + if (VIR_ALLOC(ret) < 0) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; + } + ret->magic = VIR_SECRET_MAGIC; + ret->conn = conn; + memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); + ret->usageType = usageType; + if (!(ret->usageID = strdup(usageID))) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; } + conn->refs++; ret->refs++; virMutexUnlock(&conn->lock); return ret; @@ -1294,13 +977,6 @@ virReleaseSecret(virSecretPtr secret) { virUUIDFormat(secret->uuid, uuidstr); DEBUG("release secret %p %s", secret, uuidstr); - if (virHashRemoveEntry(conn->secrets, uuidstr, NULL) < 0) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s", - _("secret missing from connection hash table")); - conn = NULL; - } - VIR_FREE(secret->usageID); secret->magic = -1; VIR_FREE(secret); @@ -1433,31 +1109,22 @@ virGetNWFilter(virConnectPtr conn, const char *name, const unsigned char *uuid) virUUIDFormat(uuid, uuidstr); - ret = (virNWFilterPtr) virHashLookup(conn->nwfilterPools, uuidstr); - if (ret == NULL) { - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->magic = VIR_NWFILTER_MAGIC; - ret->conn = conn; - memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); - - if (virHashAddEntry(conn->nwfilterPools, uuidstr, ret) < 0) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to add network filter pool to connection hash table")); - goto error; - } - conn->refs++; + if (VIR_ALLOC(ret) < 0) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; + } + ret->name = strdup(name); + if (ret->name == NULL) { + virMutexUnlock(&conn->lock); + virReportOOMError(); + goto error; } + ret->magic = VIR_NWFILTER_MAGIC; + ret->conn = conn; + memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); + + conn->refs++; ret->refs++; virMutexUnlock(&conn->lock); return(ret); @@ -1491,13 +1158,6 @@ virReleaseNWFilterPool(virNWFilterPtr pool) { virUUIDFormat(pool->uuid, uuidstr); DEBUG("release pool %p %s %s", pool, pool->name, uuidstr); - if (virHashRemoveEntry(conn->nwfilterPools, uuidstr, NULL) < 0) { - virMutexUnlock(&conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s", - _("pool missing from connection hash table")); - conn = NULL; - } - pool->magic = -1; VIR_FREE(pool->name); VIR_FREE(pool); @@ -1558,33 +1218,21 @@ virGetDomainSnapshot(virDomainPtr domain, const char *name) } virMutexLock(&domain->conn->lock); - ret = (virDomainSnapshotPtr) virHashLookup(domain->snapshots, name); - if (ret == NULL) { - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&domain->conn->lock); - virReportOOMError(); - goto error; - } - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&domain->conn->lock); - virReportOOMError(); - goto error; - } - ret->magic = VIR_SNAPSHOT_MAGIC; - ret->domain = domain; - - if (virHashAddEntry(domain->snapshots, name, ret) < 0) { - virMutexUnlock(&domain->conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, - "%s", _("failed to add snapshot to domain hash table")); - goto error; - } - domain->refs++; - DEBUG("New hash entry %p", ret); - } else { - DEBUG("Existing hash entry %p: refs now %d", ret, ret->refs+1); + if (VIR_ALLOC(ret) < 0) { + virMutexUnlock(&domain->conn->lock); + virReportOOMError(); + goto error; + } + ret->name = strdup(name); + if (ret->name == NULL) { + virMutexUnlock(&domain->conn->lock); + virReportOOMError(); + goto error; } + ret->magic = VIR_SNAPSHOT_MAGIC; + ret->domain = domain; + + domain->refs++; ret->refs++; virMutexUnlock(&domain->conn->lock); return(ret); @@ -1604,13 +1252,6 @@ virReleaseDomainSnapshot(virDomainSnapshotPtr snapshot) virDomainPtr domain = snapshot->domain; DEBUG("release snapshot %p %s", snapshot, snapshot->name); - if (virHashRemoveEntry(domain->snapshots, snapshot->name, NULL) < 0) { - virMutexUnlock(&domain->conn->lock); - virLibConnError(VIR_ERR_INTERNAL_ERROR, - "%s", _("snapshot missing from domain hash table")); - domain = NULL; - } - snapshot->magic = -1; VIR_FREE(snapshot->name); VIR_FREE(snapshot); diff --git a/src/datatypes.h b/src/datatypes.h index bbeb7cf..0409ab1 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -24,7 +24,6 @@ # include "internal.h" -# include "hash.h" # include "driver.h" # include "threads.h" @@ -188,14 +187,6 @@ struct _virConnect { virErrorFunc handler; /* associated handlet */ void *userData; /* the user data */ - virHashTablePtr domains; /* hash table for known domains */ - virHashTablePtr networks; /* hash table for known domains */ - virHashTablePtr interfaces; /* hash table for known interfaces */ - virHashTablePtr storagePools;/* hash table for known storage pools */ - virHashTablePtr storageVols;/* hash table for known storage vols */ - virHashTablePtr nodeDevices; /* hash table for known node devices */ - virHashTablePtr secrets; /* hash taboe for known secrets */ - virHashTablePtr nwfilterPools; /* hash tables ofr known nw filter pools */ int refs; /* reference count */ }; @@ -211,7 +202,6 @@ struct _virDomain { char *name; /* the domain external name */ int id; /* the domain ID */ unsigned char uuid[VIR_UUID_BUFLEN]; /* the domain unique identifier */ - virHashTablePtr snapshots; /* hash table for known snapshots */ }; /** -- 1.7.2.3

3 6

[libvirt] [RFC] Proposed API to support block device streaming
by Adam Litke 15 Nov '10

15 Nov '10

I've been working with Anthony Liguori and Stefan Hajnoczi to enable data streaming to copy-on-read disk images in qemu. This work is working its way through peer review and I expect it to be upstream soon as part of the support for the new QED disk image format. I would like to enable these commands in libvirt in order to support at least two compelling use cases: 1) Rapid deployment of domains: Creating a new domain from a central repository of images can be time consuming since a local copy of the image must be made before the domain can be started. With copy-on-read and streaming, up-front copy time is eliminated and the domain can be started immediately. Streaming can run while the domain runs to fully populate the disk image. 2) Post-copy live block migration: A qemu-nbd server is started on the source host and serves the domain's block device to the destination host. A QED image is created on the destination host with backing to the nbd server. The domain is migrated as normal. When migration completes, a stream command is executed to fully populate the destination QED image. After streaming completes, the qemu-nbd server can be shut down and the domain (including local storage) is fully independent of the source host. Qemu will support two streaming modes: full device and single sector. Full device streaming is the easiest to use because one command will cause the whole device to be streamed as fast as possible. Single sector mode can be used if one wants to throttle streaming to reduce I/O pressure. In this mode, the user issues individual commands to stream single sectors. To enable this support in libvirt, I propose the following API... virDomainStreamDisk() initiates either a full device stream or a single sector stream (depending on virDomainStreamDiskFlags). For a full device stream, it returns either 0 or -1. For a single sector stream, it returns an offset that can be used to continue streaming with a subsequent call to virDomainStreamDisk(). virDomainStreamDiskInfo() returns the status of a currently-running full device stream (the device name, current streaming position, and total size). Comments on this design would be greatly appreciated. Thanks! diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 81db3a2..d80a8b5 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1046,6 +1046,39 @@ int virDomainUpdateDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags); /* + * Disk Streaming + */ +typedef enum { + VIR_STREAM_DISK_FULL = 1, /* Stream the entire disk */ + VIR_STREAM_DISK_ONE = 2, /* Stream a single disk unit */ +} virDomainStreamDiskFlags; + +#define VIR_STREAM_PATH_BUFLEN 100 +#define VIR_STREAM_DISK_MAX_STREAMS 10 + +typedef struct _virStreamDiskState virStreamDiskState; +struct _virStreamDiskState { + char path[VIR_STREAM_PATH_BUFLEN]; + /* + * The unit of measure for size and offset is unspecified. These fields + * are meant to indicate the progress of a continuous streaming operation. + */ + unsigned long long offset; /* Current offset of active streaming */ + unsigned long long size; /* Disk size */ +}; +typedef virStreamDiskState *virStreamDiskStatePtr; + +unsigned long long virDomainStreamDisk(virDomainPtr dom, + const char *path, + unsigned long long offset, + unsigned int flags); + +int virDomainStreamDiskInfo(virDomainPtr dom, + virStreamDiskStatePtr infos, + unsigned int nr_infos, + unsigned int flags); + +/* * NUMA support */ -- Thanks, Adam

5 8

[libvirt] Ask for help about libvirt error: this function is not supported by the connection driver..
by Feng Dan Liu 15 Nov '10

15 Nov '10

Hi, I'm a libvirt user, I encountered a problem when I use the libvirt java bindings, could you please help me take a look？ Thanks in advance! Any suggestions from anyone of you will be very appreciated. Here is my scenario: Remote access machine A is a RHEL 5.4 workstation with the command "virsh version" results: Compiled against library: libvir 0.8.1 Using library: libvir 0.8.1 Using API: Xen 3.0.1 Running hypervisor: Xen 3.1.0 Hypervisor machine B is a Fedora Core 14 Linux workstation with the command "virsh version" results: Compiled against library: libvir 0.8.3 Using library: libvir 0.8.3 Using API: QEMU 0.8.3 Running hypervisor: QEMU 0.13.0 I finished the code using libvirt java bindings and run it on machine A, try to remote connect to the hypervisor on machine B. It can be connected succussfully, but when the code calls the function of conn.listDefinedStoragePools() or conn.listStoragePools, the libvirt on machine B will get the errors below: 04:19:10.467: error : phypStorageOpen:3884 : invalid argument in phypStorageOpen: unsupported flags (0x1) 04:19:11.354: warning : qemudParsePCIDeviceStrs:1422 : Unexpected exit status '1', qemu probably failed 04:19:12.128: error : virLibConnError:448 : this function is not supported by the connection driver: virConnectNumOfStoragePools With the errors above, my code can't run the functions of conn.listDefinedStoragePools() or conn.listStoragePools successfully. Could you please give me some suggestions? By the way, I try to use the libvirt java bindings 0.3.2 and 0.4.6, both of them have this problem. Best Regards, Fengdan Liu, Casey AdTech, Tivoli China Development, China Software Development Lab, IBM Greater China Group Phone: 86-10-82453004 Email: liufengd(a)cn.ibm.com Addr: No.28 Building, Zhong Guan Cun Software Park, No. 8 Dong Bei Wang West Road, ShangDi, Haidian District, Beijing 100193, P.R.China

2 2

[libvirt] [PATCH] Use python discovered through env instead of hardcoding a path
by Matthias Bolte 14 Nov '10

14 Nov '10

This is more flexible regarding the location of the python binary but doesn't allow to pass the -u flag. The -i flag can be passed from inside the script using the PYTHONINSPECT env variable. This fixes a problem with the esx_vi_generator.py on FreeBSD. --- examples/python/dominfo.py | 2 +- examples/python/domrestore.py | 2 +- examples/python/domsave.py | 2 +- examples/python/domstart.py | 2 +- examples/python/esxlist.py | 2 +- python/generator.py | 2 +- python/libvirt-override.py | 5 +++++ src/esx/esx_vi_generator.py | 2 +- 8 files changed, 12 insertions(+), 7 deletions(-) diff --git a/examples/python/dominfo.py b/examples/python/dominfo.py index e41709c..2f250e7 100755 --- a/examples/python/dominfo.py +++ b/examples/python/dominfo.py @@ -1,4 +1,4 @@ -#! /usr/bin/python +#!/usr/bin/env python # dominfo - print some information about a domain import libvirt diff --git a/examples/python/domrestore.py b/examples/python/domrestore.py index b032138..ccc82f6 100755 --- a/examples/python/domrestore.py +++ b/examples/python/domrestore.py @@ -1,4 +1,4 @@ -#! /usr/bin/python +#!/usr/bin/env python # domstart - make sure a given domU is running, if not start it import libvirt diff --git a/examples/python/domsave.py b/examples/python/domsave.py index 35e2c8a..12ffac1 100755 --- a/examples/python/domsave.py +++ b/examples/python/domsave.py @@ -1,4 +1,4 @@ -#! /usr/bin/python +#!/usr/bin/env python # domstart - make sure a given domU is running, if not start it import libvirt diff --git a/examples/python/domstart.py b/examples/python/domstart.py index 52fb79e..b1e89b8 100755 --- a/examples/python/domstart.py +++ b/examples/python/domstart.py @@ -1,4 +1,4 @@ -#! /usr/bin/python +#!/usr/bin/env python # domstart - make sure a given domU is running, if not start it import libvirt diff --git a/examples/python/esxlist.py b/examples/python/esxlist.py index a0602e8..c55424f 100755 --- a/examples/python/esxlist.py +++ b/examples/python/esxlist.py @@ -1,4 +1,4 @@ -#! /usr/bin/python +#!/usr/bin/env python # esxlist - list active domains of an ESX host and print some info. # also demonstrates how to use the libvirt.openAuth() method diff --git a/python/generator.py b/python/generator.py index 68009b9..15751bd 100755 --- a/python/generator.py +++ b/python/generator.py @@ -1010,7 +1010,7 @@ def buildWrappers(): classes = open("libvirt.py", "w") extra = open(os.path.join(srcPref,"libvirt-override.py"), "r") - classes.write("#!/usr/bin/python -i\n") + classes.write("#!/usr/bin/env python\n") classes.write("#\n") classes.write("# WARNING WARNING WARNING WARNING\n") classes.write("#\n") diff --git a/python/libvirt-override.py b/python/libvirt-override.py index d544a0e..909ebe3 100644 --- a/python/libvirt-override.py +++ b/python/libvirt-override.py @@ -2,6 +2,11 @@ # Manually written part of python bindings for libvirt # +# Specify -i commandline option after python was started +if __name__ == "__main__": + import os + os.environ["PYTHONINSPECT"] = "1" + # On cygwin, the DLL is called cygvirtmod.dll try: import libvirtmod diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 01636aa..4593379 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -1,4 +1,4 @@ -#!/usr/bin/python +#!/usr/bin/env python # # esx_vi_generator.py: generates most of the SOAP type mapping code -- 1.7.0.4

2 2

[libvirt] [PATCH] esx: Avoid warnings about breaking strict-aliasing rules on FreeBSD
by Matthias Bolte 14 Nov '10

14 Nov '10

--- src/esx/esx_vi_generator.py | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index c2f7044..01636aa 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -699,7 +699,10 @@ class Object: if self.features & Object.FEATURE__LIST: if self.extends is not None: - source += " esxVI_%s_Free((esxVI_%s **)&item->_next);\n\n" % (self.extends, self.extends) + # avoid "dereferencing type-punned pointer will break strict-aliasing rules" warnings + source += " esxVI_%s *next = (esxVI_%s *)item->_next;\n\n" % (self.extends, self.extends) + source += " esxVI_%s_Free(&next);\n" % self.extends + source += " item->_next = (esxVI_%s *)next;\n\n" % self.name else: source += " esxVI_%s_Free(&item->_next);\n\n" % self.name @@ -719,7 +722,10 @@ class Object: if self.features & Object.FEATURE__LIST: if self.extends is not None: - source += " esxVI_%s_Free((esxVI_%s **)&item->_next);\n\n" % (self.extends, self.extends) + # avoid "dereferencing type-punned pointer will break strict-aliasing rules" warnings + source += " esxVI_%s *next = (esxVI_%s *)item->_next;\n\n" % (self.extends, self.extends) + source += " esxVI_%s_Free(&next);\n" % self.extends + source += " item->_next = (esxVI_%s *)next;\n\n" % self.name else: source += " esxVI_%s_Free(&item->_next);\n\n" % self.name -- 1.7.0.4

2 2

[libvirt] [PATCH] Make sure struct sockaddr_in is defined on FreeBSD
by Matthias Bolte 14 Nov '10

14 Nov '10

--- src/util/network.h | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/util/network.h b/src/util/network.h index 0900e29..724a5f8 100644 --- a/src/util/network.h +++ b/src/util/network.h @@ -21,6 +21,10 @@ # include <netdb.h> # include <stdbool.h> +# ifdef __FreeBSD__ +# include <netinet/in.h> +# endif + typedef struct { union { struct sockaddr sa; -- 1.7.0.4

2 2

[libvirt] [PATCH] configure: Disable FS storage driver if mntent.h is not available
by Matthias Bolte 14 Nov '10

14 Nov '10

This is the case on FreeBSD. --- configure.ac | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/configure.ac b/configure.ac index 66237e1..d21d558 100644 --- a/configure.ac +++ b/configure.ac @@ -1497,6 +1497,18 @@ if test "$with_osx" = "yes"; then fi if test "$with_storage_fs" = "yes" || test "$with_storage_fs" = "check"; then + AC_CHECK_HEADER([mntent.h],, + [ + if test "$with_storage_fs" = "check"; then + with_storage_fs=no + AC_MSG_NOTICE([<mntent.h> is required for the FS storage driver, disabling it]) + else + AC_MSG_ERROR([<mntent.h> is required for the FS storage driver]) + fi + ]) +fi + +if test "$with_storage_fs" = "yes" || test "$with_storage_fs" = "check"; then AC_PATH_PROG([MOUNT], [mount], [], [$PATH:/sbin:/usr/sbin]) AC_PATH_PROG([UMOUNT], [umount], [], [$PATH:/sbin:/usr/sbin]) if test "$with_storage_fs" = "yes" ; then -- 1.7.0.4

2 2

[libvirt] [PATCH] daemon: Include stdlib.h in dispatch.c
by Matthias Bolte 14 Nov '10

14 Nov '10

Otherwise GCC complains about malloc being unknown on FreeBSD. --- daemon/dispatch.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/daemon/dispatch.c b/daemon/dispatch.c index 9731614..bf2ac73 100644 --- a/daemon/dispatch.c +++ b/daemon/dispatch.c @@ -24,6 +24,7 @@ #include <config.h> #include <stdio.h> +#include <stdlib.h> #include <stdarg.h> #include <stdbool.h> -- 1.7.0.4

2 2

Re: [libvirt] Is there an API for clone?
by Alex Jia 14 Nov '10

14 Nov '10

Hi lancer, As Osier said, libvirt hasn't a independent API for cloning an existing guest, instead of using virStorageVolGetXMLDesc API to get a guest volume xml description and then calling virStorageVolCreateXMLFrom API to clone a new volume. For python-virtinst component, you may use virt-clone to implement this, for libvirt component, you may directly use python binding API or virsh tool 'vol-clone' to implement it. I hope it's helpful for you. - Alex ----- Original Message ----- From: "黄亮" <lancerhuang(a)163.com> To: "libvir-list" <libvir-list(a)redhat.com> Sent: Wednesday, November 10, 2010 9:30:28 AM GMT +08:00 Beijing / Chongqing / Hong Kong / Urumqi Subject: [libvirt] Is there an API for clone? Hi, guys I know there is a command "virt-clone", but I couldn't find the equivalent API here : http://libvirt.org/html/libvirt-libvirt.html So, is there an API to clone an existing VM or not? Thank you! Regards 2010-11-10 Lancer -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

3 3

[libvirt] CSharp bindings patch
by arnaud.champion＠devatom.fr 14 Nov '10

14 Nov '10

?Hi, here is a patch because I have forgotten to include comments on sample Regards, Arnaud

1 0

[libvirt] Fw: CSharp bindings patch
by arnaud.champion＠devatom.fr 14 Nov '10

14 Nov '10

?Hi, here is a 3rd patch that add a sample named "virDomainStats" Regards, Arnaud

1 0

[libvirt] Fw: CSharp bindings patch
by arnaud.champion＠devatom.fr 14 Nov '10

14 Nov '10

?Hi, here is a 2nd patch to correct marshaling around virError. Regards, Arnaud

1 0