[libvirt] [Q]Why does libvirt check size of memory in xenStoreDriver?
Hi, I have a question. Why does libvirt check size of memory in xenStoreDomainSetMemory of xs_internal.c? In the xen not exclude lifecycle, when I do the following command for the domain that is shutoff, error message is output and return value is 0 and the definition file(=/etc/xen/XXX) is updated. # virsh setmem(or setmaxmem) guestdom 65535(4096-65535) libvir: Xen Store error : invalid argument in xenStoreDomainSetMemory # echo $? 0 As a result that I research this, this error message is output by the check of the memory size in xenStoreDriver. Because libvirt checks the memory size of under 4096 in libvirt.c, I think that libvirt should check memory size range of 4096-65535 in libvirt.c. Thanks, Shigeki Sakamoto. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Q]Why does libvirt check size of memory inxenStoreDriver?
Hi, Shigeki At first, libvirt.c should not check VMM-arch specific value. In this meaning, xs_internal.c do the right thing. The problem is error handling on libvirt.c(Xen Store error is not handled properly). Do you investigate it? Thanks Atsushi SAKAI S.Sakamoto [EMAIL PROTECTED] wrote: Hi, I have a question. Why does libvirt check size of memory in xenStoreDomainSetMemory of xs_internal.c? In the xen not exclude lifecycle, when I do the following command for the domain that is shutoff, error message is output and return value is 0 and the definition file(=/etc/xen/XXX) is updated. # virsh setmem(or setmaxmem) guestdom 65535(4096-65535) libvir: Xen Store error : invalid argument in xenStoreDomainSetMemory # echo $? 0 As a result that I research this, this error message is output by the check of the memory size in xenStoreDriver. Because libvirt checks the memory size of under 4096 in libvirt.c, I think that libvirt should check memory size range of 4096-65535 in libvirt.c. Thanks, Shigeki Sakamoto. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] sVirt v0.10 - initial prototype
Hi, James I have a question just for interest. The security context stores like /etc/selinux/targeted/contexts/files/file_contexts. But you are storeing the domain security label on libvirt specific XML. Of course, this is good for libvirt POV. Is it permitted for SELinux policy POV? By the way, I want to see the further discussion of the Requirements. Requirements not yet addressed include: Thanks Atsushi SAKAI James Morris [EMAIL PROTECTED] wrote: [snip] The domain security label configuration format is as follows: # virsh dumpxml sys1 domain seclabel model='selinux' labelsystem_u:system_r:virtd_t:s0/label policytypetargeted/policytype /seclabel /domain It's possible to query the security label of a running domain via virsh: # virsh dominfo sys1 Id: 1 Name: sys1 UUID: fa3c8e06-0877-2a08-06fd-f2479b7bacb4 OS Type:hvm State: running CPU(s): 1 CPU time: 11.4s Max memory: 524288 kB Used memory:524288 kB Autostart: disable Security label: system_u:system_r:virtd_t:s0 (selinux/targeted/enforcing) The security label is deterimed via the new virDomainGetSecLabel() API method, which is transported over RPC to the backend driver (qemu in this case), and is entirely independent of the local security model, if any. e.g. this command could be run remotely from an entirely different platform: you just see what's happening on the remote system, as with other attributes of the domain. Feedback on the design thus far is sought before proceeding to more comprehensive integration. In particular, I'd be interested in any thoughts on the placement of the security labeling driver within libvirt, as this seems to be the most critical architectural issue (I've already refactored this aspect once). Currently, the idea is to attach the security labeling driver to the virt driver, rather than implement it independently as a top-level component as in the case of other types of drivers (e.g. storage). This is because process-based security labeling is highly dependent on the kind of virtualization in use, and may not make sense at all in some cases (e.g. when using a non-Linux hypervisor, or containers). In the case of qemu, a security labeling driver is added to qemud: @@ -63,6 +64,7 @@ struct qemud_driver { char *vncListen; virCapsPtr caps; +virSecLabelDriverPtr secLabelDriver; }; and then initialized during qemud startup from qemudSecLabelInit(). During initialization, any available security labeling drivers are probed, and the first one which thinks it should be used is installed. Top-level libvirt API calls are then dispatched to the active security labeling driver via the backend virt driver, as necessary. Note that the security labeling framework in this release is always built-in -- it can be made a compile-time option later if desired. Requirements not yet addressed include: - Labeling of resources and generally comprehensive labeling management - Automatic labeling (e.g. for the simple isolation use-case) - Integration of labeling support into higher-level management tools such as virt-manager - Integration with the audit subsystem to help with administration and debugging - Domain of interpretation (DOI) checking/translation - Python bindings As mentioned, the goal at this stage is to get feedback on the underlying design: comments welcome! - James -- James Morris [EMAIL PROTECTED] -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 2/11: Move internal error APIs into virterror_internal.h
After Daniel's feedback that adding src/virterror.h is confusing, I've re-done the patch to instead add virterror_internal.h, making it clear that these function prototypes are related to virterror.c, but internal use only. That patch is basically moving them out of src/internal.h In addition it removes the leading __ prefix from virRaiseError since this symbol was never exported fromthe library. The diff is mis-leading - most of this is just the s/__// change. b/src/virterror_internal.h| 56 src/Makefile.am |4 +- src/conf.c|8 ++-- src/domain_conf.c |5 +- src/hash.c| 17 + src/internal.h| 24 -- src/libvirt.c | 44 - src/lxc_conf.c|1 src/lxc_conf.h|2 - src/lxc_container.c |1 src/lxc_controller.c |2 - src/lxc_driver.c |2 - src/network_conf.c|5 +- src/network_driver.c |3 + src/nodeinfo.c| 16 - src/openvz_conf.c |1 src/openvz_conf.h |2 - src/openvz_driver.c |2 - src/proxy_internal.c |5 +- src/qemu_conf.c |1 src/qemu_conf.h |2 - src/qemu_driver.c |1 src/qparams.c |3 + src/remote_internal.c | 72 +- src/sexpr.c |4 +- src/stats_linux.c |6 +-- src/storage_backend.c |2 - src/storage_backend_disk.c|2 - src/storage_backend_fs.c |2 - src/storage_backend_iscsi.c |2 - src/storage_backend_logical.c |2 - src/storage_conf.c|2 - src/storage_conf.h|2 - src/storage_driver.c |2 - src/test.c|4 +- src/util.c|4 +- src/virterror.c | 57 - src/xen_internal.c| 25 +++--- src/xen_unified.c |4 +- src/xend_internal.c |3 + src/xm_internal.c |3 + src/xml.c |5 +- src/xmlrpc.c |6 +-- src/xs_internal.c |4 +- 44 files changed, 231 insertions(+), 189 deletions(-) Daniel diff -r 786626684900 src/Makefile.am --- a/src/Makefile.am Wed Oct 29 15:23:00 2008 + +++ b/src/Makefile.am Wed Oct 29 20:09:48 2008 + @@ -50,7 +50,7 @@ stats_linux.c stats_linux.h \ uuid.c uuid.h \ util.c util.h \ - virterror.c \ + virterror.c virterror_internal.h\ xml.c xml.h # Domain driver generic impl APIs @@ -149,7 +149,7 @@ driver.h\ hash.c hash.h \ internal.h \ - libvirt.c \ + libvirt.c libvirt_internal.h\ $(GENERIC_LIB_SOURCES) \ $(DOMAIN_CONF_SOURCES) \ $(NETWORK_CONF_SOURCES) \ diff -r 786626684900 src/conf.c --- a/src/conf.cWed Oct 29 15:23:00 2008 + +++ b/src/conf.cWed Oct 29 20:09:48 2008 + @@ -18,7 +18,7 @@ #include sys/stat.h #include fcntl.h -#include internal.h +#include virterror_internal.h #include buf.h #include conf.h #include util.h @@ -96,13 +96,13 @@ /* Construct the string 'filename:line: info' if we have that. */ if (ctxt ctxt-filename) { -__virRaiseError(NULL, NULL, NULL, VIR_FROM_CONF, error, VIR_ERR_ERROR, +virRaiseError(NULL, NULL, NULL, VIR_FROM_CONF, error, VIR_ERR_ERROR, info, ctxt-filename, NULL, ctxt-line, 0, %s:%d: %s, ctxt-filename, ctxt-line, info); } else { -format = __virErrorMsg(error, info); -__virRaiseError(NULL, NULL, NULL, VIR_FROM_CONF, error, VIR_ERR_ERROR, +format = virErrorMsg(error, info); +virRaiseError(NULL, NULL, NULL, VIR_FROM_CONF, error, VIR_ERR_ERROR, info, NULL, NULL, ctxt ? ctxt-line : 0, 0, format, info); diff -r 786626684900 src/domain_conf.c --- a/src/domain_conf.c Wed Oct 29 15:23:00 2008 + +++ b/src/domain_conf.c Wed Oct 29 20:09:48 2008 + @@ -29,8 +29,7 @@ #include fcntl.h #include dirent.h -#include internal.h - +#include virterror_internal.h #include
Re: [libvirt] PATCH: 3/11: Remove unused virStateSigDispatcher() api
When we added the LXC driver, it originally needed to respond to signals. We removed that need a while ago, but left the infrastructure around. This patch removes the driver calls relating to signal progation, since they're unneccessary and we shouldn't follow this approach again in the future, because all drivers need to survive libvirtd restarts, which means they cannot rely in having signals from child processes. qemud/qemud.c|3 +-- src/driver.h |6 +- src/internal.h |2 -- src/libvirt.c| 10 -- src/network_driver.c |1 - 5 files changed, 2 insertions(+), 20 deletions(-) Daniel diff -r dbe9394acdf8 qemud/qemud.c --- a/qemud/qemud.c Wed Oct 29 15:23:01 2008 + +++ b/qemud/qemud.c Wed Oct 29 15:24:01 2008 + @@ -262,9 +262,8 @@ break; default: -qemudLog(QEMUD_INFO, _(Received signal %d, dispatching to drivers), +qemudLog(QEMUD_INFO, _(Received unexpected signal %d), siginfo.si_signo); -virStateSigDispatcher(siginfo); break; } diff -r dbe9394acdf8 src/driver.h --- a/src/driver.h Wed Oct 29 15:23:01 2008 + +++ b/src/driver.h Wed Oct 29 15:24:01 2008 + @@ -10,8 +10,6 @@ #include libvirt/virterror.h #include libxml/uri.h - -#include signal.h /* * List of registered drivers numbers @@ -595,12 +593,11 @@ virDrvStorageVolGetPath volGetPath; }; +#ifdef WITH_LIBVIRTD typedef int (*virDrvStateInitialize) (void); typedef int (*virDrvStateCleanup) (void); typedef int (*virDrvStateReload) (void); typedef int (*virDrvStateActive) (void); -#ifdef WITH_LIBVIRTD -typedef int (*virDrvSigHandler) (siginfo_t *siginfo); typedef struct _virStateDriver virStateDriver; typedef virStateDriver *virStateDriverPtr; @@ -610,7 +607,6 @@ virDrvStateCleanup cleanup; virDrvStateReload reload; virDrvStateActive active; -virDrvSigHandler sigHandler; }; #endif diff -r dbe9394acdf8 src/internal.h --- a/src/internal.hWed Oct 29 15:23:01 2008 + +++ b/src/internal.hWed Oct 29 15:24:01 2008 + @@ -333,12 +333,10 @@ int __virStateCleanup(void); int __virStateReload(void); int __virStateActive(void); -int __virStateSigDispatcher(siginfo_t *siginfo); #define virStateInitialize() __virStateInitialize() #define virStateCleanup() __virStateCleanup() #define virStateReload() __virStateReload() #define virStateActive() __virStateActive() -#define virStateSigDispatcher(s) __virStateSigDispatcher(s) #endif int __virDrvSupportsFeature (virConnectPtr conn, int feature); diff -r dbe9394acdf8 src/libvirt.c --- a/src/libvirt.c Wed Oct 29 15:23:01 2008 + +++ b/src/libvirt.c Wed Oct 29 15:24:01 2008 + @@ -625,16 +625,6 @@ return ret; } -int __virStateSigDispatcher(siginfo_t *siginfo) { -int i, ret = 0; - -for (i = 0 ; i virStateDriverTabCount ; i++) { -if (virStateDriverTab[i]-sigHandler -virStateDriverTab[i]-sigHandler(siginfo)) -ret = 1; -} -return ret; -} #endif diff -r dbe9394acdf8 src/network_driver.c --- a/src/network_driver.c Wed Oct 29 15:23:01 2008 + +++ b/src/network_driver.c Wed Oct 29 15:24:01 2008 + @@ -1144,7 +1144,6 @@ networkShutdown, networkReload, networkActive, -NULL }; int networkRegister(void) { -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 5/11: Move internal domain events APIs to their own file
This is a re-diff of the patch I sent yesterday to just move the domain events code into its own separate file. b/src/domain_event.c | 229 ++ b/src/domain_event.h | 83 ++ qemud/event.c |8 - qemud/event.h |7 + src/Makefile.am |1 src/internal.h| 72 --- src/libvirt.c | 203 src/qemu_conf.h |1 src/remote_internal.c |1 9 files changed, 327 insertions(+), 278 deletions(-) Daniel diff -r f9233a3621e6 qemud/event.c --- a/qemud/event.c Wed Oct 29 15:51:13 2008 + +++ b/qemud/event.c Wed Oct 29 15:58:26 2008 + @@ -489,7 +489,7 @@ } int -__virEventHandleTypeToPollEvent(virEventHandleType events) +virEventHandleTypeToPollEvent(int events) { int ret = 0; if(events VIR_EVENT_HANDLE_READABLE) @@ -503,10 +503,10 @@ return ret; } -virEventHandleType -__virPollEventToEventHandleType(int events) +int +virPollEventToEventHandleType(int events) { -virEventHandleType ret = 0; +int ret = 0; if(events POLLIN) ret |= VIR_EVENT_HANDLE_READABLE; if(events POLLOUT) diff -r f9233a3621e6 qemud/event.h --- a/qemud/event.h Wed Oct 29 15:51:13 2008 + +++ b/qemud/event.h Wed Oct 29 15:58:26 2008 + @@ -105,4 +105,11 @@ */ int virEventRunOnce(void); +int +virEventHandleTypeToPollEvent(int events); +int +virPollEventToEventHandleType(int events); + + + #endif /* __VIRTD_EVENT_H__ */ diff -r f9233a3621e6 src/Makefile.am --- a/src/Makefile.am Wed Oct 29 15:51:13 2008 + +++ b/src/Makefile.am Wed Oct 29 15:58:26 2008 + @@ -150,6 +150,7 @@ driver.h\ internal.h \ datatypes.c datatypes.h \ + domain_event.c domain_event.h \ libvirt.c libvirt_internal.h\ $(GENERIC_LIB_SOURCES) \ $(DOMAIN_CONF_SOURCES) \ diff -r f9233a3621e6 src/domain_event.c --- /dev/null Thu Jan 01 00:00:00 1970 + +++ b/src/domain_event.cWed Oct 29 15:58:26 2008 + @@ -0,0 +1,229 @@ +/* + * domain_event.c: domain event queue processing helpers + * + * Copyright (C) 2008 VirtualIron + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Ben Guthro + */ + +#include config.h + +#include domain_event.h +#include datatypes.h +#include memory.h + + +/** + * virDomainEventCallbackListFree: + * @list: event callback list head + * + * Free the memory in the domain event callback list + */ +void +virDomainEventCallbackListFree(virDomainEventCallbackListPtr list) +{ +int i; +for (i=0; ilist-count; i++) { +VIR_FREE(list-callbacks[i]); +} +VIR_FREE(list); +} +/** + * virDomainEventCallbackListRemove: + * @conn: pointer to the connection + * @cbList: the list + * @callback: the callback to remove + * + * Internal function to remove a callback from a virDomainEventCallbackListPtr + */ +int +virDomainEventCallbackListRemove(virConnectPtr conn, + virDomainEventCallbackListPtr cbList, + virConnectDomainEventCallback callback) +{ +int i; +for (i = 0 ; i cbList-count ; i++) { +if(cbList-callbacks[i]-cb == callback + cbList-callbacks[i]-conn == conn) { +virUnrefConnect(cbList-callbacks[i]-conn); +VIR_FREE(cbList-callbacks[i]); + +if (i (cbList-count - 1)) +memmove(cbList-callbacks + i, +cbList-callbacks + i + 1, +sizeof(*(cbList-callbacks)) * +(cbList-count - (i + 1))); + +if (VIR_REALLOC_N(cbList-callbacks, + cbList-count - 1) 0) { +; /* Failure to reduce memory allocation isn't fatal */ +} +cbList-count--; + +return 0; +} +} +return -1; +} + +/** + * virDomainEventCallbackListAdd: + * @conn: pointer to the connection + * @cbList:
Re: [libvirt] PATCH: 6/11: Change WITH_XXXX macros to love in config.h
The configure script lets users turn on/of individual drivers. Their choices get fed into a LIBVIRT_FEATURES macro, which has one or more -DWITH_XEN -DWITH_OPENVZ, etc, etc. The compiler args become rather long when we have all the drivers enabled. So this patch tweaks the configure file to instead put all these WITH_XEN, WITH_QEMU macros into the config.h file. Secondly, the Makefile.am has a few places where we do nested conditionals to determine whether to build QEMU, eg if WITH_LIBVIRTD if WITH_QEMU This patch also tweaks the configure script so that WITH_QEMU is never defined, unless WITH_LIBVIRTD is also defined. Now the makefile.am can just do if WITH_QEMU which makes things a little more readable, and helps avoid errors where we miss the WITH_LIBVIRTD wrapper. There is no functional code change here, its all just playing with the way makefile/macro conditionals are done. configure.in | 83 +- proxy/Makefile.am |4 +- qemud/Makefile.am |2 - src/Makefile.am | 29 +- src/libvirt.c |2 - tests/Makefile.am |4 -- 6 files changed, 70 insertions(+), 54 deletions(-) Daniel diff -r 0960752d24df configure.in --- a/configure.in Wed Oct 29 20:10:43 2008 + +++ b/configure.in Wed Oct 29 20:10:49 2008 + @@ -232,59 +232,69 @@ AC_PATH_PROG([IPTABLES_PATH], [iptables], /sbin/iptables, [/usr/sbin:$PATH]) AC_DEFINE_UNQUOTED([IPTABLES_PATH], $IPTABLES_PATH, [path to iptables binary]) -dnl -dnl Specify the xen-distribution directory to be able to compile on a -dnl non-xenified host -dnl -AC_ARG_WITH([xen-distdir], [AC_HELP_STRING([--with-xen-distdir=path], -[distribution directory of Xen, default /usr])]) -if test x$with_xen_distdir != x -then -CPPFLAGS=$CPPFLAGS -I$withval/install/usr/include -LDFLAGS=$LDFLAGS -L$withval/install/usr/lib -L$withval/install/usr/lib64 -fi - -LIBVIRT_FEATURES= -WITH_XEN=0 - if test $with_openvz = yes; then -LIBVIRT_FEATURES=$LIBVIRT_FEATURES -DWITH_OPENVZ +AC_DEFINE_UNQUOTED([WITH_OPENVZ], 1, [whether OpenVZ driver is enabled]) fi AM_CONDITIONAL([WITH_OPENVZ], [test $with_openvz = yes]) +if test $with_libvirtd = no ; then + with_lxc=no +fi if test $with_lxc = yes ; then -LIBVIRT_FEATURES=$LIBVIRT_FEATURES -DWITH_LXC +AC_DEFINE_UNQUOTED([WITH_LXC], 1, [whether LXC driver is enabled]) fi AM_CONDITIONAL([WITH_LXC], [test $with_lxc = yes]) +if test $with_libvirtd = no ; then + with_qemu=no +fi if test $with_qemu = yes ; then -LIBVIRT_FEATURES=$LIBVIRT_FEATURES -DWITH_QEMU +AC_DEFINE_UNQUOTED([WITH_QEMU], 1, [whether QEMU driver is enabled]) fi AM_CONDITIONAL([WITH_QEMU], [test $with_qemu = yes]) if test $with_test = yes ; then -LIBVIRT_FEATURES=$LIBVIRT_FEATURES -DWITH_TEST +AC_DEFINE_UNQUOTED([WITH_TEST], 1, [whether Test driver is enabled]) fi AM_CONDITIONAL([WITH_TEST], [test $with_test = yes]) if test $with_remote = yes ; then -LIBVIRT_FEATURES=$LIBVIRT_FEATURES -DWITH_REMOTE +AC_DEFINE_UNQUOTED([WITH_REMOTE], 1, [whether Remote driver is enabled]) fi AM_CONDITIONAL([WITH_REMOTE], [test $with_remote = yes]) if test $with_libvirtd = yes ; then -LIBVIRT_FEATURES=$LIBVIRT_FEATURES -DWITH_LIBVIRTD +AC_DEFINE_UNQUOTED([WITH_LIBVIRTD], 1, [whether libvirtd daemon is enabled]) fi AM_CONDITIONAL([WITH_LIBVIRTD], [test $with_libvirtd = yes]) -if test $with_xen = yes ; then -dnl search for the Xen store library -AC_SEARCH_LIBS(xs_read, [xenstore], - [WITH_XEN=1], - [AC_MSG_RESULT([Xen store library not found])]) -if test $WITH_XEN != 0 ; then -LIBVIRT_FEATURES=$LIBVIRT_FEATURES -DWITH_XEN +XEN_LIBS= +XEN_CFLAGS= +dnl search for the Xen store library +if test $with_xen != no ; then +if test $with_xen != yes -a $with_xen != check ; then +XEN_CFLAGS=-I$with_xen/include +XEN_LIBS=-L$with_xen/lib64 -L$with_xen/lib fi +fail=0 +old_LIBS=$LIBS +old_CFLAGS=$CFLAGS +CFLAGS=$CFLAGS $XEN_CFLAGS +LIBS=$LIBS $XEN_LIBS +AC_CHECK_LIB([xenstore], [xs_read], [ + with_xen=yes + XEN_LIBS=$XEN_LIBS -lxenstore + ],[ + if test $with_xen = check ; then + with_xen=no + else + with_xen=no + fail=1 + fi + ]) + +test $fail = 1 + AC_MSG_ERROR([You must install the Xen development package to compile Xen driver with -lxenstore]) AC_CHECK_HEADERS([xen/xen.h xen/version.h xen/dom0_ops.h],,[ AC_MSG_ERROR([Cannot find standard Xen headers. Is xen-devel installed?]) @@ -307,8 +317,15 @@ #include stdint.h #include xen/xen.h ]) +LIBS=$old_LIBS +CFLAGS=$old_CFLAGS fi -AM_CONDITIONAL([WITH_XEN], [test $WITH_XEN = 1]) +if test $with_xen = yes; then +AC_DEFINE_UNQUOTED([WITH_XEN], 1, [whether Xen driver is enabled]) +fi +AM_CONDITIONAL([WITH_XEN], [test $with_xen
Re: [libvirt] PATCH: 7/11: Build libtool convenience libraries of all drivers
A while ago we introduced a libvirt_test.so which is identical to libvirt.so, but without the linker whitelist of symbols. This did impose a little overhead in the build process - it meant we compiled all source files twice. A future patch will want to compile drivers into individual libraries to allow dlopening, and it occurred to me that if we built some more libtool convenience libraries we could avoid the double compilation. The trick is that libvirt.la must not have any source files listed against it. It must only be used to link together other convenience libraries. So, I define a while set of libraries - libvirt_util.la - generic code not related to drivers or libvirt API - libvirt_driver.la - the libvirt public API and driver support code - libvirt_driver_.la - one for each of qemu, xen, test, remote, openvz, lxc, storage, network These are all convenience libraries, so libtool will just build a local non-installed, static .a archive. Now, the actual things we care about - libvirt.la - links to all of these convenience .la libraries together and adds the libvirt_sym.version script to filter symbols - libvirt_test.la - the same, but with the version script - libvirt_lxc - links to libvirt_util.la and libvirt_driver_lxc.la - virsh - links to libvirt.la So the end result is that each source file is only ever compiled once, and each library/program just links to the .a files is actually needs. Makefile.am | 107 +++- 1 file changed, 71 insertions(+), 36 deletions(-) Daniel diff -r a5221822f20b src/Makefile.am --- a/src/Makefile.am Wed Oct 29 20:10:49 2008 + +++ b/src/Makefile.am Wed Oct 29 20:10:52 2008 + @@ -6,11 +6,7 @@ [EMAIL PROTECTED]@/include \ [EMAIL PROTECTED]@/qemud \ $(LIBXML_CFLAGS) \ - $(GNUTLS_CFLAGS) \ - $(SASL_CFLAGS) \ $(SELINUX_CFLAGS) \ - $(NUMACTL_CFLAGS) \ - $(XEN_CFLAGS) \ -DBINDIR=\$(libexecdir)\ \ -DSBINDIR=\$(sbindir)\ \ -DSYSCONF_DIR=\$(sysconfdir)\ \ @@ -18,10 +14,6 @@ -DLOCAL_STATE_DIR=\$(localstatedir)\ \ -DGETTEXT_PACKAGE=\$(PACKAGE)\ \ $(WARN_CFLAGS) - -DEPS = libvirt.la -LDADDS = @STATIC_BINARIES@ $(WARN_CFLAGS) libvirt.la ../gnulib/lib/libgnu.la -VIRSH_LIBS = @VIRSH_LIBS@ confdir = $(sysconfdir)/libvirt/ conf_DATA = qemu.conf @@ -40,7 +32,7 @@ # These files are not related to driver APIs. Simply generic # helper APIs for various purposes -GENERIC_LIB_SOURCES = \ +UTIL_SOURCES = \ bridge.c bridge.h \ buf.c buf.h \ conf.c conf.h \ @@ -53,6 +45,16 @@ util.c util.h \ virterror.c virterror_internal.h\ xml.c xml.h + +# Internal generic driver infrastructure +DRIVER_SOURCES = \ + driver.h\ + internal.h \ + datatypes.c datatypes.h \ + domain_event.c domain_event.h \ + stats_linux.c stats_linux.h \ + libvirt.c libvirt_internal.h + # Domain driver generic impl APIs DOMAIN_CONF_SOURCES = \ @@ -146,62 +148,91 @@ # # First deal with sources usable in non-daemon context -libvirt_la_SOURCES = \ - driver.h\ - internal.h \ - datatypes.c datatypes.h \ - domain_event.c domain_event.h \ - stats_linux.c stats_linux.h \ - libvirt.c libvirt_internal.h\ - $(GENERIC_LIB_SOURCES) \ +noinst_LTLIBRARIES = libvirt_util.la +libvirt_la_LIBADD = libvirt_util.la +libvirt_util_la_SOURCES = \ + $(UTIL_SOURCES) + +noinst_LTLIBRARIES += libvirt_driver.la +libvirt_la_LIBADD += libvirt_driver.la +libvirt_driver_la_SOURCES =\ + $(DRIVER_SOURCES) \ $(DOMAIN_CONF_SOURCES) \ $(NETWORK_CONF_SOURCES) \ $(STORAGE_CONF_SOURCES) if WITH_TEST -libvirt_la_SOURCES += $(TEST_DRIVER_SOURCES) +noinst_LTLIBRARIES += libvirt_driver_test.la
Re: [libvirt] PATCH: 8/11: Make better use of linker scripts
The libvirt.so file currently whitelists all the symbols in our public API. libvirtd and virsh, however, also want to use a bunch of our so called 'private' symbols which aren't in the public API. For saferead and safewrite() we dealt with this by compiling the code twice with some nasty macro tricks to give the function a different name to avoid dup symbols when statically linking. For the other APIs, we prefixed them with __ and then just added them to the exports. Neither option is very good because they both impose a burden on the source code - needing to append __ everywhere and write crazy macros. Each time we want to export another symbol, we have to make lots of manual changes to add __. This was OK if it was just a handful of symbols, but we're now upto 30 odd, and its not scaling. When we make drivers modular we'll be adding 100's more symbols. As an aside, we use ELF library versioning when linking, via the libtool -version-info flag. Since we maintain ABI compat going forwards though, the version in the eventual .so is always going to be 'libvirt.so.0'. This is sub-optimal because you may build a binary that requires 'virDomainBlockPeek' which appeared in libvirt 0.4.2, but the ELF version cannot validate this. There is nothing stopping your application launching against libvirt 0.3.0 and then aborting with a linker failure sometime while it is running when it eventually referneces the non-existant symbol. Likewise RPM automatic versioning hooks off the ELF version too, so that's not preventing you installing too old a libvirt for your app's needs. A far better solution to all these problems has been sitting in front of us the whole time. Namely we need to make full use of the linkers symbol version scripts. Our current libvirt_sym.version script is defining an unversioned set of symbols. This only enforces what's exported, and can do nothing about version compatability. So instead we need to switch to a fully versioned script, where every symbol we export is tagged with the libvirt version number at which it was introduced. Taking the virDomainBlockPeek example again, this appeared in the libvirt 0.4.2 release, so we'd add a section to the linker script LIBVIRT_0.4.2 { global: virDomainBlockPeek; virDomainMemoryPeek; } LIBVIRT_0.4.1; Then 0.4.5 added in storage pool discovery so you get another section LIBVIRT_0.4.5 { global: virConnectFindStoragePoolSources; } LIBVIRT_0.4.2; And so on for every release. The resulting libvirt.so file will thus gain metadata listing all the ABI versions for all symbols it includes. That deals with public APIs. Now we also want to export some internal APIs whose semantics/syntax may arbitrarily change between releases. Thus they should not be included in any of the formal public version sections. Instead they should be in a seperate versioned section, whose version changes on every release. This will ensure that if libvirtd or virsh link to some internal symbols, they are guareteened to only run against the exactly same libvirt.so they were linked to. This will avoid some nasty bugs our users have hit where they installed a custom libvirt version on their OS which already had another version installed, and then libvirtd crashed/behave wierdly/etc To do this we rename libvirt_sym.version to libvirt_sym.version.in and add a section called [EMAIL PROTECTED]@ { global: /* libvirt_internal.h */ debugFlag; virStateInitialize; virStateCleanup; virStateReload; virStateActive; more private symbols... } The @VERSION@ gets subsituted by the version number of the libvirt release by configure. Do a build with this all active and look at the resulting libvirt.so using objdump (or eu-readelf). # objdump -p src/.libs/libvirt.so Version definitions: 1 0x01 0x0e5a1d10 libvirt.so.0 2 0x00 0x0af6bd33 LIBVIRT_0.0.3 3 0x00 0x0af6bd35 LIBVIRT_0.0.5 LIBVIRT_0.0.3 4 0x00 0x0af6be30 LIBVIRT_0.1.0 LIBVIRT_0.0.5 5 0x00 0x0af6be31 LIBVIRT_0.1.1 LIBVIRT_0.1.0 6 0x00 0x0af6be34 LIBVIRT_0.1.4 LIBVIRT_0.1.1 7 0x00 0x0af6be35 LIBVIRT_0.1.5 LIBVIRT_0.1.4 8 0x00 0x0af6be39 LIBVIRT_0.1.9 LIBVIRT_0.1.5 9 0x00 0x0af6bb30 LIBVIRT_0.2.0 LIBVIRT_0.1.9 10 0x00 0x0af6bb31 LIBVIRT_0.2.1 LIBVIRT_0.2.0 11 0x00 0x0af6bb33 LIBVIRT_0.2.3 LIBVIRT_0.2.1 12 0x00 0x0af6bc30 LIBVIRT_0.3.0 LIBVIRT_0.2.3 13 0x00 0x0af6bc32 LIBVIRT_0.3.2 LIBVIRT_0.3.0 14 0x00 0x0af6bc33 LIBVIRT_0.3.3 LIBVIRT_0.3.2 15 0x00 0x0af6b930 LIBVIRT_0.4.0 LIBVIRT_0.3.3 16 0x00 0x0af6b931 LIBVIRT_0.4.1 LIBVIRT_0.4.0 17 0x00 0x0af6b932 LIBVIRT_0.4.2 LIBVIRT_0.4.1 18 0x00 0x0af6b935 LIBVIRT_0.4.5 LIBVIRT_0.4.2 19 0x00 0x0af6ba30 LIBVIRT_0.5.0 LIBVIRT_0.4.5 20 0x00 0x09e39b06 LIBVIRT_PRIVATE_0.4.6 You can see that as well as the main SONAME libvirt.so.0, we've got version info for each release which introduced new public API symbols, as
Re: [libvirt] PATCH: 10/11: Build stateful drivers into libvirtd instead of libvirt.so
This patches changes the way some of the drivers are built. Specifically the stateful drivers (QEMU, LXC, Network and Storage) are no longer compiled into libvirt.so Instead they are linked directly to the libvirt binary. They could only ever be executed as part of the daemon, and we had code checks to ensure this, so this just makes it sane at build time. More importantly this is required for David Lively's host device support. That support uses HAL, which uses DBus, which is GPL/AFL licensed. As such we cannot ever link HAL/DBus against libvirt.so without loosing our LGPL status. With this patch, HAL/DBus bits will only ever be linked into the libvirtd daemon binary, so there is no LGPL compat problems in libvirt.so The changes this patch does are fairly simple - qemud/Makefile.am - add the libtool convenience libraries for qemu, lxc, storage and network drivers to libvirtd - src/Makefile.am - remove the libtool convenience libraries for qemu, lxc, storage network drivers from libvirt.la - src/libvirt.c: Do not invoke the register methods for qemu, lxc, storage or network drivers in virInitialize() - qemud/qemud.c: Invoke the register methods for qemu, lxc, storage and network drivers after first calling virInitialize() - libvirt_sym.version.in: add all the internal symbols needed to be exported for qemu, lxc, storage network drivers The end result here is functionally identical to before, with one annoying exception. If you passed a 'NULL' uri to virConnectOpen it would manually probe each driver to find a suitable URI. Since QEMU, LXC drivers are not in libvirt.so anymore this probing ceases to work. I've not figured out best way to deal with this yet. One option is to not actually move the drivers out of libvirt.so at all. Just do it for the forthcoming node devices driver. ANother option is to just have a tiny QEMU/LXC stub driver in libvirt.so solely containing the probe() code. A further option is to make the remote driver implement the probe() method, so it actally talks to libvirtd and asks that for a probed URI. qemud/Makefile.am | 20 + qemud/qemud.c | 29 +++ src/Makefile.am| 13 ++- src/libvirt.c | 22 - src/libvirt_sym.version.in | 168 + tests/Makefile.am |4 - 6 files changed, 227 insertions(+), 29 deletions(-) Daniel diff -r f19d084a0d4f qemud/Makefile.am --- a/qemud/Makefile.am Thu Oct 30 10:46:21 2008 + +++ b/qemud/Makefile.am Thu Oct 30 11:04:27 2008 + @@ -88,7 +88,25 @@ $(POLKIT_LIBS) libvirtd_DEPENDENCIES = ../src/libvirt.la -libvirtd_LDADD = ../src/libvirt.la ../gnulib/lib/libgnu.la +libvirtd_LDADD = \ + ../gnulib/lib/libgnu.la \ + ../src/libvirt.la + +if WITH_QEMU +libvirtd_LDADD += ../src/libvirt_driver_qemu.la +endif + +if WITH_LXC +libvirtd_LDADD += ../src/libvirt_driver_lxc.la +endif + +if WITH_STORAGE_DIR +libvirtd_LDADD += ../src/libvirt_driver_storage.la +endif + +if WITH_NETWORK +libvirtd_LDADD += ../src/libvirt_driver_network.la +endif if HAVE_POLKIT policydir = $(datadir)/PolicyKit/policy diff -r f19d084a0d4f qemud/qemud.c --- a/qemud/qemud.c Thu Oct 30 10:46:21 2008 + +++ b/qemud/qemud.c Thu Oct 30 11:04:27 2008 + @@ -60,6 +60,20 @@ #ifdef HAVE_AVAHI #include mdns.h #endif + +#ifdef WITH_QEMU +#include qemu_driver.h +#endif +#ifdef WITH_LXC +#include lxc_driver.h +#endif +#ifdef WITH_NETWORK +#include network_driver.h +#endif +#ifdef WITH_STORAGE_DIR +#include storage_driver.h +#endif + static int godaemon = 0;/* -d: Be a daemon */ static int verbose = 0; /* -v: Verbose mode */ @@ -727,6 +741,21 @@ } server-sigread = sigread; + +virInitialize(); + +#ifdef WITH_QEMU +qemudRegister(); +#endif +#ifdef WITH_LXC +lxcRegister(); +#endif +#ifdef WITH_NETWORK +networkRegister(); +#endif +#ifdef WITH_STORAGE_DIR +storageRegister(); +#endif virEventRegisterImpl(virEventAddHandleImpl, virEventUpdateHandleImpl, diff -r f19d084a0d4f src/Makefile.am --- a/src/Makefile.am Thu Oct 30 10:46:21 2008 + +++ b/src/Makefile.am Thu Oct 30 11:04:27 2008 + @@ -195,7 +195,8 @@ if WITH_QEMU noinst_LTLIBRARIES += libvirt_driver_qemu.la -libvirt_la_LIBADD += libvirt_driver_qemu.la +# Stateful, so linked to daemon instead +#libvirt_la_LIBADD += libvirt_driver_qemu.la libvirt_driver_qemu_la_CFLAGS = $(NUMACTL_CFLAGS) libvirt_driver_qemu_la_LDFLAGS = $(NUMACTL_LIBS) libvirt_driver_qemu_la_SOURCES = $(QEMU_DRIVER_SOURCES) @@ -203,14 +204,16 @@ if WITH_LXC noinst_LTLIBRARIES += libvirt_driver_lxc.la -libvirt_la_LIBADD += libvirt_driver_lxc.la +# Stateful, so linked to daemon instead +#libvirt_la_LIBADD += libvirt_driver_lxc.la libvirt_driver_lxc_la_SOURCES = $(LXC_DRIVER_SOURCES) endif if WITH_NETWORK
Re: [libvirt] PATCH: 11/11: Optional dlopen() support for all drivers
This patch adds full support for dlopen()'ing of individual drivers. - configure.in gains a new command line flag --with-driver-modules. It defaults to off, producing a monolithic build. If enabled, it checks dlopen() in -ldl and dlfcn.h - src/Makefile.am if the WITH_DRIVER_MODULES conditional is defined, then instead of building each driver as a libtool convenience library and linking them into libvirt.la , they are instead build as libtool modules, and installed to $PREFIX/lib/libvirt/drivers/ - qemud/Makefile.am if the WITH_DRIVER_MODULES conditional is defined then don't link to the libtool convenience library for qemu/lxc/etc - libvirt_sym.version.in: Since we're loading drivers at runtime, we need to export more of our internal symbols. We'd already exported the vast majority in the previous patch to enable libvirtd to link to lxc, qemu, storage network drivers. This just adds the few more used by the xen, remote test drivers. - Renames qemudRegister() to qemuRegister() and xenUnifiedRegister() to just xenRegister(), so the prefix matches the module name - driver.c adds a virDriverLoadModule() method which given a module name like 'qemu', loads a file libvirt_driver_$NAME.so and runs the symbols ${NAME}Register(). It looks in $PREFIX/lib/libvirt/drivers/ by default, but can be override with LIBVIRT_DRIVER_DIR env variable eg, point it to $SOURCETREE/src/.libs to run against a non-installed libvirt build. - libvirt.c/qemud.c, if WITH_DRIVER_MODULES is defined, then instead of calling the XXXRegister() methods directly, call virDriverLoadModule() for each module So, as an example, having built with --enable-driver-modules, you can run the daemon directly from source tree with # LIBVIRT_DRIVER_DIR=`pwd`/src/.libs LIBVIRT_DEBUG=1 ./qemud/libvirtd DEBUG: libvirt.c: virInitialize (register drivers) DEBUG: libvirt.c: virRegisterDriver (registering Test as driver 0) DEBUG: libvirt.c: virRegisterNetworkDriver (registering Test as network driver 0) DEBUG: libvirt.c: virRegisterStorageDriver (registering Test as storage driver 0) DEBUG: libvirt.c: virRegisterDriver (registering Xen as driver 1) DEBUG: libvirt.c: virRegisterDriver (registering OPENVZ as driver 2) DEBUG: libvirt.c: virRegisterDriver (registering remote as driver 3) DEBUG: libvirt.c: virRegisterNetworkDriver (registering remote as network driver 1) DEBUG: libvirt.c: virRegisterStorageDriver (registering remote as storage driver 1) DEBUG: libvirt.c: virRegisterDriver (registering QEMU as driver 4) DEBUG: libvirt.c: virRegisterDriver (registering LXC as driver 5) DEBUG: libvirt.c: virRegisterNetworkDriver (registering Network as network driver 2) DEBUG: libvirt.c: virRegisterStorageDriver (registering storage as storage driver 2) And run virsh directly from source tree with $ LIBVIRT_DEBUG=1 LIBVIRT_DRIVER_DIR=`pwd`/.libs ./virsh --readonly --connect qemu:///system DEBUG: libvirt.c: virInitialize (register drivers) DEBUG: libvirt.c: virRegisterDriver (registering Test as driver 0) DEBUG: libvirt.c: virRegisterNetworkDriver (registering Test as network driver 0) DEBUG: libvirt.c: virRegisterStorageDriver (registering Test as storage driver 0) DEBUG: libvirt.c: virRegisterDriver (registering Xen as driver 1) DEBUG: libvirt.c: virRegisterDriver (registering OPENVZ as driver 2) DEBUG: libvirt.c: virRegisterDriver (registering remote as driver 3) DEBUG: libvirt.c: virRegisterNetworkDriver (registering remote as network driver 1) DEBUG: libvirt.c: virRegisterStorageDriver (registering remote as storage driver 1) DEBUG: libvirt.c: virConnectOpenAuth (name=qemu:///system, auth=0x3945e0, flags=1) DEBUG: libvirt.c: do_open (name qemu:///system to URI components: scheme qemu opaque (null) authority (null) server (null) user (null) port 0 path /system ) DEBUG: libvirt.c: do_open (trying driver 0 (Test) ...) DEBUG: libvirt.c: do_open (driver 0 Test returned DECLINED) DEBUG: libvirt.c: do_open (trying driver 1 (Xen) ...) DEBUG: libvirt.c: do_open (driver 1 Xen returned DECLINED) DEBUG: libvirt.c: do_open (trying driver 2 (OPENVZ) ...) DEBUG: libvirt.c: do_open (driver 2 OPENVZ returned DECLINED) DEBUG: libvirt.c: do_open (trying driver 3 (remote) ...) DEBUG: remote_internal.c: doRemoteOpen (proceeding with name = qemu:///system) DEBUG: remote_internal.c: doRemoteOpen (Adding Handler for remote events) DEBUG: remote_internal.c: doRemoteOpen (virEventAddHandle failed: No addHandleImpl defined. continuing without events.) DEBUG: libvirt.c: do_open (driver 3 remote returned SUCCESS) DEBUG: libvirt.c: do_open (network driver 0 Test returned DECLINED) DEBUG: libvirt.c: do_open (network driver 1 remote returned SUCCESS) DEBUG: libvirt.c: do_open (storage driver 0 Test returned DECLINED) DEBUG: libvirt.c: do_open (storage driver 1 remote returned SUCCESS) Welcome to lt-virsh, the virtualization interactive terminal. Type: 'help' for help with commands 'quit'
Re: [libvirt] PATCH: 0/11: Fully modular drivers and optional dlopen support
As a convenience I've placed the entire patch series online for direct download avoiding email client mangling :-) http://berrange.fedorapeople.org/libvirt-dlopen/ On Thu, Oct 30, 2008 at 01:33:31PM +, Daniel P. Berrange wrote: The following series of patches clean up our internal modularization to remove unneccessary dependancies between source files, and make everything follow a consistent pattern of .h declaring stuff in .c. Later in the series is plays some games with the linker scripts, and finally makes all hypervisor drivers fully modular, and optionally dlopen'able. The 10'th patch in this series is a pre-requisite to enable us to merge David's host device patches, since it lets us avoid GPL/LGPL compat issues from DBus/HAL. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 1/11: Remove use of virStringList
On Thu, Oct 30, 2008 at 01:35:24PM +, Daniel P. Berrange wrote: This patch updates the storage drivers to make full use of virStorageSourceList and thus removes the now uneeded virStringList code. No change since last time, just re-diffed for new CVS. No problem, +1 Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 2/11: Move internal error APIs into virterror_internal.h
On Thu, Oct 30, 2008 at 01:36:10PM +, Daniel P. Berrange wrote: After Daniel's feedback that adding src/virterror.h is confusing, I've re-done the patch to instead add virterror_internal.h, making it clear that these function prototypes are related to virterror.c, but internal use only. That patch is basically moving them out of src/internal.h In addition it removes the leading __ prefix from virRaiseError since this symbol was never exported fromthe library. The diff is mis-leading - most of this is just the s/__// change. Okay, I'm surprized that there is no more usage of internal.h, i would have though it was still used widely for things like ATTRIBUTE_UNUSED, but apparently not, +1 Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 2/11: Move internal error APIs into virterror_internal.h
On Thu, Oct 30, 2008 at 03:19:11PM +0100, Daniel Veillard wrote: On Thu, Oct 30, 2008 at 01:36:10PM +, Daniel P. Berrange wrote: After Daniel's feedback that adding src/virterror.h is confusing, I've re-done the patch to instead add virterror_internal.h, making it clear that these function prototypes are related to virterror.c, but internal use only. That patch is basically moving them out of src/internal.h In addition it removes the leading __ prefix from virRaiseError since this symbol was never exported fromthe library. The diff is mis-leading - most of this is just the s/__// change. Okay, I'm surprized that there is no more usage of internal.h, i would have though it was still used widely for things like ATTRIBUTE_UNUSED, but apparently not, Actually, that's just a minor simplification on my part - virterror_internal.h will include internal.h, so there was no need to keep the explicit internal.h, though we could if you wanted. Ultimately every single file uses the internal.h, via one or more of its includs. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 5/11: Move internal domain events APIs to their own file
On Thu, Oct 30, 2008 at 01:38:08PM +, Daniel P. Berrange wrote: This is a re-diff of the patch I sent yesterday to just move the domain events code into its own separate file. Yup, fine +1, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 6/11: Change WITH_XXXX macros to love in config.h
On Thu, Oct 30, 2008 at 01:38:56PM +, Daniel P. Berrange wrote: The configure script lets users turn on/of individual drivers. Their choices get fed into a LIBVIRT_FEATURES macro, which has one or more -DWITH_XEN -DWITH_OPENVZ, etc, etc. The compiler args become rather long when we have all the drivers enabled. So this patch tweaks the configure file to instead put all these WITH_XEN, WITH_QEMU macros into the config.h file. Secondly, the Makefile.am has a few places where we do nested conditionals to determine whether to build QEMU, eg if WITH_LIBVIRTD if WITH_QEMU This patch also tweaks the configure script so that WITH_QEMU is never defined, unless WITH_LIBVIRTD is also defined. Now the makefile.am can just do if WITH_QEMU which makes things a little more readable, and helps avoid errors where we miss the WITH_LIBVIRTD wrapper. There is no functional code change here, its all just playing with the way makefile/macro conditionals are done. Hum, this can get a bit crazy, I hope we will never get to the 2 pages of --with/without options of libxml2 (but we are getting closer every month) and while it easilly get impossible to test all combination, since we are doing background builds of HEAD every night, would it be possible to at least test compilation with some of the main options enabled or disabled. The mingw compile tests the bare minimum. Maybe a compile with only each of the hypervisor activated, and maybe one without avahi/SELinux/polkit/numactl to test compilation in a restricted environment, would be useful to catch this kind of troubles. Patch looks fine, the Xen configure.in changes are a bit hard to follow, I hope we will be able to give it some testing ... +1 Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 6/11: Change WITH_XXXX macros to love in config.h
On Thu, Oct 30, 2008 at 03:44:20PM +0100, Daniel Veillard wrote: On Thu, Oct 30, 2008 at 01:38:56PM +, Daniel P. Berrange wrote: The configure script lets users turn on/of individual drivers. Their choices get fed into a LIBVIRT_FEATURES macro, which has one or more -DWITH_XEN -DWITH_OPENVZ, etc, etc. The compiler args become rather long when we have all the drivers enabled. So this patch tweaks the configure file to instead put all these WITH_XEN, WITH_QEMU macros into the config.h file. Secondly, the Makefile.am has a few places where we do nested conditionals to determine whether to build QEMU, eg if WITH_LIBVIRTD if WITH_QEMU This patch also tweaks the configure script so that WITH_QEMU is never defined, unless WITH_LIBVIRTD is also defined. Now the makefile.am can just do if WITH_QEMU which makes things a little more readable, and helps avoid errors where we miss the WITH_LIBVIRTD wrapper. There is no functional code change here, its all just playing with the way makefile/macro conditionals are done. Hum, this can get a bit crazy, I hope we will never get to the 2 pages of --with/without options of libxml2 (but we are getting closer every month) and while it easilly get impossible to test all combination, since we are doing background builds of HEAD every night, would it be possible to at least test compilation with some of the main options enabled or disabled. The mingw compile tests the bare minimum. Maybe a compile with only each of the hypervisor activated, and maybe one without avahi/SELinux/polkit/numactl to test compilation in a restricted environment, would be useful to catch this kind of troubles. The later patches in this series which move every driver into a libtool convenience library, and then the subsquent dlopen support could help us here by making cross-file dependancies much more obvious. So we will catch many more problem at compile time even with all drivers enabled. I can probably tweak the compile find even more problems in this way, as well as documenting where the boundaries are in our internal APIs. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 7/11: Build libtool convenience libraries of all drivers
On Thu, Oct 30, 2008 at 01:39:33PM +, Daniel P. Berrange wrote: A while ago we introduced a libvirt_test.so which is identical to libvirt.so, but without the linker whitelist of symbols. This did impose a little overhead in the build process - it meant we compiled all source files twice. A future patch will want to compile drivers into individual libraries to allow dlopening, and it occurred to me that if we built some more libtool convenience libraries we could avoid the double compilation. The trick is that libvirt.la must not have any source files listed against it. It must only be used to link together other convenience libraries. So, I define a while set of libraries - libvirt_util.la - generic code not related to drivers or libvirt API - libvirt_driver.la - the libvirt public API and driver support code - libvirt_driver_.la - one for each of qemu, xen, test, remote, openvz, lxc, storage, network These are all convenience libraries, so libtool will just build a local non-installed, static .a archive. Now, the actual things we care about - libvirt.la - links to all of these convenience .la libraries together and adds the libvirt_sym.version script to filter symbols - libvirt_test.la - the same, but with the version script - libvirt_lxc - links to libvirt_util.la and libvirt_driver_lxc.la - virsh - links to libvirt.la So the end result is that each source file is only ever compiled once, and each library/program just links to the .a files is actually needs. In principle it's fine, it's equivalent, but it's a bit complex. Maybe this description should go somewhere in the HACKING file too i assume we can still compile with --disable-shared and everything will just build and link fine, just that the .la will be replaced with good old .a and the binaries will have them statically linked in. I we can just double check this, +1 Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 2/11: Move internal error APIs into virterror_internal.h
On Thu, Oct 30, 2008 at 02:21:35PM +, Daniel P. Berrange wrote: On Thu, Oct 30, 2008 at 03:19:11PM +0100, Daniel Veillard wrote: On Thu, Oct 30, 2008 at 01:36:10PM +, Daniel P. Berrange wrote: After Daniel's feedback that adding src/virterror.h is confusing, I've re-done the patch to instead add virterror_internal.h, making it clear that these function prototypes are related to virterror.c, but internal use only. That patch is basically moving them out of src/internal.h In addition it removes the leading __ prefix from virRaiseError since this symbol was never exported fromthe library. The diff is mis-leading - most of this is just the s/__// change. Okay, I'm surprized that there is no more usage of internal.h, i would have though it was still used widely for things like ATTRIBUTE_UNUSED, but apparently not, Actually, that's just a minor simplification on my part - virterror_internal.h will include internal.h, so there was no need to keep the explicit internal.h, though we could if you wanted. Ultimately every single file uses the internal.h, via one or more of its includs. Ah, right, i missed that, now it's clear ! thanks, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PATCH: don't print uninitialized integer in diagnostic
In adding a test of the vcpu cpuset parsing code (another patch coming separately), I noticed a bogus diagnostic: virsh --connect test:///default dumpxml 1 xml sed s/vcpu/vcpu cpuset='aaa'/ xml xml-invalid ./virsh --connect test:///default define xml-invalid 21 |head -1 libvir: Domain Config error : failed Xen syscall topology cpuset syntax error -2027441560 With the patch below, I get this output instead: (i.e., same, but without the trailing negative number) $ ./virsh --connect test:///default define xml-invalid 21 |head -1 libvir: Domain Config error : failed Xen syscall topology cpuset syntax error diff --git a/src/virterror.c b/src/virterror.c index 21c7339..be809a0 100644 --- a/src/virterror.c +++ b/src/virterror.c @@ -1,7 +1,7 @@ /* * virterror.c: implements error handling and reporting code for libvirt * - * Copy: Copyright (C) 2006 Red Hat, Inc. + * Copy: Copyright (C) 2006, 2008 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -516,7 +516,7 @@ __virErrorMsg(virErrorNumber error, const char *info) errmsg = _(could not connect to Xen Store %s); break; case VIR_ERR_XEN_CALL: -errmsg = _(failed Xen syscall %s %d); +errmsg = _(failed Xen syscall %s); break; case VIR_ERR_OS_TYPE: if (info == NULL) -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] * tests/cpuset: New script. Test for today's fix.
From c0beb0ed6dd3d392b11161c565d7dfd52ed2aceb Mon Sep 17 00:00:00 2001 From: Jim Meyering [EMAIL PROTECTED] Date: Mon, 2 Jun 2008 11:54:34 +0200 Subject: [PATCH 1/2] * tests/cpuset: New script. Test for today's fix. * tests/Makefile.am (test_scripts): Add cpuset. --- tests/Makefile.am |1 + tests/cpuset | 45 + 2 files changed, 46 insertions(+), 0 deletions(-) create mode 100755 tests/cpuset diff --git a/tests/Makefile.am b/tests/Makefile.am index 02b7970..ea577df 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -48,6 +48,7 @@ test_scripts = domainschematest if WITH_LIBVIRTD test_scripts += \ test_conf.sh \ + cpuset \ daemon-conf \ int-overflow \ read-bufsiz \ diff --git a/tests/cpuset b/tests/cpuset new file mode 100755 index 000..8722c81 --- /dev/null +++ b/tests/cpuset @@ -0,0 +1,45 @@ +#!/bin/sh +# ensure that defining with an invalid vCPU cpuset elicits a diagnostic + +# Copyright (C) 2008 Free Software Foundation, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. + +if test $VERBOSE = yes; then + set -x + virsh --version +fi + +. $srcdir/test-lib.sh + +fail=0 + +# generate input +virsh --connect test:///default dumpxml 1 xml || fail=1 + +# require the presence of the string we'll transform +grep 'vcpu' xml /dev/null || fail=1 + +sed s/vcpu/vcpu cpuset='aaa'/ xml xml-invalid || fail=1 + +# Require failure and a diagnostic. +virsh --connect test:///default define xml-invalid out 21 fail=1 +cat \EOF exp || fail=1 +libvir: Domain Config error : failed Xen syscall topology cpuset syntax error +error: Failed to define domain from xml-invalid + +EOF +compare out exp || fail=1 + +(exit $fail); exit $fail -- 1.6.0.3.756.gb776d -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 8/11: Make better use of linker scripts
On Thu, Oct 30, 2008 at 01:41:14PM +, Daniel P. Berrange wrote: The libvirt.so file currently whitelists all the symbols in our public API. libvirtd and virsh, however, also want to use a bunch of our so called 'private' symbols which aren't in the public API. For saferead and safewrite() we dealt with this by compiling the code twice with some nasty macro tricks to give the function a different name to avoid dup symbols when statically linking. For the other APIs, we prefixed them with __ and then just added them to the exports. Neither option is very good because they both impose a burden on the source code - needing to append __ everywhere and write crazy macros. Each time we want to export another symbol, we have to make lots of manual changes to add __. This was OK if it was just a handful of symbols, but we're now upto 30 odd, and its not scaling. When we make drivers modular we'll be adding 100's more symbols. As an aside, we use ELF library versioning when linking, via the libtool -version-info flag. Since we maintain ABI compat going forwards though, the version in the eventual .so is always going to be 'libvirt.so.0'. This is sub-optimal because you may build a binary that requires 'virDomainBlockPeek' which appeared in libvirt 0.4.2, but the ELF version cannot validate this. There is nothing stopping your application launching against libvirt 0.3.0 and then aborting with a linker failure sometime while it is running when it eventually referneces the non-existant symbol. Likewise RPM automatic versioning hooks off the ELF version too, so that's not preventing you installing too old a libvirt for your app's needs. A far better solution to all these problems has been sitting in front of us the whole time. Namely we need to make full use of the linkers symbol version scripts. Our current libvirt_sym.version script is defining an unversioned set of symbols. This only enforces what's exported, and can do nothing about version compatability. So instead we need to switch to a fully versioned script, where every symbol we export is tagged with the libvirt version number at which it was introduced. Taking the virDomainBlockPeek example again, this appeared in the libvirt 0.4.2 release, so we'd add a section to the linker script LIBVIRT_0.4.2 { global: virDomainBlockPeek; virDomainMemoryPeek; } LIBVIRT_0.4.1; Then 0.4.5 added in storage pool discovery so you get another section LIBVIRT_0.4.5 { global: virConnectFindStoragePoolSources; } LIBVIRT_0.4.2; And so on for every release. The resulting libvirt.so file will thus gain metadata listing all the ABI versions for all symbols it includes. That deals with public APIs. Now we also want to export some internal APIs whose semantics/syntax may arbitrarily change between releases. Thus they should not be included in any of the formal public version sections. Instead they should be in a seperate versioned section, whose version changes on every release. This will ensure that if libvirtd or virsh link to some internal symbols, they are guareteened to only run against the exactly same libvirt.so they were linked to. This will avoid some nasty bugs our users have hit where they installed a custom libvirt version on their OS which already had another version installed, and then libvirtd crashed/behave wierdly/etc To do this we rename libvirt_sym.version to libvirt_sym.version.in and add a section called [EMAIL PROTECTED]@ { global: /* libvirt_internal.h */ debugFlag; virStateInitialize; virStateCleanup; virStateReload; virStateActive; more private symbols... } The @VERSION@ gets subsituted by the version number of the libvirt release by configure. Do a build with this all active and look at the resulting libvirt.so using objdump (or eu-readelf). # objdump -p src/.libs/libvirt.so Version definitions: 1 0x01 0x0e5a1d10 libvirt.so.0 2 0x00 0x0af6bd33 LIBVIRT_0.0.3 3 0x00 0x0af6bd35 LIBVIRT_0.0.5 LIBVIRT_0.0.3 4 0x00 0x0af6be30 LIBVIRT_0.1.0 LIBVIRT_0.0.5 5 0x00 0x0af6be31 LIBVIRT_0.1.1 LIBVIRT_0.1.0 6 0x00 0x0af6be34 LIBVIRT_0.1.4 LIBVIRT_0.1.1 7 0x00 0x0af6be35 LIBVIRT_0.1.5 LIBVIRT_0.1.4 8 0x00 0x0af6be39 LIBVIRT_0.1.9 LIBVIRT_0.1.5 9 0x00 0x0af6bb30 LIBVIRT_0.2.0 LIBVIRT_0.1.9 10 0x00 0x0af6bb31 LIBVIRT_0.2.1 LIBVIRT_0.2.0 11 0x00 0x0af6bb33 LIBVIRT_0.2.3 LIBVIRT_0.2.1 12 0x00 0x0af6bc30 LIBVIRT_0.3.0 LIBVIRT_0.2.3 13 0x00 0x0af6bc32 LIBVIRT_0.3.2 LIBVIRT_0.3.0 14 0x00 0x0af6bc33 LIBVIRT_0.3.3 LIBVIRT_0.3.2 15 0x00 0x0af6b930 LIBVIRT_0.4.0 LIBVIRT_0.3.3 16 0x00 0x0af6b931 LIBVIRT_0.4.1 LIBVIRT_0.4.0 17 0x00 0x0af6b932 LIBVIRT_0.4.2 LIBVIRT_0.4.1 18 0x00 0x0af6b935 LIBVIRT_0.4.5 LIBVIRT_0.4.2 19 0x00 0x0af6ba30 LIBVIRT_0.5.0
Re: [libvirt] [PATCH] Add storage API support to test driver
Cole Robinson wrote: The attached patch implements the storage driver routines for the test driver. Most of the code is identical to storage_driver.c with all the references to backends removed. One caveat of this is that storage pools are hardcoded to a specific size when they are defined: I figure someone could expand this to read sizes from xml at definition time if they wanted, but for now hardcoded values is sufficient. I've done some decent testing with it all, so I'm pretty confident it isn't too broken. Pushed now. Thanks, Cole -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3]: Better debug output from virExec/virRun
The following three patches add improved debug output for the virExec and virRun commands. The full argv is now logged, and virRun reads the command stdout and stderr for use in error reporting. Thanks, Cole -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3]: Log argv passed to virExec and virRun
The attached patch logs the the argv's passed to the virExec and virRun functions. There's a bit of trickery here: since virRun is just a wrapper for virExec, we don't want the argv string to be logged twice. I addressed this by renaming virExec to __virExec, and keeping the original function name to simply debug the argv and then hand off control. This means anytime virExec is explictly called, the argv will be logged, but if functions wish to by pass that they can just call __virExec (which is what virRun does.) Please let me know if there are any problems with that approach. Thanks, Cole commit 8330537d24ed7d924e3caecff92402e9dc57dd4f Author: Cole Robinson [EMAIL PROTECTED] Date: Tue Oct 28 11:59:56 2008 -0400 Log argv string passed to virExec and virRun diff --git a/src/util.c b/src/util.c index 4476323..e59e25c 100644 --- a/src/util.c +++ b/src/util.c @@ -136,14 +136,14 @@ static int virSetNonBlock(int fd) { return 0; } -int -virExec(virConnectPtr conn, -const char *const*argv, -const char *const*envp, -const fd_set *keepfd, -int *retpid, -int infd, int *outfd, int *errfd, -int flags) { +static int +__virExec(virConnectPtr conn, + const char *const*argv, + const char *const*envp, + const fd_set *keepfd, + int *retpid, + int infd, int *outfd, int *errfd, + int flags) { int pid, null, i, openmax; int pipeout[2] = {-1,-1}; int pipeerr[2] = {-1,-1}; @@ -393,6 +393,27 @@ virExec(virConnectPtr conn, return -1; } +int +virExec(virConnectPtr conn, +const char *const*argv, +const char *const*envp, +const fd_set *keepfd, +int *retpid, +int infd, int *outfd, int *errfd, +int flags) { +char *argv_str; + +if ((argv_str = virArgvToString(argv)) == NULL) { +ReportError(conn, VIR_ERR_NO_MEMORY, _(command debug string)); +return -1; +} +DEBUG0(argv_str); +VIR_FREE(argv_str); + +return __virExec(conn, argv, envp, keepfd, retpid, infd, outfd, errfd, + flags); +} + /** * @conn connection to report errors against * @argv NULL terminated argv to run @@ -413,9 +434,17 @@ virRun(virConnectPtr conn, const char *const*argv, int *status) { int childpid, exitstatus, ret; +char *argv_str; + +if ((argv_str = virArgvToString(argv)) == NULL) { +ReportError(conn, VIR_ERR_NO_MEMORY, _(command debug string)); +return -1; +} +DEBUG0(argv_str); +VIR_FREE(argv_str); -if ((ret = virExec(conn, argv, NULL, NULL, - childpid, -1, NULL, NULL, VIR_EXEC_NONE)) 0) +if ((ret = __virExec(conn, argv, NULL, NULL, + childpid, -1, NULL, NULL, VIR_EXEC_NONE)) 0) return ret; while ((ret = waitpid(childpid, exitstatus, 0) == -1) errno == EINTR); -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3]: Read cmd stdout + stderr in virRun
The attached patch is my second cut at reading stdout and stderr of the command virRun kicks off. There is no hard limit to the amount of data we read now, and we use a poll loop to avoid any possible full buffer issues. If stdout or stderr had any content, we DEBUG it, and if the command appears to fail we return stderr in the error message. So now, trying to stop a logical pool with active volumes will return: $ sudo virsh pool-destroy vgdata libvir: error : internal error '/sbin/vgchange -an vgdata' exited with non-zero status 5 and signal 0: Can't deactivate volume group vgdata with 2 open logical volume(s) error: Failed to destroy pool vgdata Thanks, Cole commit af7d94392bc89fd0645514cd13a2186ca5224dfc Author: Cole Robinson [EMAIL PROTECTED] Date: Thu Oct 30 13:46:06 2008 -0400 Capture command stdout and stderr in virRun diff --git a/src/util.c b/src/util.c index e59e25c..8d624b2 100644 --- a/src/util.c +++ b/src/util.c @@ -33,6 +33,9 @@ #include errno.h #include sys/types.h #include sys/stat.h +#if HAVE_SYS_POLL_H +#include sys/poll.h +#endif #if HAVE_SYS_WAIT_H #include sys/wait.h #endif @@ -433,43 +436,129 @@ int virRun(virConnectPtr conn, const char *const*argv, int *status) { -int childpid, exitstatus, ret; -char *argv_str; +int childpid, exitstatus, execret, waitret, i; +int ret = -1; +int errfd = -1, outfd = -1; +char *outbuf = NULL; +char *errbuf = NULL; +char *argv_str = NULL; +struct pollfd fds[2]; +int finished[2]; if ((argv_str = virArgvToString(argv)) == NULL) { ReportError(conn, VIR_ERR_NO_MEMORY, _(command debug string)); -return -1; +goto error; } DEBUG0(argv_str); -VIR_FREE(argv_str); -if ((ret = __virExec(conn, argv, NULL, NULL, - childpid, -1, NULL, NULL, VIR_EXEC_NONE)) 0) -return ret; +if ((execret = __virExec(conn, argv, NULL, NULL, + childpid, -1, outfd, errfd, + VIR_EXEC_NONE)) 0) { +ret = execret; +goto error; +} + +fds[0].fd = outfd; +fds[0].events = POLLIN; +finished[0] = 0; +fds[1].fd = errfd; +fds[1].events = POLLIN; +finished[1] = 0; + +while(!(finished[0] finished[1])) { + +if (poll(fds, ARRAY_CARDINALITY(fds), -1) 0) { +if (errno == EAGAIN) +continue; +goto pollerr; +} + +for (i = 0; i ARRAY_CARDINALITY(fds); ++i) { +char data[1024], **buf; +int got, size; + +if (!(fds[i].revents)) +continue; +else if (fds[i].revents POLLHUP) +finished[i] = 1; + +if (!(fds[i].revents POLLIN)) { +if (fds[i].revents POLLHUP) +continue; + +ReportError(conn, VIR_ERR_INTERNAL_ERROR, +%s, _(Unknown poll response.)); +goto error; +} + +got = read(fds[i].fd, data, sizeof(data)); + +if (got == 0) { +finished[i] = 1; +continue; +} +if (got 0) { +if (errno == EINTR) +continue; +if (errno == EAGAIN) +break; +goto pollerr; +} -while ((ret = waitpid(childpid, exitstatus, 0) == -1) errno == EINTR); -if (ret == -1) { +buf = ((fds[i].fd == outfd) ? outbuf : errbuf); +size = (*buf ? strlen(*buf) : 0); +if (VIR_REALLOC_N(*buf, size+got+1) 0) { +ReportError(conn, VIR_ERR_NO_MEMORY, %s, realloc buf); +goto error; +} +memmove(*buf+size, data, got); +(*buf)[size+got] = '\0'; +} +continue; + +pollerr: +ReportError(conn, VIR_ERR_INTERNAL_ERROR, +_(poll error: %s), strerror(errno)); +goto error; +} + +if (outbuf) +DEBUG(Command stdout: %s, outbuf); +if (errbuf) +DEBUG(Command stderr: %s, errbuf); + +while ((waitret = waitpid(childpid, exitstatus, 0) == -1) +errno == EINTR); +if (waitret == -1) { ReportError(conn, VIR_ERR_INTERNAL_ERROR, _(cannot wait for '%s': %s), argv[0], strerror(errno)); -return -1; +goto error; } if (status == NULL) { errno = EINVAL; -if (WIFEXITED(exitstatus) WEXITSTATUS(exitstatus) == 0) -return 0; +if (WIFEXITED(exitstatus) WEXITSTATUS(exitstatus) != 0) { -ReportError(conn, VIR_ERR_INTERNAL_ERROR, -_(%s exited with non-zero status %d and signal %d), -argv[0], -WIFEXITED(exitstatus) ? WEXITSTATUS(exitstatus) : 0, -WIFSIGNALED(exitstatus) ?
[libvirt] [PATCH 1/3]: Move argvToString to util
The attached patch moves the argvToString function from iptables.c to util.c for use in later patches. Thanks, Cole commit c115b34455789910597061c7b100e056cb3fe787 Author: Cole Robinson [EMAIL PROTECTED] Date: Fri Oct 24 10:10:11 2008 -0400 Move argvToString to virArgvToString in util diff --git a/src/iptables.c b/src/iptables.c index 726141a..53e0f41 100644 --- a/src/iptables.c +++ b/src/iptables.c @@ -384,32 +384,6 @@ iptRulesNew(const char *table, return NULL; } -static char * -argvToString(const char *const *argv) -{ -int len, i; -char *ret, *p; - -for (len = 1, i = 0; argv[i]; i++) -len += strlen(argv[i]) + 1; - -if (VIR_ALLOC_N(ret, len) 0) -return NULL; -p = ret; - -for (i = 0; argv[i]; i++) { -if (i != 0) -*(p++) = ' '; - -strcpy(p, argv[i]); -p += strlen(argv[i]); -} - -*p = '\0'; - -return ret; -} - static int iptablesAddRemoveRule(iptRules *rules, int action, const char *arg, ...) { @@ -464,7 +438,7 @@ iptablesAddRemoveRule(iptRules *rules, int action, const char *arg, ...) va_end(args); -if (!(rule = argvToString(argv[command_idx]))) +if (!(rule = virArgvToString(argv[command_idx]))) goto error; if (action == REMOVE) { diff --git a/src/util.c b/src/util.c index aa21653..4476323 100644 --- a/src/util.c +++ b/src/util.c @@ -87,6 +87,33 @@ int virFileStripSuffix(char *str, return 1; } +char * +virArgvToString(const char *const *argv) +{ +int len, i; +char *ret, *p; + +for (len = 1, i = 0; argv[i]; i++) +len += strlen(argv[i]) + 1; + +if (VIR_ALLOC_N(ret, len) 0) +return NULL; +p = ret; + +for (i = 0; argv[i]; i++) { +if (i != 0) +*(p++) = ' '; + +strcpy(p, argv[i]); +p += strlen(argv[i]); +} + +*p = '\0'; + +return ret; +} + + #ifndef __MINGW32__ static int virSetCloseExec(int fd) { diff --git a/src/util.h b/src/util.h index 093ef46..8e91dd7 100644 --- a/src/util.h +++ b/src/util.h @@ -87,6 +87,8 @@ int virFileReadPid(const char *dir, int virFileDeletePid(const char *dir, const char *name); +char *virArgvToString(const char *const *argv); + int __virStrToLong_i(char const *s, char **end_ptr, int base, -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] sVirt v0.10 - initial prototype
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Atsushi SAKAI wrote: Hi, James I have a question just for interest. The security context stores like /etc/selinux/targeted/contexts/files/file_contexts. But you are storeing the domain security label on libvirt specific XML. Of course, this is good for libvirt POV. Is it permitted for SELinux policy POV? By the way, I want to see the further discussion of the Requirements. Requirements not yet addressed include: Thanks Atsushi SAKAI Both /etc/selinux/targeted/contexts/files/file_contexts and libvirt specific XML are just indicators of the way the system should be setup. The kernel policy is the final arbiter of the policy. So if the kernel does not understand virt_image_t in /etc/selinux/targeted/contexts/files/file_contexts, it will not allow tools like rpm or restorecon to set it as a file context. Similarly if the process label qemu_t is specified in the libvirt xml, and the kernel policy does not know what a qemu_t is, it will not allow libvirt to start a processes labeled qemu_t. James Morris [EMAIL PROTECTED] wrote: [snip] The domain security label configuration format is as follows: # virsh dumpxml sys1 domain seclabel model='selinux' labelsystem_u:system_r:virtd_t:s0/label policytypetargeted/policytype /seclabel /domain It's possible to query the security label of a running domain via virsh: # virsh dominfo sys1 Id: 1 Name: sys1 UUID: fa3c8e06-0877-2a08-06fd-f2479b7bacb4 OS Type:hvm State: running CPU(s): 1 CPU time: 11.4s Max memory: 524288 kB Used memory:524288 kB Autostart: disable Security label: system_u:system_r:virtd_t:s0 (selinux/targeted/enforcing) The security label is deterimed via the new virDomainGetSecLabel() API method, which is transported over RPC to the backend driver (qemu in this case), and is entirely independent of the local security model, if any. e.g. this command could be run remotely from an entirely different platform: you just see what's happening on the remote system, as with other attributes of the domain. Feedback on the design thus far is sought before proceeding to more comprehensive integration. In particular, I'd be interested in any thoughts on the placement of the security labeling driver within libvirt, as this seems to be the most critical architectural issue (I've already refactored this aspect once). Currently, the idea is to attach the security labeling driver to the virt driver, rather than implement it independently as a top-level component as in the case of other types of drivers (e.g. storage). This is because process-based security labeling is highly dependent on the kind of virtualization in use, and may not make sense at all in some cases (e.g. when using a non-Linux hypervisor, or containers). In the case of qemu, a security labeling driver is added to qemud: @@ -63,6 +64,7 @@ struct qemud_driver { char *vncListen; virCapsPtr caps; +virSecLabelDriverPtr secLabelDriver; }; and then initialized during qemud startup from qemudSecLabelInit(). During initialization, any available security labeling drivers are probed, and the first one which thinks it should be used is installed. Top-level libvirt API calls are then dispatched to the active security labeling driver via the backend virt driver, as necessary. Note that the security labeling framework in this release is always built-in -- it can be made a compile-time option later if desired. Requirements not yet addressed include: - Labeling of resources and generally comprehensive labeling management - Automatic labeling (e.g. for the simple isolation use-case) - Integration of labeling support into higher-level management tools such as virt-manager - Integration with the audit subsystem to help with administration and debugging - Domain of interpretation (DOI) checking/translation - Python bindings As mentioned, the goal at this stage is to get feedback on the underlying design: comments welcome! - James -- James Morris [EMAIL PROTECTED] -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [EMAIL PROTECTED] with the words unsubscribe selinux without quotes as the message. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org iEYEARECAAYFAkkKCHgACgkQrlYvE4MpobO44ACdG8CX5Y6ptaUTd2RtP4rtjaTo u1sAniwNi1WoYUuxkO3zM9A8hNUGLhTO =jtTR -END PGP SIGNATURE- -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3]: Read cmd stdout + stderr in virRun
Cole Robinson [EMAIL PROTECTED] wrote: The attached patch is my second cut at reading stdout and stderr of the command virRun kicks off. There is no hard limit to the amount of data we read now, and we use a poll loop to avoid any possible full buffer issues. If stdout or stderr had any content, we DEBUG it, and if the command appears to fail we return stderr in the error message. So now, trying to stop a logical pool with active volumes will return: $ sudo virsh pool-destroy vgdata libvir: error : internal error '/sbin/vgchange -an vgdata' exited with non-zero status 5 and signal 0: Can't deactivate volume group vgdata with 2 open logical volume(s) error: Failed to destroy pool vgdata Thanks, Cole commit af7d94392bc89fd0645514cd13a2186ca5224dfc Author: Cole Robinson [EMAIL PROTECTED] Date: Thu Oct 30 13:46:06 2008 -0400 Capture command stdout and stderr in virRun diff --git a/src/util.c b/src/util.c index e59e25c..8d624b2 100644 --- a/src/util.c +++ b/src/util.c @@ -33,6 +33,9 @@ #include errno.h #include sys/types.h #include sys/stat.h +#if HAVE_SYS_POLL_H +#include sys/poll.h +#endif Hi Cole, This looks fine to me. One minor suggestion: You can replace the above 3 lines with just #include poll.h poll.h is POSIX-specified, and guaranteed to be usable, since we're now using gnulib's poll module. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Domain Events Python Bindings (Round 2)
Attached are the python bindings for domain events as previously submitted: https://www.redhat.com/archives/libvir-list/2008-October/msg00707.html https://www.redhat.com/archives/libvir-list/2008-October/msg00668.html I have resolved most of the issues Daniel V. commented on I also addressed a number of ref counting problems. examples/domain-events/events-python/event-test.py | 186 python/Makefile.am |5 python/generator.py| 19 python/libvir.c| 441 + python/libvir.py | 20 python/libvirt_wrap.h | 27 + python/types.c | 47 ++ python/virConnect.py | 43 ++ 8 files changed, 782 insertions(+), 6 deletions(-) diff --git a/examples/domain-events/events-python/event-test.py b/examples/domain-events/events-python/event-test.py new file mode 100755 index 000..7082db0 --- /dev/null +++ b/examples/domain-events/events-python/event-test.py @@ -0,0 +1,186 @@ +#!/usr/bin/python -u +import sys,getopt,os +import libvirt +import select + +mypoll = select.poll() +TIMEOUT_MS = 1000 + +# handle globals +h_fd = 0 +h_events = 0 +h_cb = None +h_opaque = None + +# timeout globals +t_active = 0 +t_timeout = -1 +t_cb = None +t_opaque = None + +# +# Callback Functions +# +def eventToString(event): +eventStrings = ( Added, + Removed, + Started, + Suspended, + Resumed, + Stopped, + Saved, + Restored ); +return eventStrings[event]; + +def myDomainEventCallback1 (conn, dom, event, opaque): +print myDomainEventCallback1 EVENT: Domain %s(%s) %s % (dom.name(), dom.ID(), eventToString(event)) + +def myDomainEventCallback2 (conn, dom, event, opaque): +print myDomainEventCallback2 EVENT: Domain %s(%s) %s % (dom.name(), dom.ID(), eventToString(event)) + +# +# EventImpl Functions +# +def myEventHandleTypeToPollEvent(events): +ret = 0 +if events libvirt.VIR_EVENT_HANDLE_READABLE: +ret |= select.POLLIN +if events libvirt.VIR_EVENT_HANDLE_WRITABLE: +ret |= select.POLLOUT +if events libvirt.VIR_EVENT_HANDLE_ERROR: +ret |= select.POLLERR; +if events libvirt.VIR_EVENT_HANDLE_HANGUP: +ret |= select.POLLHUP; +return ret + +def myPollEventToEventHandleType(events): +ret = 0; +if events select.POLLIN: +ret |= libvirt.VIR_EVENT_HANDLE_READABLE; +if events select.POLLOUT: +ret |= libvirt.VIR_EVENT_HANDLE_WRITABLE; +if events select.POLLERR: +ret |= libvirt.VIR_EVENT_HANDLE_ERROR; +if events select.POLLHUP: +ret |= libvirt.VIR_EVENT_HANDLE_HANGUP; +return ret; + +def myAddHandle(fd, events, cb, opaque): +global h_fd, h_events, h_cb, h_opaque +#print Adding Handle %s %s %s %s % (str(fd), str(events), str(cb), str(opaque)) +h_fd = fd +h_events = events +h_cb = cb +h_opaque = opaque + +mypoll.register(fd, myEventHandleTypeToPollEvent(events)) + +def myUpdateHandle(fd, event): +global h_fd, h_events +#print Updating Handle %s %s % (str(fd), str(events)) +h_fd = fd +h_events = event +mypoll.unregister(fd) +mypoll.register(fd, myEventHandleTypeToPollEvent(event)) + +def myRemoveHandle(fd): +global h_fd +#print Removing Handle %s % str(fd) +h_fd = 0 +mypoll.unregister(fd) + +def myAddTimeout(timeout, cb, opaque): +global t_active, t_timeout, t_cb, t_opaque +#print Adding Timeout %s %s %s % (str(timeout), str(cb), str(opaque)) +t_active = 1; +t_timeout = timeout; +t_cb = cb; +t_opaque = opaque; + +def myUpdateTimeout(timer, timeout): +global t_timeout +#print Updating Timeout %s % (str(timer), str(timeout)) +t_timeout = timeout; + +def myRemoveTimeout(timer): +global t_active +#print Removing Timeout %s % str(timer) +t_active = 0; + +## +# Main +## + +def usage(): +print usage: +os.path.basename(sys.argv[0])+ [uri] +printuri will default to qemu:///system + +def main(): +try: +opts, args = getopt.getopt(sys.argv[1:], h, [help] ) +except getopt.GetoptError, err: +# print help information and exit: +print str(err) # will print something like option -a not recognized +usage() +sys.exit(2) +for o, a in opts: +if o in (-h, --help): +usage() +sys.exit() + +if len(sys.argv) 1: +uri =
[libvirt] virt-viewer with fullscreen?
Hi, I am using virt-viewer to view an Windows VM. However, I cannot get fullscreen with virt-viewer when using menu View - Fullscreen. When I did that, nothing happenes. Is that a bug? The version of virt-viewer is 0.0.3 Thanks, J -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list